Bug 202457

Summary:

Canvas drawImage should not alter the input source or the destination rectangles

Product:

WebKit

Reporter:

Chris Lord <clord>

Component:

Canvas

Assignee:

Said Abou-Hallawa <sabouhallawa>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

ahmad.saleem792, dino, junov, karlcow, sabouhallawa, webkit-bug-importer

Priority:

Keywords:

BrowserCompat, InRadar, WPTImpact

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Linux

URL:

http://wpt.live/html/canvas/element/manual/drawing-images-to-the-canvas/drawimage_canvas.html

See Also:

https://bugs.webkit.org/show_bug.cgi?id=149989
https://bugs.webkit.org/show_bug.cgi?id=116991
https://bugs.webkit.org/show_bug.cgi?id=274299

Bug Depends on:

179191

Bug Blocks:

Attachments:

Description	Flags
drawn canvas blown up.	none
rendering in safari, firefox, chrome	none
testcase	none
testcase 2	none
rendering in safari, firefox, chrome for testcase 2	none

Chris Lord

Reported 2019-10-02 05:58:54 PDT

imported/w3c/web-platform-tests/2dcontext/drawing-images-to-the-canvas/drawimage_canvas.html fails with: FAIL Test scenario 9: sx = 0, sy = 0, sw = 50, sh = 50, dx = 100, dy = 100, dw = -50, dh = -50 --- Pixel 50,50 should be blue. assert_array_equals: property 0, expected 0 but got 255 FAIL Test scenario 9: sx = 0, sy = 0, sw = 50, sh = 50, dx = 100, dy = 100, dw = -50, dh = -50 --- Pixel 50,99 should be blue. assert_array_equals: property 0, expected 0 but got 255 FAIL Test scenario 9: sx = 0, sy = 0, sw = 50, sh = 50, dx = 100, dy = 100, dw = -50, dh = -50 --- Pixel 99,50 should be blue. assert_array_equals: property 0, expected 0 but got 255 FAIL Test scenario 9: sx = 0, sy = 0, sw = 50, sh = 50, dx = 100, dy = 100, dw = -50, dh = -50 --- Pixel 99,99 should be blue. assert_array_equals: property 0, expected 0 but got 255 FAIL Test scenario 9: sx = 0, sy = 0, sw = 50, sh = 50, dx = 100, dy = 100, dw = -50, dh = -50 --- Pixel 55,55 should be black. assert_array_equals: property 0, expected 0 but got 255 FAIL Test scenario 9: sx = 0, sy = 0, sw = 50, sh = 50, dx = 100, dy = 100, dw = -50, dh = -50 --- Pixel 55,94 should be black. assert_array_equals: property 0, expected 0 but got 255 FAIL Test scenario 9: sx = 0, sy = 0, sw = 50, sh = 50, dx = 100, dy = 100, dw = -50, dh = -50 --- Pixel 94,55 should be black. assert_array_equals: property 0, expected 0 but got 255 FAIL Test scenario 9: sx = 0, sy = 0, sw = 50, sh = 50, dx = 100, dy = 100, dw = -50, dh = -50 --- Pixel 94,94 should be black. assert_array_equals: property 0, expected 0 but got 255 This test passes on Mac and iOS.

Attachments
drawn canvas blown up. (84.57 KB, image/png) 2024-05-12 21:58 PDT, Karl Dubost	no flags	Details
rendering in safari, firefox, chrome (213.49 KB, image/png) 2024-05-12 22:41 PDT, Karl Dubost	no flags	Details
testcase (937 bytes, text/html) 2024-05-12 23:36 PDT, Karl Dubost	no flags	Details
testcase 2 (1.40 KB, text/html) 2024-05-13 01:46 PDT, Karl Dubost	no flags	Details
rendering in safari, firefox, chrome for testcase 2 (181.77 KB, image/png) 2024-05-13 01:47 PDT, Karl Dubost	no flags	Details
View All Add attachment proposed patch, testcase, etc.

Karl Dubost

Comment 1 2024-05-12 21:44:53 PDT

This is a weird failure. It fails currently on macOS. So this is not only about GTK.

Radar WebKit Bug Importer

Comment 2 2024-05-12 21:45:03 PDT

<rdar://problem/127982607>

Karl Dubost

Comment 3 2024-05-12 21:46:30 PDT

This is specifically weird because tests about red are working for example. Test scenario 11: sx = 0, sy = 0, sw = 100, sh = 100, dx = 0, dy = 0, dw = 50, dh = 50 --- Pixel 99,99 should be red. Asserts run Pass assert_array_equals(object "255,0,0,255", [255, 0, 0, 255]) checkPixel /html/canvas/element/manual/drawing-images-to-the-canvas/drawimage_canvas.html:21:24 While: Test scenario 12: sx = -20, sy = -20, sw = 50, sh = 50, dx = 20, dy = 20, dw = 125, dh = 125 --- Pixel 70,70 should be blue. assert_array_equals: expected property 2 to be 255 but got 0 (expected array [0, 0, 255, 255] got object "0,0,0,255") Asserts run Fail assert_array_equals(object "0,0,0,255", [0, 0, 255, 255]) checkPixel /html/canvas/element/manual/drawing-images-to-the-canvas/drawimage_canvas.html:21:24

Karl Dubost

Comment 4 2024-05-12 21:54:45 PDT

