Fix assorted issues with remote CoreAnimation

Fix assorted issues with remote CoreAnimation

These issues came up while running for a few days with the
flag --enable-remote-core-animation.

Fix flashes of old frames by hooking up the DiscardBackbuffer (which
happens when being made non-visible) to re-set the CAContext and
CALayer (so the browser gets a new one with new content for the next
frame).

Add support for disabling vsync by using setNeedsDisplay and
displayIfNeeded pairs to draw.

Add a timeout to un-block the command buffer if the display callback
for a frame never happens, to prevent hangs (we have to do the same
hack for the non-remote CALayer in the browser process, for the
IOSurface path).

Add a ScopedSetGLToRealGLApi structure to ensure that we are talking
to the real GL API while in the CoreAnimation callback.

BUG=312462

[ccameron, 2014-08-26 00:43:45]

ccameron@chromium.org changed reviewers:
+ piman@chromium.org

[ccameron, 2014-08-26 00:48:40]

Slightly-related cluster of issues that came up during testing.

[piman, 2014-08-26 01:44:39]

The timeout makes me *really* sad. Do we not have ways of identifying the cases
that lead to CA not calling us back?
What happens if CA calls us back late (after the timer expired)?

https://codereview.chromium.org/505053002/diff/20001/content/common/gpu/image...
File content/common/gpu/image_transport_surface_calayer_mac.mm (right):

https://codereview.chromium.org/505053002/diff/20001/content/common/gpu/image...
content/common/gpu/image_transport_surface_calayer_mac.mm:174: uint64 
CALayerStorageProvider::SwapBuffersAndGetSurfaceHandle() {
nit: only 1 space beween uint64 and CALayerStorageProvider

[ccameron, 2014-08-26 02:00:55]

On 2014/08/26 01:44:39, piman (OOO) wrote:
> The timeout makes me *really* sad. Do we not have ways of identifying the
cases
> that lead to CA not calling us back?

This makes me very sad too -- I was very unhappy when I had to add it to the
browser's CALayer in [1]. The callback never comes in when the layer isn't
visible (sometimes), so I was hoping that the remote-process version wouldn't
have this issue. It also happens when multiple setNeedsDisplay calls are made in
rapid succession (e.g, with vsync disabled).

> Do we not have ways of identifying the cases that lead to CA not
> calling us back?

So far I haven't been able to figure out a way to do this. I can generally
reproduce this when opening lots of windows in rapid succession, or when opening
and closing tabs in rapid succession.

> What happens if CA calls us back late (after the timer expired)?

Nothing bad happens -- we just draw the frame and keep going. If we've received
a new frame, then the frame that didn't get drawn is skipped, and if we haven't
received a new frame, then we draw the old frame.

I'd like to see if I can find a way to prevent this from happening (especially
needed if we want to do something more complicated than a blit), but the timer
mechanism has proven itself to be reliable (if sadmaking).

[1]
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/re...

https://codereview.chromium.org/505053002/diff/20001/content/common/gpu/image...
File content/common/gpu/image_transport_surface_calayer_mac.mm (right):

