Closed Bug 1034115 Opened 5 years ago Closed 5 years ago

Stop needlessly creating CSS-px-to-dev-pixel matrices when doing SVG hit-testing

Categories

(Core :: SVG, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla33

People

(Reporter: jwatt, Assigned: jwatt)

References

Details

Attachments

(1 file)

Right now the various GetCanvasTM implementations usually return nsSVGIntegrationUtils::GetCSSPxToDevPxMatrix() when passed FOR_HIT_TESTING. There is no good reason for this, and it is very undesirable when we convert SVG hit-testing to Moz2D since it necessitates doing a Path::TransformedCopyToBuilder() call, which can be expensive. I'd like to fix this before fixing bug 988808.

Basically when GetCanvasTM() returns GetCSSPxToDevPxMatrix() for FOR_HIT_TESTING we should instead return the identity matrix and then adjust the consumers to understand that the transform no longer has the pointless css-to-dev-px component.
Summary: Stop needlessly creating CSS-px-to-dev-pixel matricies when doing SVG hit-testing → Stop needlessly creating CSS-px-to-dev-pixel matrices when doing SVG hit-testing
Attached patch patchSplinter Review
Attachment #8450242 - Flags: review?(longsonr)
These code paths seem to be fairly well tested BTW, since I made a couple of typos when writing this patch and blew up a bunch of our mochitests.
Comment on attachment 8450242 [details] [diff] [review]
patch

> 
>+  SVGBBox& operator*=(float aFactor) {

Should make this a Scale method to match http://mxr.mozilla.org/mozilla-central/source/gfx/2d/BaseRect.h#385

>+    mBBox.x *= aFactor;
>+    mBBox.y *= aFactor;
>+    mBBox.width *= aFactor;
>+    mBBox.height *= aFactor;
>+    return *this;
>+  }
>+

r=longsonr with that
Attachment #8450242 - Flags: review?(longsonr) → review+
BTW did you test this at all with SVGDisplayListHitTestingEnabled false? Maybe it's time to bite the bullet and remove the false codepaths.
(In reply to Robert Longson from comment #4)
> BTW did you test this at all with SVGDisplayListHitTestingEnabled false?

I did a bit of testing.

> Maybe it's time to bite the bullet and remove the false codepaths.

See bug 829802 comment 6.
https://hg.mozilla.org/mozilla-central/rev/340b176d8f6b
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
Blocks: 1036400
You need to log in before you can comment on or make changes to this bug.