Bug 49225 - Philip Canvas test 2d.path.rect.winding fails
Summary: Philip Canvas test 2d.path.rect.winding fails
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Canvas (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL: http://philip.html5.org/tests/canvas/...
Keywords:
Depends on:
Blocks: 46506
  Show dependency treegraph
 
Reported: 2010-11-08 16:58 PST by Helder Correia
Modified: 2011-01-06 13:05 PST (History)
5 users (show)

See Also:


Attachments
Patch (5.08 KB, patch)
2010-11-08 17:06 PST, Helder Correia
no flags Details | Formatted Diff | Diff
Patch (4.21 KB, patch)
2010-11-09 15:04 PST, Helder Correia
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Helder Correia 2010-11-08 16:58:45 PST
The direction of the subpath is not taken into account.  This breaks the nonzero winding rule when using either a negative width or a negative height (but not both), in which case the rect should be counterclockwise.
Comment 1 Helder Correia 2010-11-08 17:06:36 PST
Created attachment 73312 [details]
Patch
Comment 2 Andreas Kling 2010-11-09 00:18:41 PST
Comment on attachment 73312 [details]
Patch

This patch can be simplified to remove the "if (width < 0) {" and "if (height < 0) {" blocks and just leaving the Path::addRect() call as-is.

The Path objects generated by the two code paths in your patch are equivalent AFAICT.
Comment 3 Helder Correia 2010-11-09 00:48:04 PST
@Andreas: my initial patch consisted in removing the two 'if' blocks only, but then I saw that you introduced them in http://trac.webkit.org/changeset/63270. If you're sure that removing them won't break anything, I can just do that.
Comment 4 Andreas Kling 2010-11-09 00:51:15 PST
(In reply to comment #3)
> @Andreas: my initial patch consisted in removing the two 'if' blocks only, but then I saw that you introduced them in http://trac.webkit.org/changeset/63270. If you're sure that removing them won't break anything, I can just do that.

That should be fine. r63270 simply factored those checks out of validateRectForCanvas() and into rect(). :-)
Comment 5 Helder Correia 2010-11-09 15:04:20 PST
Created attachment 73428 [details]
Patch
Comment 6 WebKit Commit Bot 2010-11-09 21:07:07 PST
Comment on attachment 73428 [details]
Patch

Clearing flags on attachment: 73428

Committed r71716: <http://trac.webkit.org/changeset/71716>
Comment 7 WebKit Commit Bot 2010-11-09 21:07:12 PST
All reviewed patches have been landed.  Closing bug.
Comment 8 Ademar Reis 2010-12-28 13:38:05 PST
As discussed (in pvt) with Suresh, this should not block qtwebkit-2.1. Marking as a nice-to-have fix for qtwebkit-2.2 (later it can be triaged and promoted to blocker if necessary)
Comment 9 Ademar Reis 2011-01-03 06:21:02 PST
Revision r71716 cherry-picked into qtwebkit-2.2 with commit dd6e586 <http://gitorious.org/webkit/qtwebkit/commit/dd6e586>
Comment 10 Prasad 2011-01-06 13:05:53 PST
Related QtWRT Bugzilla error ID: http://bugs.nokia-boston.com/bugzilla/show_bug.cgi?id=7147