Closed
Bug 523481
Opened 15 years ago
Closed 15 years ago
The patch for bug 455984 broke our opacity optimization
Categories
(Core :: SVG, defect)
Core
SVG
Tracking
()
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
status1.9.2 | --- | beta1-fixed |
People
(Reporter: jwatt, Assigned: jwatt)
References
Details
Attachments
(2 files, 1 obsolete file)
5.08 KB,
patch
|
longsonr
:
review+
roc
:
approval1.9.2+
|
Details | Diff | Splinter Review |
144.70 KB,
image/svg+xml
|
Details |
The patch for bug 455984 broke our opacity optimization.
Assignee | ||
Comment 1•15 years ago
|
||
Assignee | ||
Comment 2•15 years ago
|
||
Need to add a Tsvg test - will do that tomorrow when I'm more awake.
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 &&
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.
Updated•15 years ago
|
Attachment #407416 -
Flags: review?(longsonr) → review+
Assignee | ||
Comment 5•15 years ago
|
||
Pushed http://hg.mozilla.org/mozilla-central/rev/573a37f0d13c
Flags: in-testsuite?
Assignee | ||
Updated•15 years ago
|
Attachment #407416 -
Flags: approval1.9.2?
Assignee | ||
Updated•15 years ago
|
Flags: wanted1.9.2+
Flags: blocking1.9.2?
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.
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.
Attachment #407510 -
Attachment is obsolete: true
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.
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?
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Yes.
Flags: blocking1.9.2? → blocking1.9.2+
Attachment #407416 -
Flags: approval1.9.2? → approval1.9.2+
Assignee | ||
Comment 11•15 years ago
|
||
Pushed to 1.9.2 http://hg.mozilla.org/releases/mozilla-1.9.2/rev/7e6838c2051f
status1.9.2:
--- → beta1-fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•