https://codereview.chromium.org/505053002/diff/20001/content/common/gpu/image...
content/common/gpu/image_transport_surface_calayer_mac.mm:174: uint64 
CALayerStorageProvider::SwapBuffersAndGetSurfaceHandle() {
On 2014/08/26 01:44:38, piman (OOO) wrote:
> nit: only 1 space beween uint64 and CALayerStorageProvider

Done.

[piman, 2014-08-26 03:09:59]

On Mon, Aug 25, 2014 at 7:00 PM, <ccameron@chromium.org> wrote:

> On 2014/08/26 01:44:39, piman (OOO) wrote:
>
>> The timeout makes me *really* sad. Do we not have ways of identifying the
>>
> cases
>
>> that lead to CA not calling us back?
>>
>
> This makes me very sad too -- I was very unhappy when I had to add it to
> the
> browser's CALayer in [1]. The callback never comes in when the layer isn't
> visible (sometimes), so I was hoping that the remote-process version
> wouldn't
> have this issue. It also happens when multiple setNeedsDisplay calls are
> made in
> rapid succession (e.g, with vsync disabled).
>
>
>  Do we not have ways of identifying the cases that lead to CA not
>> calling us back?
>>
>
> So far I haven't been able to figure out a way to do this. I can generally
> reproduce this when opening lots of windows in rapid succession, or when
> opening
> and closing tabs in rapid succession.
>
>
>  What happens if CA calls us back late (after the timer expired)?
>>
>
> Nothing bad happens -- we just draw the frame and keep going. If we've
> received
> a new frame, then the frame that didn't get drawn is skipped, and if we
> haven't
> received a new frame, then we draw the old frame.
>

As far as I can tell, the backing texture isn't double-buffered. What
prevents LayerDoDraw from being called in the middle of a frame, where the
(next) frame may be half done?

>
> I'd like to see if I can find a way to prevent this from happening
> (especially
> needed if we want to do something more complicated than a blit), but the
> timer
> mechanism has proven itself to be reliable (if sadmaking).
>
> [1]
> https://code.google.com/p/chromium/codesearch#chromium/
> src/content/browser/renderer_host/compositing_iosurface_layer_mac.h&l=106
>
>
>
> https://codereview.chromium.org/505053002/diff/20001/
> content/common/gpu/image_transport_surface_calayer_mac.mm
> File content/common/gpu/image_transport_surface_calayer_mac.mm (right):
>
> https://codereview.chromium.org/505053002/diff/20001/
> content/common/gpu/image_transport_surface_calayer_mac.mm#newcode174
> content/common/gpu/image_transport_surface_calayer_mac.mm:174: uint64
> CALayerStorageProvider::SwapBuffersAndGetSurfaceHandle() {
> On 2014/08/26 01:44:38, piman (OOO) wrote:
>
>> nit: only 1 space beween uint64 and CALayerStorageProvider
>>
>
> Done.
>
> https://codereview.chromium.org/505053002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[ccameron, 2014-08-26 03:27:25]

On 2014/08/26 03:09:59, piman (OOO) wrote:
> On Mon, Aug 25, 2014 at 7:00 PM, <mailto:ccameron@chromium.org> wrote:
> 
> > On 2014/08/26 01:44:39, piman (OOO) wrote:
> >
> >> The timeout makes me *really* sad. Do we not have ways of identifying the
> >>
> > cases
> >
> >> that lead to CA not calling us back?
> >>
> >
> > This makes me very sad too -- I was very unhappy when I had to add it to
> > the
> > browser's CALayer in [1]. The callback never comes in when the layer isn't
> > visible (sometimes), so I was hoping that the remote-process version
> > wouldn't
> > have this issue. It also happens when multiple setNeedsDisplay calls are
> > made in
> > rapid succession (e.g, with vsync disabled).
> >
> >
> >  Do we not have ways of identifying the cases that lead to CA not
> >> calling us back?
> >>
> >
> > So far I haven't been able to figure out a way to do this. I can generally
> > reproduce this when opening lots of windows in rapid succession, or when
> > opening
> > and closing tabs in rapid succession.
> >
> >
> >  What happens if CA calls us back late (after the timer expired)?
> >>
> >
> > Nothing bad happens -- we just draw the frame and keep going. If we've
> > received
> > a new frame, then the frame that didn't get drawn is skipped, and if we
> > haven't
> > received a new frame, then we draw the old frame.
> >
> 
> As far as I can tell, the backing texture isn't double-buffered. What
> prevents LayerDoDraw from being called in the middle of a frame, where the
> (next) frame may be half done?

This has actually been a potential-bug in Mac for a long time (in particular,
there was always a risk that we would need to re-display the IOSurface while
re-painting it in the GPU process). This risk has gone down somewhat with the
transition to CoreAnimation (because we re-paint less often). Somehow, while
this is entirely possible in theory, we've never seen it in practice (I think
that we got some assurance at some point that our placement of glFlush calls
does prevent this in practice).

There may be a workaround for this that doesn't run the above risk (in
particular, transition to setAsynchronous:NO mode on the timeout and then blast
setNeedsDisplay calls in the message loop until one takes) which appears to be
working in prototypes. I have a lot of faith in what's implemented (cause it has
been running for quite some time in the regular CoreAnimation path), so I'd
prefer to do things this way, sad as it may be, then try putting other schemes
on top of this one.

