Bug 169547 – REGRESSION (r213764): Background image from sprite sheet incorrectly scaled

• Component: Images
• Status: RESOLVED FIXED

[Jamie White]

Created attachment 304246 [details]
Minimal HTML demonstrating the bug

The reproduction is extracted from Slack, which uses a large sprite sheet for its emojis. On the current nightly, the sprites in question appear to be scaled much larger than is intended. To demonstrate the effect: load the sample page and resize the browser window.

[Jamie White]

Created attachment 304247 [details]
Demo video

[Radar WebKit Bug Importer]

<rdar://problem/31011842>

[Said Abou-Hallawa]

Created attachment 304302 [details]
Patch

[Said Abou-Hallawa]

Comment on attachment 304302 [details]
Patch

This patch is for EWS.

[Build Bot]

Comment on attachment 304302 [details]
Patch

Attachment 304302 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/3310670

New failing tests:
fast/images/sprite-sheet-image-draw.html

[Build Bot]

Created attachment 304315 [details]
Archive of layout-test-results from ews116 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews116  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 304302 [details]
Patch

Attachment 304302 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/3310856

New failing tests:
fast/images/sprite-sheet-image-draw.html

[Build Bot]

Created attachment 304316 [details]
Archive of layout-test-results from ews103 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Said Abou-Hallawa]

Created attachment 304324 [details]
Patch

[Build Bot]

Comment on attachment 304324 [details]
Patch

Attachment 304324 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/3311648

New failing tests:
fast/images/sprite-sheet-image-draw.html

[Build Bot]

Created attachment 304326 [details]
Archive of layout-test-results from ews113 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews113  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 304324 [details]
Patch

Attachment 304324 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/3311784

New failing tests:
fast/images/sprite-sheet-image-draw.html

[Build Bot]

Created attachment 304328 [details]
Archive of layout-test-results from ews102 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Said Abou-Hallawa]

Created attachment 304388 [details]
Patch

[Said Abou-Hallawa]

Created attachment 304399 [details]
Patch

[Said Abou-Hallawa]

Created attachment 304400 [details]
Patch

[Build Bot]

Comment on attachment 304400 [details]
Patch

Attachment 304400 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/3322956

New failing tests:
fast/images/sprite-sheet-image-draw.html

[Build Bot]

Created attachment 304413 [details]
Archive of layout-test-results from ews101 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 304400 [details]
Patch

Attachment 304400 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/3324962

New failing tests:
fast/images/sprite-sheet-image-draw.html

[Build Bot]

Created attachment 304446 [details]
Archive of layout-test-results from ews116 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews116  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Said Abou-Hallawa]

Created attachment 304477 [details]
Patch

[Said Abou-Hallawa]

Created attachment 304479 [details]
Patch

[Build Bot]

Comment on attachment 304479 [details]
Patch

Attachment 304479 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/3327922

New failing tests:
fast/images/sprite-sheet-image-draw.html

[Build Bot]

Created attachment 304484 [details]
Archive of layout-test-results from ews101 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 304479 [details]
Patch

Attachment 304479 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/3327909

New failing tests:
fast/images/sprite-sheet-image-draw.html

[Build Bot]

Created attachment 304485 [details]
Archive of layout-test-results from ews114 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Said Abou-Hallawa]

Created attachment 304503 [details]
Patch

[Said Abou-Hallawa]

I gave up adding a test to this patch. The test runs all the times locally even with --repeat-each=100. But it's never passed on the mac bots. The bots refused to show the image when it is included as an <img> element or as a css background-image.

[Said Abou-Hallawa]

Comment on attachment 304503 [details]
Patch

The tests may not be showing the images because of https://bugs.webkit.org/show_bug.cgi?id=169771.

[Said Abou-Hallawa]

Created attachment 305651 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 305651 [details]
Patch

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

> LayoutTests/fast/images/sprite-sheet-image-draw.html:19
> +            }, 50);

Are you racing with image decode? This could be flakey on the bots.

[Build Bot]

Comment on attachment 305651 [details]
Patch

Attachment 305651 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/3429832

New failing tests:
fast/images/sprite-sheet-image-draw.html
fast/images/async-image-background-image.html

[Build Bot]

Created attachment 305667 [details]
Archive of layout-test-results from ews100 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Said Abou-Hallawa]

Created attachment 305668 [details]
Patch

