Bug 42190 - Failing 2d.path.stroke.prune.curve philip canvas test
Summary: Failing 2d.path.stroke.prune.curve philip canvas test
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: All OS X 10.6
: P2 Normal
Assignee: Matthew Delaney
URL:
Keywords:
Depends on: 42188
Blocks:
  Show dependency treegraph
 
Reported: 2010-07-13 13:30 PDT by Matthew Delaney
Modified: 2010-07-20 00:07 PDT (History)
3 users (show)

See Also:


Attachments
Patch (4.77 KB, patch)
2010-07-16 22:00 PDT, Matthew Delaney
no flags Details | Formatted Diff | Diff
Patch (5.73 KB, patch)
2010-07-17 19:13 PDT, Matthew Delaney
kenneth: review+
commit-queue: commit-queue-
Details | Formatted Diff | Diff
Previous Patch (without extra whitespace deletion) (4.89 KB, patch)
2010-07-19 13:23 PDT, Matthew Delaney
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Matthew Delaney 2010-07-13 13:30:46 PDT
Safari 5 fails 2d.path.stroke.prune.curve
Comment 1 Matthew Delaney 2010-07-16 22:00:11 PDT
Created attachment 61874 [details]
Patch
Comment 2 Andreas Kling 2010-07-17 18:41:01 PDT
Comment on attachment 61874 [details]
Patch

>WebCore/html/canvas/CanvasRenderingContext2D.cpp:562
> +      
Stray whitespace.

>WebCore/html/canvas/CanvasRenderingContext2D.cpp:599
> +      
Ditto.

>WebCore/html/canvas/CanvasRenderingContext2D.cpp:613
> +      
Ditto.

>WebCore/html/canvas/CanvasRenderingContext2D.cpp:563
> +      FloatRect boundRect = m_path.boundingRect();
It's a bit sad that we have to do this, getting the boundingRect() can be rather expensive.
Qt will cache its results internally and reuse them when stroking/filling the path later on, but other back-ends may not be so lucky.
Unfortunately I don't have a better suggestion. ;(
Comment 3 Matthew Delaney 2010-07-17 19:13:55 PDT
Created attachment 61896 [details]
Patch
Comment 4 Kenneth Rohde Christiansen 2010-07-19 06:25:32 PDT
Comment on attachment 61896 [details]
Patch

Normally we keep style changes in separate patches. But the rest seems sensible to me, and Andreas Kling has had a look as well, so r=me.
Comment 5 WebKit Commit Bot 2010-07-19 08:25:40 PDT
Comment on attachment 61896 [details]
Patch

Rejecting patch 61896 from commit-queue.

Failed to run "[u'/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/svn-apply', u'--reviewer', u'Kenneth Rohde Christiansen', u'--force']" exit_code: 1
Last 500 characters of output:
Core/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file WebCore/html/canvas/CanvasRenderingContext2D.cpp
Hunk #5 FAILED at 621.
Hunk #6 succeeded at 646 (offset 2 lines).
1 out of 6 hunks FAILED -- saving rejects to file WebCore/html/canvas/CanvasRenderingContext2D.cpp.rej
patching file WebCore/platform/graphics/cg/PathCG.cpp
patching file LayoutTests/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file LayoutTests/platform/mac/Skipped
Hunk #1 succeeded at 211 (offset -2 lines).

Full output: http://queues.webkit.org/results/3588225
Comment 6 Matthew Delaney 2010-07-19 13:23:46 PDT
Created attachment 61986 [details]
Previous Patch (without extra whitespace deletion)

Ugh, it looks like deleting some whitespace in one of the otherwise untouched methods causes svn-apply to fail. Here's a new patch without those whitespace deletes that patches on my machine with no problem.
Comment 7 WebKit Commit Bot 2010-07-20 00:07:27 PDT
Comment on attachment 61986 [details]
Previous Patch (without extra whitespace deletion)

Clearing flags on attachment: 61986

Committed r63727: <http://trac.webkit.org/changeset/63727>
Comment 8 WebKit Commit Bot 2010-07-20 00:07:33 PDT
All reviewed patches have been landed.  Closing bug.