[piman, 2014-08-26 04:24:49]

On Mon, Aug 25, 2014 at 8:27 PM, <ccameron@chromium.org> wrote:

> On 2014/08/26 03:09:59, piman (OOO) wrote:
>
>  On Mon, Aug 25, 2014 at 7:00 PM, <mailto:ccameron@chromium.org> wrote:
>>
>
>  > On 2014/08/26 01:44:39, piman (OOO) wrote:
>> >
>> >> The timeout makes me *really* sad. Do we not have ways of identifying
>> the
>> >>
>> > cases
>> >
>> >> that lead to CA not calling us back?
>> >>
>> >
>> > This makes me very sad too -- I was very unhappy when I had to add it to
>> > the
>> > browser's CALayer in [1]. The callback never comes in when the layer
>> isn't
>> > visible (sometimes), so I was hoping that the remote-process version
>> > wouldn't
>> > have this issue. It also happens when multiple setNeedsDisplay calls are
>> > made in
>> > rapid succession (e.g, with vsync disabled).
>> >
>> >
>> >  Do we not have ways of identifying the cases that lead to CA not
>> >> calling us back?
>> >>
>> >
>> > So far I haven't been able to figure out a way to do this. I can
>> generally
>> > reproduce this when opening lots of windows in rapid succession, or when
>> > opening
>> > and closing tabs in rapid succession.
>> >
>> >
>> >  What happens if CA calls us back late (after the timer expired)?
>> >>
>> >
>> > Nothing bad happens -- we just draw the frame and keep going. If we've
>> > received
>> > a new frame, then the frame that didn't get drawn is skipped, and if we
>> > haven't
>> > received a new frame, then we draw the old frame.
>> >
>>
>
>  As far as I can tell, the backing texture isn't double-buffered. What
>> prevents LayerDoDraw from being called in the middle of a frame, where the
>> (next) frame may be half done?
>>
>
> This has actually been a potential-bug in Mac for a long time (in
> particular,
> there was always a risk that we would need to re-display the IOSurface
> while
> re-painting it in the GPU process). This risk has gone down somewhat with
> the
> transition to CoreAnimation (because we re-paint less often). Somehow,
> while
> this is entirely possible in theory, we've never seen it in practice (I
> think
> that we got some assurance at some point that our placement of glFlush
> calls
> does prevent this in practice).
>

I don't think flushes help - on the contrary, they would cause an
interruption in the command buffers, possibly allowing CA to squeeze in a
draw in the middle of a frame.


> There may be a workaround for this that doesn't run the above risk (in
> particular, transition to setAsynchronous:NO mode on the timeout and then
> blast
> setNeedsDisplay calls in the message loop until one takes) which appears
> to be
> working in prototypes. I have a lot of faith in what's implemented (cause
> it has
> been running for quite some time in the regular CoreAnimation path), so I'd
> prefer to do things this way, sad as it may be, then try putting other
> schemes
> on top of this one.
>

What I'd suggest is to keep the old texture around until we produced a new
one. That way you always have a correct texture ready to display. You could
limit that to the cases where you know you requested a draw but gave up
waiting for it (timeout or disable-vsync) to mitigate the memory
consumption if that's an issue.

