cocoa: Improvements to VSync implementation:

- Use futex instead of condition var
- Prepare for adaptive sync support (#1553)
- Support in tinydisplay

panda/src/cocoadisplay/cocoaGraphicsPipe.h

@@ -16,8 +16,10 @@
 
 #include "pandabase.h"
 #include "graphicsPipe.h"
+#include "patomic.h"
 
 #include <ApplicationServices/ApplicationServices.h>
+#include <CoreVideo/CoreVideo.h>
 
 /**
  * This graphics pipe represents the base class for pipes that create
@@ -28,17 +30,28 @@ public:
   CocoaGraphicsPipe(CGDirectDisplayID display = CGMainDisplayID());
   virtual ~CocoaGraphicsPipe();
 
-  INLINE CGDirectDisplayID get_display_id() const;
-
-public:
   virtual PreferredWindowThread get_preferred_window_thread() const;
 
+  INLINE CGDirectDisplayID get_display_id() const;
+
+  bool init_vsync(uint32_t &counter);
+  void wait_vsync(uint32_t &counter, bool adaptive=false);
+
 private:
+  static CVReturn display_link_cb(CVDisplayLinkRef link, const CVTimeStamp *now,
+                                  const CVTimeStamp *output_time,
+                                  CVOptionFlags flags_in, CVOptionFlags *flags_out,
+                                  void *context);
+
   void load_display_information();
 
   // This is the Quartz display identifier.
   CGDirectDisplayID _display;
 
+  CVDisplayLinkRef _display_link = nullptr;
+  patomic<int> _last_wait_frame {0};
+  uint32_t _vsync_counter = 0;
+
 public:
   static TypeHandle get_class_type() {
     return _type_handle;

panda/src/cocoadisplay/cocoaGraphicsPipe.mm

@@ -76,6 +76,13 @@ CocoaGraphicsPipe(CGDirectDisplayID display) : _display(display) {
     cocoadisplay_cat.debug()
       << "Creating CocoaGraphicsPipe for display ID " << _display << "\n";
   }
+
+  // It takes a while to fire up the display link, so let's fire it up now if
+  // we expect to need VSync.
+  if (sync_video) {
+    uint32_t counter;
+    init_vsync(counter);
+  }
 }
 
 /**
@@ -171,6 +178,14 @@ load_display_information() {
  */
 CocoaGraphicsPipe::
 ~CocoaGraphicsPipe() {
+  if (_display_link != nil) {
+    CVDisplayLinkRelease(_display_link);
+    _display_link = nil;
+
+    // Unblock any threads that may be waiting on the VSync counter.
+    __atomic_fetch_add(&_vsync_counter, 1u, __ATOMIC_SEQ_CST);
+    patomic_notify_all(&_vsync_counter);
+  }
 }
 
 /**
@@ -184,3 +199,112 @@ CocoaGraphicsPipe::get_preferred_window_thread() const {
   // only be called from the main thread!
   return PWT_app;
 }
+
+/**
+ * Ensures a CVDisplayLink is created, which tells us when the display will
+ * want a frame, to avoid tearing (vertical blanking interval).
+ * Initializes the counter with the value that can be passed to wait_vsync
+ * to wait for the next interval.
+ */
+bool CocoaGraphicsPipe::
+init_vsync(uint32_t &counter) {
+  if (_display_link != nil) {
+    // Already set up.
+    __atomic_load(&_vsync_counter, &counter, __ATOMIC_SEQ_CST);
+    return true;
+  }
+
+  counter = 0;
+  _vsync_counter = 0;
+
+  CVDisplayLinkRef display_link;
+  CVReturn result = CVDisplayLinkCreateWithActiveCGDisplays(&display_link);
+  if (result != kCVReturnSuccess) {
+    cocoadisplay_cat.error() << "Failed to create CVDisplayLink.\n";
+    display_link = nil;
+    return false;
+  }
+
+  result = CVDisplayLinkSetCurrentCGDisplay(display_link, _display);
+  if (result != kCVReturnSuccess) {
+    cocoadisplay_cat.error() << "Failed to set CVDisplayLink's current display.\n";
+    CVDisplayLinkRelease(display_link);
+    display_link = nil;
+    return false;
+  }
+
+  result = CVDisplayLinkSetOutputCallback(display_link, &display_link_cb, this);
+  if (result != kCVReturnSuccess) {
+    cocoadisplay_cat.error() << "Failed to set CVDisplayLink output callback.\n";
+    CVDisplayLinkRelease(display_link);
+    display_link = nil;
+    return false;
+  }
+
+  result = CVDisplayLinkStart(display_link);
+  if (result != kCVReturnSuccess) {
+    cocoadisplay_cat.error() << "Failed to start the CVDisplayLink.\n";
+    CVDisplayLinkRelease(display_link);
+    display_link = nil;
+    return false;
+  }
+
+  _display_link = display_link;
+  return true;
+}
+
+/**
+ * The first time this method is called in a frame, waits for the vertical
+ * blanking interval.  If init_vsync has not first been called, does nothing.
+ *
+ * The given counter will be updated with the vblank counter.  If adaptive is
+ * true and the value differs from the current, no wait will occur.
+ */
+void CocoaGraphicsPipe::
+wait_vsync(uint32_t &counter, bool adaptive) {
+  if (_display_link == nil) {
+    return;
+  }
+
+  // Use direct atomic operations since we need this to be thread-safe even
+  // when compiling without thread support.
+  uint32_t current_count = __atomic_load_n(&_vsync_counter, __ATOMIC_SEQ_CST);
+  uint32_t diff = current_count - counter;
+  if (diff > 0) {
+    if (cocoadisplay_cat.is_spam()) {
+      cocoadisplay_cat.spam()
+        << "Missed vertical blanking interval by " << diff << " frames.\n";
+    }
+    if (adaptive) {
+      counter = current_count;
+      return;
+    }
+  }
+
+  // We only wait for the first window that gets flipped in a single frame,
+  // otherwise we end up halving our FPS when we have multiple windows!
+  int cur_frame = ClockObject::get_global_clock()->get_frame_count();
+  if (_last_wait_frame.exchange(cur_frame) == cur_frame) {
+    counter = current_count;
+    return;
+  }
+
+  patomic_wait(&_vsync_counter, current_count);
+  __atomic_load(&_vsync_counter, &counter, __ATOMIC_SEQ_CST);
+}
+
+/**
+ * Called whenever a display wants a frame.  The context argument contains the
+ * applicable CocoaGraphicsPipe.
+ */
+CVReturn CocoaGraphicsPipe::
+display_link_cb(CVDisplayLinkRef link, const CVTimeStamp *now,
+                const CVTimeStamp *output_time, CVOptionFlags flags_in,
+                CVOptionFlags *flags_out, void *context) {
+
+  CocoaGraphicsPipe *pipe = (CocoaGraphicsPipe *)context;
+  __atomic_fetch_add(&pipe->_vsync_counter, 1u, __ATOMIC_SEQ_CST);
+  patomic_notify_all(&pipe->_vsync_counter);
+
+  return kCVReturnSuccess;
+}

panda/src/cocoagldisplay/cocoaGLGraphicsStateGuardian.h

@@ -20,7 +20,6 @@
 
 #import <AppKit/NSOpenGL.h>
 #import <OpenGL/OpenGL.h>
-#import <CoreVideo/CoreVideo.h>
 
 /**
  * A tiny specialization on GLGraphicsStateGuardian to add some Cocoa-specific
@@ -39,7 +38,6 @@ public:
                                CocoaGLGraphicsStateGuardian *share_with);
 
   virtual ~CocoaGLGraphicsStateGuardian();
-  bool setup_vsync();
 
   INLINE void lock_context();
   INLINE void unlock_context();
@@ -49,11 +47,6 @@ public:
   NSOpenGLPixelFormat *_format = nullptr;
   FrameBufferProperties _fbprops;
 
-  CVDisplayLinkRef _display_link = nullptr;
-  TrueMutexImpl _swap_lock;
-  TrueConditionVarImpl _swap_condition;
-  AtomicAdjust::Integer _last_wait_frame = 0;
-
 protected:
   virtual void query_gl_version();
   virtual void *do_get_extension_func(const char *name);

panda/src/cocoagldisplay/cocoaGLGraphicsStateGuardian.mm

@@ -28,21 +28,6 @@
 #define NSAppKitVersionNumber10_7 1138
 #endif
 
-/**
- * Called whenever a display wants a frame.  The context argument contains the
- * applicable CocoaGLGraphicsStateGuardian.
- */
-static CVReturn
-display_link_cb(CVDisplayLinkRef link, const CVTimeStamp *now,
-                const CVTimeStamp* output_time, CVOptionFlags flags_in,
-                CVOptionFlags *flags_out, void *context) {
-  CocoaGLGraphicsStateGuardian *gsg = (CocoaGLGraphicsStateGuardian *)context;
-  gsg->_swap_lock.lock();
-  gsg->_swap_condition.notify();
-  gsg->_swap_lock.unlock();
-  return kCVReturnSuccess;
-}
-
 TypeHandle CocoaGLGraphicsStateGuardian::_type_handle;
 
 /**
@@ -51,8 +36,7 @@ TypeHandle CocoaGLGraphicsStateGuardian::_type_handle;
 CocoaGLGraphicsStateGuardian::
 CocoaGLGraphicsStateGuardian(GraphicsEngine *engine, GraphicsPipe *pipe,
                            CocoaGLGraphicsStateGuardian *share_with) :
-  GLGraphicsStateGuardian(engine, pipe),
-  _swap_condition(_swap_lock)
+  GLGraphicsStateGuardian(engine, pipe)
 {
   _share_context = nil;
   _context = nil;
@@ -71,57 +55,12 @@ CocoaGLGraphicsStateGuardian::
   if (_format != nil) {
     [_format release];
   }
-  if (_display_link != nil) {
-    CVDisplayLinkRelease(_display_link);
-    _display_link = nil;
-    _swap_lock.lock();
-    _swap_condition.notify();
-    _swap_lock.unlock();
-  }
   if (_context != nil) {
     [_context clearDrawable];
     [_context release];
   }
 }
 
-/**
- * Creates a CVDisplayLink, which tells us when the display the window is on
- * will want a frame.
- */
-bool CocoaGLGraphicsStateGuardian::
-setup_vsync() {
-  if (_display_link != nil) {
-    // Already set up.
-    return true;
-  }
-
-  CVReturn result = CVDisplayLinkCreateWithActiveCGDisplays(&_display_link);
-  if (result != kCVReturnSuccess) {
-    cocoadisplay_cat.error() << "Failed to create CVDisplayLink.\n";
-    return false;
-  }
-
-  result = CVDisplayLinkSetCurrentCGDisplayFromOpenGLContext(_display_link, (CGLContextObj)[_context CGLContextObj], (CGLPixelFormatObj)[_format CGLPixelFormatObj]);
-  if (result != kCVReturnSuccess) {
-    cocoadisplay_cat.error() << "Failed to set CVDisplayLink's current display.\n";
-    return false;
-  }
-
-  result = CVDisplayLinkSetOutputCallback(_display_link, &display_link_cb, this);
-  if (result != kCVReturnSuccess) {
-    cocoadisplay_cat.error() << "Failed to set CVDisplayLink output callback.\n";
-    return false;
-  }
-
-  result = CVDisplayLinkStart(_display_link);
-  if (result != kCVReturnSuccess) {
-    cocoadisplay_cat.error() << "Failed to start the CVDisplayLink.\n";
-    return false;
-  }
-
-  return true;
-}
-
 /**
  * Gets the FrameBufferProperties to match the indicated config.
  */

panda/src/cocoagldisplay/cocoaGLGraphicsWindow.h

@@ -42,6 +42,7 @@ protected:
 
 private:
   bool _vsync_enabled = false;
+  uint32_t _vsync_counter = 0;
 
 public:
   static TypeHandle get_class_type() {

panda/src/cocoagldisplay/cocoaGLGraphicsWindow.mm

@@ -159,13 +159,16 @@ end_flip() {
     CocoaGLGraphicsStateGuardian *cocoagsg;
     DCAST_INTO_V(cocoagsg, _gsg);
 
-    if (_vsync_enabled) {
-      AtomicAdjust::Integer cur_frame = ClockObject::get_global_clock()->get_frame_count();
-      if (AtomicAdjust::set(cocoagsg->_last_wait_frame, cur_frame) != cur_frame) {
-        cocoagsg->_swap_lock.lock();
-        cocoagsg->_swap_condition.wait();
-        cocoagsg->_swap_lock.unlock();
+    if (sync_video) {
+      CocoaGraphicsPipe *cocoapipe = (CocoaGraphicsPipe *)_pipe.p();
+      if (!_vsync_enabled) {
+        // If this fails, we don't keep trying.
+        cocoapipe->init_vsync(_vsync_counter);
+        _vsync_enabled = true;
       }
+      cocoapipe->wait_vsync(_vsync_counter);
+    } else {
+      _vsync_enabled = false;
     }
 
     cocoagsg->lock_context();
@@ -237,8 +240,6 @@ open_window() {
   }
   _fb_properties = cocoagsg->get_fb_properties();
 
-  _vsync_enabled = sync_video && cocoagsg->setup_vsync();
-
   return true;
 }
 

panda/src/tinydisplay/tinyCocoaGraphicsWindow.h

@@ -62,6 +62,8 @@ private:
 
   small_vector<SwapBuffer, 2> _swap_chain;
   int _swap_index = 0;
+  uint32_t _vsync_counter = 0;
+  bool _vsync_enabled = false;
 
 public:
   static TypeHandle get_class_type() {

panda/src/tinydisplay/tinyCocoaGraphicsWindow.mm

@@ -114,6 +114,21 @@ end_flip() {
   if (_flip_ready) {
     do_present();
     _swap_index = (_swap_index + 1) % _swap_chain.size();
+
+    // We don't really support proper VSync because we just update the backing
+    // store and let the OS update it when needed, but we still need to wait
+    // for VBlank so that the frame rate is appropriately limited.
+    if (sync_video) {
+      CocoaGraphicsPipe *cocoapipe = (CocoaGraphicsPipe *)_pipe.p();
+      if (!_vsync_enabled) {
+        // If this fails, we don't keep trying.
+        cocoapipe->init_vsync(_vsync_counter);
+        _vsync_enabled = true;
+      }
+      cocoapipe->wait_vsync(_vsync_counter);
+    } else {
+      _vsync_enabled = false;
+    }
   }
 
   GraphicsWindow::end_flip();