Clear out pending SwapBufferComplete count when activating the compositor.

Clear out pending SwapBufferComplete count when activating the compositor.
Any SBC's that are still pending are from a previous page / compositor tree
and should be ignored.
This will help avoid flashes (compositing of page contents before they are ready)
when navigating from one url to another.
 
BUG=153452

[Vangelis Kokkevis, 2012-10-03 01:35:35]

I decided to clear this out on activation of the compositor rather than
de-activation in case the page load that does activate the compositor takes a
while and we do need to actually paint and ack the previous page in the
meantime.

The theory is that even if a swapbufferscomplete callback wedges itself between
the de-activation and the activation, we will still early out since accelerated
compositing will be off.

[jamesr, 2012-10-03 16:39:13]

On 2012/10/03 01:35:35, Vangelis Kokkevis wrote:
> I decided to clear this out on activation of the compositor rather than
> de-activation in case the page load that does activate the compositor takes a
> while and we do need to actually paint and ack the previous page in the
> meantime.

In force-compositing-mode, the deactivate and activate calls happen under the
same callstack with no significant processing in between.  We don't wait until
load to activate the compositor, and we don't deactivate the compositor for the
previous page early.

> 
> The theory is that even if a swapbufferscomplete callback wedges itself
between
> the de-activation and the activation, we will still early out since
accelerated
> compositing will be off.

Not possible in force-compositing-mode - the deactivate and activate call are in
the same runloop invocation.

Without force-compositing-mode things get a bit more interesting - it's possible
that there is a significant gap between the deactivate and the activate. 
However, it never makes sense to produce a frame from a SwapBuffersComplete
callback if we aren't in compositing mode when we get the call.

Another thought about this I had was if we are swap-limited (say it's a
gpu-heavy page) and we navigate to another swap-limited page that starts putting
frames in before we get acks this will effectively put us 4 swaps back instead
of 2.  I think in this case the command buffer itself will eventually block us
from inserting more swaps into the stream, but we should probably verify this.

[Vangelis Kokkevis, 2012-10-03 22:07:08]

On 2012/10/03 16:39:13, jamesr wrote:
> On 2012/10/03 01:35:35, Vangelis Kokkevis wrote:
> > I decided to clear this out on activation of the compositor rather than
> > de-activation in case the page load that does activate the compositor takes
a
> > while and we do need to actually paint and ack the previous page in the
> > meantime.
> 
> In force-compositing-mode, the deactivate and activate calls happen under the
> same callstack with no significant processing in between.  We don't wait until
> load to activate the compositor, and we don't deactivate the compositor for
the
> previous page early.
> 
> > 
> > The theory is that even if a swapbufferscomplete callback wedges itself
> between
> > the de-activation and the activation, we will still early out since
> accelerated
> > compositing will be off.
> 
> Not possible in force-compositing-mode - the deactivate and activate call are
in
> the same runloop invocation.
> 
> Without force-compositing-mode things get a bit more interesting - it's
possible
> that there is a significant gap between the deactivate and the activate. 
> However, it never makes sense to produce a frame from a SwapBuffersComplete
> callback if we aren't in compositing mode when we get the call.
>

Thanks.  I was actually thinking about the non-fcm case where there could be an
appreciable gap between the de-activate and the activate. If we zero out the
pending SBC's on de-activate we may miss ack-ing back input events and doing
repaints while the next page is still getting loaded / painted.  Not sure if
that's a big deal or not.

Also, we won't produce a frame as there's already a check that we're indeed in
accelerated_compositing mode in the SBC callback (line 532).
 
> Another thought about this I had was if we are swap-limited (say it's a
> gpu-heavy page) and we navigate to another swap-limited page that starts
putting
> frames in before we get acks this will effectively put us 4 swaps back instead
> of 2.  I think in this case the command buffer itself will eventually block us
> from inserting more swaps into the stream, but we should probably verify this.

I doubt this could easily happen in practice.  Load and repaint times should
generally be longer than a typical long frame on the gpu so we should have
enough time to flush the pipeline. And even in the event that we do have 2 swaps
pending from the previous page, we still want to put in the new page ASAP. We
won't gain much by waiting for a SBC callback from the previous page.

[jamesr, 2012-10-16 04:15:23]