[Build Bot]

Comment on attachment 305668 [details]
Patch

Attachment 305668 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/3430402

New failing tests:
fast/images/sprite-sheet-image-draw.html
fast/images/async-image-background-image.html

[Build Bot]

Created attachment 305687 [details]
Archive of layout-test-results from ews115 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 305668 [details]
Patch

Attachment 305668 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/3430473

New failing tests:
fast/images/sprite-sheet-image-draw.html
fast/images/async-image-background-image.html

[Build Bot]

Created attachment 305689 [details]
Archive of layout-test-results from ews102 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 305668 [details]
Patch

Attachment 305668 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/3430420

New failing tests:
fast/images/sprite-sheet-image-draw.html

[Build Bot]

Created attachment 305691 [details]
Archive of layout-test-results from ews122 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews122  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 305668 [details]
Patch

Attachment 305668 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/3430594

New failing tests:
fast/images/sprite-sheet-image-draw.html

[Build Bot]

Created attachment 305693 [details]
Archive of layout-test-results from ews104 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6

[Said Abou-Hallawa]

Created attachment 305754 [details]
Patch

[Build Bot]

Comment on attachment 305754 [details]
Patch

Attachment 305754 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/3434740

New failing tests:
fast/images/sprite-sheet-image-draw.html

[Build Bot]

Created attachment 305761 [details]
Archive of layout-test-results from ews104 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6

[Said Abou-Hallawa]

Created attachment 305764 [details]
Patch

[Build Bot]

Comment on attachment 305764 [details]
Patch

Attachment 305764 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/3435105

New failing tests:
fast/images/sprite-sheet-image-draw.html
fast/images/async-image-background-image.html

[Build Bot]

Created attachment 305772 [details]
Archive of layout-test-results from ews117 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews117  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 305764 [details]
Patch

Attachment 305764 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/3435135

New failing tests:
fast/images/sprite-sheet-image-draw.html
fast/images/async-image-background-image.html

[Build Bot]

Created attachment 305773 [details]
Archive of layout-test-results from ews100 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Said Abou-Hallawa]

Created attachment 305775 [details]
Patch

[Build Bot]

Comment on attachment 305775 [details]
Patch

Attachment 305775 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/3435536

New failing tests:
fast/images/sprite-sheet-image-draw.html
fast/images/async-image-background-image.html

[Build Bot]

Created attachment 305784 [details]
Archive of layout-test-results from ews102 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 305775 [details]
Patch

Attachment 305775 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/3435558

New failing tests:
fast/images/sprite-sheet-image-draw.html
fast/images/async-image-background-image.html

[Build Bot]

Created attachment 305789 [details]
Archive of layout-test-results from ews113 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews113  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Said Abou-Hallawa]

Created attachment 305796 [details]
Patch

[Build Bot]

Comment on attachment 305796 [details]
Patch

Attachment 305796 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/3436145

New failing tests:
fast/images/sprite-sheet-image-draw.html

[Build Bot]

Created attachment 305804 [details]
Archive of layout-test-results from ews126 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews126  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.6

[Said Abou-Hallawa]

Created attachment 305818 [details]
Patch

[Build Bot]

Comment on attachment 305818 [details]
Patch

Attachment 305818 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/3437057

New failing tests:
fast/images/sprite-sheet-image-draw.html
fast/images/async-image-background-image.html

[Build Bot]

Created attachment 305821 [details]
Archive of layout-test-results from ews102 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 305818 [details]
Patch

Attachment 305818 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/3437055

New failing tests:
fast/images/sprite-sheet-image-draw.html
fast/images/async-image-background-image.html

[Build Bot]

Created attachment 305822 [details]
Archive of layout-test-results from ews115 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Said Abou-Hallawa]

Created attachment 305870 [details]
Patch

[Said Abou-Hallawa]

There were three issues with the test:

1. There was a bug in the code. BitmapImage::stopAnimation() was stopping the decoding thread for all images. This is not needed for large images since the decoding thread will stop automatically once the frame is decoded. The fix is to make BitmapImage::stopAnimation() stops the decoding thread for the animated images only.

2. There was a problem with the test when running it on 2x display. The width of the <div> element was created with the same width as the background image. But on 2x display, the number of pixels of the <div> element are doubled for each dimension. And this was causing the image to be resized and the pixels on the edges get blurred. The solution was use @media css rules for the 2x display.

