[Webkit-unassigned] [Bug 105508] Proposal: Add fillRule to canvas

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=105508





--- Comment #23 from James Ascroft-Leigh <jwal at jwal.me.uk>  2013-01-04 09:20:58 PST ---
(From update of attachment 180692)
View in context: https://bugs.webkit.org/attachment.cgi?id=180692&action=review

>> Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp:427
>> +    c->setFillRule(rule);
> 
> This is the only interesting bit of this whole change.  I don't feel up-to-date enough on GraphicsContext to review this part, but otherwise thsi chagne LGTM.

Indeed, and even this doesn't do very much apart from passing the call onto the lower level API.  To be honest I didn't think too hard about what should be done at this point but I took the other similar functions in the file as a template for this function.  For example https://github.com/jwal/webkit/blob/e8701c215ca234c43b119d3a3a1fe93f28deda34/Source/WebCore/platform/graphics/GraphicsTypes.cpp#L85

I tested this by creating a build of Chromium based on the modified WebKit and ran it on

  a) A modified version of the original webapp which originally led me to this missing feature
  b) Hand-crafted test case HTML files specifically for this new feature, based on canvas calls generated directly from the example SVG files in the SVG spec

I thought it was sufficient, for now, to perform just these directed tests on Chromium on GNU/Linux on x86_64.

>> Source/WebCore/platform/graphics/GraphicsTypes.cpp:111
>> +    return names[rule];
> 
> I might have written this as an if (rule == NonZero) return "nonzero"; return "evenodd", to avoid any possible OOB, but this is also fine.

I wrote it like this to be consistent with the other similar functions in this file.

Sorry the patch does not apply cleanly any more, the base version was: https://github.com/jwal/webkit/blob/e8701c215ca234c43b119d3a3a1fe93f28deda34/Source/WebCore/platform/graphics/GraphicsTypes.cpp#L85

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.