>
> https://codereview.chromium.org/505053002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[ccameron, 2014-08-26 19:57:55]

On 2014/08/26 04:24:49, piman (OOO) wrote:
> On Mon, Aug 25, 2014 at 8:27 PM, <mailto:ccameron@chromium.org> wrote:
> 
> > On 2014/08/26 03:09:59, piman (OOO) wrote:
> >
> >  On Mon, Aug 25, 2014 at 7:00 PM, <mailto:ccameron@chromium.org> wrote:
> >>
> >
> >  > On 2014/08/26 01:44:39, piman (OOO) wrote:
> >> >
> >> >> The timeout makes me *really* sad. Do we not have ways of identifying
> >> the
> >> >>
> >> > cases
> >> >
> >> >> that lead to CA not calling us back?
> >> >>
> >> >
> >> > This makes me very sad too -- I was very unhappy when I had to add it to
> >> > the
> >> > browser's CALayer in [1]. The callback never comes in when the layer
> >> isn't
> >> > visible (sometimes), so I was hoping that the remote-process version
> >> > wouldn't
> >> > have this issue. It also happens when multiple setNeedsDisplay calls are
> >> > made in
> >> > rapid succession (e.g, with vsync disabled).
> >> >
> >> >
> >> >  Do we not have ways of identifying the cases that lead to CA not
> >> >> calling us back?
> >> >>
> >> >
> >> > So far I haven't been able to figure out a way to do this. I can
> >> generally
> >> > reproduce this when opening lots of windows in rapid succession, or when
> >> > opening
> >> > and closing tabs in rapid succession.
> >> >
> >> >
> >> >  What happens if CA calls us back late (after the timer expired)?
> >> >>
> >> >
> >> > Nothing bad happens -- we just draw the frame and keep going. If we've
> >> > received
> >> > a new frame, then the frame that didn't get drawn is skipped, and if we
> >> > haven't
> >> > received a new frame, then we draw the old frame.
> >> >
> >>
> >
> >  As far as I can tell, the backing texture isn't double-buffered. What
> >> prevents LayerDoDraw from being called in the middle of a frame, where the
> >> (next) frame may be half done?
> >>
> >
> > This has actually been a potential-bug in Mac for a long time (in
> > particular,
> > there was always a risk that we would need to re-display the IOSurface
> > while
> > re-painting it in the GPU process). This risk has gone down somewhat with
> > the
> > transition to CoreAnimation (because we re-paint less often). Somehow,
> > while
> > this is entirely possible in theory, we've never seen it in practice (I
> > think
> > that we got some assurance at some point that our placement of glFlush
> > calls
> > does prevent this in practice).
> >
> 
> I don't think flushes help - on the contrary, they would cause an
> interruption in the command buffers, possibly allowing CA to squeeze in a
> draw in the middle of a frame.
> 
> 
> > There may be a workaround for this that doesn't run the above risk (in
> > particular, transition to setAsynchronous:NO mode on the timeout and then
> > blast
> > setNeedsDisplay calls in the message loop until one takes) which appears
> > to be
> > working in prototypes. I have a lot of faith in what's implemented (cause
> > it has
> > been running for quite some time in the regular CoreAnimation path), so I'd
> > prefer to do things this way, sad as it may be, then try putting other
> > schemes
> > on top of this one.
> >
> 
> What I'd suggest is to keep the old texture around until we produced a new
> one. That way you always have a correct texture ready to display. You could
> limit that to the cases where you know you requested a draw but gave up
> waiting for it (timeout or disable-vsync) to mitigate the memory
> consumption if that's an issue.
> 
> >
> > https://codereview.chromium.org/505053002/
> >
> 
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

Hmm, this is proving more difficult than I'd expected -- blocking the GPU
process (by de-scheduling the command buffer) can cause hangs and deadlocks in
various places. I'm going to see if I can move the backpressure to the same
place as the IOSurface path (in the browser process). Closing this out for now.