926618 - Fix resize flickering on Linux OMTC

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: blah.patch

nsWindow::OnExpose will perform two composites when the window is resized: the first one to force a paint (bug 915809), and the second after layout has been updated. This results in really weird flickering.

This patch changes OnExpose to only force a composite if WillPaint didn't already schedule one.

Comment 1

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 816772 [details] [diff] [review]
blah.patch

Review of attachment 816772 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/layers/Layers.h
@@ +581,5 @@
> +  void SetNeedsComposite(bool aNeedsComposite)
> +  {
> +    mNeedsComposite = aNeedsComposite;
> +  }
> +  bool NeedsComposite() const { return mNeedsComposite; }

Can't this go in ClientLayerManager.h?

::: gfx/layers/client/ClientLayerManager.cpp
@@ +351,5 @@
>        }
>      }
> +
> +    if (sent)
> +      mNeedsComposite = false;

{}

::: widget/gtk/nsWindow.cpp
@@ +1985,5 @@
> +        !gdk_screen_is_composited(gdk_window_get_screen(mGdkWindow)))
> +    {
> +        // We need to paint to the screen even if nothing changed, since if we
> +        // don't have a compositing window manager, our pixels could be stale.
> +        mLayerManager->SetNeedsComposite(true);

Seems to me you're subverting the intent of the existing code, which was to get *something* on the screen in the resized window ASAP. Now we could be doing expensive painting before anything is composited. Right?

@@ +2008,5 @@
>      }
>  
> +    if (GetLayerManager()->GetBackendType() == LAYERS_CLIENT && mCompositorParent &&
> +        mLayerManager->NeedsComposite())
> +    {

{ on previous line

Comment 2

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: fix

Yeah, it technically means it may take longer to perform the paint. But that hack was added for redrawing on non-compositing WMs when the only thing happening is invalidation (like when windows overlap). So it should be fast. The flickering is pretty awful so the original hack was not good.

FWIW, Chrome seems to keep a backbuffer available to the UI process so it can repaint without the compositor.

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 816798 [details] [diff] [review]
fix

Review of attachment 816798 [details] [diff] [review]:
-----------------------------------------------------------------

OK, I think Matt should look over this.

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to David Anderson [:dvander] from comment #2)
> FWIW, Chrome seems to keep a backbuffer available to the UI process so it
> can repaint without the compositor.

We effectively have that in the layer tree owned by the compositor. That's why scheduling an async composite was helpful. I don't understand why what Chrome is doing is different/better.

Comment 6

[Karl Tomlinson (:karlt)]

(In reply to David Anderson [:dvander] from comment #2)
> Yeah, it technically means it may take longer to perform the paint. But that
> hack was added for redrawing on non-compositing WMs when the only thing
> happening is invalidation (like when windows overlap).

The length of the paint shouldn't be a problem.  So long as the new area is repainted before onexpose returns, I'd expect the _NET_WM_SYNC_REQUEST synchronization to sort things out.

Painting old content and then updated content, however, in the non-resize case at least, however could be an issue with both compositing and non-compositing window managers.

Comment 7

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 816798 [details] [diff] [review]
fix

Review of attachment 816798 [details] [diff] [review]:
-----------------------------------------------------------------

::: widget/gtk/nsWindow.cpp
@@ +2011,5 @@
>              return FALSE;
>      }
>  
> +    if (clientLayers && mCompositorParent && clientLayers->NeedsComposite()) {
> +        mCompositorParent->ScheduleRenderOnCompositorThread();

This does a PostTask to the compositor thread, and then in PaintWindow we send a synchronous IPC message (FlushRendering), are we guaranteed ordering for these two things?

Comment 8

[David Anderson [:dvander] - inactive, e-mail if emergency]

(In reply to Matt Woodrow (:mattwoodrow) from comment #7)
> ::: widget/gtk/nsWindow.cpp
> @@ +2011,5 @@
> >              return FALSE;
> >      }
> >  
> > +    if (clientLayers && mCompositorParent && clientLayers->NeedsComposite()) {
> > +        mCompositorParent->ScheduleRenderOnCompositorThread();
> 
> This does a PostTask to the compositor thread, and then in PaintWindow we
> send a synchronous IPC message (FlushRendering), are we guaranteed ordering
> for these two things?

Theoretically, yes. The compositor's IPC channel will receive the message on the main thread, and post a task to the compositor loop that it should process one message. So the tasks should happen in-order. With some IPDL features you can process multiple messages out of turn with the event loop, but the compositor doesn't use those.

Comment 9

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 816798 [details] [diff] [review]
fix

Review of attachment 816798 [details] [diff] [review]:
-----------------------------------------------------------------

Cool.

Comment 10

[David Anderson [:dvander] - inactive, e-mail if emergency]

https://hg.mozilla.org/integration/mozilla-inbound/rev/110171f903b1

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/110171f903b1