929021 - Avoid calling nsDisplayTransform::GetFrameBoundsForTransform

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Robert O'Callahan (:roc) (email my personal email if necessary)]

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.

Comment 1

[Robert O'Callahan (:roc) (email my personal email if necessary)]

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.

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a61d898ea4fa

Comment 4

[Ed Morley [:emorley]]

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

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

http://hg.mozilla.org/integration/mozilla-inbound/rev/75ee2a0bc5d3

Comment 6

[Ed Morley [:emorley]]

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.

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

https://tbpl.mozilla.org/?tree=Try&rev=c52e80b74d53
https://hg.mozilla.org/integration/mozilla-inbound/rev/c1578a4fc86d

Comment 8

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/c1578a4fc86d

Comment 9

[Matt Brubeck (:mbrubeck)]

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 10

[Tracy Walker [:tracy]]

Where is data for this regression this bug is tracking?

Comment 11

[Jonathan Watt [:jwatt]]

(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

[Jonathan Watt [:jwatt]]

(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

[Matt Brubeck (:mbrubeck)]

I filed bug 933354 to track the remaining regression.

Comment 14

[Robert O'Callahan (:roc) (email my personal email if necessary)]

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

[Ryan VanderMeulen [:RyanVM]]

Merge of backout:
https://hg.mozilla.org/mozilla-central/rev/3fb151446ec5

Comment 16

[Ioana (away)]

Removing keyword per comment 12.