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.
Created attachment 304247 [details] Demo video
<rdar://problem/31011842>
Created attachment 304302 [details] Patch
Comment on attachment 304302 [details] Patch This patch is for EWS.
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
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
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
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
Created attachment 304324 [details] Patch
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
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
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
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
Created attachment 304388 [details] Patch
Created attachment 304399 [details] Patch
Created attachment 304400 [details] Patch
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
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
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
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
Created attachment 304477 [details] Patch
Created attachment 304479 [details] Patch
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
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
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
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
Created attachment 304503 [details] Patch
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.
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.
Created attachment 305651 [details] Patch
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.
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
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
Created attachment 305668 [details] Patch
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
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
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
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
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
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
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
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
Created attachment 305754 [details] Patch
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
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
Created attachment 305764 [details] Patch
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
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
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
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
Created attachment 305775 [details] Patch
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
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
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
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
Created attachment 305796 [details] Patch
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
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
Created attachment 305818 [details] Patch
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
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
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
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
Created attachment 305870 [details] Patch
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.
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.
*** Bug 169574 has been marked as a duplicate of this bug. ***
Created attachment 305910 [details] Patch
(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.
Comment on attachment 305910 [details] Patch Clearing flags on attachment: 305910 Committed r214635: <http://trac.webkit.org/changeset/214635>
All reviewed patches have been landed. Closing bug.
I logged https://bugs.webkit.org/show_bug.cgi?id=170353 to track making nativeImageDrawingScale() a cross platform function.