Closed
Bug 926618
Opened 11 years ago
Closed 11 years ago
Fix resize flickering on Linux OMTC
Categories
(Core :: Graphics, defect)
Tracking
()
RESOLVED
FIXED
mozilla27
People
(Reporter: dvander, Assigned: dvander)
References
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
mattwoodrow
:
review+
roc
:
feedback+
|
Details | Diff | Splinter Review |
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.
Attachment #816772 -
Flags: review?(roc)
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
Attachment #816772 -
Flags: review?(roc) → review-
Assignee | ||
Comment 2•11 years ago
|
||
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.
Attachment #816772 -
Attachment is obsolete: true
Attachment #816798 -
Flags: review?(roc)
Comment on attachment 816798 [details] [diff] [review] fix Review of attachment 816798 [details] [diff] [review]: ----------------------------------------------------------------- OK, I think Matt should look over this.
Attachment #816798 -
Flags: review?(roc)
Attachment #816798 -
Flags: review?(matt.woodrow)
Attachment #816798 -
Flags: feedback+
(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•11 years ago
|
||
(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•11 years ago
|
||
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?
Assignee | ||
Comment 8•11 years ago
|
||
(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•11 years ago
|
||
Comment on attachment 816798 [details] [diff] [review] fix Review of attachment 816798 [details] [diff] [review]: ----------------------------------------------------------------- Cool.
Attachment #816798 -
Flags: review?(matt.woodrow) → review+
Assignee | ||
Comment 10•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/110171f903b1
https://hg.mozilla.org/mozilla-central/rev/110171f903b1
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
Updated•4 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•