Bug 47868

Summary: Pixel cracks when using background image sprite on transformed element at certain scales.
Product: WebKit Reporter: Yongjun Zhang <yongjun_zhang>
Component: ImagesAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, ddkilzer, joepeck, mitz, simon.fraser
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: OS X 10.5   
Attachments:
Description Flags
Add a screenshot to show the unwanted pixels around div element.
none
Change to sub-image path when CGContext has scale and sourceRect is not the same as destRect.
mitz: review-
revised patch, use AffineTransform::isIdentityOrTranslationOrFlipped() to test if sub-image is needed.
none
Revised patch: Use GraphicsContext::getCTM() instead of CGContextGetUserSpaceToDeviceSpaceTransform() since getCTM() could be more efficient. none

Description Yongjun Zhang 2010-10-18 18:54:38 PDT
The following html code snippet will cause unwanted blue pixels around the div element in TOT at certain zoom levels:

<div style="-webkit-transform: scale(2.659); background-image: url(http://www.google.com/reader/ui/1225119760-lhn-sprite.png); height: 16px; width: 16px;"></div>
Comment 1 Yongjun Zhang 2010-10-18 18:57:14 PDT
Created attachment 71112 [details]
Add a screenshot to show the unwanted pixels around div element.

Added a screenshot to show the pixel bleeding at the edge of div element.   Please note the two short blue lines at the right and bottom edge.
Comment 2 Yongjun Zhang 2010-10-18 18:58:21 PDT
<rdar://problem/8020939>
Comment 3 Yongjun Zhang 2010-10-18 19:20:20 PDT
Created attachment 71117 [details]
Change to sub-image path when CGContext has scale and sourceRect is not the same as destRect.

This patch has pixel test.   When I run it, the expected result is generated automatically under mac directory.  However, I am not sure what I should do for other ports.   Do I need to create expected png result for other ports?  Or at least the expected.txt file?
Comment 4 Yongjun Zhang 2010-10-18 21:50:26 PDT
Dan suggested I should run pixel tests for fast/borders/border-image-rotate-transform.html.  However, in my machine, fast/borders/border-image-rotate-transform.html fails with or without this patch (the image diff is 0.2% with this patch, and 0.19% without this patch).
Comment 5 mitz 2010-10-19 10:48:13 PDT
Comment on attachment 71117 [details]
Change to sub-image path when CGContext has scale and sourceRect is not the same as destRect.

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

> WebCore/platform/graphics/cg/ImageCG.cpp:189
> +        bool hasScale = transform.a != 1.0f || transform.d != 1.0f;

WebKit style is to omit the “.0f”. It serves no useful purpose whatsoever.

This test criteria seem to be wrong. On the one hand, the (very common) flip transform is included, and on the other hand, a shear or skew transform, which is likely to require interpolation and therefore introduce bleeding, is excluded. AffineTransform::isIdentityOrTranslationOrFlipped() may be the right test here.
Comment 6 Yongjun Zhang 2010-10-19 12:13:46 PDT
Created attachment 71191 [details]
revised patch, use AffineTransform::isIdentityOrTranslationOrFlipped() to test if sub-image is needed.
Comment 7 Simon Fraser (smfr) 2010-10-19 13:08:05 PDT
Comment on attachment 71191 [details]
revised patch, use AffineTransform::isIdentityOrTranslationOrFlipped() to test if sub-image is needed.

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

> WebCore/platform/graphics/cg/ImageCG.cpp:189
> +            CGAffineTransform transform = CGContextGetUserSpaceToDeviceSpaceTransform(context);

Is CGContextGetUserSpaceToDeviceSpaceTransform() the right thing to test? What if you're rendering into a compositing layer (CALayer)? Why not just check the GraphicsContext getCTM?
Comment 8 Yongjun Zhang 2010-10-19 16:11:44 PDT
Created attachment 71217 [details]
Revised patch: Use GraphicsContext::getCTM() instead of CGContextGetUserSpaceToDeviceSpaceTransform() since getCTM() could be more efficient.
Comment 9 WebKit Commit Bot 2010-10-19 18:44:25 PDT
Comment on attachment 71217 [details]
Revised patch: Use GraphicsContext::getCTM() instead of CGContextGetUserSpaceToDeviceSpaceTransform() since getCTM() could be more efficient.

Clearing flags on attachment: 71217

Committed r70118: <http://trac.webkit.org/changeset/70118>
Comment 10 WebKit Commit Bot 2010-10-19 18:44:32 PDT
All reviewed patches have been landed.  Closing bug.