Jonathan Watt [:jwatt] Assignee
	Description • 15 years ago

The patch for bug 455984 broke our opacity optimization.

	Jonathan Watt [:jwatt] Assignee
	Comment 2 • 15 years ago

Need to add a Tsvg test - will do that tomorrow when I'm more awake.

	Robert Longson [:longsonr]
	Comment 3 • 15 years ago

Comment on attachment 407416 [details] [diff] [review]
patch

>   if (!aFrame->GetStyleSVGReset()->mFilter) {
>     nsIAtom *type = aFrame->GetType();
>     if (type == nsGkAtoms::svgImageFrame)
>       return PR_TRUE;
>     if (type == nsGkAtoms::svgPathGeometryFrame) {
>       const nsStyleSVG *style = aFrame->GetStyleSVG();
>-      if (style->mFill.mType == eStyleSVGPaintType_None &&
>-          style->mStroke.mType == eStyleSVGPaintType_None)
>+      if (style->mFill.mType == eStyleSVGPaintType_None ||
>+          !static_cast<nsSVGPathGeometryFrame*>(aFrame)->HasStroke())
>         return PR_TRUE;

Shouldn't the logic operator stay as &&

	Jonathan Watt [:jwatt] Assignee
	Comment 4 • 15 years ago

No. If the fill OR the stroke paints nothing, then the 'opacity' value can be combined with the 'stroke-opacity' or 'fill-opacity' respectively.

If fill AND stroke paint nothing, then we'll be returning from both those operations early, although it's still useful to avoid the push-group which this logic will do.

	Jonathan Watt [:jwatt] Assignee
	Comment 5 • 15 years ago

Pushed http://hg.mozilla.org/mozilla-central/rev/573a37f0d13c

	Jonathan Watt [:jwatt] Assignee
	Comment 6 • 15 years ago

Here's a testcase for Tsvg. It has 10,000 little rects testing the opacity optimization. On my laptop it hangs the browser for over 2 minutes when trying to repaint after a browser resize without the patch, but "only" takes about 1 second to repaint with the patch.

	Jonathan Watt [:jwatt] Assignee
	Comment 7 • 15 years ago

500 KB it probably a little big. Here's a 50x50 testcase that "only" hangs for 20 seconds without the patch.

	Jonathan Watt [:jwatt] Assignee
	Comment 8 • 15 years ago

Given that this gives optimizable cases in excess of a 2 orders of magnitude speedup, maybe we should also land this on Firefox 3.5? I heard from three people at SVG Open and from others prior to that that Firefox is really, really slow with opacity compared to Safari/Opera, but I'd just put that down to bug 309782 rather than a regression.

	Jonathan Watt [:jwatt] Assignee
	Comment 9 • 15 years ago

I filed bug 524089 about Tsvg tests, so marking this fixed.

roc, what do you think about getting this landed on branches?

	Robert O'Callahan (:roc) (email my personal email if necessary)
	Comment 10 • 15 years ago

Yes.

	Jonathan Watt [:jwatt] Assignee
	Comment 11 • 15 years ago

Pushed to 1.9.2 http://hg.mozilla.org/releases/mozilla-1.9.2/rev/7e6838c2051f