1852420 - createImageBitmap() and OffscreenCanvas drawImage() dows not work on Firefox 118

Status: RESOLVED FIXED

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

Description

[tkr-oita]

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

Steps to reproduce:

1. crop an image: const newImage = createBitmapImage(oldImage, x, y, w, h)
2. create a new OffscreenCanvas(w, h)
3. drawImage(newImage, 0, 0) against to its 2d context
4. convertToBlob() and save it as a file.

I wrote this codepen: https://codepen.io/tkuri2010/pen/bGOqojL

Actual results:

I got a blank image file.

Expected results:

I want to get a cropped image file.

Comment 1

[Mayank Bansal]

Can repro with D2d-canvas

Comment 2

[Timothy Nikkel (:tnikkel)]

Might be same issue as bug 1851943.

Comment 3

[Andrew Osmond [:aosmond] (he/him)]

These are not related. This bug has existed since OffscreenCanvas landed as far as I can tell.

Comment 4

[Andrew Osmond [:aosmond] (he/him)]

It appears that if we create an ImageBitmap, draw it to a canvas and draw it again to an OffscreenCanvas, then it only draws successfully into the first canvas.

Comment 5

[Andrew Osmond [:aosmond] (he/him)]

This is happening because the first canvas is a D2D canvas, and the OffscreenCanvas is a Skia canvas. When we crop the underlying source surface with the D2D canvas, we end up getting a SourceSurfaceRecording out of it, without a link to the original surface:
https://searchfox.org/mozilla-central/rev/09f7e00da9eaef55c80620c17e8575ddd4532451/dom/canvas/ImageBitmap.cpp#810
https://searchfox.org/mozilla-central/rev/09f7e00da9eaef55c80620c17e8575ddd4532451/dom/canvas/ImageBitmap.cpp#817

Which the Skia canvas cannot draw.

If we don't crop, we hit the OptimizeSourceSurface path, which for a DrawTargetRecording means it creates a SourceSurfaceRecording with a link to the original surface which we can use to draw with Skia:
https://searchfox.org/mozilla-central/rev/09f7e00da9eaef55c80620c17e8575ddd4532451/dom/canvas/ImageBitmap.cpp#870
https://searchfox.org/mozilla-central/rev/09f7e00da9eaef55c80620c17e8575ddd4532451/gfx/2d/DrawTargetRecording.cpp#601

Comment 6

[Andrew Osmond [:aosmond] (he/him)]

Attachment: Bug 1852420 - Ensure surface and crop rect used in ImageBitmap are valid.

When ImageBitmap::PrepareForDrawTarget is called, we must ensure
that the surface returned is acceptable to the given DrawTarget. If
it is not, then we need to recreate the surface from mData. We must
also ensure similar when we are creating a new ImageBitmap from
another ImageBitmap, as we don't know in what context it may be
used.

Additionally, we now no longer clear mPictureRect when we set
mSurface with the cached, cropped data. Since we may need to clear
the cached surface, we need to know the original crop rect.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a34e138c35ad
Ensure surface and crop rect used in ImageBitmap are valid. r=gfx-reviewers,lsalzman

Comment 8

[Andrew Osmond [:aosmond] (he/him)]

I'm going to wontfix this for beta because we've lived with this for a while. The patch could have been simpler but there were a number of similar/related bugs to the surface management I wanted to fix at the same time that would be hard to track. Better for this to ride the trains and consider an uplift to 115 ESR once we make it out to release.

Comment 9

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/a34e138c35ad

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Is this ready for an esr115 approval request? Please nominate if so.

Comment 11

[Andrew Osmond [:aosmond] (he/him)]

Comment on attachment 9352802 [details]
Bug 1852420 - Ensure surface and crop rect used in ImageBitmap are valid.

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Fixes canvas rendering bugs on Windows
• User impact if declined: Broken canvas depending on usage
• Fix Landed on Version: 119
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): No known regressions, been exercised in release for quite some time, changes are fairly localized.

Comment 12

[Donal Meehan [:dmeehan]]

Comment on attachment 9352802 [details]
Bug 1852420 - Ensure surface and crop rect used in ImageBitmap are valid.

Approved for 115.7esr.

Comment 13

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr115/rev/90599daace8d