All the tests failing are in the negative coordinates but not for all colors. Only for Blue: [70,70], [70,99], [99,70], [82,82] Black: [84,99], [99,84], [99,99] ..........but not [84,84] Red: [69,69] ............................but not [0,69], [69, 0] // Negative coordinates of the source rectangle. // The source area is clipped to fit the source image and the destination area // is clipped in the same proportion. In this specific test: // - source is clipped by 20 from top and left. // - destination will get proportionally clipped by 50 from top and left as we // are scaling the source image 2.5 times. // - the rect will be drawn from 70,70 to 100,100. testDescription = 'Test scenario 12: sx = -20, sy = -20, sw = 50, sh = 50, dx = 20, dy = 20, dw = 125, dh = 125 --- '; sourceRect = {x: -20, y: -20, w: 50, h: 50}; destRect = {x: 20, y: 20, w: 125, h: 125}; blueCheck = [[70,70], [70,99], [99,70], [82,82]]; blackCheck = [[84,84], [84,99], [99,84], [99,99]]; redCheck = [[0,69], [69, 0], [69,69]]; drawCanvasOnCanvasUsingDrawImage(sourceRect, destRect, blueCheck, blackCheck, redCheck, testDescription);

Karl Dubost

Comment 5 2024-05-12 21:58:34 PDT

Created attachment 471379 [details] drawn canvas blown up. the squarea are not totally square.

Karl Dubost

Comment 6 2024-05-12 22:13:38 PDT

For example the first value failing. var actual = destCtx.getImageData(location[0], location[1], 1, 1).data; with location: [70, 70] actual: Uint8ClampedArray (4) 0 0 1 0 2 0 3 255 expected: [0, 0, 255, 255] (4) so we got a black pixel instead of a blue pixel.

Karl Dubost

Comment 7 2024-05-12 22:41:16 PDT

Created attachment 471380 [details] rendering in safari, firefox, chrome The scaling is really wrong

Karl Dubost

Comment 8 2024-05-12 22:42:09 PDT

So probably an issue with the math when the origin of the sourceRect has negative units.

Karl Dubost

Comment 9 2024-05-12 23:36:04 PDT

Created attachment 471381 [details] testcase OK found. Not sure if it depends on Bug 149989 But basically this is because of drawImage(image, sx, sy, sw, sh, dx, dy, dw, dh) https://html.spec.whatwg.org/multipage/canvas.html#dom-context-2d-drawimage * if sx and sy are positive integers this is working. * if sx and xy are NEGATIVE integers this is FAILING.

Karl Dubost

Comment 10 2024-05-12 23:40:11 PDT

I see the IDL https://searchfox.org/wubkat/source/Source/WebCore/html/canvas/CanvasDrawImage.idl#47 but I don't see where the maths are done. Or is it outside of WebKit.

Karl Dubost

Comment 11 2024-05-12 23:42:28 PDT

Ah found. https://searchfox.org/wubkat/rev/af7bd70a44bb1e3adae77f36bcc34a47daeeb9a4/Source/WebCore/html/canvas/CanvasRenderingContext2DBase.cpp#1569-1576

Karl Dubost

Comment 12 2024-05-13 01:41:02 PDT

So if I understand correctly. Safari seems to clip and drop the part of the image which is outside the source Canvas, while Firefox and Chrome keeps the part which is outside the initial canvas before positioning inside the destination. According to > The source rectangle is the rectangle whose corners are the four points (sx, sy), (sx+sw, sy), (sx+sw, sy+sh), (sx, sy+sh). > The destination rectangle is the rectangle whose corners are the four points (dx, dy), (dx+dw, dy), (dx+dw, dy+dh), (dx, dy+dh). Firefox and Chrome are doing the right thing. BUT there is also this sentence just after, which seems to contradict the math above. > 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. So according to this sentence Safari does the right thing and then Firefox/Chrome are wrong.

Karl Dubost

Comment 13 2024-05-13 01:46:34 PDT

Created attachment 471382 [details] testcase 2 Though the rendering of Safari is still wrong because in my green yellow example it should extend to the bottom. I will modify my testcase to better show the issue. testcase 2

Karl Dubost

Comment 14 2024-05-13 01:47:47 PDT

Created attachment 471383 [details] rendering in safari, firefox, chrome for testcase 2 Rendering of the second test case

Ahmad Saleem

Comment 15 2024-05-13 05:30:44 PDT

https://github.com/whatwg/html/issues/6306 ? Blink - https://chromium.googlesource.com/chromium/src/+/88434d80a2e6ef4ae9850740d04ba138c94c8cb1 ?

Said Abou-Hallawa

Comment 16 2024-05-13 14:43:29 PDT

I think we misinterpreted the specs ttp://www.whatwg.org/specs/web-apps/current-work/multipage/the-canvas-element.html#dom-context-2d-drawimage in our implementation for the canvas drawImage(). I think the specs means we should limit the physical pixel rectangle to the source image rectangle. But it does not mean we can alter the input source rectangle or the input destination rectangle because they are used to scale the image when drawing it to the destination context. The platform SDK should handle the clipping properly when getting the pixels from the source image and when copying these pixels to the destination context.

Said Abou-Hallawa

Comment 17 2024-05-13 14:44:47 PDT

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

Said Abou-Hallawa

Comment 18 2024-05-13 14:58:01 PDT

Pull request: https://github.com/WebKit/WebKit/pull/28501

EWS

Comment 19 2024-05-15 08:15:13 PDT

Committed 278807@main (5dd01bad437e): <https://commits.webkit.org/278807@main> Reviewed commits have been landed. Closing PR #28501 and removing active labels.

Note You need to log in before you can comment on or make changes to this bug.