On 2012/10/03 22:07:08, Vangelis Kokkevis wrote:
> On 2012/10/03 16:39:13, jamesr wrote:
> > On 2012/10/03 01:35:35, Vangelis Kokkevis wrote:
> > > I decided to clear this out on activation of the compositor rather than
> > > de-activation in case the page load that does activate the compositor
takes
> a
> > > while and we do need to actually paint and ack the previous page in the
> > > meantime.
> > 
> > In force-compositing-mode, the deactivate and activate calls happen under
the
> > same callstack with no significant processing in between.  We don't wait
until
> > load to activate the compositor, and we don't deactivate the compositor for
> the
> > previous page early.
> > 
> > > 
> > > The theory is that even if a swapbufferscomplete callback wedges itself
> > between
> > > the de-activation and the activation, we will still early out since
> > accelerated
> > > compositing will be off.
> > 
> > Not possible in force-compositing-mode - the deactivate and activate call
are
> in
> > the same runloop invocation.
> > 
> > Without force-compositing-mode things get a bit more interesting - it's
> possible
> > that there is a significant gap between the deactivate and the activate. 
> > However, it never makes sense to produce a frame from a SwapBuffersComplete
> > callback if we aren't in compositing mode when we get the call.
> >
> 
> Thanks.  I was actually thinking about the non-fcm case where there could be
an
> appreciable gap between the de-activate and the activate. If we zero out the
> pending SBC's on de-activate we may miss ack-ing back input events and doing
> repaints while the next page is still getting loaded / painted.  Not sure if
> that's a big deal or not.
> 

I don't think that's possible - when exiting compositing mode we immediately
generate a normal paint invalidation and don't rate-limit on swap acks any more,
so we can't miss a frame that way.

Overall I think clearing on deactivate is simpler to understand but will produce
the same end result.

LGTM

[Vangelis Kokkevis, 2012-11-05 20:01:19]

On 2012/10/16 04:15:23, jamesr wrote:
> On 2012/10/03 22:07:08, Vangelis Kokkevis wrote:
> > On 2012/10/03 16:39:13, jamesr wrote:
> > > On 2012/10/03 01:35:35, Vangelis Kokkevis wrote:
> > > > I decided to clear this out on activation of the compositor rather than
> > > > de-activation in case the page load that does activate the compositor
> takes
> > a
> > > > while and we do need to actually paint and ack the previous page in the
> > > > meantime.
> > > 
> > > In force-compositing-mode, the deactivate and activate calls happen under
> the
> > > same callstack with no significant processing in between.  We don't wait
> until
> > > load to activate the compositor, and we don't deactivate the compositor
for
> > the
> > > previous page early.
> > > 
> > > > 
> > > > The theory is that even if a swapbufferscomplete callback wedges itself
> > > between
> > > > the de-activation and the activation, we will still early out since
> > > accelerated
> > > > compositing will be off.
> > > 
> > > Not possible in force-compositing-mode - the deactivate and activate call
> are
> > in
> > > the same runloop invocation.
> > > 
> > > Without force-compositing-mode things get a bit more interesting - it's
> > possible
> > > that there is a significant gap between the deactivate and the activate. 
> > > However, it never makes sense to produce a frame from a
SwapBuffersComplete
> > > callback if we aren't in compositing mode when we get the call.
> > >
> > 
> > Thanks.  I was actually thinking about the non-fcm case where there could be
> an
> > appreciable gap between the de-activate and the activate. If we zero out the
> > pending SBC's on de-activate we may miss ack-ing back input events and doing
> > repaints while the next page is still getting loaded / painted.  Not sure if
> > that's a big deal or not.
> > 
> 
> I don't think that's possible - when exiting compositing mode we immediately
> generate a normal paint invalidation and don't rate-limit on swap acks any
more,
> so we can't miss a frame that way.
> 
> Overall I think clearing on deactivate is simpler to understand but will
produce
> the same end result.
> 
> LGTM

James, is this CL still useful after the latest refactoring you've done for
switching between software and composited modes?

[jamesr, 2012-11-05 20:17:14]

On 2012/11/05 20:01:19, Vangelis Kokkevis wrote:
> On 2012/10/16 04:15:23, jamesr wrote:> > LGTM
> 
> James, is this CL still useful after the latest refactoring you've done for
> switching between software and composited modes?

No, I don't think this is needed any more - if we get a SwapComplete ack while
compositing is not active we just ignore it.

[Vangelis Kokkevis, 2012-11-05 21:30:30]

On 2012/11/05 20:17:14, jamesr wrote:
> On 2012/11/05 20:01:19, Vangelis Kokkevis wrote:
> > On 2012/10/16 04:15:23, jamesr wrote:> > LGTM
> > 
> > James, is this CL still useful after the latest refactoring you've done for
> > switching between software and composited modes?
> 
> No, I don't think this is needed any more - if we get a SwapComplete ack while
> compositing is not active we just ignore it.

Ok, thanks.  Closing it then.