skia: fix stroking of zero-height rectangles

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Mike Reed]

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).

[Mike Reed]

Created attachment 94426 [details]
patch

[WebKit Review Bot]

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]

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 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]

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]

Created attachment 94456 [details]
use path that doubles-back on itself so we can respect the lineJoin

[Kenneth Russell]

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 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]

Created attachment 94604 [details]
Patch

[Mike Reed]

rebase ChangeLog (grrrr) -- no code changes

[Eric Seidel (no email)]

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]

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]

Created attachment 94617 [details]
Patch

[Mike Reed]

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 on attachment 94617 [details]
Patch

OK.

[WebKit Commit Bot]

The commit-queue encountered the following flaky tests while processing attachment 94617 [details]:

animations/suspend-resume-animation.html bug 48161 (authors: [e-mail] and [e-mail])
The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

Comment on attachment 94617 [details]
Patch

Clearing flags on attachment: 94617

Committed r87174: <http://trac.webkit.org/changeset/87174>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.