From 3237222b62116179d069a1e9dafe6b384c757751 Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Thu, 3 Apr 2025 17:25:17 +0200 Subject: [PATCH] [macOS] Implement merged UI and platform thread (#162883) Original issue: https://github.com/flutter/flutter/issues/150525 This PR lets the macOS embedder run both with and without UI and platform thread merged. Thread merging is controlled through `FLTEnableMergedPlatformUIThread` `info.plist` option similar to iOS embedder, though the default value is currently `false`. Changes in the resize / vsync synchronization: - Added `FlutterRunLoop` class to schedule Flutter tasks on main thread in a way where it is possible to only process these (Flutter posted) tasks while waiting for correct frame size during resizing. This significantly simplifies the resize synchronization and makes the same code work both with separate UI thread and with UI and platform thread merged. - `FlutterThreadSynchronizer` has been renamed to `FlutterResizeSynchronizer` vastly simplified, mutex and conditions are removed and the blocking is now done by only processing Flutter messages while waiting for resizing. It is now per view (instead of storing a viewId->Size map internally) and owned by the view itself, instead of engine. - This approach to resize synchronization will work for Windows and Linux as well. This will allow us to conceptually consolidate the way we do threading and resize synchronization on all three desktop embedders. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md --- .../ci/licenses_golden/licenses_flutter | 14 +- .../shell/platform/darwin/macos/BUILD.gn | 8 +- .../framework/Source/FlutterDisplayLink.h | 14 +- .../framework/Source/FlutterDisplayLink.mm | 186 +++------ .../Source/FlutterDisplayLinkTest.mm | 82 ++-- .../macos/framework/Source/FlutterEngine.mm | 138 ++++--- .../framework/Source/FlutterEngineTest.mm | 36 +- .../framework/Source/FlutterEngine_Internal.h | 4 - .../Source/FlutterResizeSynchronizer.h | 43 ++ .../Source/FlutterResizeSynchronizer.mm | 61 +++ .../Source/FlutterResizeSynchronizerTest.mm | 121 ++++++ .../macos/framework/Source/FlutterRunLoop.h | 46 +++ .../macos/framework/Source/FlutterRunLoop.mm | 127 ++++++ .../framework/Source/FlutterSurfaceManager.h | 6 +- .../framework/Source/FlutterSurfaceManager.mm | 38 +- .../Source/FlutterSurfaceManagerTest.mm | 4 +- .../Source/FlutterThreadSynchronizer.h | 112 ------ .../Source/FlutterThreadSynchronizer.mm | 205 ---------- .../Source/FlutterThreadSynchronizerTest.mm | 374 ------------------ .../framework/Source/FlutterVSyncWaiter.h | 5 +- .../framework/Source/FlutterVSyncWaiter.mm | 110 +++--- .../Source/FlutterVSyncWaiterTest.mm | 12 +- .../macos/framework/Source/FlutterView.h | 8 +- .../macos/framework/Source/FlutterView.mm | 38 +- .../framework/Source/FlutterViewController.mm | 13 +- .../Source/FlutterViewController_Internal.h | 3 +- .../macos/framework/Source/FlutterViewTest.mm | 6 - 27 files changed, 719 insertions(+), 1095 deletions(-) create mode 100644 engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterResizeSynchronizer.h create mode 100644 engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterResizeSynchronizer.mm create mode 100644 engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterResizeSynchronizerTest.mm create mode 100644 engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterRunLoop.h create mode 100644 engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterRunLoop.mm delete mode 100644 engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.h delete mode 100644 engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.mm delete mode 100644 engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizerTest.mm diff --git a/engine/src/flutter/ci/licenses_golden/licenses_flutter b/engine/src/flutter/ci/licenses_golden/licenses_flutter index 6ecd230631..050de2116a 100644 --- a/engine/src/flutter/ci/licenses_golden/licenses_flutter +++ b/engine/src/flutter/ci/licenses_golden/licenses_flutter @@ -52934,6 +52934,9 @@ ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterPla ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterPlatformViewControllerTest.mm + ../../../flutter/LICENSE ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterRenderer.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterRenderer.mm + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterResizeSynchronizer.h + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterResizeSynchronizer.mm + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterResizeSynchronizerTest.mm + ../../../flutter/LICENSE ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterSurface.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterSurface.mm + ../../../flutter/LICENSE ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h + ../../../flutter/LICENSE @@ -52947,9 +52950,6 @@ ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterTex ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterTextInputSemanticsObjectTest.mm + ../../../flutter/LICENSE ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterTextureRegistrar.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterTextureRegistrar.mm + ../../../flutter/LICENSE -ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.h + ../../../flutter/LICENSE -ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.mm + ../../../flutter/LICENSE -ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizerTest.mm + ../../../flutter/LICENSE ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterTimeConverter.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterTimeConverter.mm + ../../../flutter/LICENSE ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterUmbrellaImportTests.m + ../../../flutter/LICENSE @@ -55943,6 +55943,11 @@ FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterPlatf FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterPlatformViewControllerTest.mm FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterRenderer.h FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterRenderer.mm +FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterResizeSynchronizer.h +FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterResizeSynchronizer.mm +FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterResizeSynchronizerTest.mm +FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterRunLoop.h +FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterRunLoop.mm FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterSurface.h FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterSurface.mm FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h @@ -55956,9 +55961,6 @@ FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterTextI FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterTextInputSemanticsObjectTest.mm FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterTextureRegistrar.h FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterTextureRegistrar.mm -FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.h -FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.mm -FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizerTest.mm FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterTimeConverter.h FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterTimeConverter.mm FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterUmbrellaImportTests.m diff --git a/engine/src/flutter/shell/platform/darwin/macos/BUILD.gn b/engine/src/flutter/shell/platform/darwin/macos/BUILD.gn index cacda84d67..09fd490246 100644 --- a/engine/src/flutter/shell/platform/darwin/macos/BUILD.gn +++ b/engine/src/flutter/shell/platform/darwin/macos/BUILD.gn @@ -94,6 +94,10 @@ source_set("flutter_framework_source") { "framework/Source/FlutterPlatformViewController.mm", "framework/Source/FlutterRenderer.h", "framework/Source/FlutterRenderer.mm", + "framework/Source/FlutterResizeSynchronizer.h", + "framework/Source/FlutterResizeSynchronizer.mm", + "framework/Source/FlutterRunLoop.h", + "framework/Source/FlutterRunLoop.mm", "framework/Source/FlutterSurface.h", "framework/Source/FlutterSurface.mm", "framework/Source/FlutterSurfaceManager.h", @@ -104,8 +108,6 @@ source_set("flutter_framework_source") { "framework/Source/FlutterTextInputSemanticsObject.mm", "framework/Source/FlutterTextureRegistrar.h", "framework/Source/FlutterTextureRegistrar.mm", - "framework/Source/FlutterThreadSynchronizer.h", - "framework/Source/FlutterThreadSynchronizer.mm", "framework/Source/FlutterTimeConverter.h", "framework/Source/FlutterTimeConverter.mm", "framework/Source/FlutterVSyncWaiter.h", @@ -194,10 +196,10 @@ executable("flutter_desktop_darwin_unittests") { "framework/Source/FlutterMutatorViewTest.mm", "framework/Source/FlutterPlatformNodeDelegateMacTest.mm", "framework/Source/FlutterPlatformViewControllerTest.mm", + "framework/Source/FlutterResizeSynchronizerTest.mm", "framework/Source/FlutterSurfaceManagerTest.mm", "framework/Source/FlutterTextInputPluginTest.mm", "framework/Source/FlutterTextInputSemanticsObjectTest.mm", - "framework/Source/FlutterThreadSynchronizerTest.mm", "framework/Source/FlutterVSyncWaiterTest.mm", "framework/Source/FlutterViewControllerTest.mm", "framework/Source/FlutterViewControllerTestUtils.h", diff --git a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterDisplayLink.h b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterDisplayLink.h index fa3c0925a7..f595817496 100644 --- a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterDisplayLink.h +++ b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterDisplayLink.h @@ -4,6 +4,7 @@ #import @protocol FlutterDisplayLinkDelegate +/// This will be called on main thread. - (void)onDisplayLink:(CFTimeInterval)timestamp targetTimestamp:(CFTimeInterval)targetTimestamp; @end @@ -12,27 +13,28 @@ /// Internally FlutterDisplayLink will use at most one CVDisplayLink per /// screen shared for all views belonging to that screen. This is necessary /// because each CVDisplayLink comes with its own thread. +/// +/// All methods must be called on main thread. @interface FlutterDisplayLink : NSObject /// Creates new instance tied to provided NSView. FlutterDisplayLink /// will track view display changes transparently to synchronize /// update with display refresh. -/// This function must be called on the main thread. + (instancetype)displayLinkWithView:(NSView*)view; -/// Delegate must be set on main thread. Delegate method will be called on -/// on display link thread. +/// Delegate must be set on main thread. +/// Delegate method will be also called on main thread. @property(nonatomic, weak) id delegate; -/// Pauses and resumes the display link. May be called from any thread. +/// Pauses and resumes the display link. @property(readwrite) BOOL paused; /// Returns the nominal refresh period of the display to which the view /// currently belongs (in seconds). If view does not belong to any display, -/// returns 0. Can be called from any thread. +/// returns 0. @property(readonly) CFTimeInterval nominalOutputRefreshPeriod; -/// Invalidates the display link. Must be called on the main thread. +/// Invalidates the display link. - (void)invalidate; @end diff --git a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterDisplayLink.mm b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterDisplayLink.mm index af6643ed40..0fd314ddee 100644 --- a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterDisplayLink.mm +++ b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterDisplayLink.mm @@ -1,4 +1,5 @@ #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterDisplayLink.h" +#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterRunLoop.h" #include "flutter/fml/logging.h" @@ -8,23 +9,6 @@ #include #include -// Note on thread safety and locking: -// -// There are three mutexes used within the scope of this file: -// - CVDisplayLink internal mutex. This is locked during every CVDisplayLink method -// and is also held while display link calls the output handler. -// - DisplayLinkManager mutex. -// - _FlutterDisplayLink mutex (through @synchronized blocks). -// -// Special care must be taken to avoid deadlocks. Because CVDisplayLink holds the -// mutex for the entire duration of the output handler, it is necessary for -// DisplayLinkManager to not call any CVDisplayLink methods while holding its -// mutex. Instead it must retain the display link instance and then call the -// appropriate method with the mutex unlocked. -// -// Similarly _FlutterDisplayLink must not call any DisplayLinkManager methods -// within the @synchronized block. - @class _FlutterDisplayLinkView; @interface _FlutterDisplayLink : FlutterDisplayLink { @@ -61,11 +45,7 @@ class DisplayLinkManager { struct ScreenEntry { CGDirectDisplayID display_id; std::vector<_FlutterDisplayLink*> clients; - - /// Display link for this screen. It is not safe to call display link methods - /// on this object while holding the mutex. Instead the instance should be - /// retained, mutex unlocked and then released. - CVDisplayLinkRef display_link_locked; + CVDisplayLinkRef display_link; bool ShouldBeRunning() { return std::any_of(clients.begin(), clients.end(), @@ -73,37 +53,19 @@ class DisplayLinkManager { } }; std::vector entries_; - std::mutex mutex_; }; void RunOrStopDisplayLink(CVDisplayLinkRef display_link, bool should_be_running) { bool is_running = CVDisplayLinkIsRunning(display_link); if (should_be_running && !is_running) { - if (CVDisplayLinkStart(display_link) == kCVReturnError) { - // CVDisplayLinkStart will fail if it was called from the display link thread. - // The problem is that it CVDisplayLinkStop doesn't clean the pthread_t value in the display - // link itself. If the display link is started and stopped before before the UI thread is - // started (*), pthread_self() of the UI thread may have same value as the one stored in - // CVDisplayLink. Because this can happen at most once starting the display link from a - // temporary thread is a reasonable workaround. - // - // (*) Display link is started before UI thread because FlutterVSyncWaiter will run display - // link for one tick at the beginning to determine vsync phase. - // - // http://www.openradar.me/radar?id=5520107644125184 - CVDisplayLinkRef retained = CVDisplayLinkRetain(display_link); - [NSThread detachNewThreadWithBlock:^{ - CVDisplayLinkStart(retained); - CVDisplayLinkRelease(retained); - }]; - } + CVDisplayLinkStart(display_link); } else if (!should_be_running && is_running) { CVDisplayLinkStop(display_link); } } void DisplayLinkManager::UnregisterDisplayLink(_FlutterDisplayLink* display_link) { - std::unique_lock lock(mutex_); + FML_DCHECK(NSThread.isMainThread); for (auto entry = entries_.begin(); entry != entries_.end(); ++entry) { auto it = std::find(entry->clients.begin(), entry->clients.end(), display_link); if (it != entry->clients.end()) { @@ -111,18 +73,12 @@ void DisplayLinkManager::UnregisterDisplayLink(_FlutterDisplayLink* display_link if (entry->clients.empty()) { // Erasing the entry - take the display link instance and stop / release it // outside of the mutex. - CVDisplayLinkRef display_link = entry->display_link_locked; + CVDisplayLinkStop(entry->display_link); + CVDisplayLinkRelease(entry->display_link); entries_.erase(entry); - lock.unlock(); - CVDisplayLinkStop(display_link); - CVDisplayLinkRelease(display_link); } else { // Update the display link state outside of the mutex. - bool should_be_running = entry->ShouldBeRunning(); - CVDisplayLinkRef display_link = CVDisplayLinkRetain(entry->display_link_locked); - lock.unlock(); - RunOrStopDisplayLink(display_link, should_be_running); - CVDisplayLinkRelease(display_link); + RunOrStopDisplayLink(entry->display_link, entry->ShouldBeRunning()); } return; } @@ -131,15 +87,11 @@ void DisplayLinkManager::UnregisterDisplayLink(_FlutterDisplayLink* display_link void DisplayLinkManager::RegisterDisplayLink(_FlutterDisplayLink* display_link, CGDirectDisplayID display_id) { - std::unique_lock lock(mutex_); + FML_DCHECK(NSThread.isMainThread); for (ScreenEntry& entry : entries_) { if (entry.display_id == display_id) { entry.clients.push_back(display_link); - bool should_be_running = entry.ShouldBeRunning(); - CVDisplayLinkRef display_link = CVDisplayLinkRetain(entry.display_link_locked); - lock.unlock(); - RunOrStopDisplayLink(display_link, should_be_running); - CVDisplayLinkRelease(display_link); + RunOrStopDisplayLink(entry.display_link, entry.ShouldBeRunning()); return; } } @@ -147,10 +99,10 @@ void DisplayLinkManager::RegisterDisplayLink(_FlutterDisplayLink* display_link, ScreenEntry entry; entry.display_id = display_id; entry.clients.push_back(display_link); - CVDisplayLinkCreateWithCGDisplay(display_id, &entry.display_link_locked); + CVDisplayLinkCreateWithCGDisplay(display_id, &entry.display_link); CVDisplayLinkSetOutputHandler( - entry.display_link_locked, + entry.display_link, ^(CVDisplayLinkRef display_link, const CVTimeStamp* in_now, const CVTimeStamp* in_output_time, CVOptionFlags flags_in, CVOptionFlags* flags_out) { OnDisplayLink(display_link, in_now, in_output_time, flags_in, flags_out); @@ -158,34 +110,24 @@ void DisplayLinkManager::RegisterDisplayLink(_FlutterDisplayLink* display_link, }); // This is a new display link so it is safe to start it with mutex held. - bool should_be_running = entry.ShouldBeRunning(); - RunOrStopDisplayLink(entry.display_link_locked, should_be_running); + RunOrStopDisplayLink(entry.display_link, entry.ShouldBeRunning()); entries_.push_back(entry); } void DisplayLinkManager::PausedDidChange(_FlutterDisplayLink* display_link) { - std::unique_lock lock(mutex_); for (ScreenEntry& entry : entries_) { auto it = std::find(entry.clients.begin(), entry.clients.end(), display_link); if (it != entry.clients.end()) { - bool running = entry.ShouldBeRunning(); - CVDisplayLinkRef display_link = CVDisplayLinkRetain(entry.display_link_locked); - lock.unlock(); - RunOrStopDisplayLink(display_link, running); - CVDisplayLinkRelease(display_link); + RunOrStopDisplayLink(entry.display_link, entry.ShouldBeRunning()); return; } } } CFTimeInterval DisplayLinkManager::GetNominalOutputPeriod(CGDirectDisplayID display_id) { - std::unique_lock lock(mutex_); for (ScreenEntry& entry : entries_) { if (entry.display_id == display_id) { - CVDisplayLinkRef display_link = CVDisplayLinkRetain(entry.display_link_locked); - lock.unlock(); - CVTime latency = CVDisplayLinkGetNominalOutputVideoRefreshPeriod(display_link); - CVDisplayLinkRelease(display_link); + CVTime latency = CVDisplayLinkGetNominalOutputVideoRefreshPeriod(entry.display_link); return (CFTimeInterval)latency.timeValue / (CFTimeInterval)latency.timeScale; } } @@ -197,25 +139,25 @@ void DisplayLinkManager::OnDisplayLink(CVDisplayLinkRef display_link, const CVTimeStamp* in_output_time, CVOptionFlags flags_in, CVOptionFlags* flags_out) { - // Hold the mutex only while copying clients. - std::vector<_FlutterDisplayLink*> clients; - { - std::lock_guard lock(mutex_); + CVTimeStamp inNow = *in_now; + CVTimeStamp inOutputTime = *in_output_time; + [FlutterRunLoop.mainRunLoop performBlock:^{ + std::vector<_FlutterDisplayLink*> clients; for (ScreenEntry& entry : entries_) { - if (entry.display_link_locked == display_link) { + if (entry.display_link == display_link) { clients = entry.clients; break; } } - } - CFTimeInterval timestamp = (CFTimeInterval)in_now->hostTime / CVGetHostClockFrequency(); - CFTimeInterval target_timestamp = - (CFTimeInterval)in_output_time->hostTime / CVGetHostClockFrequency(); + CFTimeInterval timestamp = (CFTimeInterval)inNow.hostTime / CVGetHostClockFrequency(); + CFTimeInterval target_timestamp = + (CFTimeInterval)inOutputTime.hostTime / CVGetHostClockFrequency(); - for (_FlutterDisplayLink* client : clients) { - [client didFireWithTimestamp:timestamp targetTimestamp:target_timestamp]; - } + for (_FlutterDisplayLink* client : clients) { + [client didFireWithTimestamp:timestamp targetTimestamp:target_timestamp]; + } + }]; } } // namespace @@ -242,7 +184,7 @@ static NSString* const kFlutterDisplayLinkViewDidMoveToWindow = @synthesize delegate = _delegate; - (instancetype)initWithView:(NSView*)view { - FML_DCHECK([NSThread isMainThread]); + FML_DCHECK(NSThread.isMainThread); if (self = [super init]) { self->_view = [[_FlutterDisplayLinkView alloc] initWithFrame:CGRectZero]; [view addSubview:self->_view]; @@ -261,39 +203,38 @@ static NSString* const kFlutterDisplayLinkViewDidMoveToWindow = } - (void)invalidate { - @synchronized(self) { - FML_DCHECK([NSThread isMainThread]); - // Unregister observer before removing the view to ensure - // that the viewDidChangeWindow notification is not received - // while in @synchronized block. - [[NSNotificationCenter defaultCenter] removeObserver:self]; - [_view removeFromSuperview]; - _view = nil; - _delegate = nil; - } + FML_DCHECK(NSThread.isMainThread); + // Unregister observer before removing the view to ensure + // that the viewDidChangeWindow notification is not received + // while in @synchronized block. + [[NSNotificationCenter defaultCenter] removeObserver:self]; + [_view removeFromSuperview]; + _view = nil; + _delegate = nil; DisplayLinkManager::Instance().UnregisterDisplayLink(self); } - (void)updateScreen { + FML_DCHECK(NSThread.isMainThread); DisplayLinkManager::Instance().UnregisterDisplayLink(self); std::optional displayId; - @synchronized(self) { - NSScreen* screen = _view.window.screen; - if (screen != nil) { - // https://developer.apple.com/documentation/appkit/nsscreen/1388360-devicedescription?language=objc - _display_id = (CGDirectDisplayID)[ - [[screen deviceDescription] objectForKey:@"NSScreenNumber"] unsignedIntValue]; - } else { - _display_id = std::nullopt; - } - displayId = _display_id; + NSScreen* screen = _view.window.screen; + if (screen != nil) { + // https://developer.apple.com/documentation/appkit/nsscreen/1388360-devicedescription?language=objc + _display_id = (CGDirectDisplayID)[ + [[screen deviceDescription] objectForKey:@"NSScreenNumber"] unsignedIntValue]; + } else { + _display_id = std::nullopt; } + displayId = _display_id; + if (displayId.has_value()) { DisplayLinkManager::Instance().RegisterDisplayLink(self, *displayId); } } - (void)viewDidChangeWindow:(NSNotification*)notification { + FML_DCHECK(NSThread.isMainThread); NSView* view = notification.object; if (_view == view) { [self updateScreen]; @@ -301,6 +242,7 @@ static NSString* const kFlutterDisplayLinkViewDidMoveToWindow = } - (void)windowDidChangeScreen:(NSNotification*)notification { + FML_DCHECK(NSThread.isMainThread); NSWindow* window = notification.object; if (_view.window == window) { [self updateScreen]; @@ -309,38 +251,34 @@ static NSString* const kFlutterDisplayLinkViewDidMoveToWindow = - (void)didFireWithTimestamp:(CFTimeInterval)timestamp targetTimestamp:(CFTimeInterval)targetTimestamp { - @synchronized(self) { - if (!_paused) { - id delegate = _delegate; - [delegate onDisplayLink:timestamp targetTimestamp:targetTimestamp]; - } + FML_DCHECK(NSThread.isMainThread); + if (!_paused) { + id delegate = _delegate; + [delegate onDisplayLink:timestamp targetTimestamp:targetTimestamp]; } } - (BOOL)paused { - @synchronized(self) { - return _paused; - } + FML_DCHECK(NSThread.isMainThread); + return _paused; } - (void)setPaused:(BOOL)paused { - @synchronized(self) { - if (_paused == paused) { - return; - } - _paused = paused; + FML_DCHECK(NSThread.isMainThread); + if (_paused == paused) { + return; } + _paused = paused; DisplayLinkManager::Instance().PausedDidChange(self); } - (CFTimeInterval)nominalOutputRefreshPeriod { + FML_DCHECK(NSThread.isMainThread); CGDirectDisplayID display_id; - @synchronized(self) { - if (_display_id.has_value()) { - display_id = *_display_id; - } else { - return 0; - } + if (_display_id.has_value()) { + display_id = *_display_id; + } else { + return 0; } return DisplayLinkManager::Instance().GetNominalOutputPeriod(display_id); } diff --git a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterDisplayLinkTest.mm b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterDisplayLinkTest.mm index 8357e2e973..e0455ded62 100644 --- a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterDisplayLinkTest.mm +++ b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterDisplayLinkTest.mm @@ -3,6 +3,7 @@ // found in the LICENSE file. #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterDisplayLink.h" +#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterRunLoop.h" #import #include @@ -33,7 +34,12 @@ @end -TEST(FlutterDisplayLinkTest, ViewAddedToWindowFirst) { +class FlutterDisplayLinkTest : public testing::Test { + public: + void SetUp() override { [FlutterRunLoop ensureMainLoopInitialized]; } +}; + +TEST_F(FlutterDisplayLinkTest, ViewAddedToWindowFirst) { NSWindow* window = [[NSWindow alloc] initWithContentRect:NSMakeRect(0, 0, 100, 100) styleMask:NSWindowStyleMaskTitled backing:NSBackingStoreNonretained @@ -41,30 +47,32 @@ TEST(FlutterDisplayLinkTest, ViewAddedToWindowFirst) { NSView* view = [[NSView alloc] initWithFrame:NSMakeRect(0, 0, 100, 100)]; [window setContentView:view]; - auto event = std::make_shared(); + __block BOOL signalled = NO; TestDisplayLinkDelegate* delegate = [[TestDisplayLinkDelegate alloc] initWithBlock:^(CFTimeInterval timestamp, CFTimeInterval targetTimestamp) { - event->Signal(); + signalled = YES; }]; FlutterDisplayLink* displayLink = [FlutterDisplayLink displayLinkWithView:view]; displayLink.delegate = delegate; displayLink.paused = NO; - event->Wait(); + while (!signalled) { + [FlutterRunLoop.mainRunLoop pollFlutterMessagesOnce]; + } [displayLink invalidate]; } -TEST(FlutterDisplayLinkTest, ViewAddedToWindowLater) { +TEST_F(FlutterDisplayLinkTest, ViewAddedToWindowLater) { NSView* view = [[NSView alloc] initWithFrame:NSMakeRect(0, 0, 100, 100)]; - auto event = std::make_shared(); + __block BOOL signalled = NO; TestDisplayLinkDelegate* delegate = [[TestDisplayLinkDelegate alloc] initWithBlock:^(CFTimeInterval timestamp, CFTimeInterval targetTimestamp) { - event->Signal(); + signalled = YES; }]; FlutterDisplayLink* displayLink = [FlutterDisplayLink displayLinkWithView:view]; @@ -77,12 +85,14 @@ TEST(FlutterDisplayLinkTest, ViewAddedToWindowLater) { defer:NO]; [window setContentView:view]; - event->Wait(); + while (!signalled) { + [FlutterRunLoop.mainRunLoop pollFlutterMessagesOnce]; + } [displayLink invalidate]; } -TEST(FlutterDisplayLinkTest, ViewRemovedFromWindow) { +TEST_F(FlutterDisplayLinkTest, ViewRemovedFromWindow) { NSWindow* window = [[NSWindow alloc] initWithContentRect:NSMakeRect(0, 0, 100, 100) styleMask:NSWindowStyleMaskTitled backing:NSBackingStoreNonretained @@ -90,67 +100,39 @@ TEST(FlutterDisplayLinkTest, ViewRemovedFromWindow) { NSView* view = [[NSView alloc] initWithFrame:NSMakeRect(0, 0, 100, 100)]; [window setContentView:view]; - auto event = std::make_shared(); + __block BOOL signalled = NO; TestDisplayLinkDelegate* delegate = [[TestDisplayLinkDelegate alloc] initWithBlock:^(CFTimeInterval timestamp, CFTimeInterval targetTimestamp) { - event->Signal(); + signalled = YES; }]; FlutterDisplayLink* displayLink = [FlutterDisplayLink displayLinkWithView:view]; displayLink.delegate = delegate; displayLink.paused = NO; - event->Wait(); + while (!signalled) { + [FlutterRunLoop.mainRunLoop pollFlutterMessagesOnce]; + } displayLink.paused = YES; - event->Reset(); + signalled = false; displayLink.paused = NO; [window setContentView:nil]; - EXPECT_TRUE(event->WaitWithTimeout(fml::TimeDelta::FromMilliseconds(100))); - EXPECT_FALSE(event->IsSignaledForTest()); + CFTimeInterval start = CACurrentMediaTime(); + while (CACurrentMediaTime() < start + 0.1) { + [FlutterRunLoop.mainRunLoop pollFlutterMessagesOnce]; + } + + EXPECT_FALSE(signalled); [displayLink invalidate]; } -TEST(FlutterDisplayLinkTest, WorkaroundForFB13482573) { - NSWindow* window = [[NSWindow alloc] initWithContentRect:NSMakeRect(0, 0, 100, 100) - styleMask:NSWindowStyleMaskTitled - backing:NSBackingStoreNonretained - defer:NO]; - NSView* view = [[NSView alloc] initWithFrame:NSMakeRect(0, 0, 100, 100)]; - [window setContentView:view]; - - auto event = std::make_shared(); - - TestDisplayLinkDelegate* delegate = [[TestDisplayLinkDelegate alloc] - initWithBlock:^(CFTimeInterval timestamp, CFTimeInterval targetTimestamp) { - event->Signal(); - }]; - - FlutterDisplayLink* displayLink = [FlutterDisplayLink displayLinkWithView:view]; - displayLink.delegate = delegate; - displayLink.paused = NO; - - event->Wait(); - displayLink.paused = YES; - - event->Reset(); - [NSThread detachNewThreadWithBlock:^{ - // Here pthread_self() will be same as pthread_self inside first invocation of - // display link callback, causing CVDisplayLinkStart to return error. - displayLink.paused = NO; - }]; - - event->Wait(); - - [displayLink invalidate]; -} - -TEST(FlutterDisplayLinkTest, CVDisplayLinkInterval) { +TEST_F(FlutterDisplayLinkTest, CVDisplayLinkInterval) { CVDisplayLinkRef link; CVDisplayLinkCreateWithCGDisplay(CGMainDisplayID(), &link); __block CFTimeInterval last = 0; diff --git a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm index 221dbee2ad..a6fb4de099 100644 --- a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm +++ b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm @@ -24,6 +24,7 @@ #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterMouseCursorPlugin.h" #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterPlatformViewController.h" #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterRenderer.h" +#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterRunLoop.h" #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterTimeConverter.h" #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterVSyncWaiter.h" #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterViewController_Internal.h" @@ -458,8 +459,6 @@ static void OnPlatformMessage(const FlutterPlatformMessage* message, void* user_ // A method channel for miscellaneous platform functionality. FlutterMethodChannel* _platformChannel; - FlutterThreadSynchronizer* _threadSynchronizer; - // Whether the application is currently the active application. BOOL _active; @@ -508,6 +507,9 @@ static void SetThreadPriority(FlutterThreadPriority priority) { allowHeadlessExecution:(BOOL)allowHeadlessExecution { self = [super init]; NSAssert(self, @"Super init cannot be nil"); + + [FlutterRunLoop ensureMainLoopInitialized]; + _pasteboard = [[FlutterPasteboard alloc] init]; _active = NO; _visible = NO; @@ -538,7 +540,6 @@ static void SetThreadPriority(FlutterThreadPriority priority) { object:nil]; _platformViewController = [[FlutterPlatformViewController alloc] init]; - _threadSynchronizer = [[FlutterThreadSynchronizer alloc] init]; // The macOS compositor must be initialized in the initializer because it is // used when adding views, which might happen before runWithEntrypoint. _macOSCompositor = std::make_unique( @@ -595,6 +596,31 @@ static void SetThreadPriority(FlutterThreadPriority priority) { } } +- (FlutterTaskRunnerDescription)createPlatformThreadTaskDescription { + static size_t sTaskRunnerIdentifiers = 0; + FlutterTaskRunnerDescription cocoa_task_runner_description = { + .struct_size = sizeof(FlutterTaskRunnerDescription), + // Retain for use in post_task_callback. Released in destruction_callback. + .user_data = (__bridge_retained void*)self, + .runs_task_on_current_thread_callback = [](void* user_data) -> bool { + return [[NSThread currentThread] isMainThread]; + }, + .post_task_callback = [](FlutterTask task, uint64_t target_time_nanos, + void* user_data) -> void { + FlutterEngine* engine = (__bridge FlutterEngine*)user_data; + [engine postMainThreadTask:task targetTimeInNanoseconds:target_time_nanos]; + }, + .identifier = ++sTaskRunnerIdentifiers, + .destruction_callback = + [](void* user_data) { + // Balancing release for the retain when setting user_data above. + FlutterEngine* engine = (__bridge_transfer FlutterEngine*)user_data; + engine = nil; + }, + }; + return cocoa_task_runner_description; +} + - (BOOL)runWithEntrypoint:(NSString*)entrypoint { if (self.running) { return NO; @@ -655,31 +681,33 @@ static void SetThreadPriority(FlutterThreadPriority priority) { flutterArguments.engine_id = reinterpret_cast((__bridge void*)self); - static size_t sTaskRunnerIdentifiers = 0; - const FlutterTaskRunnerDescription cocoa_task_runner_description = { - .struct_size = sizeof(FlutterTaskRunnerDescription), - // Retain for use in post_task_callback. Released in destruction_callback. - .user_data = (__bridge_retained void*)self, - .runs_task_on_current_thread_callback = [](void* user_data) -> bool { - return [[NSThread currentThread] isMainThread]; - }, - .post_task_callback = [](FlutterTask task, uint64_t target_time_nanos, - void* user_data) -> void { - FlutterEngine* engine = (__bridge FlutterEngine*)user_data; - [engine postMainThreadTask:task targetTimeInNanoseconds:target_time_nanos]; - }, - .identifier = ++sTaskRunnerIdentifiers, - .destruction_callback = - [](void* user_data) { - // Balancing release for the retain when setting user_data above. - FlutterEngine* engine = (__bridge_transfer FlutterEngine*)user_data; - engine = nil; - }, - }; + BOOL mergedPlatformUIThread = NO; + NSNumber* enableMergedPlatformUIThread = + [[NSBundle mainBundle] objectForInfoDictionaryKey:@"FLTEnableMergedPlatformUIThread"]; + if (enableMergedPlatformUIThread != nil) { + mergedPlatformUIThread = enableMergedPlatformUIThread.boolValue; + } + + if (mergedPlatformUIThread) { + NSLog(@"Running with merged UI and platform thread. Experimental."); + } + + // The task description needs to be created separately for platform task + // runner and UI task runner because each one has their own __bridge_retained + // engine user data. + FlutterTaskRunnerDescription platformTaskRunnerDescription = + [self createPlatformThreadTaskDescription]; + std::optional uiTaskRunnerDescription; + if (mergedPlatformUIThread) { + uiTaskRunnerDescription = [self createPlatformThreadTaskDescription]; + } + const FlutterCustomTaskRunners custom_task_runners = { .struct_size = sizeof(FlutterCustomTaskRunners), - .platform_task_runner = &cocoa_task_runner_description, - .thread_priority_setter = SetThreadPriority}; + .platform_task_runner = &platformTaskRunnerDescription, + .thread_priority_setter = SetThreadPriority, + .ui_task_runner = uiTaskRunnerDescription ? &uiTaskRunnerDescription.value() : nullptr, + }; flutterArguments.custom_task_runners = &custom_task_runners; [self loadAOTData:_project.assetsPath]; @@ -766,9 +794,7 @@ static void SetThreadPriority(FlutterThreadPriority priority) { NSAssert([_viewControllers objectForKey:@(viewIdentifier)] == nil, @"The requested view ID is occupied."); [_viewControllers setObject:controller forKey:@(viewIdentifier)]; - [controller setUpWithEngine:self - viewIdentifier:viewIdentifier - threadSynchronizer:_threadSynchronizer]; + [controller setUpWithEngine:self viewIdentifier:viewIdentifier]; NSAssert(controller.viewIdentifier == viewIdentifier, @"Failed to assign view ID."); // Verify that the controller's property are updated accordingly. Failing the // assertions is likely because either the FlutterViewController or the @@ -796,13 +822,7 @@ static void SetThreadPriority(FlutterThreadPriority priority) { [timeConverter CAMediaTimeToEngineTime:targetTimestamp]; FlutterEngine* engine = weakSelf; if (engine) { - // It is a bit unfortunate that embedder requires OnVSync call on - // platform thread just to immediately redispatch it to UI thread. - // We are already on UI thread right now, but have to do the - // extra hop to main thread. - [engine->_threadSynchronizer performOnPlatformThread:^{ - engine->_embedderAPI.OnVsync(_engine, baton, timeNanos, targetTimeNanos); - }]; + engine->_embedderAPI.OnVsync(_engine, baton, timeNanos, targetTimeNanos); } }]; FML_DCHECK([_vsyncWaiters objectForKey:@(viewController.viewIdentifier)] == nil); @@ -1135,12 +1155,19 @@ static void SetThreadPriority(FlutterThreadPriority priority) { _keyboardManager = [[FlutterKeyboardManager alloc] initWithDelegate:self]; } +// This will be called on UI thread, which maybe or may not be platform thread, +// depending on the configuration. - (void)onVSync:(uintptr_t)baton { - @synchronized(_vsyncWaiters) { + auto block = ^{ // TODO(knopp): Use vsync waiter for correct view. // https://github.com/flutter/flutter/issues/142845 FlutterVSyncWaiter* waiter = [_vsyncWaiters objectForKey:@(kFlutterImplicitViewId)]; [waiter waitForVSync:baton]; + }; + if ([NSThread isMainThread]) { + block(); + } else { + [FlutterRunLoop.mainRunLoop performBlock:block]; } } @@ -1152,9 +1179,6 @@ static void SetThreadPriority(FlutterThreadPriority priority) { return; } - [_threadSynchronizer shutdown]; - _threadSynchronizer = nil; - FlutterEngineResult result = _embedderAPI.Deinitialize(_engine); if (result != kSuccess) { NSLog(@"Could not de-initialize the Flutter engine: error %d", result); @@ -1353,10 +1377,6 @@ static void SetThreadPriority(FlutterThreadPriority priority) { return flutter::GetSwitchesFromEnvironment(); } -- (FlutterThreadSynchronizer*)testThreadSynchronizer { - return _threadSynchronizer; -} - #pragma mark - FlutterAppLifecycleDelegate - (void)setApplicationState:(flutter::AppLifecycleState)state { @@ -1540,29 +1560,21 @@ static void SetThreadPriority(FlutterThreadPriority priority) { #pragma mark - Task runner integration -- (void)runTaskOnEmbedder:(FlutterTask)task { - if (_engine) { - auto result = _embedderAPI.RunTask(_engine, &task); - if (result != kSuccess) { - NSLog(@"Could not post a task to the Flutter engine."); - } - } -} - - (void)postMainThreadTask:(FlutterTask)task targetTimeInNanoseconds:(uint64_t)targetTime { __weak FlutterEngine* weakSelf = self; - auto worker = ^{ - [weakSelf runTaskOnEmbedder:task]; - }; const auto engine_time = _embedderAPI.GetCurrentTime(); - if (targetTime <= engine_time) { - dispatch_async(dispatch_get_main_queue(), worker); - - } else { - dispatch_after(dispatch_time(DISPATCH_TIME_NOW, targetTime - engine_time), - dispatch_get_main_queue(), worker); - } + [FlutterRunLoop.mainRunLoop + performBlock:^{ + FlutterEngine* self = weakSelf; + if (self != nil && self->_engine != nil) { + auto result = _embedderAPI.RunTask(self->_engine, &task); + if (result != kSuccess) { + NSLog(@"Could not post a task to the Flutter engine."); + } + } + } + afterDelay:(targetTime - (double)engine_time) / NSEC_PER_SEC]; } // Getter used by test harness, only exposed through the FlutterEngine(Test) category diff --git a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm index 5a7727a50b..4c3d7ed6ac 100644 --- a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm +++ b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm @@ -4,6 +4,7 @@ #import "flutter/shell/platform/darwin/macos/framework/Headers/FlutterEngine.h" #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterEngine_Internal.h" +#include "shell/platform/darwin/macos/framework/Source/FlutterResizeSynchronizer.h" #include @@ -33,8 +34,6 @@ // CREATE_NATIVE_ENTRY and MOCK_ENGINE_PROC are leaky by design // NOLINTBEGIN(clang-analyzer-core.StackAddressEscape) -constexpr int64_t kImplicitViewId = 0ll; - @interface FlutterEngine (Test) /** * The FlutterCompositor object currently in use by the FlutterEngine. @@ -526,9 +525,15 @@ TEST_F(FlutterEngineTest, Compositor) { result:^(id result){ }]; - [engine.testThreadSynchronizer blockUntilFrameAvailable]; - + // Wait up to 1 second for Flutter to emit a frame. + CFAbsoluteTime start = CFAbsoluteTimeGetCurrent(); CALayer* rootLayer = viewController.flutterView.layer; + while (rootLayer.sublayers.count == 0) { + CFRunLoopRunInMode(kCFRunLoopDefaultMode, 1, YES); + if (CFAbsoluteTimeGetCurrent() - start > 1) { + break; + } + } // There are two layers with Flutter contents and one view EXPECT_EQ(rootLayer.sublayers.count, 2u); @@ -618,6 +623,7 @@ TEST_F(FlutterEngineTest, DartEntrypointArguments) { EXPECT_TRUE([engine runWithEntrypoint:@"main"]); EXPECT_TRUE(called); + [engine shutDownEngine]; } // Verify that the engine is not retained indirectly via the binary messenger held by channels and @@ -888,15 +894,15 @@ TEST_F(FlutterEngineTest, CanGetEngineForId) { ShutDownEngine(); } -TEST_F(FlutterEngineTest, ThreadSynchronizerNotBlockingRasterThreadAfterShutdown) { - FlutterThreadSynchronizer* threadSynchronizer = [[FlutterThreadSynchronizer alloc] init]; - [threadSynchronizer shutdown]; +TEST_F(FlutterEngineTest, ResizeSynchronizerNotBlockingRasterThreadAfterShutdown) { + FlutterResizeSynchronizer* threadSynchronizer = [[FlutterResizeSynchronizer alloc] init]; + [threadSynchronizer shutDown]; std::thread rasterThread([&threadSynchronizer] { - [threadSynchronizer performCommitForView:kImplicitViewId - size:CGSizeMake(100, 100) + [threadSynchronizer performCommitForSize:CGSizeMake(100, 100) notify:^{ - }]; + } + delay:0]; }); rasterThread.join(); @@ -980,7 +986,15 @@ TEST_F(FlutterEngineTest, RemovingViewDisposesCompositorResources) { viewController.flutterView.frame = CGRectMake(0, 0, 800, 600); EXPECT_TRUE([engine runWithEntrypoint:@"drawIntoAllViews"]); - [engine.testThreadSynchronizer blockUntilFrameAvailable]; + // Wait up to 1 second for Flutter to emit a frame. + CFTimeInterval start = CACurrentMediaTime(); + while (engine.macOSCompositor->DebugNumViews() == 0) { + CFRunLoopRunInMode(kCFRunLoopDefaultMode, 1, YES); + if (CACurrentMediaTime() - start > 1) { + break; + } + } + EXPECT_EQ(engine.macOSCompositor->DebugNumViews(), 1u); engine.viewController = nil; diff --git a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterEngine_Internal.h b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterEngine_Internal.h index 567430a6df..ec34b267a9 100644 --- a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterEngine_Internal.h +++ b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterEngine_Internal.h @@ -240,10 +240,6 @@ typedef NS_ENUM(NSInteger, FlutterAppExitResponse) { + (nullable FlutterEngine*)engineForIdentifier:(int64_t)identifier; @end -@interface FlutterEngine (Tests) -- (nonnull FlutterThreadSynchronizer*)testThreadSynchronizer; -@end - NS_ASSUME_NONNULL_END #endif // FLUTTER_SHELL_PLATFORM_DARWIN_MACOS_FRAMEWORK_SOURCE_FLUTTERENGINE_INTERNAL_H_ diff --git a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterResizeSynchronizer.h b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterResizeSynchronizer.h new file mode 100644 index 0000000000..3b15aca2fd --- /dev/null +++ b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterResizeSynchronizer.h @@ -0,0 +1,43 @@ +// Copyright 2013 The Flutter 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 FLUTTER_SHELL_PLATFORM_DARWIN_MACOS_FRAMEWORK_SOURCE_FLUTTERRESIZESYNCHRONIZER_H_ +#define FLUTTER_SHELL_PLATFORM_DARWIN_MACOS_FRAMEWORK_SOURCE_FLUTTERRESIZESYNCHRONIZER_H_ + +#import + +#import "flutter/shell/platform/darwin/macos/framework/Headers/FlutterViewController.h" +#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterRunLoop.h" + +/** + * Class responsible for coordinating window resize with content update. + */ +@interface FlutterResizeSynchronizer : NSObject + +/** + * Begins a resize operation for the given size. Block the thread until + * performCommitForSize: with the same size is called. + * While the thread is blocked Flutter messages are being pumped. + * See [FlutterRunLoop pollFlutterMessagesOnce]. + */ +- (void)beginResizeForSize:(CGSize)size notify:(nonnull dispatch_block_t)notify; + +/** + * Called from raster thread. Schedules the given block on platform thread + * at given delay and unblocks the platform thread if waiting for the surface + * during resize. + */ +- (void)performCommitForSize:(CGSize)size + notify:(nonnull dispatch_block_t)notify + delay:(NSTimeInterval)delay; + +/** + * Called when the view is shut down. Unblocks platform thread if blocked + * during resize. + */ +- (void)shutDown; + +@end + +#endif // FLUTTER_SHELL_PLATFORM_DARWIN_MACOS_FRAMEWORK_SOURCE_FLUTTERRESIZESYNCHRONIZER_H_ diff --git a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterResizeSynchronizer.mm b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterResizeSynchronizer.mm new file mode 100644 index 0000000000..1effdab11a --- /dev/null +++ b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterResizeSynchronizer.mm @@ -0,0 +1,61 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterResizeSynchronizer.h" +#import "flutter/fml/logging.h" + +#include + +@implementation FlutterResizeSynchronizer { + std::atomic_bool _inResize; + BOOL _shuttingDown; + BOOL _didReceiveFrame; + CGSize _contentSize; +} + +- (void)beginResizeForSize:(CGSize)size notify:(nonnull dispatch_block_t)notify { + if (!_didReceiveFrame || _shuttingDown) { + notify(); + return; + } + + _inResize = true; + _contentSize = CGSizeMake(-1, -1); + notify(); + CFAbsoluteTime start = CFAbsoluteTimeGetCurrent(); + while (true) { + if (CGSizeEqualToSize(_contentSize, size) || _shuttingDown) { + break; + } + if (CFAbsoluteTimeGetCurrent() - start > 1.0) { + FML_LOG(ERROR) << "Resize timed out."; + break; + } + [FlutterRunLoop.mainRunLoop pollFlutterMessagesOnce]; + } + _inResize = false; +} + +- (void)performCommitForSize:(CGSize)size + notify:(nonnull dispatch_block_t)notify + delay:(NSTimeInterval)delay { + if (_inResize) { + delay = 0; + } + [FlutterRunLoop.mainRunLoop + performBlock:^{ + _didReceiveFrame = YES; + _contentSize = size; + notify(); + } + afterDelay:delay]; +} + +- (void)shutDown { + [FlutterRunLoop.mainRunLoop performBlock:^{ + _shuttingDown = YES; + }]; +} + +@end diff --git a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterResizeSynchronizerTest.mm b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterResizeSynchronizerTest.mm new file mode 100644 index 0000000000..9d50046ab5 --- /dev/null +++ b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterResizeSynchronizerTest.mm @@ -0,0 +1,121 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "flutter/fml/synchronization/waitable_event.h" +#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterResizeSynchronizer.h" +#import "flutter/testing/testing.h" + +TEST(FlutterThreadSynchronizerTest, NotBlocked) { + [FlutterRunLoop ensureMainLoopInitialized]; + + FlutterResizeSynchronizer* synchronizer = [[FlutterResizeSynchronizer alloc] init]; + __block BOOL performed = NO; + + [NSThread detachNewThreadWithBlock:^{ + [synchronizer performCommitForSize:CGSizeMake(100, 100) + notify:^{ + performed = YES; + } + delay:0]; + }]; + + CFTimeInterval start = CFAbsoluteTimeGetCurrent(); + + while (!performed && CFAbsoluteTimeGetCurrent() - start < 1.0) { + [FlutterRunLoop.mainRunLoop pollFlutterMessagesOnce]; + } + EXPECT_EQ(performed, YES); +} + +TEST(FlutterThreadSynchronizerTest, WaitForResize) { + [FlutterRunLoop ensureMainLoopInitialized]; + + FlutterResizeSynchronizer* synchronizer = [[FlutterResizeSynchronizer alloc] init]; + + __block BOOL commit1 = NO; + __block BOOL commit2 = NO; + + // Capturing c++ objects in blocks requires copy constructor, that also applies + // to __block variables where the copy is made on heap. + fml::AutoResetWaitableEvent latch_; + fml::AutoResetWaitableEvent& latch = latch_; + + // Resize synchronizer must have at received one frame in order to block. + __block BOOL didReceiveFrame = NO; + [synchronizer performCommitForSize:CGSizeMake(10, 10) + notify:^{ + didReceiveFrame = YES; + } + delay:0]; + + CFTimeInterval start = CFAbsoluteTimeGetCurrent(); + while (!didReceiveFrame && CFAbsoluteTimeGetCurrent() - start < 1.0) { + [FlutterRunLoop.mainRunLoop pollFlutterMessagesOnce]; + } + + // Now resize should block until it has received expected size. + + [NSThread detachNewThreadWithBlock:^{ + latch.Wait(); + + [synchronizer performCommitForSize:CGSizeMake(50, 100) + notify:^{ + commit1 = YES; + } + delay:0]; + + [synchronizer performCommitForSize:CGSizeMake(100, 100) + notify:^{ + commit2 = YES; + } + delay:0]; + }]; + + [synchronizer beginResizeForSize:CGSizeMake(100, 100) + notify:^{ + latch.Signal(); + }]; + + EXPECT_EQ(commit1, YES); + EXPECT_EQ(commit2, YES); +} + +TEST(FlutterThreadSynchronizerTest, UnblocksOnShutDown) { + [FlutterRunLoop ensureMainLoopInitialized]; + + FlutterResizeSynchronizer* synchronizer = [[FlutterResizeSynchronizer alloc] init]; + + // Resize synchronizer must have at received one frame in order to block. + __block BOOL didReceiveFrame = NO; + [synchronizer performCommitForSize:CGSizeMake(10, 10) + notify:^{ + didReceiveFrame = YES; + } + delay:0]; + + CFTimeInterval start = CFAbsoluteTimeGetCurrent(); + while (!didReceiveFrame && CFAbsoluteTimeGetCurrent() - start < 1.0) { + [FlutterRunLoop.mainRunLoop pollFlutterMessagesOnce]; + } + + fml::AutoResetWaitableEvent latch_; + fml::AutoResetWaitableEvent& latch = latch_; + + [NSThread detachNewThreadWithBlock:^{ + latch.Wait(); + + [synchronizer shutDown]; + }]; + + [synchronizer beginResizeForSize:CGSizeMake(100, 100) + notify:^{ + // Unblock resize + latch.Signal(); + }]; + + // Subsequent calls should not block. + [synchronizer beginResizeForSize:CGSizeMake(100, 100) + notify:^{ + }]; +} diff --git a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterRunLoop.h b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterRunLoop.h new file mode 100644 index 0000000000..86535c7f98 --- /dev/null +++ b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterRunLoop.h @@ -0,0 +1,46 @@ +#ifndef FLUTTER_SHELL_PLATFORM_DARWIN_MACOS_FRAMEWORK_SOURCE_FLUTTERRUNLOOP_H_ +#define FLUTTER_SHELL_PLATFORM_DARWIN_MACOS_FRAMEWORK_SOURCE_FLUTTERRUNLOOP_H_ + +#import + +/** + * Interface for scheduling tasks on the run loop. + * + * Main difference between using `FlutterRunLoop` to schedule tasks compared to + * `dispatch_async` or `[NSRunLoop performBlock:]` is that `FlutterRunLoop` + * schedules the task in both common run loop mode and a private run loop mode, + * which allows it to run in mode where it only processes Flutter messages + * (`[FlutterRunLoop pollFlutterMessagesOnce]`). + */ +@interface FlutterRunLoop : NSObject + +/** + * Ensures that the `FlutterRunLoop` for main thread is initialized. Only + * needs to be called once and must be called on the main thread. + */ ++ (void)ensureMainLoopInitialized; + +/** + * Returns the `FlutterRunLoop` for the main thread. + */ ++ (FlutterRunLoop*)mainRunLoop; + +/** + * Schedules a block to be executed on the main thread. + */ +- (void)performBlock:(void (^)(void))block; + +/** + * Schedules a block to be executed on the main thread after a delay. + */ +- (void)performBlock:(void (^)(void))block afterDelay:(NSTimeInterval)delay; + +/** + * Executes single iteration of the run loop in the mode where only Flutter + * messages are processed. + */ +- (void)pollFlutterMessagesOnce; + +@end + +#endif // FLUTTER_SHELL_PLATFORM_DARWIN_MACOS_FRAMEWORK_SOURCE_FLUTTERRUNLOOP_H_ diff --git a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterRunLoop.mm b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterRunLoop.mm new file mode 100644 index 0000000000..ddd38897ef --- /dev/null +++ b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterRunLoop.mm @@ -0,0 +1,127 @@ +#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterRunLoop.h" +#include +#include "fml/logging.h" + +namespace { +struct Task { + void (^block)(void); + CFAbsoluteTime target_time; + + Task(void (^block)(void), CFAbsoluteTime target_time) : block(block), target_time(target_time) {} +}; + +const CFStringRef kFlutterRunLoopMode = CFSTR("FlutterRunLoopMode"); + +FlutterRunLoop* mainLoop; + +} // namespace + +@implementation FlutterRunLoop { + CFRunLoopRef _runLoop; + CFRunLoopSourceRef _source; + CFRunLoopTimerRef _timer; + std::vector _tasks; +} + +static void Perform(void* info) { + FlutterRunLoop* runner = (__bridge FlutterRunLoop*)info; + [runner performExpiredTasks]; +} + +static void PerformTimer(CFRunLoopTimerRef timer, void* info) { + FlutterRunLoop* runner = (__bridge FlutterRunLoop*)info; + [runner performExpiredTasks]; +} + +- (instancetype)init { + if (self = [super init]) { + _runLoop = CFRunLoopGetCurrent(); + CFRunLoopSourceContext sourceContext = { + .info = (__bridge void*)self, + .perform = Perform, + }; + _source = CFRunLoopSourceCreate(kCFAllocatorDefault, 0, &sourceContext); + CFRunLoopAddSource(_runLoop, _source, kCFRunLoopCommonModes); + CFRunLoopAddSource(_runLoop, _source, kFlutterRunLoopMode); + + CFRunLoopTimerContext timerContext = { + .info = (__bridge void*)self, + }; + _timer = CFRunLoopTimerCreate(kCFAllocatorDefault, HUGE_VALF, HUGE_VALF, 0, 0, PerformTimer, + &timerContext); + CFRunLoopAddTimer(_runLoop, _timer, kCFRunLoopCommonModes); + CFRunLoopAddTimer(_runLoop, _timer, kFlutterRunLoopMode); + } + return self; +} + +- (void)dealloc { + CFRunLoopTimerInvalidate(_timer); + CFRunLoopRemoveTimer(_runLoop, _timer, kCFRunLoopCommonModes); + CFRunLoopRemoveTimer(_runLoop, _timer, kFlutterRunLoopMode); + CFRunLoopSourceInvalidate(_source); + CFRunLoopRemoveSource(_runLoop, _source, kCFRunLoopCommonModes); + CFRunLoopRemoveSource(_runLoop, _source, kFlutterRunLoopMode); +} + +- (void)rearmTimer { + CFAbsoluteTime nextFireTime = HUGE_VALF; + for (const auto& task : _tasks) { + nextFireTime = std::min(nextFireTime, task.target_time); + } + CFRunLoopTimerSetNextFireDate(_timer, nextFireTime); +} + +- (void)performExpiredTasks { + std::vector expiredTasks; + @synchronized(self) { + CFAbsoluteTime now = CFAbsoluteTimeGetCurrent(); + std::vector::iterator it = _tasks.begin(); + while (it != _tasks.end()) { + if (it->target_time <= now) { + expiredTasks.push_back(std::move(*it)); + it = _tasks.erase(it); + } else { + ++it; + } + } + [self rearmTimer]; + } + for (const auto& task : expiredTasks) { + task.block(); + } +} + +- (void)performBlock:(void (^)(void))block afterDelay:(NSTimeInterval)delay { + @synchronized(self) { + _tasks.emplace_back(block, CFAbsoluteTimeGetCurrent() + delay); + if (delay > 0) { + [self rearmTimer]; + } else { + CFRunLoopSourceSignal(_source); + CFRunLoopWakeUp(_runLoop); + } + } +} + +- (void)performBlock:(void (^)(void))block { + [self performBlock:block afterDelay:0]; +} + ++ (void)ensureMainLoopInitialized { + FML_DCHECK(NSRunLoop.currentRunLoop == NSRunLoop.mainRunLoop); + if (mainLoop == nil) { + mainLoop = [[FlutterRunLoop alloc] init]; + } +} + ++ (FlutterRunLoop*)mainRunLoop { + FML_DCHECK(mainLoop != nil); + return mainLoop; +} + +- (void)pollFlutterMessagesOnce { + CFRunLoopRunInMode(kFlutterRunLoopMode, 0.1, YES); +} + +@end diff --git a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h index d515741538..888b41e181 100644 --- a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h +++ b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h @@ -27,11 +27,13 @@ @protocol FlutterSurfaceManagerDelegate /* - * Schedules the block on the platform thread and blocks until the block is executed. + * Schedules the block on the platform thread. * Provided `frameSize` is used to unblock the platform thread if it waits for * a certain frame size during resizing. */ -- (void)onPresent:(CGSize)frameSize withBlock:(nonnull dispatch_block_t)block; +- (void)onPresent:(CGSize)frameSize + withBlock:(nonnull dispatch_block_t)block + delay:(NSTimeInterval)delay; @end diff --git a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.mm b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.mm index d87aed4c9e..a021a8b625 100644 --- a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.mm +++ b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.mm @@ -222,18 +222,9 @@ static CGSize GetRequiredFrameSize(NSArray* surfaces [commandBuffer commit]; [commandBuffer waitUntilScheduled]; - dispatch_block_t presentBlock = ^{ - // Get the actual dimensions of the frame (relevant for thread synchronizer). - CGSize size = GetRequiredFrameSize(surfaces); - [_delegate onPresent:size - withBlock:^{ - _lastPresentationTime = presentationTime; - [self commit:surfaces]; - if (notify != nil) { - notify(); - } - }]; - }; + CGSize size = GetRequiredFrameSize(surfaces); + + CFTimeInterval delay = 0; if (presentationTime > 0) { // Enforce frame pacing. It seems that the target timestamp of CVDisplayLink does not @@ -249,17 +240,20 @@ static CGSize GetRequiredFrameSize(NSArray* surfaces // as a average_frame_rasterizer_time_millis regresson. CFTimeInterval minPresentationTime = (presentationTime + _lastPresentationTime) / 2.0; CFTimeInterval now = CACurrentMediaTime(); - if (now < minPresentationTime) { - NSTimer* timer = [NSTimer timerWithTimeInterval:minPresentationTime - now - repeats:NO - block:^(NSTimer* timer) { - presentBlock(); - }]; - [[NSRunLoop currentRunLoop] addTimer:timer forMode:NSDefaultRunLoopMode]; - return; - } + delay = std::max(minPresentationTime - now, 0.0); } - presentBlock(); + [_delegate onPresent:size + withBlock:^{ + _lastPresentationTime = presentationTime; + [CATransaction begin]; + [CATransaction setDisableActions:YES]; + [self commit:surfaces]; + if (notify != nil) { + notify(); + } + [CATransaction commit]; + } + delay:delay]; } @end diff --git a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManagerTest.mm b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManagerTest.mm index 8d6996280e..30a2248b93 100644 --- a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManagerTest.mm +++ b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManagerTest.mm @@ -29,7 +29,9 @@ return self; } -- (void)onPresent:(CGSize)frameSize withBlock:(nonnull dispatch_block_t)block { +- (void)onPresent:(CGSize)frameSize + withBlock:(nonnull dispatch_block_t)block + delay:(NSTimeInterval)delay { self.presentedFrameSize = frameSize; block(); } diff --git a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.h b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.h deleted file mode 100644 index a7a1020077..0000000000 --- a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.h +++ /dev/null @@ -1,112 +0,0 @@ -// Copyright 2013 The Flutter 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 FLUTTER_SHELL_PLATFORM_DARWIN_MACOS_FRAMEWORK_SOURCE_FLUTTERTHREADSYNCHRONIZER_H_ -#define FLUTTER_SHELL_PLATFORM_DARWIN_MACOS_FRAMEWORK_SOURCE_FLUTTERTHREADSYNCHRONIZER_H_ - -#import - -#import "flutter/shell/platform/darwin/macos/framework/Headers/FlutterViewController.h" - -/** - * Takes care of synchronization between raster and platform thread. - * - * All methods of this class must be called from the platform thread, - * except for performCommitForView:size:notify:. - */ -@interface FlutterThreadSynchronizer : NSObject - -/** - * Creates a FlutterThreadSynchronizer that uses the OS main thread as the - * platform thread. - */ -- (nullable instancetype)init; - -/** - * Blocks until all views have a commit with their given sizes (or empty) is requested. - */ -- (void)beginResizeForView:(FlutterViewIdentifier)viewIdentifier - size:(CGSize)size - notify:(nonnull dispatch_block_t)notify; - -/** - * Called from raster thread. Schedules the given block on platform thread - * and blocks until it is performed. - * - * If platform thread is blocked in `beginResize:` for given size (or size is empty), - * unblocks platform thread. - * - * The notify block is guaranteed to be called within a core animation transaction. - */ -- (void)performCommitForView:(FlutterViewIdentifier)viewIdentifier - size:(CGSize)size - notify:(nonnull dispatch_block_t)notify; - -/** - * Schedules the given block to be performed on the platform thread. - * The block will be performed even if the platform thread is blocked waiting - * for a commit. - */ -- (void)performOnPlatformThread:(nonnull dispatch_block_t)block; - -/** - * Requests the synchronizer to track another view. - * - * A view must be registered before calling begineResizeForView: or - * performCommitForView:. It is typically done when the view controller is - * created. - */ -- (void)registerView:(FlutterViewIdentifier)viewIdentifier; - -/** - * Requests the synchronizer to no longer track a view. - * - * It is typically done when the view controller is destroyed. - */ -- (void)deregisterView:(FlutterViewIdentifier)viewIdentifier; - -/** - * Called when the engine shuts down. - * - * Prevents any further synchronization and no longer blocks any threads. - */ -- (void)shutdown; - -@end - -@interface FlutterThreadSynchronizer (TestUtils) - -/** - * Creates a FlutterThreadSynchronizer that uses the specified queue as the - * platform thread. - */ -- (nullable instancetype)initWithMainQueue:(nonnull dispatch_queue_t)queue; - -/** - * Blocks current thread until the mutex is available, then return whether the - * synchronizer is waiting for a correct commit during resizing. - * - * After calling an operation of the thread synchronizer, call this method, - * and when it returns, the thread synchronizer can be at one of the following 3 - * states: - * - * 1. The operation has not started at all (with a return value FALSE.) - * 2. The operation has ended (with a return value FALSE.) - * 3. beginResizeForView: is in progress, waiting (with a return value TRUE.) - * - * By eliminating the 1st case (such as using the notify callback), we can use - * this return value to decide whether the synchronizer is in case 2 or case 3, - * that is whether the resizing is blocked by a mismatching commit. - */ -- (BOOL)isWaitingWhenMutexIsAvailable; - -/** - * Blocks current thread until there is frame available. - * Used in FlutterEngineTest. - */ -- (void)blockUntilFrameAvailable; - -@end - -#endif // FLUTTER_SHELL_PLATFORM_DARWIN_MACOS_FRAMEWORK_SOURCE_FLUTTERTHREADSYNCHRONIZER_H_ diff --git a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.mm b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.mm deleted file mode 100644 index b1de918a9a..0000000000 --- a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.mm +++ /dev/null @@ -1,205 +0,0 @@ -// Copyright 2013 The Flutter Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.h" - -#import - -#include -#include -#include - -#import "flutter/fml/logging.h" -#import "flutter/fml/synchronization/waitable_event.h" - -@interface FlutterThreadSynchronizer () { - dispatch_queue_t _mainQueue; - std::mutex _mutex; - BOOL _shuttingDown; - std::unordered_map _contentSizes; - std::vector _scheduledBlocks; - - BOOL _beginResizeWaiting; - - // Used to block [beginResize:]. - std::condition_variable _condBlockBeginResize; -} - -/** - * Returns true if all existing views have a non-zero size. - * - * If there are no views, still returns true. - */ -- (BOOL)allViewsHaveFrame; - -/** - * Returns true if there are any views that have a non-zero size. - * - * If there are no views, returns false. - */ -- (BOOL)someViewsHaveFrame; - -@end - -@implementation FlutterThreadSynchronizer - -- (instancetype)init { - return [self initWithMainQueue:dispatch_get_main_queue()]; -} - -- (instancetype)initWithMainQueue:(dispatch_queue_t)queue { - self = [super init]; - if (self != nil) { - _mainQueue = queue; - } - return self; -} - -- (BOOL)allViewsHaveFrame { - for (auto const& [viewIdentifier, contentSize] : _contentSizes) { - if (CGSizeEqualToSize(contentSize, CGSizeZero)) { - return NO; - } - } - return YES; -} - -- (BOOL)someViewsHaveFrame { - for (auto const& [viewIdentifier, contentSize] : _contentSizes) { - if (!CGSizeEqualToSize(contentSize, CGSizeZero)) { - return YES; - } - } - return NO; -} - -- (void)drain { - dispatch_assert_queue(_mainQueue); - - [CATransaction begin]; - [CATransaction setDisableActions:YES]; - for (dispatch_block_t block : _scheduledBlocks) { - block(); - } - [CATransaction commit]; - _scheduledBlocks.clear(); -} - -- (void)blockUntilFrameAvailable { - std::unique_lock lock(_mutex); - [self drain]; - - _beginResizeWaiting = YES; - while (![self someViewsHaveFrame] && !_shuttingDown) { - _condBlockBeginResize.wait(lock); - [self drain]; - } - - _beginResizeWaiting = NO; -} - -- (void)beginResizeForView:(FlutterViewIdentifier)viewIdentifier - size:(CGSize)size - notify:(nonnull dispatch_block_t)notify { - dispatch_assert_queue(_mainQueue); - std::unique_lock lock(_mutex); - - if (![self allViewsHaveFrame] || _shuttingDown) { - // No blocking until framework produces at least one frame - notify(); - return; - } - - [self drain]; - - notify(); - - _contentSizes[viewIdentifier] = CGSizeMake(-1, -1); - - _beginResizeWaiting = YES; - - while (true) { - if (_shuttingDown) { - break; - } - const CGSize& contentSize = _contentSizes[viewIdentifier]; - if (CGSizeEqualToSize(contentSize, size) || CGSizeEqualToSize(contentSize, CGSizeZero)) { - break; - } - _condBlockBeginResize.wait(lock); - [self drain]; - } - - _beginResizeWaiting = NO; -} - -- (void)performCommitForView:(FlutterViewIdentifier)viewIdentifier - size:(CGSize)size - notify:(nonnull dispatch_block_t)notify { - dispatch_assert_queue_not(_mainQueue); - fml::AutoResetWaitableEvent event; - { - std::unique_lock lock(_mutex); - if (_shuttingDown) { - // Engine is shutting down, main thread may be blocked by the engine - // waiting for raster thread to finish. - return; - } - fml::AutoResetWaitableEvent& e = event; - _scheduledBlocks.push_back(^{ - notify(); - _contentSizes[viewIdentifier] = size; - e.Signal(); - }); - if (_beginResizeWaiting) { - _condBlockBeginResize.notify_all(); - } else { - dispatch_async(_mainQueue, ^{ - std::unique_lock lock(_mutex); - [self drain]; - }); - } - } - event.Wait(); -} - -- (void)performOnPlatformThread:(nonnull dispatch_block_t)block { - std::unique_lock lock(_mutex); - _scheduledBlocks.push_back(block); - if (_beginResizeWaiting) { - _condBlockBeginResize.notify_all(); - } else { - dispatch_async(_mainQueue, ^{ - std::unique_lock lock(_mutex); - [self drain]; - }); - } -} - -- (void)registerView:(FlutterViewIdentifier)viewIdentifier { - dispatch_assert_queue(_mainQueue); - std::unique_lock lock(_mutex); - _contentSizes[viewIdentifier] = CGSizeZero; -} - -- (void)deregisterView:(FlutterViewIdentifier)viewIdentifier { - dispatch_assert_queue(_mainQueue); - std::unique_lock lock(_mutex); - _contentSizes.erase(viewIdentifier); -} - -- (void)shutdown { - dispatch_assert_queue(_mainQueue); - std::unique_lock lock(_mutex); - _shuttingDown = YES; - _condBlockBeginResize.notify_all(); - [self drain]; -} - -- (BOOL)isWaitingWhenMutexIsAvailable { - std::unique_lock lock(_mutex); - return _beginResizeWaiting; -} - -@end diff --git a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizerTest.mm b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizerTest.mm deleted file mode 100644 index efa5fce563..0000000000 --- a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizerTest.mm +++ /dev/null @@ -1,374 +0,0 @@ -// Copyright 2013 The Flutter Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.h" - -#import "flutter/fml/synchronization/waitable_event.h" -#import "flutter/testing/testing.h" - -@interface FlutterThreadSynchronizerTestScaffold : NSObject - -@property(nonatomic, readonly, nonnull) FlutterThreadSynchronizer* synchronizer; - -- (nullable instancetype)init; -- (void)dispatchMainTask:(nonnull void (^)())task; -- (void)dispatchRenderTask:(nonnull void (^)())task; -- (void)joinMain; -- (void)joinRender; -@end - -@implementation FlutterThreadSynchronizerTestScaffold { - dispatch_queue_t _mainQueue; - std::shared_ptr _mainLatch; - - dispatch_queue_t _renderQueue; - std::shared_ptr _renderLatch; - - FlutterThreadSynchronizer* _synchronizer; -} - -- (nullable instancetype)init { - self = [super init]; - if (self != nil) { - _mainQueue = dispatch_queue_create("MAIN", DISPATCH_QUEUE_SERIAL); - _renderQueue = dispatch_queue_create("RENDER", DISPATCH_QUEUE_SERIAL); - _synchronizer = [[FlutterThreadSynchronizer alloc] initWithMainQueue:_mainQueue]; - } - return self; -} - -- (void)dispatchMainTask:(nonnull void (^)())task { - dispatch_async(_mainQueue, task); -} - -- (void)dispatchRenderTask:(nonnull void (^)())task { - dispatch_async(_renderQueue, task); -} - -- (void)joinMain { - fml::AutoResetWaitableEvent latch; - fml::AutoResetWaitableEvent* pLatch = &latch; - dispatch_async(_mainQueue, ^{ - pLatch->Signal(); - }); - latch.Wait(); -} - -- (void)joinRender { - fml::AutoResetWaitableEvent latch; - fml::AutoResetWaitableEvent* pLatch = &latch; - dispatch_async(_renderQueue, ^{ - pLatch->Signal(); - }); - latch.Wait(); -} - -@end - -TEST(FlutterThreadSynchronizerTest, RegularCommit) { - FlutterThreadSynchronizerTestScaffold* scaffold = - [[FlutterThreadSynchronizerTestScaffold alloc] init]; - FlutterThreadSynchronizer* synchronizer = scaffold.synchronizer; - - // Initial resize: does not block until the first frame. - __block int notifiedResize = 0; - [scaffold dispatchMainTask:^{ - [synchronizer registerView:1]; - [synchronizer beginResizeForView:1 - size:CGSize{5, 5} - notify:^{ - notifiedResize += 1; - }]; - }]; - EXPECT_FALSE([synchronizer isWaitingWhenMutexIsAvailable]); - [scaffold joinMain]; - EXPECT_EQ(notifiedResize, 1); - - // Still does not block. - [scaffold dispatchMainTask:^{ - [synchronizer beginResizeForView:1 - size:CGSize{7, 7} - notify:^{ - notifiedResize += 1; - }]; - }]; - EXPECT_FALSE([synchronizer isWaitingWhenMutexIsAvailable]); - [scaffold joinMain]; - EXPECT_EQ(notifiedResize, 2); - - // First frame - __block int notifiedCommit = 0; - [scaffold dispatchRenderTask:^{ - [synchronizer performCommitForView:1 - size:CGSize{7, 7} - notify:^{ - notifiedCommit += 1; - }]; - }]; - EXPECT_FALSE([synchronizer isWaitingWhenMutexIsAvailable]); - [scaffold joinRender]; - EXPECT_EQ(notifiedCommit, 1); -} - -TEST(FlutterThreadSynchronizerTest, ResizingBlocksRenderingUntilSizeMatches) { - FlutterThreadSynchronizerTestScaffold* scaffold = - [[FlutterThreadSynchronizerTestScaffold alloc] init]; - FlutterThreadSynchronizer* synchronizer = scaffold.synchronizer; - // A latch to ensure that a beginResizeForView: call has at least executed - // something, so that the isWaitingWhenMutexIsAvailable: call correctly stops - // at either when beginResizeForView: finishes or waits half way. - fml::AutoResetWaitableEvent begunResizingLatch; - fml::AutoResetWaitableEvent* begunResizing = &begunResizingLatch; - - // Initial resize: does not block until the first frame. - [scaffold dispatchMainTask:^{ - [synchronizer registerView:1]; - [synchronizer beginResizeForView:1 - size:CGSize{5, 5} - notify:^{ - }]; - }]; - [scaffold joinMain]; - EXPECT_FALSE([synchronizer isWaitingWhenMutexIsAvailable]); - - // First frame. - [scaffold dispatchRenderTask:^{ - [synchronizer performCommitForView:1 - size:CGSize{5, 5} - notify:^{ - }]; - }]; - [scaffold joinRender]; - EXPECT_FALSE([synchronizer isWaitingWhenMutexIsAvailable]); - - // Resize to (7, 7): blocks until the next frame. - [scaffold dispatchMainTask:^{ - [synchronizer beginResizeForView:1 - size:CGSize{7, 7} - notify:^{ - begunResizing->Signal(); - }]; - }]; - begunResizing->Wait(); - EXPECT_TRUE([synchronizer isWaitingWhenMutexIsAvailable]); - - // Render with old size. - [scaffold dispatchRenderTask:^{ - [synchronizer performCommitForView:1 - size:CGSize{5, 5} - notify:^{ - }]; - }]; - [scaffold joinRender]; - EXPECT_TRUE([synchronizer isWaitingWhenMutexIsAvailable]); - - // Render with new size. - [scaffold dispatchRenderTask:^{ - [synchronizer performCommitForView:1 - size:CGSize{7, 7} - notify:^{ - }]; - }]; - [scaffold joinRender]; - EXPECT_FALSE([synchronizer isWaitingWhenMutexIsAvailable]); - - [scaffold joinMain]; -} - -TEST(FlutterThreadSynchronizerTest, ShutdownMakesEverythingNonBlocking) { - FlutterThreadSynchronizerTestScaffold* scaffold = - [[FlutterThreadSynchronizerTestScaffold alloc] init]; - FlutterThreadSynchronizer* synchronizer = scaffold.synchronizer; - fml::AutoResetWaitableEvent begunResizingLatch; - fml::AutoResetWaitableEvent* begunResizing = &begunResizingLatch; - - // Initial resize - [scaffold dispatchMainTask:^{ - [synchronizer registerView:1]; - [synchronizer beginResizeForView:1 - size:CGSize{5, 5} - notify:^{ - }]; - }]; - [scaffold joinMain]; - EXPECT_FALSE([synchronizer isWaitingWhenMutexIsAvailable]); - - // Push a frame. - [scaffold dispatchRenderTask:^{ - [synchronizer performCommitForView:1 - size:CGSize{5, 5} - notify:^{ - }]; - }]; - [scaffold joinRender]; - EXPECT_FALSE([synchronizer isWaitingWhenMutexIsAvailable]); - - [scaffold dispatchMainTask:^{ - [synchronizer shutdown]; - }]; - - // Resize to (7, 7). Should not block any frames since it has shut down. - [scaffold dispatchMainTask:^{ - [synchronizer beginResizeForView:1 - size:CGSize{7, 7} - notify:^{ - begunResizing->Signal(); - }]; - }]; - begunResizing->Wait(); - EXPECT_FALSE([synchronizer isWaitingWhenMutexIsAvailable]); - [scaffold joinMain]; - - // All further calls should be unblocking. - [scaffold dispatchRenderTask:^{ - [synchronizer performCommitForView:1 - size:CGSize{9, 9} - notify:^{ - }]; - }]; - [scaffold joinRender]; - EXPECT_FALSE([synchronizer isWaitingWhenMutexIsAvailable]); -} - -TEST(FlutterThreadSynchronizerTest, RegularCommitForMultipleViews) { - FlutterThreadSynchronizerTestScaffold* scaffold = - [[FlutterThreadSynchronizerTestScaffold alloc] init]; - FlutterThreadSynchronizer* synchronizer = scaffold.synchronizer; - fml::AutoResetWaitableEvent begunResizingLatch; - fml::AutoResetWaitableEvent* begunResizing = &begunResizingLatch; - - // Initial resize: does not block until the first frame. - [scaffold dispatchMainTask:^{ - [synchronizer registerView:1]; - [synchronizer registerView:2]; - [synchronizer beginResizeForView:1 - size:CGSize{5, 5} - notify:^{ - }]; - [synchronizer beginResizeForView:2 - size:CGSize{15, 15} - notify:^{ - begunResizing->Signal(); - }]; - }]; - begunResizing->Wait(); - EXPECT_FALSE([synchronizer isWaitingWhenMutexIsAvailable]); - [scaffold joinMain]; - - // Still does not block. - [scaffold dispatchMainTask:^{ - [synchronizer beginResizeForView:1 - size:CGSize{7, 7} - notify:^{ - begunResizing->Signal(); - }]; - }]; - begunResizing->Signal(); - EXPECT_FALSE([synchronizer isWaitingWhenMutexIsAvailable]); - [scaffold joinMain]; - - // First frame - [scaffold dispatchRenderTask:^{ - [synchronizer performCommitForView:1 - size:CGSize{7, 7} - notify:^{ - }]; - [synchronizer performCommitForView:2 - size:CGSize{15, 15} - notify:^{ - }]; - }]; - [scaffold joinRender]; - EXPECT_FALSE([synchronizer isWaitingWhenMutexIsAvailable]); -} - -TEST(FlutterThreadSynchronizerTest, ResizingForMultipleViews) { - FlutterThreadSynchronizerTestScaffold* scaffold = - [[FlutterThreadSynchronizerTestScaffold alloc] init]; - FlutterThreadSynchronizer* synchronizer = scaffold.synchronizer; - fml::AutoResetWaitableEvent begunResizingLatch; - fml::AutoResetWaitableEvent* begunResizing = &begunResizingLatch; - - // Initial resize: does not block until the first frame. - [scaffold dispatchMainTask:^{ - [synchronizer registerView:1]; - [synchronizer registerView:2]; - [synchronizer beginResizeForView:1 - size:CGSize{5, 5} - notify:^{ - }]; - [synchronizer beginResizeForView:2 - size:CGSize{15, 15} - notify:^{ - }]; - }]; - [scaffold joinMain]; - EXPECT_FALSE([synchronizer isWaitingWhenMutexIsAvailable]); - - // First frame. - [scaffold dispatchRenderTask:^{ - [synchronizer performCommitForView:1 - size:CGSize{5, 5} - notify:^{ - }]; - [synchronizer performCommitForView:2 - size:CGSize{15, 15} - notify:^{ - }]; - }]; - [scaffold joinRender]; - EXPECT_FALSE([synchronizer isWaitingWhenMutexIsAvailable]); - - // Resize view 2 to (17, 17): blocks until the next frame. - [scaffold dispatchMainTask:^{ - [synchronizer beginResizeForView:2 - size:CGSize{17, 17} - notify:^{ - begunResizing->Signal(); - }]; - }]; - begunResizing->Wait(); - EXPECT_TRUE([synchronizer isWaitingWhenMutexIsAvailable]); - - // Render view 1 with the size. Still blocking. - [scaffold dispatchRenderTask:^{ - [synchronizer performCommitForView:1 - size:CGSize{5, 5} - notify:^{ - }]; - }]; - [scaffold joinRender]; - EXPECT_TRUE([synchronizer isWaitingWhenMutexIsAvailable]); - - // Render view 2 with the old size. Still blocking. - [scaffold dispatchRenderTask:^{ - [synchronizer performCommitForView:1 - size:CGSize{15, 15} - notify:^{ - }]; - }]; - [scaffold joinRender]; - EXPECT_TRUE([synchronizer isWaitingWhenMutexIsAvailable]); - - // Render view 1 with the size. - [scaffold dispatchRenderTask:^{ - [synchronizer performCommitForView:1 - size:CGSize{5, 5} - notify:^{ - }]; - }]; - [scaffold joinRender]; - EXPECT_TRUE([synchronizer isWaitingWhenMutexIsAvailable]); - - // Render view 2 with the new size. Unblocks. - [scaffold dispatchRenderTask:^{ - [synchronizer performCommitForView:2 - size:CGSize{17, 17} - notify:^{ - }]; - }]; - [scaffold joinRender]; - [scaffold joinMain]; - EXPECT_FALSE([synchronizer isWaitingWhenMutexIsAvailable]); -} diff --git a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterVSyncWaiter.h b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterVSyncWaiter.h index 2bb574f775..712a7bcfef 100644 --- a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterVSyncWaiter.h +++ b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterVSyncWaiter.h @@ -10,15 +10,14 @@ /// Creates new waiter instance tied to provided NSView. /// This function must be called on the main thread. /// -/// Provided |block| will be invoked on same thread as -waitForVSync:. +/// Provided |block| will be invoked on main thread. - (instancetype)initWithDisplayLink:(FlutterDisplayLink*)displayLink block:(void (^)(CFTimeInterval timestamp, CFTimeInterval targetTimestamp, uintptr_t baton))block; /// Schedules |baton| to be signaled on next display refresh. -/// The block provided in the initializer will be invoked on same thread -/// as this method (there must be a run loop associated with current thread). +/// This function must be called on the main thread. - (void)waitForVSync:(uintptr_t)baton; @end diff --git a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterVSyncWaiter.mm b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterVSyncWaiter.mm index eb4d6fb9b3..884ae11216 100644 --- a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterVSyncWaiter.mm +++ b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterVSyncWaiter.mm @@ -1,5 +1,6 @@ #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterVSyncWaiter.h" #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterDisplayLink.h" +#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterRunLoop.h" #include "flutter/fml/logging.h" @@ -37,7 +38,6 @@ static const CFTimeInterval kTimerLatencyCompensation = 0.001; std::optional _pendingBaton; FlutterDisplayLink* _displayLink; void (^_block)(CFTimeInterval, CFTimeInterval, uintptr_t); - NSRunLoop* _runLoop; CFTimeInterval _lastTargetTimestamp; BOOL _warmUpFrame; } @@ -59,75 +59,52 @@ static const CFTimeInterval kTimerLatencyCompensation = 0.001; return self; } -// Called on same thread as the vsync request (UI thread). -- (void)processDisplayLink:(CFTimeInterval)timestamp - targetTimestamp:(CFTimeInterval)targetTimestamp { - FML_DCHECK([NSRunLoop currentRunLoop] == _runLoop); - - _lastTargetTimestamp = targetTimestamp; - - // CVDisplayLink callback is called one and a half frame before the target - // timestamp. That can cause frame-pacing issues if the frame is rendered too early, - // it may also trigger frame start before events are processed. - CFTimeInterval minStart = targetTimestamp - _displayLink.nominalOutputRefreshPeriod; - CFTimeInterval current = CACurrentMediaTime(); - CFTimeInterval remaining = std::max(minStart - current - kTimerLatencyCompensation, 0.0); - - TRACE_VSYNC("DisplayLinkCallback-Original", _pendingBaton.value_or(0)); - - NSTimer* timer = [NSTimer - timerWithTimeInterval:remaining - repeats:NO - block:^(NSTimer* _Nonnull timer) { - if (!_pendingBaton.has_value()) { - TRACE_VSYNC("DisplayLinkPaused", size_t(0)); - _displayLink.paused = YES; - return; - } - TRACE_VSYNC("DisplayLinkCallback-Delayed", _pendingBaton.value_or(0)); - _block(minStart, targetTimestamp, *_pendingBaton); - _pendingBaton = std::nullopt; - }]; - [_runLoop addTimer:timer forMode:NSRunLoopCommonModes]; -} - // Called from display link thread. - (void)onDisplayLink:(CFTimeInterval)timestamp targetTimestamp:(CFTimeInterval)targetTimestamp { - @synchronized(self) { - if (_runLoop == nil) { - // Initial vsync - timestamp will be used to determine vsync phase. - _lastTargetTimestamp = targetTimestamp; - _displayLink.paused = YES; - } else { - [_runLoop performBlock:^{ - [self processDisplayLink:timestamp targetTimestamp:targetTimestamp]; - }]; - } + if (_lastTargetTimestamp == 0) { + // Initial vsync - timestamp will be used to determine vsync phase. + _lastTargetTimestamp = targetTimestamp; + _displayLink.paused = YES; + } else { + _lastTargetTimestamp = targetTimestamp; + + // CVDisplayLink callback is called one and a half frame before the target + // timestamp. That can cause frame-pacing issues if the frame is rendered too early, + // it may also trigger frame start before events are processed. + CFTimeInterval minStart = targetTimestamp - _displayLink.nominalOutputRefreshPeriod; + CFTimeInterval current = CACurrentMediaTime(); + CFTimeInterval remaining = std::max(minStart - current - kTimerLatencyCompensation, 0.0); + + TRACE_VSYNC("DisplayLinkCallback-Original", _pendingBaton.value_or(0)); + + [FlutterRunLoop.mainRunLoop + performBlock:^{ + if (!_pendingBaton.has_value()) { + TRACE_VSYNC("DisplayLinkPaused", size_t(0)); + _displayLink.paused = YES; + return; + } + TRACE_VSYNC("DisplayLinkCallback-Delayed", _pendingBaton.value_or(0)); + _block(minStart, targetTimestamp, *_pendingBaton); + _pendingBaton = std::nullopt; + } + afterDelay:remaining]; } } // Called from UI thread. - (void)waitForVSync:(uintptr_t)baton { + FML_DCHECK([NSThread isMainThread]); // CVDisplayLink start -> callback latency is two frames, there is // no need to delay the warm-up frame. if (_warmUpFrame) { _warmUpFrame = NO; TRACE_VSYNC("WarmUpFrame", baton); - [[NSRunLoop currentRunLoop] performBlock:^{ - CFTimeInterval now = CACurrentMediaTime(); - _block(now, now, baton); - }]; + CFTimeInterval now = CACurrentMediaTime(); + _block(now, now, baton); return; } - // RunLoop is accessed both from main thread and from the display link thread. - @synchronized(self) { - if (_runLoop == nil) { - _runLoop = [NSRunLoop currentRunLoop]; - } - } - - FML_DCHECK(_runLoop == [NSRunLoop currentRunLoop]); if (_pendingBaton.has_value()) { FML_LOG(WARNING) << "Engine requested vsync while another was pending"; _block(0, 0, *_pendingBaton); @@ -161,15 +138,13 @@ static const CFTimeInterval kTimerLatencyCompensation = 0.001; delay = std::max(start - now - kTimerLatencyCompensation, 0.0); } - NSTimer* timer = [NSTimer timerWithTimeInterval:delay - repeats:NO - block:^(NSTimer* timer) { - CFTimeInterval targetTimestamp = - start + tick_interval; - TRACE_VSYNC("SynthesizedInitialVSync", baton); - _block(start, targetTimestamp, baton); - }]; - [_runLoop addTimer:timer forMode:NSRunLoopCommonModes]; + [FlutterRunLoop.mainRunLoop + performBlock:^{ + CFTimeInterval targetTimestamp = start + tick_interval; + TRACE_VSYNC("SynthesizedInitialVSync", baton); + _block(start, targetTimestamp, baton); + } + afterDelay:delay]; _displayLink.paused = NO; } else { _pendingBaton = baton; @@ -180,7 +155,12 @@ static const CFTimeInterval kTimerLatencyCompensation = 0.001; if (_pendingBaton.has_value()) { FML_LOG(WARNING) << "Deallocating FlutterVSyncWaiter with a pending vsync"; } - [_displayLink invalidate]; + // It is possible that block running on UI thread held the last reference to + // the waiter, in which case reschedule to main thread. + FlutterDisplayLink* link = _displayLink; + dispatch_async(dispatch_get_main_queue(), ^{ + [link invalidate]; + }); } @end diff --git a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterVSyncWaiterTest.mm b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterVSyncWaiterTest.mm index b7637104aa..6e3882243f 100644 --- a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterVSyncWaiterTest.mm +++ b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterVSyncWaiterTest.mm @@ -3,6 +3,7 @@ // found in the LICENSE file. #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterDisplayLink.h" +#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterRunLoop.h" #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterVSyncWaiter.h" #import "flutter/testing/testing.h" @@ -35,7 +36,12 @@ @end -TEST(FlutterVSyncWaiterTest, RequestsInitialVSync) { +class FlutterVSyncWaiterTest : public testing::Test { + public: + void SetUp() override { [FlutterRunLoop ensureMainLoopInitialized]; } +}; + +TEST_F(FlutterVSyncWaiterTest, RequestsInitialVSync) { TestDisplayLink* displayLink = [[TestDisplayLink alloc] init]; EXPECT_TRUE(displayLink.paused); // When created waiter requests a reference vsync to determine vsync phase. @@ -60,7 +66,7 @@ static void BusyWait(CFTimeInterval duration) { // See FlutterVSyncWaiter.mm for the original definition. static const CFTimeInterval kTimerLatencyCompensation = 0.001; -TEST(FlutterVSyncWaiterTest, FirstVSyncIsSynthesized) { +TEST_F(FlutterVSyncWaiterTest, FirstVSyncIsSynthesized) { TestDisplayLink* displayLink = [[TestDisplayLink alloc] init]; displayLink.nominalOutputRefreshPeriod = 1.0 / 60.0; @@ -114,7 +120,7 @@ TEST(FlutterVSyncWaiterTest, FirstVSyncIsSynthesized) { test(0.040, 3 * displayLink.nominalOutputRefreshPeriod); } -TEST(FlutterVSyncWaiterTest, VSyncWorks) { +TEST_F(FlutterVSyncWaiterTest, VSyncWorks) { TestDisplayLink* displayLink = [[TestDisplayLink alloc] init]; displayLink.nominalOutputRefreshPeriod = 1.0 / 60.0; const uintptr_t kWarmUpBaton = 0xFFFFFFFF; diff --git a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterView.h b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterView.h index e22e50f6af..35b3430d9d 100644 --- a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterView.h +++ b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterView.h @@ -7,8 +7,8 @@ #import +#import "flutter/shell/platform/darwin/macos/framework/Headers/FlutterViewController.h" #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h" -#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.h" #include @@ -40,7 +40,6 @@ - (nullable instancetype)initWithMTLDevice:(nonnull id)device commandQueue:(nonnull id)commandQueue delegate:(nonnull id)delegate - threadSynchronizer:(nonnull FlutterThreadSynchronizer*)threadSynchronizer viewIdentifier:(FlutterViewIdentifier)viewIdentifier NS_DESIGNATED_INITIALIZER; @@ -72,6 +71,11 @@ */ - (void)didUpdateMouseCursor:(nonnull NSCursor*)cursor; +/** + * Called from the controller to unblock resize synchronizer when shutting down. + */ +- (void)shutDown; + @end #endif // FLUTTER_SHELL_PLATFORM_DARWIN_MACOS_FRAMEWORK_SOURCE_FLUTTERVIEW_H_ diff --git a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterView.mm b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterView.mm index 21a1115e73..756e9e9d71 100644 --- a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterView.mm +++ b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterView.mm @@ -4,15 +4,15 @@ #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterView.h" +#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterResizeSynchronizer.h" #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h" -#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.h" #import @interface FlutterView () { FlutterViewIdentifier _viewIdentifier; __weak id _viewDelegate; - FlutterThreadSynchronizer* _threadSynchronizer; + FlutterResizeSynchronizer* _resizeSynchronizer; FlutterSurfaceManager* _surfaceManager; NSCursor* _lastCursor; } @@ -24,7 +24,6 @@ - (instancetype)initWithMTLDevice:(id)device commandQueue:(id)commandQueue delegate:(id)delegate - threadSynchronizer:(FlutterThreadSynchronizer*)threadSynchronizer viewIdentifier:(FlutterViewIdentifier)viewIdentifier { self = [super initWithFrame:NSZeroRect]; if (self) { @@ -33,30 +32,25 @@ [self setLayerContentsRedrawPolicy:NSViewLayerContentsRedrawDuringViewResize]; _viewIdentifier = viewIdentifier; _viewDelegate = delegate; - _threadSynchronizer = threadSynchronizer; _surfaceManager = [[FlutterSurfaceManager alloc] initWithDevice:device commandQueue:commandQueue layer:self.layer delegate:self]; + _resizeSynchronizer = [[FlutterResizeSynchronizer alloc] init]; } return self; } -- (void)onPresent:(CGSize)frameSize withBlock:(dispatch_block_t)block { - [_threadSynchronizer performCommitForView:_viewIdentifier size:frameSize notify:block]; +- (void)onPresent:(CGSize)frameSize withBlock:(dispatch_block_t)block delay:(NSTimeInterval)delay { + [_resizeSynchronizer performCommitForSize:frameSize notify:block delay:delay]; } - (FlutterSurfaceManager*)surfaceManager { return _surfaceManager; } -- (void)reshaped { - CGSize scaledSize = [self convertSizeToBacking:self.bounds.size]; - [_threadSynchronizer beginResizeForView:_viewIdentifier - size:scaledSize - notify:^{ - [_viewDelegate viewDidReshape:self]; - }]; +- (void)shutDown { + [_resizeSynchronizer shutDown]; } - (void)setBackgroundColor:(NSColor*)color { @@ -67,7 +61,11 @@ - (void)setFrameSize:(NSSize)newSize { [super setFrameSize:newSize]; - [self reshaped]; + CGSize scaledSize = [self convertSizeToBacking:self.bounds.size]; + [_resizeSynchronizer beginResizeForSize:scaledSize + notify:^{ + [_viewDelegate viewDidReshape:self]; + }]; } /** @@ -99,12 +97,12 @@ _lastCursor = cursor; } -// Restores mouse cursor. There are few cases when this is needed and framework will not handle this -// automatically: -// - When mouse cursor leaves subview of FlutterView (technically still within bound of FlutterView -// tracking area so the framework won't be notified) -// - When context menu above FlutterView is closed. Context menu will change current cursor to arrow -// and will not restore it back. +// Restores mouse cursor. There are few cases when this is needed and framework will not handle +// this automatically: +// - When mouse cursor leaves subview of FlutterView (technically still within bound of +// FlutterView tracking area so the framework won't be notified) +// - When context menu above FlutterView is closed. Context menu will change current cursor to +// arrow and will not restore it back. - (void)cursorUpdate:(NSEvent*)event { // Make sure to not override cursor when over a platform view. NSPoint mouseLocation = [[self superview] convertPoint:event.locationInWindow fromView:nil]; diff --git a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm index 176053dd8b..5346c452da 100644 --- a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm +++ b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm @@ -295,10 +295,6 @@ struct MouseState { FlutterDartProject* _project; std::shared_ptr _bridge; - - // FlutterViewController does not actually uses the synchronizer, but only - // passes it to FlutterView. - FlutterThreadSynchronizer* _threadSynchronizer; } // Synthesize properties declared readonly. @@ -426,6 +422,7 @@ static void CommonInit(FlutterViewController* controller, FlutterEngine* engine) if ([self attached]) { [_engine removeViewController:self]; } + [self.flutterView shutDown]; } #pragma mark - Public methods @@ -472,19 +469,14 @@ static void CommonInit(FlutterViewController* controller, FlutterEngine* engine) } - (void)setUpWithEngine:(FlutterEngine*)engine - viewIdentifier:(FlutterViewIdentifier)viewIdentifier - threadSynchronizer:(FlutterThreadSynchronizer*)threadSynchronizer { + viewIdentifier:(FlutterViewIdentifier)viewIdentifier { NSAssert(_engine == nil, @"Already attached to an engine %@.", _engine); _engine = engine; _viewIdentifier = viewIdentifier; - _threadSynchronizer = threadSynchronizer; - [_threadSynchronizer registerView:_viewIdentifier]; } - (void)detachFromEngine { NSAssert(_engine != nil, @"Not attached to any engine."); - [_threadSynchronizer deregisterView:_viewIdentifier]; - _threadSynchronizer = nil; _engine = nil; } @@ -801,7 +793,6 @@ static void CommonInit(FlutterViewController* controller, FlutterEngine* engine) return [[FlutterView alloc] initWithMTLDevice:device commandQueue:commandQueue delegate:self - threadSynchronizer:_threadSynchronizer viewIdentifier:_viewIdentifier]; } diff --git a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterViewController_Internal.h b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterViewController_Internal.h index 1db2c06cc2..301d584fdf 100644 --- a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterViewController_Internal.h +++ b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterViewController_Internal.h @@ -32,8 +32,7 @@ * before being used, and must be set up only once until detachFromEngine:. */ - (void)setUpWithEngine:(nonnull FlutterEngine*)engine - viewIdentifier:(FlutterViewIdentifier)viewIdentifier - threadSynchronizer:(nonnull FlutterThreadSynchronizer*)threadSynchronizer; + viewIdentifier:(FlutterViewIdentifier)viewIdentifier; /** * Reset the `engine` and `id` of this controller. diff --git a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterViewTest.mm b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterViewTest.mm index 3c84116188..d64c6d1603 100644 --- a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterViewTest.mm +++ b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterViewTest.mm @@ -29,11 +29,9 @@ TEST(FlutterView, ShouldInheritContentsScaleReturnsYes) { id device = MTLCreateSystemDefaultDevice(); id queue = [device newCommandQueue]; TestFlutterViewDelegate* delegate = [[TestFlutterViewDelegate alloc] init]; - FlutterThreadSynchronizer* threadSynchronizer = [[FlutterThreadSynchronizer alloc] init]; FlutterView* view = [[FlutterView alloc] initWithMTLDevice:device commandQueue:queue delegate:delegate - threadSynchronizer:threadSynchronizer viewIdentifier:kImplicitViewId]; EXPECT_EQ([view layer:view.layer shouldInheritContentsScale:3.0 fromWindow:view.window], YES); } @@ -74,11 +72,9 @@ TEST(FlutterView, CursorUpdateDoesHitTest) { id device = MTLCreateSystemDefaultDevice(); id queue = [device newCommandQueue]; TestFlutterViewDelegate* delegate = [[TestFlutterViewDelegate alloc] init]; - FlutterThreadSynchronizer* threadSynchronizer = [[FlutterThreadSynchronizer alloc] init]; TestFlutterView* view = [[TestFlutterView alloc] initWithMTLDevice:device commandQueue:queue delegate:delegate - threadSynchronizer:threadSynchronizer viewIdentifier:kImplicitViewId]; NSWindow* window = [[NSWindow alloc] initWithContentRect:NSMakeRect(0, 0, 800, 600) styleMask:NSBorderlessWindowMask @@ -118,11 +114,9 @@ TEST(FlutterView, CursorUpdateDoesNotOverridePlatformView) { id device = MTLCreateSystemDefaultDevice(); id queue = [device newCommandQueue]; TestFlutterViewDelegate* delegate = [[TestFlutterViewDelegate alloc] init]; - FlutterThreadSynchronizer* threadSynchronizer = [[FlutterThreadSynchronizer alloc] init]; TestFlutterView* view = [[TestFlutterView alloc] initWithMTLDevice:device commandQueue:queue delegate:delegate - threadSynchronizer:threadSynchronizer viewIdentifier:kImplicitViewId]; NSWindow* window = [[NSWindow alloc] initWithContentRect:NSMakeRect(0, 0, 800, 600) styleMask:NSBorderlessWindowMask