1912211 - Inaccurate texture sampling

Status: RESOLVED FIXED

(Core :: Graphics: CanvasWebGL, defect)

Description

[Jake Archibald]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/127.0.0.0 Safari/537.36

Steps to reproduce:

https://static-misc-3.glitch.me/alpha-video-wc/

This demo takes a double-height video (https://cdn.glitch.global/2f4b4e08-395c-470d-aad5-9ba01f7c4597/split-av1.mp4?v=1722702194835) and uses WebGL to apply the bottom half as an alpha mask to the top half.

Actual results:

The alpha mask appears offset in Firefox 129 for Android (observed on a Pixel 8a and 8 pro).

It rendered fine in Firefox 128 for Android, so this is a recent regression.
It renders fine in Firefox 129 for MacOS.
It renders fine in Safari & Chrome.

Expected results:

The alpha channel should not be offset.

Comment 1

[Jake Archibald]

Attachment: Firefox 128 screenshot

Comment 2

[Jake Archibald]

Attachment: Firefox 129 screenshot

Comment 3

[Jamie Nicol [:jnicol] out of office until 6th Jan]

Thanks for reporting this bug, Jake!

I can reproduce on my Pixel 8. Note to reproduce in geckoview_example I need to set media.geckoview.autoplay.request to false, otherwise nothing is displayed at all. Using mozregression I found this regression range: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=ed59a57d89d8a7cb727c9a02b7bc95e288250b6d&tochange=101b67d76547ca0b75ead3668fa0780d0b0bfef4

I confirmed manually that reverting bug 1880346 resolves the issue.

Comment 4

[Jake Archibald]

Thank you for jumping on this so quickly!

Comment 5

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

:jhlin, since you are the author of the regressor, bug 1880346, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Comment 6

[Jamie Nicol [:jnicol] out of office until 6th Jan]

The issue is that SurfaceTextures provide a transform that should be applied to texture coordinates when rendering from the surface texture. Usually this is just a simple y-flip, but not all of the time. And it varies device to device, decoder to decoder. We handle this correctly in our regular rendering path, but not in GLBlitHelper which is used when uploading a video frame to a webgl texture. We still just hard code a y-flip there, see bug 1811456.

It seems prior to bug 1880346, GLBlitHelper wasn't being used at all. John, is it possible we were software decoding video prior to that? And now we're hardware decoding we are provided with a transform which is not just a y-flip, it also includes a scale and offset. So we must handle that correctly.

Comment 7

[Jamie Nicol [:jnicol] out of office until 6th Jan]

Attachment: Bug 1912211 - Handle SurfaceTexture transform in GLBlitHelper. r?jgilbert

Android SurfaceTextures provide a transform that should be applied to
texture coordinates when rendering. Usually this is simply a y-flip,
but not always. While we handle the transform correctly in our regular
rendering path, GLBlitHelper (used for example when using a video as a
webgl texture) currently hard-codes a y-flip. This patch replaces the
hardcoded y-flip with the provided transform.

Comment 8

[John Lin [:jhlin][:jolin]]

Thanks, Jamie! It's exactly what you said: before bug 1880346 software decoding was used and it didn't involve GLBlitHelper.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

FYI, RC week for Fx130 is next week. If we want to get this landed and uplifted in time, it'll need review pretty quick.

Comment 10

[Jamie Nicol [:jnicol] out of office until 6th Jan]

Attachment: Bug 1912211 - Handle SurfaceTexture transform in GLBlitHelper. r?jgilbert

Android SurfaceTextures provide a transform that should be applied to
texture coordinates when rendering. Usually this is simply a y-flip,
but not always. While we handle the transform correctly in our regular
rendering path, GLBlitHelper (used for example when using a video as a
webgl texture) currently hard-codes a y-flip. This patch replaces the
hardcoded y-flip with the provided transform.

Original Revision: https://phabricator.services.mozilla.com/D218834

Comment 11

[Phabricator Automation]

beta Uplift Approval Request

• User impact if declined: Webgl content using video data may be rendered incorrectly (eg offset or upside down) for some video formats on certain devices
• Code covered by automated testing: yes
• Fix verified in Nightly: no
• Needs manual QE test: no
• Steps to reproduce for manual QE testing: N/A
• Risk associated with taking this patch: Low
• Explanation of risk level: Well understood problem, simple fix that has already been verified in normal (non webgl) rendering path
• String changes made/needed: N/A
• Is Android affected?: yes

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/54746e417c74
Handle SurfaceTexture transform in GLBlitHelper. r=gfx-reviewers,bradwerth

Comment 13

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/54746e417c74

Comment 14

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/5af8ee5e1246