Route D3D VSync signal to Compositor

content/browser/compositor/gpu_browser_compositor_output_surface.h

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef CONTENT_BROWSER_COMPOSITOR_GPU_BROWSER_COMPOSITOR_OUTPUT_SURFACE_H_
 #define CONTENT_BROWSER_COMPOSITOR_GPU_BROWSER_COMPOSITOR_OUTPUT_SURFACE_H_
 
 #include <memory>
 
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "build/build_config.h"
 #include "content/browser/compositor/browser_compositor_output_surface.h"
+#include "content/browser/compositor/gpu_vsync_begin_frame_source.h"
 #include "ui/gfx/swap_result.h"
 
 namespace display_compositor {
 class CompositorOverlayCandidateValidator;
 }
 
 namespace gpu {
 class CommandBufferProxyImpl;
 struct GpuProcessHostedCALayerTreeParamsMac;
 }
 
 namespace ui {
 class ContextProviderCommandBuffer;
 class LatencyInfo;
 }
 
 namespace content {
 class ReflectorTexture;
 
 // Adapts a WebGraphicsContext3DCommandBufferImpl into a
 // cc::OutputSurface that also handles vsync parameter updates
 // arriving from the GPU process.
-class GpuBrowserCompositorOutputSurface
-    : public BrowserCompositorOutputSurface {
+class GpuBrowserCompositorOutputSurface : public BrowserCompositorOutputSurface,
+                                          public GpuVSyncControl {
  public:
   GpuBrowserCompositorOutputSurface(
       scoped_refptr<ui::ContextProviderCommandBuffer> context,
       const UpdateVSyncParametersCallback& update_vsync_parameters_callback,
       std::unique_ptr<display_compositor::CompositorOverlayCandidateValidator>
           overlay_candidate_validator);
 
   ~GpuBrowserCompositorOutputSurface() override;
 
   // Called when a swap completion is sent from the GPU process.
@@ skipping 20 matching lines @@
   void Reshape(const gfx::Size& size,
                float device_scale_factor,
                const gfx::ColorSpace& color_space,
                bool has_alpha) override;
   void SwapBuffers(cc::OutputSurfaceFrame frame) override;
   uint32_t GetFramebufferCopyTextureFormat() override;
   bool IsDisplayedAsOverlayPlane() const override;
   unsigned GetOverlayTextureId() const override;
   bool SurfaceIsSuspendForRecycle() const override;
 
+  GpuVSyncControl* GetVSyncControl() override;

[enne (OOO), 2017/01/13 21:14:11]

I don't know that this really needs to be part of the interface here.  It seems
like you can derive from GpuVSyncControl and then just grab this inside of
GpuProcessTransportFactory when the GpuBrowserCompositorOutputSurface is
created.

[stanisc, 2017/01/14 00:32:20]

This was my attempt to simplify the logic in the factory code which creates 7
different types of surface derived from BrowserCompositorOutputSurface. Some of
them would support this new VSync mechanism, and only on OS_WIN, and some won't.
And to make it more complex all newly created instances are immediately assigned
to unique_ptr<BrowserCompositorOutputSurface>. The alternative that I've tried
is to have some code that will assign GpuVSyncControl* in some (3?) of those 7
cases and then the outcome would have to be different depending on whether it
was on OS_WIN or not. And because on creation these instances are assigned to
the base class unique_ptr I needed to either downcast it or use an intermediate
GpuBrowserCompositorOutputSurface* pointer. So the code didn't look pretty. It
is far more straightforward this way. I'll give it another shot.

[stanisc, 2017/01/24 21:23:25]

OK, I've removed this function.

+
+  // GpuVSyncControl
+  void EnableVSync(bool enabled) override;

[enne (OOO), 2017/01/13 21:14:11]

Similarly, EnableVSync doesn't need to be virtual either.  I don't think that's
something any other BrowserCompositorOutputSurface will handle.  Also, is there
some better name than EnableVSync? It's not like we're ignoring vsync in other
cases?

[stanisc, 2017/01/14 00:32:20]

Well, I guess if GpuBrowserCompositorOutputSurface* was passed to
GpuVSyncBeginFrameSource I could avoid  having this method virtua (and needing
GpuVsyncControl interface). I thought that decoupling BFS from the actual
implementation of sending Enable/Disable VSync was a cleaner approach.

Regarding the name, I agree, I've already realized that that would conflict with
a notion of disabling VSync completely via --disable-vsync flag. How about
SetNeedsVSync which is consistent with other similar named methods
(SetNeedsCommit, SetNeedsRedraw, etc)?

[stanisc, 2017/01/24 21:23:25]

I've kept this as virtual for now, mostly for the purpose of decoupling
GpuVSyncControl facet of functionality from the rest of
GpuBrowserCompositorOutputSurface. I could get rid of GpuVSyncControl if I made
GpuVsyncBeginFrameSource depend directly on this class.

+
  protected:
   gpu::CommandBufferProxyImpl* GetCommandBufferProxy();
 
   cc::OutputSurfaceClient* client_ = nullptr;
   std::unique_ptr<ReflectorTexture> reflector_texture_;
   base::WeakPtrFactory<GpuBrowserCompositorOutputSurface> weak_ptr_factory_;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(GpuBrowserCompositorOutputSurface);
 };
 
 }  // namespace content
 
 #endif  // CONTENT_BROWSER_COMPOSITOR_GPU_BROWSER_COMPOSITOR_OUTPUT_SURFACE_H_