1430610 - OMTV causes display list reconstruction with WebRender

Status: RESOLVED FIXED

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

Description

[Markus Stange [:mstange]]

With WebRender, off-main-thread video isn't keeping the main thread as idle as it should be. We cause the display list to be rebuilt all the time, from this stack:

    mozilla::PresShell::ScheduleViewManagerFlush(nsIPresShell::PaintType)
    InvalidateFrameInternal(nsIFrame*, bool)
    nsIFrame::InvalidateLayer(DisplayItemType, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const*, nsRect const*, unsigned int)
    mozilla::dom::HTMLMediaElement::Invalidate(bool, mozilla::Maybe<mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits> >&, bool)
    mozilla::VideoFrameContainer::Invalidate()
    mozilla::MediaDecoder::UpdateLogicalPositionInternal()
    mozilla::WatchManager<mozilla::MediaDecoder>::PerCallbackWatcher::DoNotify()
    mozilla::detail::RunnableMethodImpl<mozilla::WatchManager<mozilla::MediaDecoder>::PerCallbackWatcher*, void (mozilla::WatchManager<mozilla::MediaDecoder>::PerCallbackWatcher::*)(), true, (mozilla::RunnableKind)0>::Run()
    mozilla::AutoTaskDispatcher::DrainDirectTasks()
    mozilla::AutoTaskDispatcher::TaskGroupRunnable::Run()
    mozilla::EventTargetWrapper::Runner::Run()
    mozilla::SchedulerGroup::Runnable::Run()
    nsThread::ProcessNextEvent(bool, bool*)

Without WebRender, nsIFrame::InvalidateLayer has a fast path where it returns early if the frame has a layer that "supports async update":

  // If the layer is being updated asynchronously, and it's being forwarded
  // to a compositor, then we don't need to invalidate.
  if ((aFlags & UPDATE_IS_ASYNC) && layer && layer->SupportsAsyncUpdate()) {
    return layer;
  }

We're not hitting that fast path and are calling SchedulePaintInternal later down in the function.

Comment 1

[Vincent Liu[:vliu]]

Based on Bug 1420430 Comment 4, I will take it for further study.

Comment 2

[Vincent Liu[:vliu]]

(In reply to Vincent Liu[:vliu] from comment #1)
> Based on Bug 1420430 Comment 4, I will take it for further study.

After discussed with Jerry, it lacks of dlbi with wr enabled to be cause the rebuild all the time. I would untake it because it still has some discussions for this.

Comment 3

[Sotaro Ikeda [:sotaro]]

(In reply to Markus Stange [:mstange] from comment #0)
> 
> Without WebRender, nsIFrame::InvalidateLayer has a fast path where it
> returns early if the frame has a layer that "supports async update":
> 

Ah, It seems like a regression of layer free with WebRender. When WebRender uses layers Bug 1359993 reduced the invalidation.

Comment 4

[Sotaro Ikeda [:sotaro]]

Attachment: patch - Change nsIFrame::InvalidateLayer() as to check WebRender's async update

Comment 5

[Sotaro Ikeda [:sotaro]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=d19ef500feb31b94d62eb6bbac6fb3efea6c498a

Comment 6

[Sotaro Ikeda [:sotaro]]

Comment on attachment 8942839 [details] [diff] [review]
patch - Change nsIFrame::InvalidateLayer() as to check WebRender's async update

mstange, can you feedback to the patch?

Comment 7

[Markus Stange [:mstange]]

Comment on attachment 8942839 [details] [diff] [review]
patch - Change nsIFrame::InvalidateLayer() as to check WebRender's async update

This patch seems to address the problem. Thanks!

Comment 8

[Sotaro Ikeda [:sotaro]]

(In reply to Vincent Liu[:vliu] from comment #2)
> (In reply to Vincent Liu[:vliu] from comment #1)
> > Based on Bug 1420430 Comment 4, I will take it for further study.
> 
> After discussed with Jerry, it lacks of dlbi with wr enabled to be cause the
> rebuild all the time. I would untake it because it still has some
> discussions for this.

I am not sure why this bug is related to dlbi. We do not need to think about if  nsIFrame::InvalidateLayer() is called with UPDATE_IS_ASYNC and WebRenderImageData for video already has async image pipeline.

Comment 9

[Sotaro Ikeda [:sotaro]]

Attachment: patch - Change nsIFrame::InvalidateLayer() as to check WebRender's async update

Comment 10

[Sotaro Ikeda [:sotaro]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=52c17765627ae32ba2e503a3e2695dc23d5ea82c

Comment 11

[Nicolas Silva [:nical]]

Comment on attachment 8943088 [details] [diff] [review]
patch - Change nsIFrame::InvalidateLayer() as to check WebRender's async update

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

I don't know this well enough to give a proper review, redirecting to Morris.

Comment 12

[Morris Tseng [:mtseng] [:Morris] (Inactive)]

Comment on attachment 8943088 [details] [diff] [review]
patch - Change nsIFrame::InvalidateLayer() as to check WebRender's async update

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

LGTM

Comment 13

[Sotaro Ikeda [:sotaro]]

Attachment: patch - Change nsIFrame::InvalidateLayer() as to check WebRender's async update

Rebased.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/dac05eadb245
Change nsIFrame::InvalidateLayer() as to check WebRender's async update r=mtseng

Comment 15

[Dorel Luca [:dluca]]

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