Closed Bug 1751205 Opened 4 years ago Closed 4 years ago

Display of OffscreenCanvas is Y-flipped

Categories

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

Product:
Core
Component:
Graphics: Canvas2D
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
98 Branch
Tracking Flags:
Tracking Status
firefox98 --- fixed

People

(Reporter: aosmond, Assigned: aosmond)

References

Details

Attachments

(2 files)

Bug 1751205 - Part 1. Add Swizzle/PremultiplyYFlipData helper methods.
48 bytes, text/x-phabricator-request
Details | Review
Bug 1751205 - Part 2. Ensure we y-flip surfaces with OffscreenCanvas if needed.
48 bytes, text/x-phabricator-request
Details | Review

When we hit the path that we are pulling pixels out of the WebGL context for display instead of being able to use the system specific buffers, we actually forget that we need to do a Y flip since we don't use the texture flags:

https://searchfox.org/mozilla-central/rev/c3cbf7e56630d12443459a6bd7938358c231ce3b/dom/canvas/OffscreenCanvasDisplayHelper.cpp#122

This can be reproduced by enabled OffscreenCanvas, disabling the webcompat intervention, and joining a Zoom call from Firefox.

These new methods do both swizzle/premultiply and a Y-flip operation.

On the path that we need to read the pixels from the canvas for display
purposes, instead of using a platform buffer handle, we need to take
into account the need to y-flip for WebGL. This patch ensures that we do
so.

Depends on D136503

Blocks: 1751721
Pushed by aosmond@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/2a1e5a439a3b Part 1. Add Swizzle/PremultiplyYFlipData helper methods. r=gfx-reviewers,lsalzman,jgilbert https://hg.mozilla.org/integration/autoland/rev/211ffc98eb32 Part 2. Ensure we y-flip surfaces with OffscreenCanvas if needed. r=gfx-reviewers,jgilbert

https://hg.mozilla.org/mozilla-central/rev/2a1e5a439a3b
https://hg.mozilla.org/mozilla-central/rev/211ffc98eb32

Status: NEW → RESOLVED
Closed: 4 years ago
status-firefox98: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 98 Branch

It appears to be failing only for the reftest-snapshot variant. Very interesting.

We are hitting this failure condition:

https://searchfox.org/mozilla-central/rev/d5113f9c616b75b7919916f5fe4b4f8f78191ac9/dom/canvas/OffscreenCanvasDisplayHelper.cpp#200

I think the child ID was legitimately 0. We should just use Maybe instead of a sentinel value.

Actually it was not legitimately zero. The main thread context on Linux doesn't go through IPC, and instead runs in the content process. It isn't backed by a WebGLChild so it doesn't go through CanvasManagerParent. Instead we go on this path:

https://searchfox.org/mozilla-central/rev/d5113f9c616b75b7919916f5fe4b4f8f78191ac9/dom/canvas/ClientWebGLContext.cpp#654

This is a bit annoying that we have both main thread in content process, and off main thread in compositor process situations for OffscreenCanvas.

I don't understand why this passes on regular reftests.

I think to be consistent we should actually punt main thread contexts through the parent process. That way OffscreenCanvas always works the same and main thread testing is closer to worker thread testing.

For now, I think we can probably just go to the HTMLCanvasElement to get the OffscreenCanvas object. If it is on the main thread, then we will have access to the nsICanvasRenderingContextInternal. From there, we can get the snapshot directly. This only works for the main thread case, but that is all we need it to work for. I'll have to rethink how this should work.

Flags: needinfo?(aosmond)
Pushed by aosmond@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/e97271d90795 Part 1. Add Swizzle/PremultiplyYFlipData helper methods. r=gfx-reviewers,lsalzman,jgilbert https://hg.mozilla.org/integration/autoland/rev/70e5f13d08cc Part 2. Ensure we y-flip surfaces with OffscreenCanvas if needed. r=gfx-reviewers,jgilbert

Backed out for causing multiple wpt failures on offscreencanvas.resize.html

Backout link

Push with failures

Failure log crash // Failure log timeout

Failure line crash: PROCESS-CRASH | /html/canvas/offscreen/manual/the-offscreen-canvas/offscreencanvas.resize.html | application crashed [@ mozilla::gl::AndroidSharedBlitGL::Blit(mozilla::jni::Ref<mozilla::java::GeckoSurfaceTexture, _jobject*> const&, mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits> const&)]

Failure line timeout: TEST-UNEXPECTED-TIMEOUT | /html/canvas/offscreen/manual/the-offscreen-canvas/offscreencanvas.resize.html | TestRunner hit external timeout (this may indicate a hang)

Flags: needinfo?(aosmond)

After discussing this with jnicol, it seems likely this test case won't work on Android. We are displaying some of the canvases in html/canvas/offscreen/manual/the-offscreen-canvas/offscreencanvas.resize.html and then we try to readback the pixels. Apparently this won't work on Android due to platform limitations.

https://searchfox.org/mozilla-central/rev/7056a708787621758bef9793a93aa7ca8375eeef/gfx/gl/AndroidSurfaceTexture.cpp#66

For now, I may just skip this test on Android and we can see if we can do better in a follow up.

Flags: needinfo?(aosmond)
Pushed by aosmond@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/659ee118b977 Part 1. Add Swizzle/PremultiplyYFlipData helper methods. r=gfx-reviewers,lsalzman,jgilbert https://hg.mozilla.org/integration/autoland/rev/1ce43bc56fee Part 2. Ensure we y-flip surfaces with OffscreenCanvas if needed. r=gfx-reviewers,jgilbert

https://hg.mozilla.org/mozilla-central/rev/659ee118b977
https://hg.mozilla.org/mozilla-central/rev/1ce43bc56fee

Status: REOPENED → RESOLVED
Closed: 4 years ago → 4 years ago
status-firefox98: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 98 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: