Bug 202457 – Canvas drawImage should not alter the input source or the destination rectangles

• Status: RESOLVED
• Resolution: FIXED
• Component: Canvas

[Chris Lord]

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.

[Karl Dubost]

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

[Radar WebKit Bug Importer]

<rdar://problem/127982607>

[Karl Dubost]

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]

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]

Created attachment 471379 [details]
drawn canvas blown up.

the squarea are not totally square.

[Karl Dubost]

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]

Created attachment 471380 [details]
rendering in safari, firefox, chrome

The scaling is really wrong

[Karl Dubost]

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

[Karl Dubost]

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]

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]

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

[Karl Dubost]

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]

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]

Created attachment 471383 [details]
rendering in safari, firefox, chrome for testcase 2

Rendering of the second test case

[Ahmad Saleem]

https://github.com/whatwg/html/issues/6306 ?

Blink - https://chromium.googlesource.com/chromium/src/+/88434d80a2e6ef4ae9850740d04ba138c94c8cb1 ?

[Said Abou-Hallawa]

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]

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

[Said Abou-Hallawa]

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

[EWS]

Committed 278807@main (5dd01bad437e): <https://commits.webkit.org/278807@main>

Reviewed commits have been landed. Closing PR #28501 and removing active labels.