1663732 - Set initial loading content page color to non-white when using dark mode

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, --, P2)

Description

[Chenxia Liu [:liuche]]

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

Why/User Benefit/User Problem

Currently opening a new page temporarily shows a pure white page before loading actual content regardless of the browser theme.

What/Requirements

Fenix does not give a way to control this while on Windows this can be controlled by browser.in-content.dark-mode pref (which is on by default on Nightly 71 and makes the page gray).

The white flash is the page clear color, which Fenix can already set clear color using GV API CompositorController.setClearColor.

Fenix clear color should probably change the clear color depending on Fenix's or Android's dark mode settings.

This bug was originally filed in Bugzilla:
https://bugzilla.mozilla.org/show_bug.cgi?id=1589576

Acceptance Criteria (how do I know when I’m done?)

Opening a new page when dark mode is enabled does not flash a pure white page before loading actual content.

Comment 1

[Emily Kager [:ekager]]

Originally we were looking into using CompositorController.setClearColor but after chatting with the GV team on Riot and they let me knw this doesn't work with WR anymore. I tried a workaround to keep the GV GONE until we get the first contentful paint callback, but it doesn't seem to be super reliable :/ and I haven't had time to keep messing with it since then.
It would be ideal to be able to have a similar GV API to CompositorController.setClearColor that would work with WR in the same way (ie we can pass in dark/light to the GV settings).

Comment 2

[James Willcox (:snorp) ([e-mail]) (he/him)]

We need to make Compositor::SetDefaultClearColor() work with WR.

Comment 3

[Jamie Nicol [:jnicol]]

Sorry this hasn't been implemented in webrender yet, I wasn't aware of it being required, and only vaguely aware of it even existing for the layers compositor.

Digging in to how this works for layers, UiCompositorControllerParent::RecvDefaultClearColor() calls Compositor::SetDefaultClearColor(). Then in LayerManagerComposite::Render() we check if we have a root layer. If we do, we call ScrollMetadata::GetBackgroundColor() which I think basically gives us the page's background colour, and call Compositor::SetClearColor() with that. If we have no root layer, then we call Compositor::SetClearColorToDefault() which sets it to the value geckoview has set. Then in CompositorOGL::BeginFrame() we call glClearColor() with the selected color. (There is also another path here where we use a clear color override pref, which appears to be for VR and we can probably ignore for now.)

So it seems like implementing the default clear color in webrender will be enough to prevent these flashes, but additionally overriding that with the ScrollMetadata's background colour will give us a better colour, I think during page load, for example.

The current situation in webrender, is that the clear color is decided by the Renderer::clear_color field, which is set at initialization time by RendererOptions::clear_color. We set this to white on android, and black on other platforms. I imagine we can probably just add an API to set this at runtime, and hook it up to UiCompositorControllerParent::RecvDefaultClearColor() via WebRenderBridgeParent.

Webrender also allows a clear color to be set when sending a new display list, which gets passed through scene building and ends up in the BuiltScene structure. It used to then be set as a property of the Frame and was in use until bug 1441308, then was later removed in bug 1571971. So currently setting the display list's clear color appears to do nothing. It seems like it was used as a fallback if Renderer::clear_color was None, but perhaps we could instead add it back and use it to override the renderer's clear color, to handle the case where we get the background color from the FrameMetrics.

But to start with we can just do the default clear color and see how far that gets us.

Comment 4

[Jamie Nicol [:jnicol]]

Also, I've been doing some testing. I'm using layers rather than webrender, and I hard coded it so that we always clear with red so that it's easy to see. I still see black and white (as well as some red) flashes when navigating around Fenix. At least one source of these is here - if you set that color to blue then you see a mixture of black, white, red, and blue flashes. I'm not sure whether the remaining ones come from geckoview or fenix.

Snorp, maybe geckoview should hook up the color from setClearColor() to be used for coverUntilFirstPaint() too?

Comment 5

[Jamie Nicol [:jnicol]]

Attachment: Bug 1663732 - Hook up GeckoView's CompositorController.setClearColor() to webrender. r?nical

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c4b987da7cfc
Hook up GeckoView's CompositorController.setClearColor() to webrender. r=nical

Comment 7

[James Willcox (:snorp) ([e-mail]) (he/him)]

(In reply to Jamie Nicol [:jnicol] from comment #4)

Snorp, maybe geckoview should hook up the color from setClearColor() to be used for coverUntilFirstPaint() too?

Yeah, I think coverUntilFirstPaint() should just call setClearColor() as well. I filed Bug 1665068.

Comment 8

[Alexandru Michis [:malexandru]]

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