Bug 25619 – the shadow direction is negated in canvas context shadowOffsetY.

• Status: RESOLVED
• Resolution: FIXED
• Component: Canvas

[Yongjun Zhang]

The shadow is painted on the wrong direction when shadowOffsetY is set.  For example, for context.shadowOffsetY = 5, the shadow is painted 5 pix _above_ , not 5 px _below_ the shadowed object.  However, if context.shadowBlur is set, the shadow is painted correctly.

[Yongjun Zhang]

Created attachment 30103 [details]
a simple test case.

[Yongjun Zhang]

looks like simple shadowed text are painted differently than blur-shadowed text.  Blur-shadowed text is painted directly by the CG; while simple shadow is painted in FontMac.mm by offsetting the text position (line 105).

[Yongjun Zhang]

Created attachment 30104 [details]
use the right y offset value for simple shadowed text.

[Simon Fraser (smfr)]

Comment on attachment 30104 [details]
use the right y offset value for simple shadowed text.

This breaks CSS text-shadow when the blur radius is 0.

[Yongjun Zhang]

yes, indeed it breaks CSS text-shadow without blur radius. I'll keep digging :)

[Mihai Parparita]

Created attachment 77122 [details]
Patch

[Mihai Parparita]

The attached patch seems to work for both canvas and CSS text shadows. Is checking for shadowsIgnoreTransforms the right thing? If so, I'll add a layout test and clean this up for review.

[Mihai Parparita]

Created attachment 77149 [details]
Patch

[Mihai Parparita]

Created attachment 77160 [details]
Patch

[Mihai Parparita]

New version of the patch fixes things for transforms too (it actually sends that on the CG shadow path, since making shadow offsets not take into account transforms was too complex for the simple path).

Note that that was a problem before this patch too, on my test case at http://persistent.info/webkit/test-cases/webkit-25619.html the shadow in the top right is not only above (it should be below), it's also displaced by twice as much as it should be (there's a 2x scale transform on the canvas).

[Simon Fraser (smfr)]

Comment on attachment 77160 [details]
Patch

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

> WebCore/platform/graphics/mac/FontMac.mm:183
> +        // If shadows are ignoring transforms, then we haven't applied the Y coordinate flip yet, so down is negative.

You never enter this code if shadows ignore transforms, because of the !context->shadowsIgnoreTransforms() check above. So this comment is not accurate.

[Mihai Parparita]

Comment on attachment 77160 [details]
Patch

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

>> WebCore/platform/graphics/mac/FontMac.mm:183
>> +        // If shadows are ignoring transforms, then we haven't applied the Y coordinate flip yet, so down is negative.
> 
> You never enter this code if shadows ignore transforms, because of the !context->shadowsIgnoreTransforms() check above. So this comment is not accurate.

The check is "!context->shadowsIgnoreTransforms() || context->getCTM().isIdentityOrTranslationOrFlipped()", so we'll still end up here if the canvas transform is just a translation (or non-existent).

[WebKit Commit Bot]

Comment on attachment 77160 [details]
Patch

Rejecting attachment 77160 [details] from commit-queue.

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=eseidel-cq-sf', 'apply-attachment', '--no-update', '--non-interactive', 77160]" exit_code: 2
Last 500 characters of output:
le WebCore/platform/graphics/GraphicsContext.h
Hunk #1 succeeded at 242 (offset 2 lines).
patching file WebCore/platform/graphics/mac/FontMac.mm
Hunk #1 FAILED at 172.
1 out of 1 hunk FAILED -- saving rejects to file WebCore/platform/graphics/mac/FontMac.mm.rej
patching file WebCore/platform/graphics/win/FontCGWin.cpp
Hunk #1 succeeded at 357 with fuzz 2 (offset 3 lines).

Failed to run "[u'/Projects/CommitQueue/Tools/Scripts/svn-apply', u'--reviewer', u'Simon Fraser', u'--force']" exit_code: 1

Full output: http://queues.webkit.org/results/7317088

[Mihai Parparita]

Created attachment 77290 [details]
Patch for landing

[WebKit Commit Bot]

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

inspector/styles-disable-then-enable.html bug 51384 (author: [e-mail])
The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

Comment on attachment 77290 [details]
Patch for landing

Clearing flags on attachment: 77290

Committed r74532: <http://trac.webkit.org/changeset/74532>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[WebKit Review Bot]

http://trac.webkit.org/changeset/74532 might have broken SnowLeopard Intel Release (Tests)
The following tests are not passing:
http/tests/local/stylesheet-and-script-load-order-media-print.html

[Mihai Parparita]

(In reply to comment #18)
> http://trac.webkit.org/changeset/74532 might have broken SnowLeopard Intel Release (Tests)
> The following tests are not passing:
> http/tests/local/stylesheet-and-script-load-order-media-print.html

This is just a flaky test, the bot is green again. Bug 51470 is already filed for it.