Closed Bug 764860 Opened 8 years ago Closed 8 years ago

Simplify the clipPath code

Categories

(Core :: SVG, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla16

People

(Reporter: jwatt, Assigned: jwatt)

Details

Attachments

(1 file, 3 obsolete files)

We can simplify the clipPath code by making it a lot clearer what we actually do when we have a trivial clipPath, and by killing off the unused SVGAutoRenderState::CLIP.
Attached patch patch (obsolete) — Splinter Review
Attachment #633159 - Flags: review?(longsonr)
Attachment #633159 - Flags: review?(longsonr) → review+
Comment on attachment 633159 [details] [diff] [review]
patch

If we took this we'd fail:

masking-path-02-b.svg
masking-path-05-f.svg
masking-path-07-b.svg
masking-path-08-b.svg
masking-path-09-b.svg

from the SVG testsuite.
Attachment #633159 - Flags: review+ → review-
Oh and the Firefox UI is now broken as the address bar has disappeared.
Yeah, I can't get rid of CLIP. New patch coming shortly.
If the original patch passes our existing reftests, we need more reftests.
Attached patch patch (obsolete) — Splinter Review
No, it doesn't pass them.
Attachment #633159 - Attachment is obsolete: true
Attachment #633168 - Flags: review?(longsonr)
And this one does, as well as passing all the SVG testsuite masking-path tests and I get to keep the Firefox address bar ;-)
Comment 7 is really a question.
Attached patch patch (obsolete) — Splinter Review
It does if I dereference aSingleChild properly when assigning to it.
Attachment #633168 - Attachment is obsolete: true
Attachment #633168 - Flags: review?(longsonr)
Attachment #633235 - Flags: review?(longsonr)
Attachment #633235 - Attachment is obsolete: true
Attachment #633235 - Flags: review?(longsonr)
Attachment #633385 - Flags: review?(longsonr)
I also locally added this for the NORMAL value:

    /**
     * Used to inform SVG frames that they should paint as normal.
     */
Comment on attachment 633385 [details] [diff] [review]
patch + comments documenting SVGAutoRenderState::RenderMode

r=longsonr provided you've checked the SVG testsuite and all the masking-path tests still pass
Attachment #633385 - Flags: review?(longsonr) → review+
Pushed https://hg.mozilla.org/integration/mozilla-inbound/rev/52e4627306f9
Target Milestone: --- → mozilla16
https://hg.mozilla.org/mozilla-central/rev/52e4627306f9
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.