Closed Bug 1593311 Opened 5 years ago Closed 5 years ago

Pixel 2 - Flashes when switching animated slides (www.playmag.fr/heroesoftomorrow)

Categories

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

defect

Tracking

()

RESOLVED FIXED

People

(Reporter: jbonisteel, Unassigned)

References

Details

From: https://github.com/mozilla-mobile/fenix/issues/6362

I tested this with WebRender on and off

(Once you force-disable WR you need to go to Settings>Apps to force the app to stop and then re-launch the browser to ensure WR is disabled)

I could reproduce the flashing with WebRender on, but not off.

STR:

On www.playmag.fr/heroesoftomorrow use the arrow buttons in the top nav bar to flip through the different characters and the images will sometimes flicker just after switching. It doesn't always happen on every image.

I will set this as blocking the MVP for now so we can discuss and decide if it should block

Glenn, curious if you have any thoughts about this?

Flags: needinfo?(gwatson)

A reduced test case of this would be helpful unless someone has a great idea of what the problem could be.

Nothing I can think of right now - would it be possible to get a video recording of with/without WR on to see the exact bug here? (otherwise I'll dig my pixel2 device out and try to repro locally this week).

Flags: needinfo?(gwatson)

I can reproduce this on my phone. And I was thinking today that the symptoms seem suspiciously like they could be the release-build-equivalent of the debug assertion I'm running in to trying to get the crashtests running on the emulator: https://searchfox.org/mozilla-central/source/gfx/webrender_bindings/RenderThread.cpp#642

Something like "our texture has been released to early so we have nothing to display for a frame, so it flashes black". Seems plausible. And sure enough I just tested a debug build on the emulator with this page and it hits that assertion when switching character.

Could just be a coincidence, but I think that's a reasonable place to start investigating this.

Sotaro has a fix for the assertion I metioned in comment 4, in bug 1594303.

This one doesn't seem to be the cause of this flickering. But there was another intermittent assertion I was hitting with the webrender reftests which might be the cause. With Sotaro's patch applied, we bypass the first assertion and hit this new one reliably on this page: https://searchfox.org/mozilla-central/source/gfx/webrender_bindings/RenderAndroidSurfaceTextureHostOGL.cpp#61.

This makes quite a lot of sense: in release builds instead of tripping the assertion we will return an Invalid WrExternalImage. The renderer will then use GL texture 0 instead of the actual texture for that frame, which could well manifest as a black flash.

I confirmed the assertion failure at RenderAndroidSurfaceTextureHostOGL::Lock() on my android device during the STR in comment 0.
https://searchfox.org/mozilla-central/source/gfx/webrender_bindings/RenderAndroidSurfaceTextureHostOGL.cpp#61

Depends on: 1594303

The priority flag is not set for this bug.
:jbonisteel, could you have a look please?

For more information, please visit auto_nag documentation.

Flags: needinfo?(jbonisteel)
Flags: needinfo?(jbonisteel)
Priority: -- → P3

This seems to have been fixed by bug 1594303. should we close this bug?

Yup

Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.