Closed
Bug 929021
Opened 11 years ago
Closed 11 years ago
Avoid calling nsDisplayTransform::GetFrameBoundsForTransform
Categories
(Core :: SVG, defect)
Tracking
()
RESOLVED
FIXED
mozilla27
Tracking | Status | |
---|---|---|
firefox26 | --- | unaffected |
firefox27 | + | fixed |
People
(Reporter: roc, Assigned: roc)
References
Details
(Keywords: perf, regression, Whiteboard: [qa-])
Attachments
(1 file)
(deleted),
patch
|
heycam
:
review+
|
Details | Diff | Splinter Review |
Bug 923193 regressed tsvg performance.
We should fix it by avoiding calling GetFrameBoundsForTransform if possible. I think the easiest and best thing to do is to avoid calling GetDeltaToMozTransformOrigin for translation-only transforms, since it doesn't affect them.
Assignee | ||
Comment 1•11 years ago
|
||
Actually the thing to do is to avoid calling GetFrameBoundsForTransform when we have absolute coordinates for transform-orign --- which is the default for SVG, so we end up avoiding the cost of bug 923193 except where transform-origin is explicitly used.
Assignee | ||
Comment 2•11 years ago
|
||
Attachment #820213 -
Flags: review?(cam)
Updated•11 years ago
|
Attachment #820213 -
Flags: review?(cam) → review+
Assignee | ||
Comment 3•11 years ago
|
||
Comment 4•11 years ago
|
||
Whole push backed out since not clear what caused the reftest failures:
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=ffd9c7bbd2fe&jobname=reftest
remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/69d9f136cf26
remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/b71812b077de
Updated•11 years ago
|
Updated•11 years ago
|
status-firefox26:
--- → unaffected
Assignee | ||
Comment 5•11 years ago
|
||
Comment 6•11 years ago
|
||
Push backed out for reftest failures:
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&jobname=reftest&rev=75ee2a0bc5d3
remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/efe4b4053304
remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/e33a8cab1096
These are the same reftest failures that caused the backout in comment 4.
Assignee | ||
Comment 7•11 years ago
|
||
Comment 8•11 years ago
|
||
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
Comment 9•11 years ago
|
||
This only partially fixed the tsvg regression from bug 923193. The original regression was about +80ms, and the change from this patch was about -30ms.
Comment 11•11 years ago
|
||
(In reply to Matt Brubeck (:mbrubeck) from comment #9)
> This only partially fixed the tsvg regression from bug 923193. The original
> regression was about +80ms, and the change from this patch was about -30ms.
That's probably because there are a lot more places that call GetFrameBoundsForTransform than the caller that the patch for this bug guarded. Unfortunately at least some of those other callers don't look like they can be guarded since we don't know at the call point whether we will want the bounds later or not.
Comment 12•11 years ago
|
||
(In reply to [:tracy] Tracy Walker - QA Mentor from comment #10)
> Where is data for this regression this bug is tracking?
http://graphs.mozilla.org/graph.html#tests=[[281,63,29]]&sel=none&displayrange=30&datatype=running
For example. As far as verifying the regression fixed, no need though. It's not.
I'm not sure if roc would prefer to reopen this bug, or have another opened for the rest. Or maybe bug 923193 should be backed out until we implement caching of bboxes. roc?
Comment 13•11 years ago
|
||
I filed bug 933354 to track the remaining regression.
Assignee | ||
Comment 14•11 years ago
|
||
Backed out everything involving this bug:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3fb151446ec5
Leaving FIXED since there is no more work to do here.
Comment 15•11 years ago
|
||
Merge of backout:
https://hg.mozilla.org/mozilla-central/rev/3fb151446ec5
Updated•11 years ago
|
Whiteboard: [qa-]
You need to log in
before you can comment on or make changes to this bug.
Description
•