Overview: Using ctx.drawImage(image, sx, sy, swidth, sheight, dx, dy, dwidth, dheight), where ctx is a CanvasRenderingContext2D, fails if any of sx, sy, swidth or sheight are non-integers. No error is given. Steps to Reproduce: Create an HTML page with the following code, and open it in Safari or Safari Technology Preview: <canvas id='target' width='100' height='100'/> <script> image = new Image(); image.onload = function () { // Changing 0.1 to 0 will cause the image to appear target.getContext( '2d' ).drawImage( image, 0.1, 0, 100, 100, 0, 0, 100, 100 ); }; image.src = 'http://placehold.it/100x100'; </script> Actual Results: The canvas remains blank. Expected Results: The image is drawn to the canvas (albeit possibly with some sampling artifacts). Build Date & Platform: Date and platform of the build in which you first encountered the bug. Safari Version 9.1.2 (11601.7.7) on Mac OS 10. Additional Information: This works as expected in all other browsers. The dx, dy, dwidth and dheight arguments work whether or not they are integers.
I can reproduce on ToT. I'll investigate.
Not a JS bindings bug. We fail the following check in the drawImage() implementation: if (!imageRect.contains(normalizedSrcRect)) return;
(In reply to comment #2) > Not a JS bindings bug. We fail the following check in the drawImage() > implementation: > if (!imageRect.contains(normalizedSrcRect)) > return; srcRect: [0.100000, 0.000000, 100.000000 100.000000] normalizedSrcRect: [0.100000, 0.000000, 100.000000 100.000000] imageRect: [0.000000, 0.000000, 100.000000 100.000000] Technically, it is true that imageRect.contains does not fully contain normalizedSrcRect / srcRect.
In this case, the spec says: "When the source rectangle is outside the source image, the source rectangle must be clipped to the source image and the destination rectangle must be clipped in the same proportion." However, WebKit ignores the call instead of clipping it.
Created attachment 286068 [details] Patch
Comment on attachment 286068 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=286068&action=review > Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp:1403 > + normalizedDstRect.setWidth(normalizedDstRect.width() - originalNormalizedSrcRect.width() + normalizedSrcRect.width()); > + normalizedDstRect.setHeight(normalizedDstRect.height() - originalNormalizedSrcRect.height() + normalizedSrcRect.height()); From the statement "When the source rectangle is outside the source image, the source rectangle must be clipped to the source image and the destination rectangle must be clipped in the same proportion." I would expect these two lines to be normalizedDstRect.setWidth(normalizedDstRect.width() * normalizedSrcRect.width() / originalNormalizedSrcRect.width()); normalizedDstRect.setHeight(normalizedDstRect.height() * normalizedSrcRect.height() * originalNormalizedSrcRect.height()); I think the goal is ensure that ratio originalNormalizedDstRect.size() / originalNormalizedSrcRect.size() is equal to the ratio normalizedDstRect.size() / normalizedSrcRect.size()
Created attachment 286071 [details] Patch
(In reply to comment #6) > Comment on attachment 286068 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=286068&action=review > > > Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp:1403 > > + normalizedDstRect.setWidth(normalizedDstRect.width() - originalNormalizedSrcRect.width() + normalizedSrcRect.width()); > > + normalizedDstRect.setHeight(normalizedDstRect.height() - originalNormalizedSrcRect.height() + normalizedSrcRect.height()); > > From the statement "When the source rectangle is outside the source image, > the source rectangle must be clipped to the source image and the > destination rectangle must be clipped in the same proportion." I would > expect these two lines to be > > normalizedDstRect.setWidth(normalizedDstRect.width() * > normalizedSrcRect.width() / originalNormalizedSrcRect.width()); > normalizedDstRect.setHeight(normalizedDstRect.height() * > normalizedSrcRect.height() * originalNormalizedSrcRect.height()); > > I think the goal is ensure that ratio originalNormalizedDstRect.size() / > originalNormalizedSrcRect.size() is equal to the ratio > normalizedDstRect.size() / normalizedSrcRect.size() You make a good point :)
Comment on attachment 286071 [details] Patch Attachment 286071 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/1876646 New failing tests: fast/canvas/drawImage-srcRect-clipping.html
Created attachment 286077 [details] Archive of layout-test-results from ews123 for ios-simulator-elcapitan-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews123 Port: ios-simulator-elcapitan-wk2 Platform: Mac OS X 10.11.5
There is barely any visible difference between actual / expected on the iOS-simulator bot. I am not sure how to resolve except by skipping the test on this platform. Please advise.
Created attachment 286097 [details] Patch
Comment on attachment 286097 [details] Patch Attachment 286097 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/1877938 New failing tests: fast/canvas/drawImage-srcRect-clipping.html
Created attachment 286121 [details] Archive of layout-test-results from ews126 for ios-simulator-elcapitan-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews126 Port: ios-simulator-elcapitan-wk2 Platform: Mac OS X 10.11.5
So what it looks like on iOS is that the green part of the canvas is blurry along the bottom and the right side. However, it is sharp in the reference html. This difference is apparently enough.
Created attachment 286136 [details] Patch
Now using Canvas in the expected result as well, hoping this will make the iOS bot happy.
Comment on attachment 286136 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=286136&action=review > Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp:1403 > + normalizedDstRect.setWidth(normalizedDstRect.width() * normalizedSrcRect.width() / originalNormalizedSrcRect.width()); > + normalizedDstRect.setHeight(normalizedDstRect.height() * normalizedSrcRect.height() / originalNormalizedSrcRect.height()); Does not seem to correctly handle the case where the source rectangle had a negative X or Y coordinate.
(In reply to comment #18) > Comment on attachment 286136 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=286136&action=review > > > Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp:1403 > > + normalizedDstRect.setWidth(normalizedDstRect.width() * normalizedSrcRect.width() / originalNormalizedSrcRect.width()); > > + normalizedDstRect.setHeight(normalizedDstRect.height() * normalizedSrcRect.height() / originalNormalizedSrcRect.height()); > > Does not seem to correctly handle the case where the source rectangle had a > negative X or Y coordinate. width / height will always be positive here since the srcRect has been normalized already.
Comment on attachment 286136 [details] Patch Clearing flags on attachment: 286136 Committed r204517: <http://trac.webkit.org/changeset/204517>
All reviewed patches have been landed. Closing bug.