Bug 71381 – [chromium] Notify scheduler on SwapBuffers in threaded compositor mode

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[James Robinson]

[chromium] Notify scheduler on SwapBuffers in threaded compositor mode

[James Robinson]

Created attachment 113346 [details]
Patch

[Nat Duca]

Recapping the verbal conversation, I think if we move the body of drawLayersAndSwap into scheduledActionDraw, then the proxy will never draw except as directed by the scheduler. At that point, we can make CCScheduler::ProcessCommands call FrameRateControll::didBeginFrame right

Testing this might require that we make FRC fake-able. :/

[James Robinson]

Created attachment 113378 [details]
Patch

[James Robinson]

Is this what you had in mind? The magic number '2' is now more of a magic number but I'm not quite sure where to put it.

[Nat Duca]

Comment on attachment 113378 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=113378&action=review

> Source/WebCore/platform/graphics/chromium/cc/CCThreadProxy.cpp:510
> +    }

What about setting this when the frame rate controller in initializeImplOnImplThread, around line 510. Right after the frameRateController creation, just use m_layerTreeHostImpl->layerRendererCapabilities().

My [informal] goal would be that the CCScheduler doesn't ever have to know how the FrameRateController is configured.

Alternatively, if you're concerned about context-lost case, you could add a frameRateController() accessor on CCScheduler, and then grab it and configure it directly.

[James Robinson]

(In reply to comment #5)
> (From update of attachment 113378 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=113378&action=review
> 
> > Source/WebCore/platform/graphics/chromium/cc/CCThreadProxy.cpp:510
> > +    }
> 
> What about setting this when the frame rate controller in initializeImplOnImplThread, around line 510. Right after the frameRateController creation, just use m_layerTreeHostImpl->layerRendererCapabilities().
> 

Can't do that, the layer renderer isn't initialized at that point.  We initialize the proxy, including creating the frame rate controller and scheduler, before initializing LRC or any GL stuff.

I could change the initialization order around, possibly, but that might be complicated.

[Nat Duca]

> I could change the initialization order around, possibly, but that might be complicated.

Good point. I'll fix that up, b71515. This looks good to go.

[James Robinson]

Ken - mind doing an official review on this now that Nat's looked it over?

[Kenneth Russell]

Comment on attachment 113378 [details]
Patch

Looks fine to me.

[James Robinson]

Comment on attachment 113378 [details]
Patch

Clearing flags on attachment: 113378

Committed r99253: <http://trac.webkit.org/changeset/99253>

[James Robinson]

All reviewed patches have been landed.  Closing bug.