Last Comment Bug 764860 - Simplify the clipPath code
: Simplify the clipPath code
Product: Core
Classification: Components
Component: SVG (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla16
Assigned To: Jonathan Watt [:jwatt]
: Jet Villegas (:jet)
Depends on:
  Show dependency treegraph
Reported: 2012-06-14 09:09 PDT by Jonathan Watt [:jwatt]
Modified: 2012-06-16 06:53 PDT (History)
1 user (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

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

Description Jonathan Watt [:jwatt] 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] 2012-06-14 09:10:13 PDT
Created attachment 633159 [details] [diff] [review]
Comment 2 Robert Longson 2012-06-14 09:47:02 PDT
Comment on attachment 633159 [details] [diff] [review]

If we took this we'd fail:


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] 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] 2012-06-14 09:54:04 PDT
Created attachment 633168 [details] [diff] [review]

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] 2012-06-14 12:34:42 PDT
Created attachment 633235 [details] [diff] [review]

It does if I dereference aSingleChild properly when assigning to it.
Comment 10 Jonathan Watt [:jwatt] 2012-06-14 20:51:47 PDT
Passed Try:
Comment 11 Jonathan Watt [:jwatt] 2012-06-14 20:54:10 PDT
Created attachment 633385 [details] [diff] [review]
patch + comments documenting SVGAutoRenderState::RenderMode
Comment 12 Jonathan Watt [:jwatt] 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] 2012-06-15 02:08:35 PDT
Comment 15 Ryan VanderMeulen [:RyanVM] 2012-06-16 06:53:23 PDT

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