From a52744a973ff4cfbf9bd0b4e73ce5112dbcf78ec Mon Sep 17 00:00:00 2001 From: rdb Date: Sat, 21 Oct 2023 15:05:44 +0200 Subject: [PATCH] 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 | 19 ++- panda/src/cocoadisplay/cocoaGraphicsPipe.mm | 124 ++++++++++++++++++ .../cocoaGLGraphicsStateGuardian.h | 7 - .../cocoaGLGraphicsStateGuardian.mm | 63 +-------- .../cocoagldisplay/cocoaGLGraphicsWindow.h | 1 + .../cocoagldisplay/cocoaGLGraphicsWindow.mm | 17 +-- .../src/tinydisplay/tinyCocoaGraphicsWindow.h | 2 + .../tinydisplay/tinyCocoaGraphicsWindow.mm | 15 +++ 8 files changed, 168 insertions(+), 80 deletions(-) diff --git a/panda/src/cocoadisplay/cocoaGraphicsPipe.h b/panda/src/cocoadisplay/cocoaGraphicsPipe.h index 420d2e6a01..60485b735d 100644 --- a/panda/src/cocoadisplay/cocoaGraphicsPipe.h +++ b/panda/src/cocoadisplay/cocoaGraphicsPipe.h @@ -16,8 +16,10 @@ #include "pandabase.h" #include "graphicsPipe.h" +#include "patomic.h" #include +#include /** * 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 _last_wait_frame {0}; + uint32_t _vsync_counter = 0; + public: static TypeHandle get_class_type() { return _type_handle; diff --git a/panda/src/cocoadisplay/cocoaGraphicsPipe.mm b/panda/src/cocoadisplay/cocoaGraphicsPipe.mm index 83a487e9fd..ea86753305 100644 --- a/panda/src/cocoadisplay/cocoaGraphicsPipe.mm +++ b/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; +} diff --git a/panda/src/cocoagldisplay/cocoaGLGraphicsStateGuardian.h b/panda/src/cocoagldisplay/cocoaGLGraphicsStateGuardian.h index 4527648c7c..00cbe9ce55 100644 --- a/panda/src/cocoagldisplay/cocoaGLGraphicsStateGuardian.h +++ b/panda/src/cocoagldisplay/cocoaGLGraphicsStateGuardian.h @@ -20,7 +20,6 @@ #import #import -#import /** * 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); diff --git a/panda/src/cocoagldisplay/cocoaGLGraphicsStateGuardian.mm b/panda/src/cocoagldisplay/cocoaGLGraphicsStateGuardian.mm index 9f8de82705..eac4ab3053 100644 --- a/panda/src/cocoagldisplay/cocoaGLGraphicsStateGuardian.mm +++ b/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. */ diff --git a/panda/src/cocoagldisplay/cocoaGLGraphicsWindow.h b/panda/src/cocoagldisplay/cocoaGLGraphicsWindow.h index 85d0416c94..7bd612678d 100644 --- a/panda/src/cocoagldisplay/cocoaGLGraphicsWindow.h +++ b/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() { diff --git a/panda/src/cocoagldisplay/cocoaGLGraphicsWindow.mm b/panda/src/cocoagldisplay/cocoaGLGraphicsWindow.mm index c1e1821226..33a97a90f9 100644 --- a/panda/src/cocoagldisplay/cocoaGLGraphicsWindow.mm +++ b/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; } diff --git a/panda/src/tinydisplay/tinyCocoaGraphicsWindow.h b/panda/src/tinydisplay/tinyCocoaGraphicsWindow.h index 961a1749c3..f2e55d1f5d 100644 --- a/panda/src/tinydisplay/tinyCocoaGraphicsWindow.h +++ b/panda/src/tinydisplay/tinyCocoaGraphicsWindow.h @@ -62,6 +62,8 @@ private: small_vector _swap_chain; int _swap_index = 0; + uint32_t _vsync_counter = 0; + bool _vsync_enabled = false; public: static TypeHandle get_class_type() { diff --git a/panda/src/tinydisplay/tinyCocoaGraphicsWindow.mm b/panda/src/tinydisplay/tinyCocoaGraphicsWindow.mm index 38c08e88f7..6565eccac3 100644 --- a/panda/src/tinydisplay/tinyCocoaGraphicsWindow.mm +++ b/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();