Simplify the clipPath code

RESOLVED FIXED in mozilla16

Status

()

Core
SVG
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: jwatt, Assigned: jwatt)

Tracking

Trunk
mozilla16
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Assignee)

Description

5 years ago
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.
(Assignee)

Comment 1

5 years ago
Created attachment 633159 [details] [diff] [review]
patch
Attachment #633159 - Flags: review?(longsonr)

Updated

5 years ago
Attachment #633159 - Flags: review?(longsonr) → review+

Comment 2

5 years ago
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-

Comment 3

5 years ago
Oh and the Firefox UI is now broken as the address bar has disappeared.
(Assignee)

Comment 4

5 years ago
Yeah, I can't get rid of CLIP. New patch coming shortly.

Comment 5

5 years ago
If the original patch passes our existing reftests, we need more reftests.
(Assignee)

Comment 6

5 years ago
Created attachment 633168 [details] [diff] [review]
patch

No, it doesn't pass them.
Attachment #633159 - Attachment is obsolete: true
Attachment #633168 - Flags: review?(longsonr)

Comment 7

5 years ago
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 8

5 years ago
Comment 7 is really a question.
(Assignee)

Comment 9

5 years ago
Created attachment 633235 [details] [diff] [review]
patch

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)
(Assignee)

Comment 10

5 years ago
Passed Try: https://tbpl.mozilla.org/?tree=Try&rev=ba7db708631b
(Assignee)

Comment 11

5 years ago
Created attachment 633385 [details] [diff] [review]
patch + comments documenting SVGAutoRenderState::RenderMode
Attachment #633235 - Attachment is obsolete: true
Attachment #633235 - Flags: review?(longsonr)
Attachment #633385 - Flags: review?(longsonr)
(Assignee)

Comment 12

5 years ago
I also locally added this for the NORMAL value:

    /**
     * Used to inform SVG frames that they should paint as normal.
     */

Comment 13

5 years ago
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+
(Assignee)

Comment 14

5 years ago
Pushed https://hg.mozilla.org/integration/mozilla-inbound/rev/52e4627306f9
Target Milestone: --- → mozilla16
https://hg.mozilla.org/mozilla-central/rev/52e4627306f9
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.