RESOLVED FIXED 159276
Drawing an SVG image into a <canvas> that is not in the DOM draws the wrong region
https://bugs.webkit.org/show_bug.cgi?id=159276
Summary Drawing an SVG image into a <canvas> that is not in the DOM draws the wrong r...
Antoine Quint
Reported 2016-06-29 14:41:50 PDT
Created attachment 282375 [details] Testcase See the attached testcase.
Attachments
Testcase (961 bytes, text/html)
2016-06-29 14:41 PDT, Antoine Quint
no flags
Patch (5.25 KB, patch)
2016-06-29 14:48 PDT, Antoine Quint
dino: review+
Patch (5.35 KB, patch)
2016-06-29 15:30 PDT, Antoine Quint
no flags
Patch (13.34 KB, patch)
2016-06-30 01:53 PDT, Antoine Quint
no flags
Archive of layout-test-results from ews101 for mac-yosemite (922.08 KB, application/zip)
2016-06-30 02:21 PDT, Build Bot
no flags
Archive of layout-test-results from ews121 for ios-simulator-wk2 (1.21 MB, application/zip)
2016-06-30 02:33 PDT, Build Bot
no flags
Archive of layout-test-results from ews107 for mac-yosemite-wk2 (1.13 MB, application/zip)
2016-06-30 02:36 PDT, Build Bot
no flags
Archive of layout-test-results from ews113 for mac-yosemite (1.52 MB, application/zip)
2016-06-30 04:56 PDT, Build Bot
no flags
Patch (13.17 KB, patch)
2016-06-30 06:43 PDT, Antoine Quint
no flags
Test case -- The image source size is different from the SVG viewBox. (906 bytes, text/html)
2016-06-30 11:46 PDT, Said Abou-Hallawa
no flags
Antoine Quint
Comment 1 2016-06-29 14:42:08 PDT
Antoine Quint
Comment 2 2016-06-29 14:48:41 PDT
Dean Jackson
Comment 3 2016-06-29 14:50:55 PDT
Comment on attachment 282376 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=282376&action=review Didn't you want to remove a timeout from an existing test too? > Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp:1406 > + // If the <img> element is not attached to the DOM, the SVG image will not have had its > + // container sized yet and it needs to be sized to match the <img>. > + if (!imageElement.inDocument()) > + image->setContainerSize(FloatSize(imageElement.naturalWidth(), imageElement.naturalHeight())); > + else > + image->setContainerSize(normalizedSrcRect.size()); What happens if the image element doesn't have a size?
Antoine Quint
Comment 4 2016-06-29 14:55:40 PDT
(In reply to comment #3) > Comment on attachment 282376 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=282376&action=review > > Didn't you want to remove a timeout from an existing test too? Yes, for the test written for https://bugs.webkit.org/show_bug.cgi?id=148845, but it's fairly involved and didn't want to mess it up. > > Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp:1406 > > + // If the <img> element is not attached to the DOM, the SVG image will not have had its > > + // container sized yet and it needs to be sized to match the <img>. > > + if (!imageElement.inDocument()) > > + image->setContainerSize(FloatSize(imageElement.naturalWidth(), imageElement.naturalHeight())); > > + else > > + image->setContainerSize(normalizedSrcRect.size()); > > What happens if the image element doesn't have a size? It gets the SVG image's intrinsic size.
Antoine Quint
Comment 5 2016-06-29 15:08:32 PDT
Actually, this patch does not cover the case where the <img> is in the DOM but has not been rendered yet. Will fix and add a new test.
Antoine Quint
Comment 6 2016-06-29 15:30:30 PDT
Dean Jackson
Comment 7 2016-06-29 22:35:33 PDT
Comment on attachment 282383 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=282383&action=review Maybe you should add a test with an image that doesn't have a width and height. new Image(); > LayoutTests/svg/as-image/img-with-svg-resource-not-in-dom-and-drawImage.html:10 > +var svgData = 'data:image/svg+xml,<svg xmlns="http://www.w3.org/2000/svg" version="1.1" viewBox="0 0 200 200"><rect width="200" height="200" fill="red" /><rect x="20" y="20" width="40" height="40" fill="green" /></svg>'; don't need version attribute
Antoine Quint
Comment 8 2016-06-30 01:53:10 PDT
Build Bot
Comment 9 2016-06-30 02:21:14 PDT
Comment on attachment 282420 [details] Patch Attachment 282420 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/1599391 New failing tests: fast/hidpi/image-srcset-svg-canvas-2x.html
Build Bot
Comment 10 2016-06-30 02:21:19 PDT
Created attachment 282422 [details] Archive of layout-test-results from ews101 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews101 Port: mac-yosemite Platform: Mac OS X 10.10.5
Build Bot
Comment 11 2016-06-30 02:33:00 PDT
Comment on attachment 282420 [details] Patch Attachment 282420 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/1599408 New failing tests: fast/hidpi/image-srcset-svg-canvas-2x.html
Build Bot
Comment 12 2016-06-30 02:33:04 PDT
Created attachment 282423 [details] Archive of layout-test-results from ews121 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews121 Port: ios-simulator-wk2 Platform: Mac OS X 10.11.4
Build Bot
Comment 13 2016-06-30 02:36:04 PDT
Comment on attachment 282420 [details] Patch Attachment 282420 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/1599426 New failing tests: fast/hidpi/image-srcset-svg-canvas-2x.html
Build Bot
Comment 14 2016-06-30 02:36:08 PDT
Created attachment 282424 [details] Archive of layout-test-results from ews107 for mac-yosemite-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews107 Port: mac-yosemite-wk2 Platform: Mac OS X 10.10.5
Build Bot
Comment 15 2016-06-30 04:56:01 PDT
Comment on attachment 282420 [details] Patch Attachment 282420 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/1599934 New failing tests: fast/hidpi/image-srcset-svg-canvas-2x.html
Build Bot
Comment 16 2016-06-30 04:56:06 PDT
Created attachment 282431 [details] Archive of layout-test-results from ews113 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews113 Port: mac-yosemite Platform: Mac OS X 10.10.5
Antoine Quint
Comment 17 2016-06-30 06:43:41 PDT
Said Abou-Hallawa
Comment 18 2016-06-30 11:46:04 PDT
Created attachment 282453 [details] Test case -- The image source size is different from the SVG viewBox. This test case is broken in Chrome and FireFox also.
Said Abou-Hallawa
Comment 19 2016-06-30 11:47:24 PDT
Comment on attachment 282433 [details] Patch Looks good to me. Unofficial r=me. But please add the test case which I attached to the bug.
Said Abou-Hallawa
Comment 20 2016-06-30 11:49:59 PDT
(In reply to comment #18) > Created attachment 282453 [details] > Test case -- The image source size is different from the SVG viewBox. > > This test case is broken in Chrome and FireFox also. Sorry I meant it is broken in FireFox and Safari without your patch
Said Abou-Hallawa
Comment 21 2016-06-30 11:52:28 PDT
(In reply to comment #20) > (In reply to comment #18) > > Created attachment 282453 [details] > > Test case -- The image source size is different from the SVG viewBox. > > > > This test case is broken in Chrome and FireFox also. > > Sorry I meant it is broken in FireFox and Safari without your patch Actually it is broken in Chrome also. The expatiation is to have the canvas filled with black except a green rectangle (0,0)-(20,20) and a red rectangle (180,180)-(200,200).
Said Abou-Hallawa
Comment 22 2016-06-30 11:57:35 PDT
(In reply to comment #21) > (In reply to comment #20) > > (In reply to comment #18) > > > Created attachment 282453 [details] > > > Test case -- The image source size is different from the SVG viewBox. > > > > > > This test case is broken in Chrome and FireFox also. > > > > Sorry I meant it is broken in FireFox and Safari without your patch > > Actually it is broken in Chrome also. The expatiation is to have the canvas > filled with black except a green rectangle (0,0)-(20,20) and a red rectangle > (180,180)-(200,200). It is also broken with your patch in WK2. I was trying it in WK1. This is a timing issue bug.
Dean Jackson
Comment 23 2016-06-30 15:07:01 PDT
Comment on attachment 282433 [details] Patch Could we add a test that exposes the flakey/timing case and mark it as PASS FAIL? I want to make sure we've captured that bug.
WebKit Commit Bot
Comment 24 2016-06-30 15:18:37 PDT
Comment on attachment 282433 [details] Patch Clearing flags on attachment: 282433 Committed r202712: <http://trac.webkit.org/changeset/202712>
WebKit Commit Bot
Comment 25 2016-06-30 15:18:43 PDT
All reviewed patches have been landed. Closing bug.
Antoine Quint
Comment 26 2016-07-01 02:51:53 PDT
Note You need to log in before you can comment on or make changes to this bug.