1632795 - Black screen when opening GVE or loading new tab in Fenix with webrender enabled

Status: RESOLVED FIXED

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

Description

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

When opening GVE with webrender enabled, the screen is black until something forces it to render, such as navigating to a page. It used to be white. When loading a new tab in fenix I also now see a momentary black flash, that I believe has the same cause.

This was regressed by https://phabricator.services.mozilla.com/D67368, but really we were lucky it worked before then. It relied on very non-explicit behaviour:

RenderBackend::update_document() gets called with render_frame == true fairly quickly on startup. And previously this would result in us calling the new_frame_ready() callback and requesting a composite. There wouldn't be any content to render at this stage, but it would result in a glClear() and swap buffers, leaving the screen white.

However, the above change modifies the texture cache code in such a way that TextureUpdateList::is_nop() now returns true. (I guess we now delay the initial allocation until we actually need it rather than performing it at startup?). This means that we decide to not render the frame here, so the screen remains black until the first actual render.

I think we should add some explicit logic to update_document() to ensure we always request a render if we haven't already done so, rather than relying on unrelated state.

Comment 1

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

I had a thought that maybe it should be WebRenderBridgeParent that decides this frame should be rendered. And then I thought, shouldn't this already be happening? WebRenderBridgeParent::Resume() will be called at start up, which calls ScheduleForcedGenerateFrame() which sends a transaction with invalidate_rendered_frame set, so we end up in RenderBackend::update_document() with invalidate_rendered_frame == true. This will set doc.rendered_frame_is_valid = false. But then we run in to this bug: WRBP schedules a composite, which will result in update_document() being called with render_frame == true. However, because we think the frame is a nop, we set doc.rendered_frame_is_valid = true, which in turn sets render_frame = false, so we don't render the frame.

My question is, do invalidate_rendered_frame and rendered_frame_is_valid actually do anything, if we just override it when we think the frame is a noop? How is it intended to be used? Do we need an extra variable to distinguish between an externally forced invalidation and when webrender believes the frame is valid?

Comment 2

[Glenn Watson [:gw]]

I'm not sure I totally understand what's going on here - it might be easier to discuss on matrix.

It sounds like it is probably a bug in WR, and we should change the code slightly so that if invalidate_rendered_frame is set, we consider that as part of the condition when detecting a no-op. Would that fix this problem?

Comment 3

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

Attachment: Bug 1632795 - Ensure invalidate_rendered_frame is not overridden by the frame being a no-op. r?gw,sotaro

The RenderBackend can be sent an invalidate_rendered_frame flag to
indicate that the current rendered frame is invalid. This is useful
when the platform requires a render, eg when starting or resuming the
app on Android. Upon receiving this flag, the render backend will set
a variable doc.rendered_frame_is_valid = false. Later on it will
decide to skip rendering only if this variable is true, so by setting
the invalidate flag we should be able to ensure the next render will
occur.

However, the RenderBackend also tries to skip renders which it
determines are not required. Currently it does this by setting
doc.rendered_frame_is_valid = true if it decides the frame is a
no-op. This overwrites the previous value set by the
invalidate_rendered_frame flag, meaning webrender skips renderering
even though the platform has requested it.

This was resulting in the GVE app showing a black screen on startup,
and Fenix temporarily showing a black screen whilst opening a new tab,
because despite WebRenderBridgeParent requesting an invalidation
immediately on startup, webrender ignored that request until it
decided it actually had content to paint.

To fix this, the logic should be flipped. The value of
doc.rendered_frame_is_valid must be remembered across document
updates rather than defaulting to false. And instead of setting it
true if webrender thinks the frame is a no-op, we must set it false if
webrender thinks the frame is not a no-op.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2760b8a14f17
Ensure invalidate_rendered_frame is not overridden by the frame being a no-op. r=gw

Comment 5

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/2760b8a14f17