Bug 61284

Summary: skia: fix stroking of zero-height rectangles
Product: WebKit Reporter: Mike Reed <reed>
Component: Layout and RenderingAssignee: Mike Reed <reed>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, dglazkov, jamesr, junov, kbr, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
patch
kbr: review-, webkit.review.bot: commit-queue-
Archive of layout-test-results from ec2-cr-linux-01
none
use path that doubles-back on itself so we can respect the lineJoin
none
Patch
none
Patch none

Mike Reed
Reported 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).
Attachments
patch (2.83 KB, patch)
2011-05-23 07:33 PDT, Mike Reed 		kbr: review-
webkit.review.bot: commit-queue-
DetailsFormatted DiffDiff
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
DetailsFormatted DiffDiff
Patch (2.00 KB, patch)
2011-05-24 06:53 PDT, Mike Reed 		no flags
DetailsFormatted DiffDiff
Patch (2.96 KB, patch)
2011-05-24 08:47 PDT, Mike Reed 		no flags
DetailsFormatted DiffDiff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.
Mike Reed
Comment 1 2011-05-23 07:33:59 PDT
Created attachment 94426 [details] patch
WebKit Review Bot
Comment 2 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
WebKit Review Bot
Comment 3 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
Kenneth Russell
Comment 4 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.
Mike Reed
Comment 5 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);
Mike Reed
Comment 6 2011-05-23 12:22:21 PDT
Created attachment 94456 [details] use path that doubles-back on itself so we can respect the lineJoin
Kenneth Russell
Comment 7 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.
WebKit Commit Bot
Comment 8 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
Mike Reed
Comment 9 2011-05-24 06:53:29 PDT
Created attachment 94604 [details] Patch
Mike Reed
Comment 10 2011-05-24 06:54:04 PDT
rebase ChangeLog (grrrr) -- no code changes
Eric Seidel (no email)
Comment 11 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?
Mike Reed
Comment 12 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.
Mike Reed
Comment 13 2011-05-24 08:47:33 PDT
Created attachment 94617 [details] Patch
Mike Reed
Comment 14 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.
Kenneth Russell
Comment 15 2011-05-24 09:57:23 PDT
Comment on attachment 94617 [details] Patch OK.
WebKit Commit Bot
Comment 16 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.
WebKit Commit Bot
Comment 17 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>
WebKit Commit Bot
Comment 18 2011-05-24 11:25:18 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.