Implement SurfaceHolder.Callback2.surfaceRedrawNeededAsync
Categories
(GeckoView :: General, task, P3)
Tracking
(Not tracked)
People
(Reporter: jnicol, Unassigned)
References
(Blocks 1 open bug)
Details
Recently I stumbled across SurfaceHolder.Callback2, which I think could be used to solve a longstanding issue we have: sometimes when resuming the app we see a brief flash of black or transparent (the phone's wallpaper) in place of our GeckoView Surface, before the compositor has a chance to render the page.
These functions get called following an onSurfaceChanged(). In the original version of this function you don't return until after you have rendered to the surface. In the async version (which is available from Android 8.0 onwards) you can return immediately to free up the UI thread, and then you call the provided Runnable when you have rendered to the surface. The system then ensures it doesn't composite the surface until it contains valid content.
Implementing the non-async version might be tricky, but the async version should be straightforward: pass the runnable down to GeckoSession, then have it run the runnable in response to a FIRST_PAINT message. This will solve the problem on Android versions 8 and above, which is seems worthwhile even if we can't easily fix it for everyone.
|
Reporter | |
Comment 1•3 years ago
|
||
Hey, it works!
without patch: https://drive.google.com/file/d/1HH_O9Myp4vefuEI8910EaP0UPiiH7MgH/view?usp=sharing
(notice the flash of the wallpaper while the app is restoring)
with patch: https://drive.google.com/file/d/1hbSOUtGMAXmQhOlG-DtabCtLwxhVyb-J/view?usp=sharing
Unfortunately it doesn't work in Fenix. When the app is restored in Fenix, there is no Session attached to the GeckoView, so we cannot hook this callback up. This seems to be due to this call which was introduced in this bug.
|
|
Reporter | |
Comment 2•3 years ago
|
||
We might be able to implement the synchronous version of this if we were somehow able to block the android UI thread whilst polling the gecko event loop, so that UiCompositorControllerChild could still receive the FIRST_PAINT message. Do you know if doing something like that is possible, Agi?
|
||
Comment 3•3 years ago
|
||
FYI we have a mobile all hands this week, but I'll try to respond to this soon.
|
|
Reporter | |
Comment 4•3 years ago
|
||
No rush, this is just something I'm planning to chip away at in the background!
|
|
||
Comment 5•3 years ago
|
||
(In reply to Jamie Nicol [:jnicol] from comment #2)
We might be able to implement the synchronous version of this if we were somehow able to block the android UI thread whilst polling the gecko event loop, so that UiCompositorControllerChild could still receive the FIRST_PAINT message. Do you know if doing something like that is possible, Agi?
I would really try to avoid this if at all possible. Blocking the UI thread has really bad performance consequences especially if the app is also trying to paint something else at the same time (e.g. at startup). A black flash would be a much better fallback rather than waiting.
That being said, this change is awesome! That black flash has bugged me for the longest time.
|
||
Updated•3 years ago
|
|
||
Comment 6•8 months ago
•
|
||
(In reply to Jamie Nicol [:jnicol] from comment #0)
[...] which I think could be used to solve a longstanding issue we have: sometimes when resuming the app we see a brief flash of black or transparent (the phone's wallpaper) [...]
(In reply to Jamie Nicol [:jnicol] from comment #1)
(notice the flash of the wallpaper while the app is restoring)
I filed bug 1899648 on this symptom today, FWIW. Thanks to jnicol for noticing the connection to this older issue.
I've marked the new bug as a dependency of this bug (possibly/hopefully fixed-by-this-bug).
Description
•