3. In the test I using document.body.offsetHeight to force redraw. It turns out this is not sufficient since offsetHeight is now optimized such that it won't cause layout/redraw unless the element is dirty. The fix was to change the element properties temporarily, call offsetHeight and then reset the element properties back.

[Simon Fraser (smfr)]

Comment on attachment 305870 [details]
Patch

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

> Source/WebCore/ChangeLog:15
> +        To fix this issue, we must use the size of the image and not destRect.size().

You say "size of the image", but the srcRect parameter to nativeImageDrawingScale() isn't necessarily the bounds of the whole image. We may be drawing a subsection of the image.

> Source/WebCore/platform/graphics/cg/NativeImageCG.cpp:83
> -float subsamplingScale(GraphicsContext& context, const FloatRect& destRect, const FloatRect& srcRect)
> +FloatSize nativeImageDrawingScale(GraphicsContext& context, const FloatRect& destRect, const FloatRect& srcRect)
>  {
>      // Never use subsampled images for drawing into PDF contexts.
>      if (wkCGContextIsPDFContext(context.platformContext()))
> -        return 1;
> +        return { 1, 1 };
>  
>      CGRect transformedDestinationRect = CGRectApplyAffineTransform(destRect, CGContextGetCTM(context.platformContext()));
> -    return std::min<float>(1, std::max(transformedDestinationRect.size.width / srcRect.width(), transformedDestinationRect.size.height / srcRect.height()));
> +    return { static_cast<float>(transformedDestinationRect.size.width / srcRect.width()), static_cast<float>(transformedDestinationRect.size.height / srcRect.height()) };

It's weird to have this bare function for each platform, but it's mostly cross platform code (you can call getCTM on the GraphicsContext). This should become a function on maybe GraphicsContext at some point.

[Said Abou-Hallawa]

*** Bug 169574 has been marked as a duplicate of this bug. ***

[Said Abou-Hallawa]

Created attachment 305910 [details]
Patch

[Said Abou-Hallawa]

(In reply to Simon Fraser (smfr) from comment #66)
> Comment on attachment 305870 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=305870&action=review
> 
> > Source/WebCore/ChangeLog:15
> > +        To fix this issue, we must use the size of the image and not destRect.size().
> 
> You say "size of the image", but the srcRect parameter to
> nativeImageDrawingScale() isn't necessarily the bounds of the whole image.
> We may be drawing a subsection of the image.
> 

I changed the ChangeLog to include the following description for the fix:

To fix this issue, first the base size has to be size of the image and not destRect.size(). Secondly, we need to scale this base size with the context transformation multiplied by the ratio destRect / srcRect. This scaling is exactly the same scaling which is calculated in subsamplingScale(). Finally we use this scaled size as the sizeForDrawing to send to the ImageDecoder.

> > Source/WebCore/platform/graphics/cg/NativeImageCG.cpp:83
> > -float subsamplingScale(GraphicsContext& context, const FloatRect& destRect, const FloatRect& srcRect)
> > +FloatSize nativeImageDrawingScale(GraphicsContext& context, const FloatRect& destRect, const FloatRect& srcRect)
> >  {
> >      // Never use subsampled images for drawing into PDF contexts.
> >      if (wkCGContextIsPDFContext(context.platformContext()))
> > -        return 1;
> > +        return { 1, 1 };
> >  
> >      CGRect transformedDestinationRect = CGRectApplyAffineTransform(destRect, CGContextGetCTM(context.platformContext()));
> > -    return std::min<float>(1, std::max(transformedDestinationRect.size.width / srcRect.width(), transformedDestinationRect.size.height / srcRect.height()));
> > +    return { static_cast<float>(transformedDestinationRect.size.width / srcRect.width()), static_cast<float>(transformedDestinationRect.size.height / srcRect.height()) };
> 
> It's weird to have this bare function for each platform, but it's mostly
> cross platform code (you can call getCTM on the GraphicsContext). This
> should become a function on maybe GraphicsContext at some point.

Will do.

[WebKit Commit Bot]

Comment on attachment 305910 [details]
Patch

Clearing flags on attachment: 305910

Committed r214635: <http://trac.webkit.org/changeset/214635>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Said Abou-Hallawa]

I logged https://bugs.webkit.org/show_bug.cgi?id=170353 to track making nativeImageDrawingScale() a cross platform function.