Bug 61284 - skia: fix stroking of zero-height rectangles
Summary: skia: fix stroking of zero-height rectangles
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mike Reed
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-05-23 07:19 PDT by Mike Reed
Modified: 2011-05-24 11:25 PDT (History)
6 users (show)

See Also:


Attachments
patch (2.83 KB, patch)
2011-05-23 07:33 PDT, Mike Reed
kbr: review-
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ec2-cr-linux-01 (1.37 MB, application/zip)
2011-05-23 08:19 PDT, WebKit Review Bot
no flags Details
use path that doubles-back on itself so we can respect the lineJoin (2.97 KB, patch)
2011-05-23 12:22 PDT, Mike Reed
no flags Details | Formatted Diff | Diff
Patch (2.00 KB, patch)
2011-05-24 06:53 PDT, Mike Reed
no flags Details | Formatted Diff | Diff
Patch (2.96 KB, patch)
2011-05-24 08:47 PDT, Mike Reed
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mike Reed 2011-05-23 07:19:51 PDT
CSS wants stroked rectangles with degenerate dimensions (width or height == 0) to be drawn as a line. This change ensures that behavior in GraphicsContextSkia.cpp, rather than relying on skia's default handling (which differs).
Comment 1 Mike Reed 2011-05-23 07:33:59 PDT
Created attachment 94426 [details]
patch
Comment 2 WebKit Review Bot 2011-05-23 08:19:33 PDT
Comment on attachment 94426 [details]
patch

Attachment 94426 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/8726407

New failing tests:
canvas/philip/tests/2d.strokeRect.zero.5.html
Comment 3 WebKit Review Bot 2011-05-23 08:19:39 PDT
Created attachment 94430 [details]
Archive of layout-test-results from ec2-cr-linux-01

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-01  Port: Chromium  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Comment 4 Kenneth Russell 2011-05-23 12:11:13 PDT
Comment on attachment 94426 [details]
patch

Does the EWS failure indicate that more work is needed on this patch? Marking r- until this is clarified.
Comment 5 Mike Reed 2011-05-23 12:12:23 PDT
Ah, we need to respect the lineJoin: will add this


        // we are expected to respect the lineJoin, so we can't just call
        // drawLine -- we have to create a path that doubles back on itself.
        SkPath path;
        path.moveTo(r.fLeft, r.fTop);
        path.lineTo(r.fRight, r.fBottom);
        path.close();
        canvas->drawPath(path, paint);
Comment 6 Mike Reed 2011-05-23 12:22:21 PDT
Created attachment 94456 [details]
use path that doubles-back on itself so we can respect the lineJoin
Comment 7 Kenneth Russell 2011-05-23 14:27:05 PDT
Comment on attachment 94456 [details]
use path that doubles-back on itself so we can respect the lineJoin

Looks good to me.
Comment 8 WebKit Commit Bot 2011-05-23 15:16:35 PDT
Comment on attachment 94456 [details]
use path that doubles-back on itself so we can respect the lineJoin

Rejecting attachment 94456 [details] from commit-queue.

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=cr-jail-3', 'land-a..." exit_code: 2

Last 500 characters of output:
cpp
r87099 = fd444dac782ae8f8023623310ad7e53dff00e895 (refs/remotes/trunk)
	M	Source/WebCore/ChangeLog
	M	Source/WebCore/platform/graphics/IntPoint.h
	M	Source/WebCore/rendering/RenderListBox.h
	M	Source/WebCore/rendering/RenderBlock.cpp
	M	Source/WebCore/rendering/RenderBlock.h
	M	Source/WebCore/rendering/RenderListBox.cpp
r87101 = e58ec13b1e380cbe5328f45ec70deb4a326ef147 (refs/remotes/trunk)
First, rewinding head to replay your work on top of it...
Fast-forwarded master to refs/remotes/trunk.

Full output: http://queues.webkit.org/results/8722988
Comment 9 Mike Reed 2011-05-24 06:53:29 PDT
Created attachment 94604 [details]
Patch
Comment 10 Mike Reed 2011-05-24 06:54:04 PDT
rebase ChangeLog (grrrr) -- no code changes
Comment 11 Eric Seidel (no email) 2011-05-24 06:56:46 PDT
Comment on attachment 94604 [details]
Patch

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

> Source/WebCore/platform/graphics/skia/GraphicsContextSkia.cpp:1235
> +    // if width==0 && height==0, do nothing
> +    // if width==0 || height==0, then just draw line for the other dimension

Hmm... Seems we would want to share this logic at a higher level.  Every port really has to do this manually?
Comment 12 Mike Reed 2011-05-24 07:01:37 PDT
They only have to do it if their underlying graphics engine behaves differently when told to stroke a rect. I don't know the rules for Qt or CG.
Comment 13 Mike Reed 2011-05-24 08:47:33 PDT
Created attachment 94617 [details]
Patch
Comment 14 Mike Reed 2011-05-24 08:48:53 PDT
oops, lost the updated test_expectations.txt on the previous refresh. Again, no code changes from the earlier patch that was approved.
Comment 15 Kenneth Russell 2011-05-24 09:57:23 PDT
Comment on attachment 94617 [details]
Patch

OK.
Comment 16 WebKit Commit Bot 2011-05-24 11:23:08 PDT
The commit-queue encountered the following flaky tests while processing attachment 94617 [details]:

animations/suspend-resume-animation.html bug 48161 (authors: cmarrin@apple.com and simon.fraser@apple.com)
The commit-queue is continuing to process your patch.
Comment 17 WebKit Commit Bot 2011-05-24 11:25:09 PDT
Comment on attachment 94617 [details]
Patch

Clearing flags on attachment: 94617

Committed r87174: <http://trac.webkit.org/changeset/87174>
Comment 18 WebKit Commit Bot 2011-05-24 11:25:18 PDT
All reviewed patches have been landed.  Closing bug.