Avoid changing the current transform between emitting the path and clipping using that path in the SVGAutoRenderState::CLIP case

RESOLVED FIXED in mozilla27

Status

()

Core
SVG
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: jwatt, Assigned: jwatt)

Tracking

(Blocks: 2 bugs, {perf})

Trunk
mozilla27
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
To allow us to kill off gfxContext and move to Moz2D we need to stop changing the current transform between emitting a path and using it. One place that we do that is where we implement SVGAutoRenderState::CLIP clipping. In this case nsSVGClipPathFrame::Clip calls PaintSVG on the single clipping frame child, which calls nsSVGPathGeometryFrame::Render. In that Render method we Save() the context, call GeneratePath() (which applies a matrix), Restore() the context and return up to nsSVGClipPathFrame::Clip where the path is used by the gfxContext::Clip() call.
(Assignee)

Updated

5 years ago
Blocks: 882109
(Assignee)

Comment 1

5 years ago
Created attachment 820334 [details] [diff] [review]
patch
Attachment #820334 - Flags: review?(cam)
Comment on attachment 820334 [details] [diff] [review]
patch

Review of attachment 820334 [details] [diff] [review]:
-----------------------------------------------------------------

This looks good.  Can you add an assertion to check your in-comment claim that the transform will never change?
Attachment #820334 - Flags: review?(cam) → review+
(Assignee)

Comment 4

5 years ago
(In reply to Cameron McCormack (:heycam) from comment #3)
> This looks good.  Can you add an assertion to check your in-comment claim
> that the transform will never change?

Talking to Cameron in person, it seems he misunderstood the comment. I'll land without adding an assertion a bit later.
Summary: Stop changing the current transform between emitting the path and clipping in the SVGAutoRenderState::CLIP case → Avoid changing the current transform between emitting the path and clipping using that path in the SVGAutoRenderState::CLIP case
(Assignee)

Comment 5

5 years ago
The Save()/Restore() that this patch avoids is pretty expensive, so this is a perf improvement even as-is (that is, without the subsequent conversion to Moz2D).
Keywords: perf
https://hg.mozilla.org/mozilla-central/rev/b011488de9e6
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
(Assignee)

Updated

5 years ago
Blocks: 703159
(Assignee)

Updated

4 years ago
Depends on: 959128
You need to log in before you can comment on or make changes to this bug.