Bug 106872 - Update GraphicsContext to support winding rule in clip operator for Chromium
Summary: Update GraphicsContext to support winding rule in clip operator for Chromium
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Rik Cabanier
URL:
Keywords:
Depends on: 106871
Blocks: 106188
  Show dependency tree / graph
 
Reported: 2013-01-14 22:10 PST by Rik Cabanier
Modified: 2013-01-17 20:03 PST (History)
4 users (show)

See Also:

Attachments
Patch (2.80 KB, text/plain)
2013-01-17 13:19 PST, Rik Cabanier 		no flags Details
Patch (2.67 KB, patch)
2013-01-17 13:39 PST, Rik Cabanier 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Rik Cabanier 2013-01-14 22:10:44 PST 
Add support for winding operator in chromium codebase
Comment 1 Rik Cabanier 2013-01-17 13:19:57 PST 
Created attachment 183256 [details]
Patch
Comment 2 Justin Novosad 2013-01-17 13:33:10 PST 
Comment on attachment 183256 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=183256&action=review

> Source/WebCore/platform/graphics/skia/GraphicsContextSkia.cpp:361
> +        storage.setFillType(ftype);

I think you mean to set the fill type back to savedFillRule here?  We shouldn't even need to do that since storage is a temp copy, or am I missing something?
Comment 3 Rik Cabanier 2013-01-17 13:39:51 PST 
Created attachment 183262 [details]
Patch
Comment 4 Rik Cabanier 2013-01-17 13:40:58 PST 
(In reply to comment #2)
> (From update of attachment 183256 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=183256&action=review
> 
> > Source/WebCore/platform/graphics/skia/GraphicsContextSkia.cpp:361
> > +        storage.setFillType(ftype);
> 
> I think you mean to set the fill type back to savedFillRule here?  We shouldn't even need to do that since storage is a temp copy, or am I missing something?
you're correct. I thought it was somehow set in the platform path.

Fixed in latest patch (+ removed obsolete comment)
Comment 5 Philip Rogers 2013-01-17 19:25:45 PST 
Comment on attachment 183262 [details]
Patch

This patch is straightforward and looks good to me.
Comment 6 Dirk Schulze 2013-01-17 19:30:34 PST 
Comment on attachment 183262 [details]
Patch

Reviewed together with pdr. Looked good for us. r=me
Comment 7 WebKit Review Bot 2013-01-17 20:02:59 PST 
Comment on attachment 183262 [details]
Patch

Clearing flags on attachment: 183262

Committed r140098: <http://trac.webkit.org/changeset/140098>
Comment 8 WebKit Review Bot 2013-01-17 20:03:04 PST 
All reviewed patches have been landed.  Closing bug.