Bug 25619

Created attachment 30103 [details] a simple test case.

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

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

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.

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

Created attachment 77122 [details] Patch

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.

Created attachment 77149 [details] Patch

Created attachment 77160 [details] Patch

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

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.

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

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

Created attachment 77290 [details] Patch for landing

The commit-queue encountered the following flaky tests while processing attachment 77290 [details]: inspector/styles-disable-then-enable.html bug 51384 (author: pfeldman@chromium.org) The commit-queue is continuing to process your patch.

Comment on attachment 77290 [details] Patch for landing Clearing flags on attachment: 77290 Committed r74532: <http://trac.webkit.org/changeset/74532>

All reviewed patches have been landed. Closing bug.

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

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