Bug 54336 - PlatformContextSkia::applyAntiAliasedClipPaths does not work for paths which have evenOdd property
Summary: PlatformContextSkia::applyAntiAliasedClipPaths does not work for paths which ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P2 Normal
Assignee: Nobody
URL: http://www.w3.org/Graphics/SVG/Test/2...
Keywords:
Depends on:
Blocks:
 
Reported: 2011-02-12 01:16 PST by Robin Cao
Modified: 2011-02-16 16:50 PST (History)
7 users (show)

See Also:


Attachments
patch (1.58 KB, patch)
2011-02-14 17:26 PST, Robin Cao
jamesr: review-
Details | Formatted Diff | Diff
updated patch (1.47 KB, patch)
2011-02-15 18:50 PST, Robin Cao
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Robin Cao 2011-02-12 01:16:01 PST
PlatformContextSkia::applyAntiAliasedClipPaths does not work for paths which have evenOdd property.
This test (http://www.w3.org/Graphics/SVG/Test/20021112/htmlframe/full-masking-path-05-f.html) fails on Chrome.

I have a fix for this. Will upload soon.
Comment 1 Robin Cao 2011-02-14 17:26:05 PST
Created attachment 82393 [details]
patch

We need to take fill type of paths into account in applyAntiAliasedClipPaths.
Comment 2 James Robinson 2011-02-14 17:40:27 PST
I think this fine - could one of you (Mike/Stephen/Brian) sanity check it?
Comment 3 James Robinson 2011-02-14 17:43:09 PST
This is marked as a failure on win/linux in chromium currently: http://code.google.com/p/chromium/issues/detail?id=25208
Comment 4 Mike Reed 2011-02-15 07:50:16 PST
SkPath::toggleInverseFillType() is a helper that does exactly this.
Comment 5 Stephen White 2011-02-15 08:02:28 PST
+agl, since I think he wrote this code.

This looks ok to me (although we should probably use the helper fn, as Mike suggests).
Comment 6 Adam Langley 2011-02-15 13:19:18 PST
LGTM if updated with Mike's suggestion.

(Of course, I still hope that Skia gets immediate anti-aliased clipping at some point :)

Shouldn't this either include updated baselines or new layout tests? Or are they to follow in a different patch?
Comment 7 James Robinson 2011-02-15 13:52:46 PST
svg/W3C-SVG-1.1/masking-path-05-f.svg covers this test.

Robin, could you remove the line referencing this test from LayoutTests/platform/chromium/test_expectations.txt when you update this patch?  Thanks!
Comment 8 James Robinson 2011-02-15 13:53:13 PST
Comment on attachment 82393 [details]
patch

Marking this one R- since it should use the helper function.
Comment 9 Robin Cao 2011-02-15 18:13:20 PST
(In reply to comment #7)
> Robin, could you remove the line referencing this test from LayoutTests/platform/chromium/test_expectations.txt when you update this patch?  Thanks!

I can do this. One question is that i only work on linux at the moment and do i have to update baselines for chromium-win?
Comment 10 James Robinson 2011-02-15 18:15:12 PST
Oh, that is a bummer.  Just leave the test_expectations.txt line alone and we (meaning probably me) will take care of the baselines.
Comment 11 Robin Cao 2011-02-15 18:50:18 PST
Created attachment 82570 [details]
updated patch

Use the helper function to toggle fill type of paths.
Comment 12 Adam Langley 2011-02-16 11:08:52 PST
LGTM. (Note I am not a WK reviewer. You need an r+ from a real reviewer also.)
Comment 13 James Robinson 2011-02-16 11:33:26 PST
Comment on attachment 82570 [details]
updated patch

R=me. Thanks again!
Comment 14 WebKit Commit Bot 2011-02-16 16:49:12 PST
The commit-queue encountered the following flaky tests while processing attachment 82570 [details]:

http/tests/misc/uncacheable-script-repeated.html bug 51009 (author: koivisto@iki.fi)
The commit-queue is continuing to process your patch.
Comment 15 WebKit Commit Bot 2011-02-16 16:50:46 PST
Comment on attachment 82570 [details]
updated patch

Clearing flags on attachment: 82570

Committed r78751: <http://trac.webkit.org/changeset/78751>
Comment 16 WebKit Commit Bot 2011-02-16 16:50:51 PST
All reviewed patches have been landed.  Closing bug.