Mac Overlays: Add GPU back-pressure

content/common/gpu/image_transport_surface_overlay_mac.h

Index: content/common/gpu/image_transport_surface_overlay_mac.h
diff --git a/content/common/gpu/image_transport_surface_overlay_mac.h b/content/common/gpu/image_transport_surface_overlay_mac.h
index 66c0591f1ee365618d667be13d0aaefe99e5d24a..e4822db21f8e6f7fe35aa1196776cdaf7901e833 100644
--- a/content/common/gpu/image_transport_surface_overlay_mac.h
+++ b/content/common/gpu/image_transport_surface_overlay_mac.h
@@ -5,9 +5,15 @@
 #ifndef CONTENT_COMMON_GPU_IMAGE_TRANSPORT_SURFACE_OVERLAY_MAC_H_
 #define CONTENT_COMMON_GPU_IMAGE_TRANSPORT_SURFACE_OVERLAY_MAC_H_
 
+#include <deque>
+#include <IOSurface/IOSurface.h>
+#include <OpenGL/CGLTypes.h>
+
+#include "base/mac/scoped_cftyperef.h"
 #import "base/mac/scoped_nsobject.h"
 #include "content/common/gpu/gpu_command_buffer_stub.h"
 #include "content/common/gpu/image_transport_surface.h"
+#include "ui/accelerated_widget_mac/display_link_mac.h"
 #include "ui/gl/gl_surface.h"
 
 @class CAContext;
@@ -47,14 +53,36 @@ class ImageTransportSurfaceOverlayMac : public gfx::GLSurface,
   void WakeUpGpu() override;
 
  private:
+  struct PendingSwap {

[tapted, 2015/08/05 07:55:56]

Since it's private, you could forward declare this, and just have

class ImageTransportSurfaceOverlayMac::PendingSwap {
..
};

declared in the .mm

That would improve encapsulation and allow you to inline the
constructor/destructor without upsetting clang's chromium-style plugins. Many of
the added #includes could move to the .mm too.

(and the std::deque should still work with a forward dec since
ImageTransportSurfaceOverlayMac's constructor/destructor are out of line)

[ccameron, 2015/08/05 21:00:42]

Done.

+    PendingSwap();
+    ~PendingSwap();
+    // The IOSurface with new content for this swap.
+    base::ScopedCFTypeRef<IOSurfaceRef> io_surface;
+
+    // A fence object, and the CGL context it was issued in.
+    base::ScopedTypeRef<CGLContextObj> cgl_context;
+    GLuint fence;

[tapted, 2015/08/05 07:55:57]

could this be a scoped_ptr<GLFence>? (or GLFenceAPPLE). That would break
copyability and upset std::deque though. (just wondering whether there's benefit
reusing code in ui/gl/gl_fence_apple.h, but doesn't really look like it .. maybe
with cc:ScopedPtrDeque?)

Maybe something like, in ~ImageTransportSurfaceOverlayMac(),
DCHECK(pending_swaps_.empty()); is assurance enough that Destroy() cleans
everything up and the fence isn't leaked.

[ccameron, 2015/08/05 21:00:43]

I'll use a GLFenceAPPLE.

I don't really trust the generic GLFence (GL_ARB_sync) on Mac -- I'd rather not
use it if I don't have to (core profile issues need a check).

Also of note is that by inspection, GLFenceAPPLE::ServerWait unnecessarily calls
ClientWait -- all system-wide OpenGL calls are ordered by their glFlush() order.
Doing nothing in ServerWait is sufficient.

+
+    // The size of the full frame, in dip.
+    gfx::Size dip_size;
+  };
+
   ~ImageTransportSurfaceOverlayMac() override;
+
+  gfx::SwapResult SwapBuffersInternal(const gfx::Rect& pixel_damage_rect);
+
+  // Check the oldest swap in |pending_swaps_|, and, if its fence has passed,
+  // send the content to the window server by -[CALayer setContents:]. If

[tapted, 2015/08/05 07:55:56]

If... (sentence fragment)

[ccameron, 2015/08/05 21:00:42]

Meant to discuss force_immediate_display. Added.

+  void CheckAndDisplayPendingSwaps(bool force_immediate_display);
+
+  // Post a call to CheckAndDisplayPendingSwaps at the next midpoint of a

[tapted, 2015/08/05 07:55:57]

optional: I usually see "Foo()" in comments to refer to functions (a no-parens
"Foo" for a class). More below. But also  there's precedent for no-parens in
this file, so that's fine too.

[ccameron, 2015/08/05 21:00:42]

Good point. If the style guide has a remark on this, happy to follow it (leaving
for now -- there's a similar inconsistency with |variable_name_| in comments).

+  // vsync period.
+  void PostCheckAndDisplayPendingSwaps();
+
   scoped_ptr<ImageTransportHelper> helper_;
   base::scoped_nsobject<CAContext> ca_context_;
   base::scoped_nsobject<CALayer> layer_;
 
-  // A phony NSView handle used to identify this.
-  gfx::AcceleratedWidget widget_;
-
   gfx::Size pixel_size_;
   float scale_factor_;
   std::vector<ui::LatencyInfo> latency_info_;
@@ -63,6 +91,16 @@ class ImageTransportSurfaceOverlayMac : public gfx::GLSurface,
   // consumed and reset when SwapBuffers is called. For now, only one overlay
   // plane is supported.
   gfx::GLImage* pending_overlay_image_;
+
+  // A queue of all frames that have been created by SwapBuffersInternal but
+  // have not yet been displayed by CheckAndDisplayPendingSwaps.
+  std::deque<PendingSwap> pending_swaps_;
+
+  // The display link used to compute the delay for the posted task in
+  // PostCheckAndDisplayPendingSwaps.
+  scoped_refptr<ui::DisplayLinkMac> display_link_mac_;
+
+  base::WeakPtrFactory<ImageTransportSurfaceOverlayMac> weak_factory_;
 };
 
 }  // namespace content