1959648 - After an "offscreen" composite like for view transitions, painting goes blank.

Status: RESOLVED DUPLICATE of bug 1923846

(Core :: Graphics: WebRender, defect)

Description

[Emilio Cobos Álvarez [:emilio]]

I was expecting bug 1959644 to fix all the known flickering with view transitions, but it doesn't. The reason is that once we start suppressing the rendering, we can still paint blank incorrectly.

To reproduce this, apply:

diff --git a/dom/view-transitions/ViewTransition.cpp b/dom/view-transitions/ViewTransition.cpp
index 1b238df22c414..d8f6b958d362a 100644
--- a/dom/view-transitions/ViewTransition.cpp
+++ b/dom/view-transitions/ViewTransition.cpp
@@ -880,7 +880,9 @@ void ViewTransition::Activate() {
     return;
   }

-  mDocument->SetRenderingSuppressedForViewTransitions(false);
+  // Step 2: Set transition's relevant global object's associated document's
+  // rendering suppression for view transitions to false.
+  // mDocument->SetRenderingSuppressedForViewTransitions(false);

   // Step 3: If transition's initial snapshot containing block size is not
   // equal to the snapshot containing block size, then skip the view transition

On top of bug 1959644 (which actually honors that flag for painting).

The expected result is that once you click in the link https://view-transitions.chrome.dev/circle/spa/, the rendering "freezes". Instead, it goes blank.

Right now window resizing bypasses that suppression, so if you resize the window the content appears (and stays there, which is the expected behavior).

I suspect this is an issue with the "offscreen" composite by webrender wiping out some state... Nical, thoughts?

Comment 1

[Emilio Cobos Álvarez [:emilio]]

diff --git a/dom/view-transitions/ViewTransition.cpp b/dom/view-transitions/ViewTransition.cpp
index 1b238df22c414..905d11b9563aa 100644
--- a/dom/view-transitions/ViewTransition.cpp
+++ b/dom/view-transitions/ViewTransition.cpp
@@ -1174,6 +1174,21 @@ Maybe<SkipTransitionReason> ViewTransition::CaptureOldState() {
   for (auto& [f, name] : captureElements) {
     SetCaptured(f, false);
   }
+
+  if (RefPtr<PresShell> ps =
+          nsContentUtils::GetInProcessSubtreeRootDocument(mDocument)
+              ->GetPresShell()) {
+    VT_LOG(
+        "ViewTransitions::CaptureOldState(), requesting on-screen composite");
+    // Build a display list and send it to WR in order to perform the
+    // capturing of old content.
+    RefPtr<nsViewManager> vm = ps->GetViewManager();
+    ps->PaintAndRequestComposite(vm->GetRootView(), PaintFlags::None);
+    VT_LOG(
+        "ViewTransitions::CaptureOldState(), requesting on-screen composite "
+        "end");
+  }
+
   return result;
 }

A workaround like this gives me the expected behavior, but even with that there's some flicker which I suspect is about the snapshots... Afaict everything looks good on the Gecko side...

Comment 2

[Nicolas Silva [:nical]]

Attachment: Bug 1959648 - Pass new_frame_ready parameters via a struct. r=#gfx-reviewers

This patch is a preliminary refactoring to make it simpler to pass additional information to the C++ side and should not affect behavior.
In a followup patch the widget glue will have special handling for frames that need to be rendered but not presented.

Comment 3

[Nicolas Silva [:nical]]

Attachment: Bug 1959648 - Skip compositor-related work when rendering without presenting. r=#gfx-reviewers

Comment 4

[Nicolas Silva [:nical]]

The two attached patches prevent us from doing incorrect stuff at the widget/presentation level, but they are not enough to fix the issue.

What we are seeing here is that the display list of the old state creates snapshots but does not display them (dettached is set to true). We first render the snapshots offscreen without changing the contents of the window, but old-state's display list is now "current" so until it is replaced with a new display list, if anything triggers a frame (for example anything happening in the chrome), the old state's DL will be rendered without its snapshots to the window. In addition I suspect that WR is scheduling a frame after the old state is captured even though it does not need to which should not be a big issue but makes this easier to reproduce.

Setting dettached to false here fixes the issue (we have to set it to false for the old state but not the new state).

I'll move the compositor/widget fixes to another so that we can address the display list part here.

Comment 5

[Phabricator Automation]

Comment on attachment 9479112 [details]
Bug 1959648 - Pass new_frame_ready parameters via a struct. r=#gfx-reviewers

Revision D245564 was moved to bug 1961115. Setting attachment 9479112 [details] to obsolete.

Comment 6

[Phabricator Automation]

Comment on attachment 9479113 [details]
Bug 1959648 - Skip compositor-related work when rendering without presenting. r=#gfx-reviewers

Revision D245565 was moved to bug 1961115. Setting attachment 9479113 [details] to obsolete.

Comment 7

[Nicolas Silva [:nical]]

Attachment: Bug 1959648 - Only dettach stacking contexts of the live capture. r=emilio

For the display list cpaturing the old state should show the contents of the snaphotted stacking contexts in addition to saving them into snapshots.

Comment 8

[BugBot [:suhaib / :marco]]

The severity field is not set for this bug.
:gw, could you have a look please?

For more information, please visit BugBot documentation.

Comment 10

[Nicolas Silva [:nical]]

This will be fixed by the patches in bug 1923846.