Bug 99592 – Add WebGraphicsContext3D::WebGraphicsUpdateVSyncTimeCallbackCHROMIUM

• Component: New Bugs
• Status: RESOLVED WONTFIX

[Ali Juma]

Created attachment 169172 [details]
Patch

This is needed for Chromium issue 143466 (http://crbug.com/143466).

[Brian Anderson]

Comment on attachment 169172 [details]
Patch

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

> Source/Platform/chromium/public/WebGraphicsContext3D.h:147
> +        virtual void onUpdateVSyncTime(int64_t) = 0;

Can you use a more descriptive variable name so we know what the units are?
Do you need two arguments (one for phase and one for frequency)?

Also, other parts of the code have started moving towards base::TimeTicks and base::TimeDeltas.  I'm not sure those are allowed in WebGraphicsContext3D.h though.  If they are, we should use those instead.

[Nat Duca]

Comment on attachment 169172 [details]
Patch

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

> Source/Platform/chromium/public/WebGraphicsContext3D.h:145
> +    class WebGraphicsUpdateVSyncTimeCallbackCHROMIUM {

why do you need this? Can't you use the output surface system?

[Ali Juma]

(In reply to comment #2)
> (From update of attachment 169172 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=169172&action=review
> 
> > Source/Platform/chromium/public/WebGraphicsContext3D.h:145
> > +    class WebGraphicsUpdateVSyncTimeCallbackCHROMIUM {
> 
> why do you need this? Can't you use the output surface system?

One big advantage of the new approach is that it works without modification for sending vsync updates to renderers on desktop Linux (whenever OML_GLX_sync_control is available). The downside of adapting this approach to go through the existing path is that the result will be Aura-specific.

To avoid having two different paths for updating vsync state, we could move the existing code to the new path. This would mean that the GPU process would always be responsible for pushing out vsync updates to all compositors; in situations (as we currently have) where the browser process is the original source of this information, the browser would send the information to the GPU process and the GPU process would pass it on to renderers. Thoughts on this?


(In reply to comment #1)
> (From update of attachment 169172 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=169172&action=review
> 
> > Source/Platform/chromium/public/WebGraphicsContext3D.h:147
> > +        virtual void onUpdateVSyncTime(int64_t) = 0;
> 
> Do you need two arguments (one for phase and one for frequency)?

Yes, there should also be an argument for frequency (but note that OML_GLX_sync_control is currently not providing frequency information on Chrome OS devices).

[Nat Duca]

Linux platforms are the only platforms where vsync originates in the gpu process. Everywhere else, its available in the browser process. So, I see the linuxes plumbing as a special case.

We dont have a wgc3d in software mode, but we do have vsync. So, the use of outputsurface has to stay.

[Nat Duca]

Also, the gpu process pushing vsync to compositors adds additional hops in the propagation of the signal on platforms where the signal is directly available. These hops have queuing delays, which while short in the minimum case, are very noisy and can often have 5ms bursts in them. No good.

[Ali Juma]

This is no longer needed for Chromium issue 143466 since we're going to use the existing output surface system instead.