Closed
Bug 655926
Opened 13 years ago
Closed 13 years ago
Add support for even-odd-rule filling and clipping
Categories
(Core :: Graphics: Canvas2D, defect)
Core
Graphics: Canvas2D
Tracking
()
RESOLVED
FIXED
mozilla7
People
(Reporter: cjones, Assigned: cjones)
Details
(Keywords: dev-doc-complete)
Attachments
(2 files)
4.39 KB,
patch
|
vlad
:
review+
vlad
:
superreview+
|
Details | Diff | Splinter Review |
6.67 KB,
patch
|
joe
:
review+
|
Details | Diff | Splinter Review |
Andreas and I are hacking on a project that needs this. It's pretty trivial for our cairo <canvas> backend; we just need to |cairo_set_fill_rule (cairo, CAIRO_FILL_RULE_EVEN_ODD);| when requested. I'm not sure what the right API is; how about context.fillRule = "nonzero" | "evenodd". Default is "nonzero". fillRule is saved and restored in the same way as fillStyle. (Probably needs to be |mozFillRule| initially.) Does Azure support even-odd-rule fills yet?
Assignee | ||
Comment 1•13 years ago
|
||
Bas tells me that Azure doesn't support even-odd-rule fills (because canvas doesn't use it, hah), but it would trivial to add.
Assignee | ||
Comment 2•13 years ago
|
||
Bas suggests "winding" instead of "nonzero", and indeed that's what the cairo constant is called too (CAIRO_FILL_RULE_WINDING).
Comment 3•13 years ago
|
||
http://www.w3.org/TR/SVG/painting.html#FillRuleProperty uses "nonzero" and "evenodd", and consistency with web APIs seems more useful than consistency with Cairo. How would this affect clip(), and isPointInPath(), and self-overlapping strokes?
Assignee | ||
Comment 4•13 years ago
|
||
(In reply to comment #3) > http://www.w3.org/TR/SVG/painting.html#FillRuleProperty uses "nonzero" and > "evenodd", and consistency with web APIs seems more useful than consistency > with Cairo. Agreed. Thanks, I had forgotten about that. > How would this affect clip(), and isPointInPath(), These would be subject to fillRule. > and self-overlapping strokes? You mean whether the fillRule might cause them not to be stroked? I don't know; I don't think it should. http://dev.w3.org/html5/2dcontext/#dom-context-2d-stroke seems to imply it doesn't. Does SVG specify anything?
Assignee | ||
Comment 5•13 years ago
|
||
(Vlad, are you still around doing sr's?) Pretty trivial patch. Tests and spec forthcoming, if interested parties agree on this impl.
Assignee: nobody → jones.chris.g
Attachment #537275 -
Flags: superreview?(vladimir)
Attachment #537275 -
Flags: review?(joe)
Comment on attachment 537275 [details] [diff] [review] Implement canvas.mozFillRule Looks good to me
Attachment #537275 -
Flags: superreview?(vladimir)
Attachment #537275 -
Flags: superreview+
Attachment #537275 -
Flags: review?(joe)
Attachment #537275 -
Flags: review+
Assignee | ||
Comment 7•13 years ago
|
||
Simple tests, can expand as bugs arise.
Attachment #538202 -
Flags: review?(joe)
Comment 8•13 years ago
|
||
Comment on attachment 538202 [details] [diff] [review] part 2: Test even-odd fill rule Review of attachment 538202 [details] [diff] [review]: ----------------------------------------------------------------- There's a bunch of boilerplate (defining PI, assert, etc) in the test html files that can be removed if you want. ::: layout/reftests/canvas/evenodd-fill-sanity.html @@ +14,5 @@ > + assert("evenodd" == ctx.mozFillRule, > + "fillRule understands 'evenodd'"); > + ctx.mozFillRule = "nonzero"; > + > + ctx.mozFillRule = "garbageLSKJDF 29879234"; You should probably move the '= "nonzero"' to just above the garbage. @@ +23,5 @@ > + assert("evenodd" == ctx.mozFillRule, > + "Garbage fillRule string has no effect"); > + ctx.mozFillRule = "nonzero"; > + > + ctx.save(); Same here.
Attachment #538202 -
Flags: review?(joe) → review+
Assignee | ||
Comment 9•13 years ago
|
||
(In reply to comment #8) > Comment on attachment 538202 [details] [diff] [review] [review] > part 2: Test even-odd fill rule > > Review of attachment 538202 [details] [diff] [review] [review]: > ----------------------------------------------------------------- > > There's a bunch of boilerplate (defining PI, assert, etc) in the test html > files that can be removed if you want. Done. (The PI garbage was from a failed first attempt that used circles instead of rects.) > > ::: layout/reftests/canvas/evenodd-fill-sanity.html > @@ +14,5 @@ > > + assert("evenodd" == ctx.mozFillRule, > > + "fillRule understands 'evenodd'"); > > + ctx.mozFillRule = "nonzero"; > > + > > + ctx.mozFillRule = "garbageLSKJDF 29879234"; > > You should probably move the '= "nonzero"' to just above the garbage. > > @@ +23,5 @@ > > + assert("evenodd" == ctx.mozFillRule, > > + "Garbage fillRule string has no effect"); > > + ctx.mozFillRule = "nonzero"; > > + > > + ctx.save(); > > Same here. Didn't change, per IRC chat.
Assignee | ||
Comment 10•13 years ago
|
||
http://hg.mozilla.org/integration/mozilla-inbound/rev/5f63b1c2b73a http://hg.mozilla.org/integration/mozilla-inbound/rev/2c19e587bc8c This will be dev-doc-needed when it lands on m-c and we finish spec'ing on whatwg.
Whiteboard: [inbound]
Updated•13 years ago
|
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Whiteboard: [inbound]
Target Milestone: --- → mozilla7
Comment 11•13 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/5f63b1c2b73a http://hg.mozilla.org/mozilla-central/rev/2c19e587bc8c
Updated•13 years ago
|
Keywords: dev-doc-needed
Assignee | ||
Comment 12•13 years ago
|
||
Let's wait for docs until this spec is looking solid on whatwg (I'll post today). Should be quick, this is a simple change. I'd rather that devs don't use this until then, TBH.
Keywords: dev-doc-needed
Assignee | ||
Comment 13•13 years ago
|
||
No objections on whatwg. (No comments at all, actually ...)
Keywords: dev-doc-needed
Comment 14•13 years ago
|
||
Since when it was first proposed on the whatwg list, the only response I can find was <http://lists.whatwg.org/pipermail/whatwg-whatwg.org/2009-April/019503.html>. You can probably ping Hixie on IRC or email if you want an immediate response from him instead of waiting months/years. (The content of the proposal seems straightforward and uncontroversial to me, so I wouldn't be surprised if it doesn't generate much discussion on the list.)
Comment 15•13 years ago
|
||
Since this is now implemented, speccing it should be non-controversial. (The only reason I am delaying canvas features is that they all get implemented almost immediately, which means they have a high opportunity cost. This is a non-issue once they are already implemented!)
Comment 16•13 years ago
|
||
Documentation updated: https://developer.mozilla.org/en/DOM/CanvasRenderingContext2D#Attributes Added to Firefox 7 for developers.
Keywords: dev-doc-needed → dev-doc-complete
Comment 17•11 years ago
|
||
This is now in the spec. http://whatwg.org/html/#dom-context-2d-fillrule
You need to log in
before you can comment on or make changes to this bug.
Description
•