Last Comment Bug 764860 - Simplify the clipPath code
: Simplify the clipPath code
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: SVG (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla16
Assigned To: Jonathan Watt [:jwatt] (back in October - email directly if necessary)
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-06-14 09:09 PDT by Jonathan Watt [:jwatt] (back in October - email directly if necessary)
Modified: 2012-06-16 06:53 PDT (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (5.64 KB, patch)
2012-06-14 09:10 PDT, Jonathan Watt [:jwatt] (back in October - email directly if necessary)
longsonr: review-
Details | Diff | Splinter Review
patch (6.95 KB, patch)
2012-06-14 09:54 PDT, Jonathan Watt [:jwatt] (back in October - email directly if necessary)
no flags Details | Diff | Splinter Review
patch (6.95 KB, patch)
2012-06-14 12:34 PDT, Jonathan Watt [:jwatt] (back in October - email directly if necessary)
no flags Details | Diff | Splinter Review
patch + comments documenting SVGAutoRenderState::RenderMode (9.18 KB, patch)
2012-06-14 20:54 PDT, Jonathan Watt [:jwatt] (back in October - email directly if necessary)
longsonr: review+
Details | Diff | Splinter Review

Description Jonathan Watt [:jwatt] (back in October - email directly if necessary) 2012-06-14 09:09:19 PDT
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.
Comment 1 Jonathan Watt [:jwatt] (back in October - email directly if necessary) 2012-06-14 09:10:13 PDT
Created attachment 633159 [details] [diff] [review]
patch
Comment 2 Robert Longson 2012-06-14 09:47:02 PDT
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.
Comment 3 Robert Longson 2012-06-14 09:48:49 PDT
Oh and the Firefox UI is now broken as the address bar has disappeared.
Comment 4 Jonathan Watt [:jwatt] (back in October - email directly if necessary) 2012-06-14 09:49:29 PDT
Yeah, I can't get rid of CLIP. New patch coming shortly.
Comment 5 Robert Longson 2012-06-14 09:51:58 PDT
If the original patch passes our existing reftests, we need more reftests.
Comment 6 Jonathan Watt [:jwatt] (back in October - email directly if necessary) 2012-06-14 09:54:04 PDT
Created attachment 633168 [details] [diff] [review]
patch

No, it doesn't pass them.
Comment 7 Robert Longson 2012-06-14 09:56:24 PDT
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 Robert Longson 2012-06-14 09:56:51 PDT
Comment 7 is really a question.
Comment 9 Jonathan Watt [:jwatt] (back in October - email directly if necessary) 2012-06-14 12:34:42 PDT
Created attachment 633235 [details] [diff] [review]
patch

It does if I dereference aSingleChild properly when assigning to it.
Comment 10 Jonathan Watt [:jwatt] (back in October - email directly if necessary) 2012-06-14 20:51:47 PDT
Passed Try: https://tbpl.mozilla.org/?tree=Try&rev=ba7db708631b
Comment 11 Jonathan Watt [:jwatt] (back in October - email directly if necessary) 2012-06-14 20:54:10 PDT
Created attachment 633385 [details] [diff] [review]
patch + comments documenting SVGAutoRenderState::RenderMode
Comment 12 Jonathan Watt [:jwatt] (back in October - email directly if necessary) 2012-06-14 21:09:30 PDT
I also locally added this for the NORMAL value:

    /**
     * Used to inform SVG frames that they should paint as normal.
     */
Comment 13 Robert Longson 2012-06-14 23:58:54 PDT
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
Comment 14 Jonathan Watt [:jwatt] (back in October - email directly if necessary) 2012-06-15 02:08:35 PDT
Pushed https://hg.mozilla.org/integration/mozilla-inbound/rev/52e4627306f9
Comment 15 Ryan VanderMeulen [:RyanVM] 2012-06-16 06:53:23 PDT
https://hg.mozilla.org/mozilla-central/rev/52e4627306f9

Note You need to log in before you can comment on or make changes to this bug.