1741767 - Large Image fails to load when using the Image constructor.

Status: RESOLVED FIXED

(Core :: Graphics: Canvas2D, defect, P3)

Description

[lukas]

Attachment: index.html

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Firefox/91.0

Steps to reproduce:

On Windows 10 build 19043.1348, firefox version >=86
load the MNIST handwritten digit sprite file from https://storage.googleapis.com/learnjs-data/model-builder/mnist_images.png via the Image() constructor and then draw the ImageData on a canvas using drawImage.

See the attached index.html for an example.

Actual results:

The canvas is black, instead of showing the sprite file.

Expected results:

Draw the sprite file (or part of it) on the canvas.
Up to firefox 86 this worked and still works in most other browsers.

Comment 1

[Robert Longson [:longsonr]]

Perhaps you could use https://mozilla.github.io/mozregression/ to establish a precise regression window.

Comment 2

[Masatoshi Kimura [:emk]]

Regression range: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=17e3c9ff2cad9711445a6c39df85925c8f347336&tochange=afa247928245d02b1305a1850eb39f19b84a2f35

This is much earlier than Firefox 85 for me.

Comment 3

[Alice0775 White]

Further bisect:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=df4d3efbfa453cf1e0da54a44df349e130731f74&tochange=a22ce6ad4e273a47c9f4481e2462efa35b17d8eb

Comment 4

[Masatoshi Kimura [:emk]]

Confirmed that the testcase works if gfx.canvas.remote is false (restart required).
Since remote 2D canvas has been enabled since Firefox 86 in release (bug 1687276), it explains reporter's regression range.

Comment 5

[lukas]

I can confirm that with gfx.canvas.remote set to false, the real usecase, being machine learning on the MNIST dataset with tensorflow.js, works again.

Comment 6

[Sotaro Ikeda [:sotaro]]

:bobowen, can you comment to the bug?

Comment 7

[Bob Owen (:bobowen)]

(In reply to Sotaro Ikeda [:sotaro] from comment #6)

:bobowen, can you comment to the bug?

My guess is that when we are calling DrawTargetD2D1::DrawSurface (in the content process) it only creates an image to fill the canvas.
For DrawTargetRecording::DrawSurface, it records the entire surface, but that fails to be created in the GPU process, because it is too big.

So, I guess we should check to see if the surface is bigger than the canvas and if it is just create a separate surface, just for this draw, that is truncated to the width and height of the canvas.

Comment 8

[Bob Owen (:bobowen)]

I dug out an old patch that uses the shared memory images in the canvas recording/translation.
It needs a bit more work, but it fixes this issue.

Comment 9

[Bob Owen (:bobowen)]

Attachment: Bug 1741767: Use shared memory surfaces in Canvas 2D recording. r=lsalzman!

This also changes SharedSurfacesParent to use a StaticMonitor, so that we can
wait for the surface to be added.

Depends on D132602

Comment 10

[Bob Owen (:bobowen)]

https://treeherder.mozilla.org/jobs?repo=try&revision=986a3d3135548ae4f1c358a71e734fba8c9cbbf3

Comment 11

[BugBot [:suhaib / :marco/ :calixte]]

There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:bobowen, could you have a look please?
For more information, please visit auto_nag documentation.

Comment 12

[Bob Owen (:bobowen)]

(In reply to Release mgmt bot [:sylvestre / :calixte / :marco for bugbug] from comment #11)

There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:bobowen, could you have a look please?
For more information, please visit auto_nag documentation.

I was thinking that I ought to try and write a test for this before I land.

Comment 13

[Masatoshi Kimura [:emk]]

Attachment: Bug 1741767 - Reftest drawing a large image to canvas. r?bobowen

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7dc323e57fd4
Use shared memory surfaces in Canvas 2D recording. r=lsalzman
https://hg.mozilla.org/integration/autoland/rev/bc03f747b472
Reftest drawing a large image to canvas. r=bobowen

Comment 15

[Noemi Erli[:noemi_erli]]

Backed out 2 changesets (Bug 1741767) for causing assertion failures in DrawTargetD2D1.cpp CLOSED TREE

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=bc03f747b472d414f1c93ae0c97cdba268824370

Log: https://treeherder.mozilla.org/logviewer?job_id=363649913&repo=autoland&lineNumber=21067

Backout: https://hg.mozilla.org/integration/autoland/rev/8f1047161612d21b6c9798f7731ca620934f5225

Comment 16

[Masatoshi Kimura [:emk]]

The test triggers the assertion failure even without D132703 when gfx.canvas.remote is false. So this is an existing problem.

Comment 17

[Masatoshi Kimura [:emk]]

We hit this assertion when the image is too large to create a D2D1Bitmap.

Bug 1088235 introduced the assertion. Bas, do you have a way round?

Comment 18

[Masatoshi Kimura [:emk]]

I'll re-land with the test disabled on debug builds if it is not easy to fix the problem because this is an existing problem.

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1f75710950a2
Use shared memory surfaces in Canvas 2D recording. r=lsalzman
https://hg.mozilla.org/integration/autoland/rev/a1cc440f0d95
Reftest drawing a large image to canvas. r=bobowen

Comment 20

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/1f75710950a2
https://hg.mozilla.org/mozilla-central/rev/a1cc440f0d95

Comment 21

[Masatoshi Kimura [:emk]]

Filed bug 1750381 for the assertion problem.