From b5042c1afa452e70f94eef78486847903e2ddee4 Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Fri, 1 Mar 2024 11:32:01 +0100 Subject: [PATCH 1/9] Revert "Revert "[macOS] Use CVDisplayLink to drive repaint (#49159)" (#51095)" This reverts commit dd3383d1739f32ab9d19e3d5c22ed1e6b7b3834d. --- ci/licenses_golden/licenses_flutter | 8 + shell/platform/darwin/macos/BUILD.gn | 6 + .../framework/Source/FlutterCompositor.mm | 15 +- .../framework/Source/FlutterDisplayLink.h | 40 ++ .../framework/Source/FlutterDisplayLink.mm | 355 ++++++++++++++++++ .../Source/FlutterDisplayLinkTest.mm | 150 ++++++++ .../macos/framework/Source/FlutterEngine.mm | 71 +++- .../framework/Source/FlutterEngineTest.mm | 1 + .../framework/Source/FlutterEngine_Internal.h | 5 + .../framework/Source/FlutterSurfaceManager.h | 5 +- .../framework/Source/FlutterSurfaceManager.mm | 29 +- .../Source/FlutterSurfaceManagerTest.mm | 39 +- .../Source/FlutterThreadSynchronizer.h | 7 + .../Source/FlutterThreadSynchronizer.mm | 13 + .../Source/FlutterThreadSynchronizerTest.mm | 6 - .../framework/Source/FlutterVSyncWaiter.h | 26 ++ .../framework/Source/FlutterVSyncWaiter.mm | 172 +++++++++ .../Source/FlutterVSyncWaiterTest.mm | 170 +++++++++ .../framework/Source/FlutterViewController.mm | 1 + shell/platform/embedder/embedder.h | 4 + .../embedder_external_view_embedder.cc | 12 +- shell/platform/embedder/embedder_layers.cc | 9 +- shell/platform/embedder/embedder_layers.h | 4 +- 23 files changed, 1112 insertions(+), 36 deletions(-) create mode 100644 shell/platform/darwin/macos/framework/Source/FlutterDisplayLink.h create mode 100644 shell/platform/darwin/macos/framework/Source/FlutterDisplayLink.mm create mode 100644 shell/platform/darwin/macos/framework/Source/FlutterDisplayLinkTest.mm create mode 100644 shell/platform/darwin/macos/framework/Source/FlutterVSyncWaiter.h create mode 100644 shell/platform/darwin/macos/framework/Source/FlutterVSyncWaiter.mm create mode 100644 shell/platform/darwin/macos/framework/Source/FlutterVSyncWaiterTest.mm diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index 51dfd586513dd..a181c2522deb8 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -36592,6 +36592,7 @@ ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterCom ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterCompositor.mm + ../../../flutter/LICENSE ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterDartProject.mm + ../../../flutter/LICENSE ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterDartProject_Internal.h + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterDisplayLinkTest.mm + ../../../flutter/LICENSE ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterEmbedderExternalTextureTest.mm + ../../../flutter/LICENSE ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterEmbedderKeyResponder.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterEmbedderKeyResponder.mm + ../../../flutter/LICENSE @@ -36642,6 +36643,7 @@ ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterThr 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/FlutterUmbrellaImportTests.m + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterVSyncWaiterTest.mm + ../../../flutter/LICENSE ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterView.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterView.mm + ../../../flutter/LICENSE ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm + ../../../flutter/LICENSE @@ -39457,6 +39459,9 @@ FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterCompo FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterCompositor.mm FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterDartProject.mm FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterDartProject_Internal.h +FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterDisplayLink.h +FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterDisplayLink.mm +FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterDisplayLinkTest.mm FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterEmbedderExternalTextureTest.mm FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterEmbedderKeyResponder.h FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterEmbedderKeyResponder.mm @@ -39507,6 +39512,9 @@ FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterThrea 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/FlutterUmbrellaImportTests.m +FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterVSyncWaiter.h +FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterVSyncWaiter.mm +FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterVSyncWaiterTest.mm FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterView.h FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterView.mm FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm diff --git a/shell/platform/darwin/macos/BUILD.gn b/shell/platform/darwin/macos/BUILD.gn index b7a52f76babea..15aa2d757fe7f 100644 --- a/shell/platform/darwin/macos/BUILD.gn +++ b/shell/platform/darwin/macos/BUILD.gn @@ -66,6 +66,8 @@ source_set("flutter_framework_source") { "framework/Source/FlutterCompositor.mm", "framework/Source/FlutterDartProject.mm", "framework/Source/FlutterDartProject_Internal.h", + "framework/Source/FlutterDisplayLink.h", + "framework/Source/FlutterDisplayLink.mm", "framework/Source/FlutterEmbedderKeyResponder.h", "framework/Source/FlutterEmbedderKeyResponder.mm", "framework/Source/FlutterEngine.mm", @@ -101,6 +103,8 @@ source_set("flutter_framework_source") { "framework/Source/FlutterTextureRegistrar.mm", "framework/Source/FlutterThreadSynchronizer.h", "framework/Source/FlutterThreadSynchronizer.mm", + "framework/Source/FlutterVSyncWaiter.h", + "framework/Source/FlutterVSyncWaiter.mm", "framework/Source/FlutterView.h", "framework/Source/FlutterView.mm", "framework/Source/FlutterViewController.mm", @@ -173,6 +177,7 @@ executable("flutter_desktop_darwin_unittests") { "framework/Source/FlutterAppDelegateTest.mm", "framework/Source/FlutterAppLifecycleDelegateTest.mm", "framework/Source/FlutterChannelKeyResponderTest.mm", + "framework/Source/FlutterDisplayLinkTest.mm", "framework/Source/FlutterEmbedderExternalTextureTest.mm", "framework/Source/FlutterEmbedderKeyResponderTest.mm", "framework/Source/FlutterEngineTest.mm", @@ -187,6 +192,7 @@ executable("flutter_desktop_darwin_unittests") { "framework/Source/FlutterTextInputPluginTest.mm", "framework/Source/FlutterTextInputSemanticsObjectTest.mm", "framework/Source/FlutterThreadSynchronizerTest.mm", + "framework/Source/FlutterVSyncWaiterTest.mm", "framework/Source/FlutterViewControllerTest.mm", "framework/Source/FlutterViewControllerTestUtils.h", "framework/Source/FlutterViewControllerTestUtils.mm", diff --git a/shell/platform/darwin/macos/framework/Source/FlutterCompositor.mm b/shell/platform/darwin/macos/framework/Source/FlutterCompositor.mm index 307c6c9a9394c..25471599eb707 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterCompositor.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterCompositor.mm @@ -69,10 +69,17 @@ } } - [view.surfaceManager present:surfaces - notify:^{ - PresentPlatformViews(view, layers, layers_count); - }]; + CFTimeInterval presentation_time = 0; + + if (layers_count > 0 && layers[0]->presentation_time != 0) { + presentation_time = layers[0]->presentation_time / 1'000'000'000.0; + } + + [view.surfaceManager presentSurfaces:surfaces + atTime:presentation_time + notify:^{ + PresentPlatformViews(view, layers, layers_count); + }]; return true; } diff --git a/shell/platform/darwin/macos/framework/Source/FlutterDisplayLink.h b/shell/platform/darwin/macos/framework/Source/FlutterDisplayLink.h new file mode 100644 index 0000000000000..fa3c0925a7dff --- /dev/null +++ b/shell/platform/darwin/macos/framework/Source/FlutterDisplayLink.h @@ -0,0 +1,40 @@ +#ifndef FLUTTER_SHELL_PLATFORM_DARWIN_MACOS_FRAMEWORK_SOURCE_FLUTTERDISPLAYLINK_H_ +#define FLUTTER_SHELL_PLATFORM_DARWIN_MACOS_FRAMEWORK_SOURCE_FLUTTERDISPLAYLINK_H_ + +#import + +@protocol FlutterDisplayLinkDelegate +- (void)onDisplayLink:(CFTimeInterval)timestamp targetTimestamp:(CFTimeInterval)targetTimestamp; +@end + +/// Provides notifications of display refresh. +/// +/// 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. +@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. +@property(nonatomic, weak) id delegate; + +/// Pauses and resumes the display link. May be called from any thread. +@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. +@property(readonly) CFTimeInterval nominalOutputRefreshPeriod; + +/// Invalidates the display link. Must be called on the main thread. +- (void)invalidate; + +@end + +#endif // FLUTTER_SHELL_PLATFORM_DARWIN_MACOS_FRAMEWORK_SOURCE_FLUTTERDISPLAYLINK_H_ diff --git a/shell/platform/darwin/macos/framework/Source/FlutterDisplayLink.mm b/shell/platform/darwin/macos/framework/Source/FlutterDisplayLink.mm new file mode 100644 index 0000000000000..31b74ec36d4e6 --- /dev/null +++ b/shell/platform/darwin/macos/framework/Source/FlutterDisplayLink.mm @@ -0,0 +1,355 @@ +#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterDisplayLink.h" + +#include "flutter/fml/logging.h" + +#include +#include +#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 { + _FlutterDisplayLinkView* _view; + std::optional _display_id; + BOOL _paused; +} + +- (void)didFireWithTimestamp:(CFTimeInterval)timestamp + targetTimestamp:(CFTimeInterval)targetTimestamp; + +@end + +namespace { +class DisplayLinkManager { + public: + static DisplayLinkManager& Instance() { + static DisplayLinkManager instance; + return instance; + } + + void UnregisterDisplayLink(_FlutterDisplayLink* display_link); + void RegisterDisplayLink(_FlutterDisplayLink* display_link, CGDirectDisplayID display_id); + void PausedDidChange(_FlutterDisplayLink* display_link); + CFTimeInterval GetNominalOutputPeriod(CGDirectDisplayID display_id); + + private: + void OnDisplayLink(CVDisplayLinkRef display_link, + const CVTimeStamp* in_now, + const CVTimeStamp* in_output_time, + CVOptionFlags flags_in, + CVOptionFlags* flags_out); + + 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; + + bool ShouldBeRunning() { + return std::any_of(clients.begin(), clients.end(), + [](FlutterDisplayLink* link) { return !link.paused; }); + } + }; + 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); + }]; + } + } else if (!should_be_running && is_running) { + CVDisplayLinkStop(display_link); + } +} + +void DisplayLinkManager::UnregisterDisplayLink(_FlutterDisplayLink* display_link) { + std::unique_lock lock(mutex_); + 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()) { + entry->clients.erase(it); + 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; + 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); + } + return; + } + } +} + +void DisplayLinkManager::RegisterDisplayLink(_FlutterDisplayLink* display_link, + CGDirectDisplayID display_id) { + std::unique_lock lock(mutex_); + 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); + return; + } + } + + ScreenEntry entry; + entry.display_id = display_id; + entry.clients.push_back(display_link); + CVDisplayLinkCreateWithCGDisplay(display_id, &entry.display_link_locked); + + CVDisplayLinkSetOutputHandler( + entry.display_link_locked, + ^(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); + return 0; + }); + + // 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); + 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); + 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); + return (CFTimeInterval)latency.timeValue / (CFTimeInterval)latency.timeScale; + } + } + return 0; +} + +void DisplayLinkManager::OnDisplayLink(CVDisplayLinkRef display_link, + const CVTimeStamp* in_now, + 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_); + for (ScreenEntry& entry : entries_) { + if (entry.display_link_locked == display_link) { + clients = entry.clients; + break; + } + } + } + + CFTimeInterval timestamp = (CFTimeInterval)in_now->hostTime / CVGetHostClockFrequency(); + CFTimeInterval target_timestamp = + (CFTimeInterval)in_output_time->hostTime / CVGetHostClockFrequency(); + + for (_FlutterDisplayLink* client : clients) { + [client didFireWithTimestamp:timestamp targetTimestamp:target_timestamp]; + } +} +} // namespace + +@interface _FlutterDisplayLinkView : NSView { +} + +@end + +static NSString* const kFlutterDisplayLinkViewDidMoveToWindow = + @"FlutterDisplayLinkViewDidMoveToWindow"; + +@implementation _FlutterDisplayLinkView + +- (void)viewDidMoveToWindow { + [super viewDidMoveToWindow]; + [[NSNotificationCenter defaultCenter] postNotificationName:kFlutterDisplayLinkViewDidMoveToWindow + object:self]; +} + +@end + +@implementation _FlutterDisplayLink + +@synthesize delegate = _delegate; + +- (instancetype)initWithView:(NSView*)view { + FML_DCHECK([NSThread isMainThread]); + if (self = [super init]) { + self->_view = [[_FlutterDisplayLinkView alloc] initWithFrame:CGRectZero]; + [view addSubview:self->_view]; + _paused = YES; + [[NSNotificationCenter defaultCenter] addObserver:self + selector:@selector(viewDidChangeWindow:) + name:kFlutterDisplayLinkViewDidMoveToWindow + object:self->_view]; + [[NSNotificationCenter defaultCenter] addObserver:self + selector:@selector(windowDidChangeScreen:) + name:NSWindowDidChangeScreenNotification + object:nil]; + [self updateScreen]; + } + return self; +} + +- (void)invalidate { + @synchronized(self) { + FML_DCHECK([NSThread isMainThread]); + [_view removeFromSuperview]; + [[NSNotificationCenter defaultCenter] removeObserver:self]; + _view = nil; + _delegate = nil; + } + DisplayLinkManager::Instance().UnregisterDisplayLink(self); +} + +- (void)updateScreen { + 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; + } + if (displayId.has_value()) { + DisplayLinkManager::Instance().RegisterDisplayLink(self, *displayId); + } +} + +- (void)viewDidChangeWindow:(NSNotification*)notification { + NSView* view = notification.object; + if (_view == view) { + [self updateScreen]; + } +} + +- (void)windowDidChangeScreen:(NSNotification*)notification { + NSWindow* window = notification.object; + if (_view.window == window) { + [self updateScreen]; + } +} + +- (void)didFireWithTimestamp:(CFTimeInterval)timestamp + targetTimestamp:(CFTimeInterval)targetTimestamp { + @synchronized(self) { + if (!_paused) { + id delegate = _delegate; + [delegate onDisplayLink:timestamp targetTimestamp:targetTimestamp]; + } + } +} + +- (BOOL)paused { + @synchronized(self) { + return _paused; + } +} + +- (void)setPaused:(BOOL)paused { + @synchronized(self) { + if (_paused == paused) { + return; + } + _paused = paused; + } + DisplayLinkManager::Instance().PausedDidChange(self); +} + +- (CFTimeInterval)nominalOutputRefreshPeriod { + CGDirectDisplayID display_id; + @synchronized(self) { + if (_display_id.has_value()) { + display_id = *_display_id; + } else { + return 0; + } + } + return DisplayLinkManager::Instance().GetNominalOutputPeriod(display_id); +} + +@end + +@implementation FlutterDisplayLink ++ (instancetype)displayLinkWithView:(NSView*)view { + return [[_FlutterDisplayLink alloc] initWithView:view]; +} + +- (void)invalidate { + [self doesNotRecognizeSelector:_cmd]; +} + +@end diff --git a/shell/platform/darwin/macos/framework/Source/FlutterDisplayLinkTest.mm b/shell/platform/darwin/macos/framework/Source/FlutterDisplayLinkTest.mm new file mode 100644 index 0000000000000..fc68055311134 --- /dev/null +++ b/shell/platform/darwin/macos/framework/Source/FlutterDisplayLinkTest.mm @@ -0,0 +1,150 @@ +// 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/FlutterDisplayLink.h" + +#import + +#include "flutter/fml/synchronization/waitable_event.h" +#include "flutter/testing/testing.h" + +@interface TestDisplayLinkDelegate : NSObject { + void (^_block)(CFTimeInterval timestamp, CFTimeInterval targetTimestamp); +} + +- (instancetype)initWithBlock:(void (^)(CFTimeInterval timestamp, + CFTimeInterval targetTimestamp))block; + +@end + +@implementation TestDisplayLinkDelegate +- (instancetype)initWithBlock:(void (^__strong)(CFTimeInterval, CFTimeInterval))block { + if (self = [super init]) { + _block = block; + } + return self; +} + +- (void)onDisplayLink:(CFTimeInterval)timestamp targetTimestamp:(CFTimeInterval)targetTimestamp { + _block(timestamp, targetTimestamp); +} + +@end + +TEST(FlutterDisplayLinkTest, ViewAddedToWindowFirst) { + 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 invalidate]; +} + +TEST(FlutterDisplayLinkTest, ViewAddedToWindowLater) { + NSView* view = [[NSView alloc] initWithFrame:NSMakeRect(0, 0, 100, 100)]; + + 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; + + NSWindow* window = [[NSWindow alloc] initWithContentRect:NSMakeRect(0, 0, 100, 100) + styleMask:NSWindowStyleMaskTitled + backing:NSBackingStoreNonretained + defer:NO]; + [window setContentView:view]; + + event->Wait(); + + [displayLink invalidate]; +} + +TEST(FlutterDisplayLinkTest, ViewRemovedFromWindow) { + 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(); + + displayLink.paused = NO; + + [window setContentView:nil]; + + EXPECT_TRUE(event->WaitWithTimeout(fml::TimeDelta::FromMilliseconds(100))); + EXPECT_FALSE(event->IsSignaledForTest()); + + [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]; +} diff --git a/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm b/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm index e836cedd5920b..bc835ee045888 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm @@ -18,10 +18,12 @@ #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterAppDelegate_Internal.h" #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterCompositor.h" #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterDartProject_Internal.h" +#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterDisplayLink.h" #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterMenuPlugin.h" #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/FlutterVSyncWaiter.h" #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterViewController_Internal.h" #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterViewEngineProvider.h" @@ -459,12 +461,31 @@ @implementation FlutterEngine { // Proxy to allow plugins, channels to hold a weak reference to the binary messenger (self). FlutterBinaryMessengerRelay* _binaryMessenger; + + // Map from ViewId to vsync waiter. Note that this is modified on main thread + // but accessed on UI thread, so access must be @synchronized. + NSMapTable* _vsyncWaiters; } - (instancetype)initWithName:(NSString*)labelPrefix project:(FlutterDartProject*)project { return [self initWithName:labelPrefix project:project allowHeadlessExecution:YES]; } +static const int kMainThreadPriority = 47; + +static void SetThreadPriority(FlutterThreadPriority priority) { + if (priority == kDisplay || priority == kRaster) { + pthread_t thread = pthread_self(); + sched_param param; + int policy; + if (!pthread_getschedparam(thread, &policy, ¶m)) { + param.sched_priority = kMainThreadPriority; + pthread_setschedparam(thread, policy, ¶m); + } + pthread_set_qos_class_self_np(QOS_CLASS_USER_INTERACTIVE, 0); + } +} + - (instancetype)initWithName:(NSString*)labelPrefix project:(FlutterDartProject*)project allowHeadlessExecution:(BOOL)allowHeadlessExecution { @@ -515,6 +536,8 @@ - (instancetype)initWithName:(NSString*)labelPrefix _terminationHandler = nil; } + _vsyncWaiters = [NSMapTable strongToStrongObjectsMapTable]; + return self; } @@ -624,7 +647,7 @@ - (BOOL)runWithEntrypoint:(NSString*)entrypoint { const FlutterCustomTaskRunners custom_task_runners = { .struct_size = sizeof(FlutterCustomTaskRunners), .platform_task_runner = &cocoa_task_runner_description, - }; + .thread_priority_setter = SetThreadPriority}; flutterArguments.custom_task_runners = &custom_task_runners; [self loadAOTData:_project.assetsPath]; @@ -639,6 +662,11 @@ - (BOOL)runWithEntrypoint:(NSString*)entrypoint { [engine engineCallbackOnPreEngineRestart]; }; + flutterArguments.vsync_callback = [](void* user_data, intptr_t baton) { + FlutterEngine* engine = (__bridge FlutterEngine*)user_data; + [engine onVSync:baton]; + }; + FlutterRendererConfig rendererConfig = [_renderer createRendererConfig]; FlutterEngineResult result = _embedderAPI.Initialize( FLUTTER_ENGINE_VERSION, &rendererConfig, &flutterArguments, (__bridge void*)(self), &_engine); @@ -703,6 +731,36 @@ - (void)registerViewController:(FlutterViewController*)controller forId:(Flutter [controller setUpWithEngine:self viewId:viewId threadSynchronizer:_threadSynchronizer]; NSAssert(controller.viewId == viewId, @"Failed to assign view ID."); [_viewControllers setObject:controller forKey:@(viewId)]; + + if (controller.viewLoaded) { + [self viewControllerViewDidLoad:controller]; + } +} + +- (void)viewControllerViewDidLoad:(FlutterViewController*)viewController { + __weak FlutterEngine* weakSelf = self; + FlutterVSyncWaiter* waiter = [[FlutterVSyncWaiter alloc] + initWithDisplayLink:[FlutterDisplayLink displayLinkWithView:viewController.view] + block:^(CFTimeInterval timestamp, CFTimeInterval targetTimestamp, + uintptr_t baton) { + // CAMediaTime and flutter time are both mach_absolute_time. + uint64_t timeNanos = timestamp * 1000000000; + uint64_t targetTimeNanos = targetTimestamp * 1000000000; + 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); + }]; + } + }]; + FML_DCHECK([_vsyncWaiters objectForKey:@(viewController.viewId)] == nil); + @synchronized(_vsyncWaiters) { + [_vsyncWaiters setObject:waiter forKey:@(viewController.viewId)]; + } } - (void)deregisterViewControllerForId:(FlutterViewId)viewId { @@ -711,6 +769,9 @@ - (void)deregisterViewControllerForId:(FlutterViewId)viewId { [oldController detachFromEngine]; [_viewControllers removeObjectForKey:@(viewId)]; } + @synchronized(_vsyncWaiters) { + [_vsyncWaiters removeObjectForKey:@(viewId)]; + } } - (void)shutDownIfNeeded { @@ -1034,6 +1095,14 @@ - (void)engineCallbackOnPreEngineRestart { } } +- (void)onVSync:(uintptr_t)baton { + @synchronized(_vsyncWaiters) { + // TODO(knopp): Use vsync waiter for correct view. + FlutterVSyncWaiter* waiter = [_vsyncWaiters objectForKey:@(kFlutterImplicitViewId)]; + [waiter waitForVSync:baton]; + } +} + /** * Note: Called from dealloc. Should not use accessors or other methods. */ diff --git a/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm b/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm index f6e692058030b..209903d8fa549 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm @@ -508,6 +508,7 @@ @implementation MockableFlutterEngine nibName:nil bundle:nil]; [viewController loadView]; + [viewController viewDidLoad]; viewController.flutterView.frame = CGRectMake(0, 0, 800, 600); EXPECT_TRUE([engine runWithEntrypoint:@"canCompositePlatformViews"]); diff --git a/shell/platform/darwin/macos/framework/Source/FlutterEngine_Internal.h b/shell/platform/darwin/macos/framework/Source/FlutterEngine_Internal.h index a2a0b1a037396..bcaca946a1203 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterEngine_Internal.h +++ b/shell/platform/darwin/macos/framework/Source/FlutterEngine_Internal.h @@ -136,6 +136,11 @@ typedef NS_ENUM(NSInteger, FlutterAppExitResponse) { */ - (void)addViewController:(FlutterViewController*)viewController; +/** + * Notify the engine that a view for the given view controller has been loaded. + */ +- (void)viewControllerViewDidLoad:(FlutterViewController*)viewController; + /** * Dissociate the given view controller from this engine. * diff --git a/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h b/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h index 45d06f63f5778..279d580e2f4cd 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h +++ b/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h @@ -69,8 +69,9 @@ * and can be used to perform additional work, such as mutating platform views. It is guaranteed be * called in the same CATransaction. */ -- (void)present:(nonnull NSArray*)surfaces - notify:(nullable dispatch_block_t)notify; +- (void)presentSurfaces:(nonnull NSArray*)surfaces + atTime:(CFTimeInterval)presentationTime + notify:(nullable dispatch_block_t)notify; @end diff --git a/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.mm b/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.mm index ed41844529b96..b3e7ef3882e34 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.mm @@ -34,6 +34,8 @@ @interface FlutterSurfaceManager () { // FLTEnableSurfaceDebugInfo value in main bundle Info.plist. NSNumber* _enableSurfaceDebugInfo; CATextLayer* _infoLayer; + + CFTimeInterval _lastPresentationTime; } /** @@ -213,11 +215,36 @@ static CGSize GetRequiredFrameSize(NSArray* surfaces return size; } -- (void)present:(NSArray*)surfaces notify:(dispatch_block_t)notify { +- (void)presentSurfaces:(NSArray*)surfaces + atTime:(CFTimeInterval)presentationTime + notify:(dispatch_block_t)notify { id commandBuffer = [_commandQueue commandBuffer]; [commandBuffer commit]; [commandBuffer waitUntilScheduled]; + if (presentationTime > 0) { + // Enforce frame pacing. It seems that the target timestamp of CVDisplayLink does not + // exactly correspond to core animation deadline. Especially with 120hz, setting the frame + // contents too close after previous target timestamp will result in uneven frame pacing. + // Empirically setting the content in the second half of frame interval seems to work + // well for both 60hz and 120hz. + // + // The easiest way to ensure that the content is not set too early is to delay raster thread. + // At this point raster thread should be idle (the next frame vsync has not been signalled yet). + // This will show on a timeline as "FlutterCompositionPresentLayers" but should not cause jank + // because the waiting interval is calculated relative to presentation time. + // + // Alternative to blocking raster thread would be to copy all presentation info provided by + // embedder and schedule a presentation timer. This would require additional coordination with + // FlutterThreadSynchronizer. + CFTimeInterval minPresentationTime = (presentationTime + _lastPresentationTime) / 2.0; + CFTimeInterval now = CACurrentMediaTime(); + if (now < minPresentationTime) { + [NSThread sleepForTimeInterval:minPresentationTime - now]; + } + } + _lastPresentationTime = presentationTime; + // Get the actual dimensions of the frame (relevant for thread synchronizer). CGSize size = GetRequiredFrameSize(surfaces); diff --git a/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManagerTest.mm b/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManagerTest.mm index bc064ece640ef..04471eb914863 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManagerTest.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManagerTest.mm @@ -99,17 +99,17 @@ - (void)onPresent:(CGSize)frameSize withBlock:(nonnull dispatch_block_t)block { EXPECT_EQ(surfaceManager.backBufferCache.count, 0ul); auto surface1 = [surfaceManager surfaceForSize:CGSizeMake(100, 100)]; - [surfaceManager present:@[ CreatePresentInfo(surface1) ] notify:nil]; + [surfaceManager presentSurfaces:@[ CreatePresentInfo(surface1) ] atTime:0 notify:nil]; EXPECT_EQ(surfaceManager.backBufferCache.count, 0ul); auto surface2 = [surfaceManager surfaceForSize:CGSizeMake(110, 110)]; - [surfaceManager present:@[ CreatePresentInfo(surface2) ] notify:nil]; + [surfaceManager presentSurfaces:@[ CreatePresentInfo(surface2) ] atTime:0 notify:nil]; EXPECT_EQ(surfaceManager.backBufferCache.count, 1ul); auto surface3 = [surfaceManager surfaceForSize:CGSizeMake(120, 120)]; - [surfaceManager present:@[ CreatePresentInfo(surface3) ] notify:nil]; + [surfaceManager presentSurfaces:@[ CreatePresentInfo(surface3) ] atTime:0 notify:nil]; // Cache should be cleaned during present and only contain the last visible // surface(s). @@ -117,10 +117,10 @@ - (void)onPresent:(CGSize)frameSize withBlock:(nonnull dispatch_block_t)block { auto surfaceFromCache = [surfaceManager surfaceForSize:CGSizeMake(110, 110)]; EXPECT_EQ(surfaceFromCache, surface2); - [surfaceManager present:@[] notify:nil]; + [surfaceManager presentSurfaces:@[] atTime:0 notify:nil]; EXPECT_EQ(surfaceManager.backBufferCache.count, 1ul); - [surfaceManager present:@[] notify:nil]; + [surfaceManager presentSurfaces:@[] atTime:0 notify:nil]; EXPECT_EQ(surfaceManager.backBufferCache.count, 0ul); } @@ -138,7 +138,7 @@ - (void)onPresent:(CGSize)frameSize withBlock:(nonnull dispatch_block_t)block { EXPECT_EQ(surfaceManager.backBufferCache.count, 0ul); EXPECT_EQ(surfaceManager.frontSurfaces.count, 0ul); - [surfaceManager present:@[ CreatePresentInfo(surface1) ] notify:nil]; + [surfaceManager presentSurfaces:@[ CreatePresentInfo(surface1) ] atTime:0 notify:nil]; EXPECT_EQ(surfaceManager.backBufferCache.count, 0ul); EXPECT_EQ(surfaceManager.frontSurfaces.count, 1ul); @@ -151,7 +151,7 @@ - (void)onPresent:(CGSize)frameSize withBlock:(nonnull dispatch_block_t)block { EXPECT_EQ(surfaceManager.backBufferCache.count, 0ul); - [surfaceManager present:@[ CreatePresentInfo(surface2) ] notify:nil]; + [surfaceManager presentSurfaces:@[ CreatePresentInfo(surface2) ] atTime:0 notify:nil]; // Check that current front surface returns to cache. EXPECT_EQ(surfaceManager.backBufferCache.count, 1ul); @@ -174,14 +174,16 @@ - (void)onPresent:(CGSize)frameSize withBlock:(nonnull dispatch_block_t)block { EXPECT_EQ(testView.layer.sublayers.count, 0ul); auto surface1_1 = [surfaceManager surfaceForSize:CGSizeMake(50, 30)]; - [surfaceManager present:@[ CreatePresentInfo(surface1_1, CGPointMake(20, 10)) ] notify:nil]; + [surfaceManager presentSurfaces:@[ CreatePresentInfo(surface1_1, CGPointMake(20, 10)) ] + atTime:0 + notify:nil]; EXPECT_EQ(testView.layer.sublayers.count, 1ul); EXPECT_TRUE(CGSizeEqualToSize(testView.presentedFrameSize, CGSizeMake(70, 40))); auto surface2_1 = [surfaceManager surfaceForSize:CGSizeMake(50, 30)]; auto surface2_2 = [surfaceManager surfaceForSize:CGSizeMake(20, 20)]; - [surfaceManager present:@[ + [surfaceManager presentSurfaces:@[ CreatePresentInfo(surface2_1, CGPointMake(20, 10), 1), CreatePresentInfo(surface2_2, CGPointMake(40, 50), 2, { @@ -189,7 +191,8 @@ - (void)onPresent:(CGSize)frameSize withBlock:(nonnull dispatch_block_t)block { FlutterRect{40, 0, 60, 20}, }) ] - notify:nil]; + atTime:0 + notify:nil]; EXPECT_EQ(testView.layer.sublayers.count, 2ul); EXPECT_EQ(testView.layer.sublayers[0].zPosition, 1.0); @@ -208,14 +211,15 @@ - (void)onPresent:(CGSize)frameSize withBlock:(nonnull dispatch_block_t)block { EXPECT_TRUE(CGSizeEqualToSize(testView.presentedFrameSize, CGSizeMake(70, 70))); // Check second overlay sublayer is removed while first is reused and updated - [surfaceManager present:@[ + [surfaceManager presentSurfaces:@[ CreatePresentInfo(surface2_1, CGPointMake(20, 10), 1), CreatePresentInfo(surface2_2, CGPointMake(40, 50), 2, { FlutterRect{0, 10, 20, 20}, }) ] - notify:nil]; + atTime:0 + notify:nil]; EXPECT_EQ(testView.layer.sublayers.count, 2ul); { NSArray* sublayers = testView.layer.sublayers[1].sublayers; @@ -225,7 +229,7 @@ - (void)onPresent:(CGSize)frameSize withBlock:(nonnull dispatch_block_t)block { } // Check that second overlay sublayer is added back while first is reused and updated - [surfaceManager present:@[ + [surfaceManager presentSurfaces:@[ CreatePresentInfo(surface2_1, CGPointMake(20, 10), 1), CreatePresentInfo(surface2_2, CGPointMake(40, 50), 2, { @@ -233,7 +237,8 @@ - (void)onPresent:(CGSize)frameSize withBlock:(nonnull dispatch_block_t)block { FlutterRect{40, 0, 60, 20}, }) ] - notify:nil]; + atTime:0 + notify:nil]; EXPECT_EQ(testView.layer.sublayers.count, 2ul); { @@ -246,13 +251,15 @@ - (void)onPresent:(CGSize)frameSize withBlock:(nonnull dispatch_block_t)block { } auto surface3_1 = [surfaceManager surfaceForSize:CGSizeMake(50, 30)]; - [surfaceManager present:@[ CreatePresentInfo(surface3_1, CGPointMake(20, 10)) ] notify:nil]; + [surfaceManager presentSurfaces:@[ CreatePresentInfo(surface3_1, CGPointMake(20, 10)) ] + atTime:0 + notify:nil]; EXPECT_EQ(testView.layer.sublayers.count, 1ul); EXPECT_TRUE(CGSizeEqualToSize(testView.presentedFrameSize, CGSizeMake(70, 40))); // Check removal of all surfaces. - [surfaceManager present:@[] notify:nil]; + [surfaceManager presentSurfaces:@[] atTime:0 notify:nil]; EXPECT_EQ(testView.layer.sublayers.count, 0ul); EXPECT_TRUE(CGSizeEqualToSize(testView.presentedFrameSize, CGSizeMake(0, 0))); } diff --git a/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.h b/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.h index f4a9915d105a5..1144f50221843 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.h +++ b/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.h @@ -41,6 +41,13 @@ 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. * diff --git a/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.mm b/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.mm index 75a4ff5037210..9834261bff9a5 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.mm @@ -164,6 +164,19 @@ - (void)performCommitForView:(int64_t)viewId 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:(int64_t)viewId { dispatch_assert_queue(_mainQueue); std::unique_lock lock(_mutex); diff --git a/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizerTest.mm b/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizerTest.mm index 2a9dec62f15b4..a9fafe947f562 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizerTest.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizerTest.mm @@ -7,12 +7,6 @@ #import "flutter/fml/synchronization/waitable_event.h" #import "flutter/testing/testing.h" -namespace flutter::testing { - -namespace {} // namespace - -} // namespace flutter::testing - @interface FlutterThreadSynchronizerTestScaffold : NSObject @property(nonatomic, readonly, nonnull) FlutterThreadSynchronizer* synchronizer; diff --git a/shell/platform/darwin/macos/framework/Source/FlutterVSyncWaiter.h b/shell/platform/darwin/macos/framework/Source/FlutterVSyncWaiter.h new file mode 100644 index 0000000000000..2bb574f775fc5 --- /dev/null +++ b/shell/platform/darwin/macos/framework/Source/FlutterVSyncWaiter.h @@ -0,0 +1,26 @@ +#ifndef FLUTTER_SHELL_PLATFORM_DARWIN_MACOS_FRAMEWORK_SOURCE_FLUTTERVSYNCWAITER_H_ +#define FLUTTER_SHELL_PLATFORM_DARWIN_MACOS_FRAMEWORK_SOURCE_FLUTTERVSYNCWAITER_H_ + +#import + +@class FlutterDisplayLink; + +@interface FlutterVSyncWaiter : NSObject + +/// 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:. +- (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). +- (void)waitForVSync:(uintptr_t)baton; + +@end + +#endif // FLUTTER_SHELL_PLATFORM_DARWIN_MACOS_FRAMEWORK_SOURCE_FLUTTERVSYNCWAITER_H_ diff --git a/shell/platform/darwin/macos/framework/Source/FlutterVSyncWaiter.mm b/shell/platform/darwin/macos/framework/Source/FlutterVSyncWaiter.mm new file mode 100644 index 0000000000000..cf948a8b11701 --- /dev/null +++ b/shell/platform/darwin/macos/framework/Source/FlutterVSyncWaiter.mm @@ -0,0 +1,172 @@ +#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterVSyncWaiter.h" +#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterDisplayLink.h" + +#include "flutter/fml/logging.h" + +#include +#include + +#if (FLUTTER_RUNTIME_MODE == FLUTTER_RUNTIME_MODE_PROFILE) +#define VSYNC_TRACING_ENABLED 1 +#endif + +#if VSYNC_TRACING_ENABLED +#include + +// Trace vsync events using os_signpost so that they can be seen in Instruments "Points of +// Interest". +#define TRACE_VSYNC(event_type, baton) \ + do { \ + os_log_t log = os_log_create("FlutterVSync", "PointsOfInterest"); \ + os_signpost_event_emit(log, OS_SIGNPOST_ID_EXCLUSIVE, event_type, "baton %lx", baton); \ + } while (0) +#else +#define TRACE_VSYNC(event_type, baton) \ + do { \ + } while (0) +#endif + +@interface FlutterVSyncWaiter () +@end + +// It's preferable to fire the timers slightly early than too late due to scheduling latency. +// 1ms before vsync should be late enough for all events to be processed. +static const CFTimeInterval kTimerLatencyCompensation = 0.001; + +@implementation FlutterVSyncWaiter { + std::optional _pending_baton; + FlutterDisplayLink* _displayLink; + void (^_block)(CFTimeInterval, CFTimeInterval, uintptr_t); + NSRunLoop* _runLoop; + CFTimeInterval _lastTargetTimestamp; +} + +- (instancetype)initWithDisplayLink:(FlutterDisplayLink*)displayLink + block:(void (^)(CFTimeInterval timestamp, + CFTimeInterval targetTimestamp, + uintptr_t baton))block { + FML_DCHECK([NSThread isMainThread]); + if (self = [super init]) { + _block = block; + + _displayLink = displayLink; + _displayLink.delegate = self; + // Get at least one callback to initialize _lastTargetTimestamp. + _displayLink.paused = NO; + } + 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", _pending_baton.value_or(0)); + + NSTimer* timer = [NSTimer + timerWithTimeInterval:remaining + repeats:NO + block:^(NSTimer* _Nonnull timer) { + if (!_pending_baton.has_value()) { + TRACE_VSYNC("DisplayLinkPaused", size_t(0)); + _displayLink.paused = YES; + return; + } + TRACE_VSYNC("DisplayLinkCallback-Delayed", _pending_baton.value_or(0)); + _block(minStart, targetTimestamp, *_pending_baton); + _pending_baton = 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]; + }]; + } + } +} + +// Called from UI thread. +- (void)waitForVSync:(uintptr_t)baton { + // 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 (_pending_baton.has_value()) { + FML_LOG(WARNING) << "Engine requested vsync while another was pending"; + _block(0, 0, *_pending_baton); + _pending_baton = std::nullopt; + } + + TRACE_VSYNC("VSyncRequest", _pending_baton.value_or(0)); + + CFTimeInterval tick_interval = _displayLink.nominalOutputRefreshPeriod; + if (_displayLink.paused || tick_interval == 0) { + // When starting display link the first notification will come in the middle + // of next frame, which would incur a whole frame period of latency. + // To avoid that, first vsync notification will be fired using a timer + // scheduled to fire where the next frame is expected to start. + // Also use a timer if display link does not belong to any display + // (nominalOutputRefreshPeriod being 0) + + // Start of the vsync interval. + CFTimeInterval start = CACurrentMediaTime(); + + // Timer delay is calculated as the time to the next frame start. + CFTimeInterval delay = 0; + + if (tick_interval != 0 && _lastTargetTimestamp != 0) { + CFTimeInterval phase = fmod(_lastTargetTimestamp, tick_interval); + CFTimeInterval now = start; + start = now - (fmod(now, tick_interval)) + phase; + if (start < now) { + start += tick_interval; + } + 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]; + _displayLink.paused = NO; + } else { + _pending_baton = baton; + } +} + +- (void)dealloc { + if (_pending_baton.has_value()) { + FML_LOG(WARNING) << "Deallocating FlutterVSyncWaiter with a pending vsync"; + } + [_displayLink invalidate]; +} + +@end diff --git a/shell/platform/darwin/macos/framework/Source/FlutterVSyncWaiterTest.mm b/shell/platform/darwin/macos/framework/Source/FlutterVSyncWaiterTest.mm new file mode 100644 index 0000000000000..533d3c636a97a --- /dev/null +++ b/shell/platform/darwin/macos/framework/Source/FlutterVSyncWaiterTest.mm @@ -0,0 +1,170 @@ +// 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/FlutterDisplayLink.h" +#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterVSyncWaiter.h" + +#import "flutter/testing/testing.h" + +@interface TestDisplayLink : FlutterDisplayLink { +} + +@property(nonatomic) CFTimeInterval nominalOutputRefreshPeriod; + +@end + +@implementation TestDisplayLink + +@synthesize nominalOutputRefreshPeriod = _nominalOutputRefreshPeriod; +@synthesize delegate = _delegate; +@synthesize paused = _paused; + +- (instancetype)init { + if (self = [super init]) { + _paused = YES; + } + return self; +} + +- (void)tickWithTimestamp:(CFTimeInterval)timestamp + targetTimestamp:(CFTimeInterval)targetTimestamp { + [_delegate onDisplayLink:timestamp targetTimestamp:targetTimestamp]; +} + +- (void)invalidate { +} + +@end + +TEST(FlutterVSyncWaiterTest, RequestsInitialVSync) { + TestDisplayLink* displayLink = [[TestDisplayLink alloc] init]; + EXPECT_TRUE(displayLink.paused); + // When created waiter requests a reference vsync to determine vsync phase. + FlutterVSyncWaiter* waiter = [[FlutterVSyncWaiter alloc] + initWithDisplayLink:displayLink + block:^(CFTimeInterval timestamp, CFTimeInterval targetTimestamp, + uintptr_t baton){ + }]; + (void)waiter; + EXPECT_FALSE(displayLink.paused); + [displayLink tickWithTimestamp:CACurrentMediaTime() + targetTimestamp:CACurrentMediaTime() + 1.0 / 60.0]; + EXPECT_TRUE(displayLink.paused); +} + +static void BusyWait(CFTimeInterval duration) { + CFTimeInterval start = CACurrentMediaTime(); + while (CACurrentMediaTime() < start + duration) { + } +} + +// See FlutterVSyncWaiter.mm for the original definition. +static const CFTimeInterval kTimerLatencyCompensation = 0.001; + +TEST(FlutterVSyncWaiterTest, FirstVSyncIsSynthesized) { + TestDisplayLink* displayLink = [[TestDisplayLink alloc] init]; + displayLink.nominalOutputRefreshPeriod = 1.0 / 60.0; + + auto test = [&](CFTimeInterval waitDuration, CFTimeInterval expectedDelay) { + __block CFTimeInterval timestamp = 0; + __block CFTimeInterval targetTimestamp = 0; + __block size_t baton = 0; + FlutterVSyncWaiter* waiter = [[FlutterVSyncWaiter alloc] + initWithDisplayLink:displayLink + block:^(CFTimeInterval _timestamp, CFTimeInterval _targetTimestamp, + uintptr_t _baton) { + timestamp = _timestamp; + targetTimestamp = _targetTimestamp; + baton = _baton; + EXPECT_TRUE(CACurrentMediaTime() >= _timestamp - kTimerLatencyCompensation); + CFRunLoopStop(CFRunLoopGetCurrent()); + }]; + // Reference vsync to setup phase. + CFTimeInterval now = CACurrentMediaTime(); + // CVDisplayLink callback is called one and a half frame before the target. + [displayLink tickWithTimestamp:now + 0.5 * displayLink.nominalOutputRefreshPeriod + targetTimestamp:now + 2 * displayLink.nominalOutputRefreshPeriod]; + EXPECT_EQ(displayLink.paused, YES); + // Vsync was not requested yet, block should not have been called. + EXPECT_EQ(timestamp, 0); + + BusyWait(waitDuration); + + // Synthesized vsync should come in 1/60th of a second after the first. + CFTimeInterval expectedTimestamp = now + expectedDelay; + [waiter waitForVSync:1]; + + CFRunLoopRun(); + + EXPECT_DOUBLE_EQ(timestamp, expectedTimestamp); + EXPECT_DOUBLE_EQ(targetTimestamp, expectedTimestamp + displayLink.nominalOutputRefreshPeriod); + EXPECT_EQ(baton, size_t(1)); + }; + + // First argument if the wait duration after reference vsync. + // Second argument is the expected delay between reference vsync and synthesized vsync. + test(0.005, displayLink.nominalOutputRefreshPeriod); + test(0.025, 2 * displayLink.nominalOutputRefreshPeriod); + test(0.040, 3 * displayLink.nominalOutputRefreshPeriod); +} + +TEST(FlutterVSyncWaiterTest, VSyncWorks) { + TestDisplayLink* displayLink = [[TestDisplayLink alloc] init]; + displayLink.nominalOutputRefreshPeriod = 1.0 / 60.0; + + struct Entry { + CFTimeInterval timestamp; + CFTimeInterval targetTimestamp; + size_t baton; + }; + __block std::vector entries; + + FlutterVSyncWaiter* waiter = [[FlutterVSyncWaiter alloc] + initWithDisplayLink:displayLink + block:^(CFTimeInterval timestamp, CFTimeInterval targetTimestamp, + uintptr_t baton) { + entries.push_back({timestamp, targetTimestamp, baton}); + EXPECT_TRUE(CACurrentMediaTime() >= timestamp - kTimerLatencyCompensation); + CFRunLoopStop(CFRunLoopGetCurrent()); + }]; + + // Reference vsync to setup phase. + CFTimeInterval now = CACurrentMediaTime(); + // CVDisplayLink callback is called one and a half frame before the target. + [displayLink tickWithTimestamp:now + 0.5 * displayLink.nominalOutputRefreshPeriod + targetTimestamp:now + 2 * displayLink.nominalOutputRefreshPeriod]; + EXPECT_EQ(displayLink.paused, YES); + + [waiter waitForVSync:1]; + CFRunLoopRun(); + + [waiter waitForVSync:2]; + [displayLink tickWithTimestamp:now + 1.5 * displayLink.nominalOutputRefreshPeriod + targetTimestamp:now + 3 * displayLink.nominalOutputRefreshPeriod]; + CFRunLoopRun(); + + [waiter waitForVSync:3]; + [displayLink tickWithTimestamp:now + 2.5 * displayLink.nominalOutputRefreshPeriod + targetTimestamp:now + 4 * displayLink.nominalOutputRefreshPeriod]; + CFRunLoopRun(); + + EXPECT_FALSE(displayLink.paused); + // Vsync without baton should pause the display link. + [displayLink tickWithTimestamp:now + 3.5 * displayLink.nominalOutputRefreshPeriod + targetTimestamp:now + 5 * displayLink.nominalOutputRefreshPeriod]; + // Make sure to run the timer scheduled in display link callback. + CFRunLoopRunInMode(kCFRunLoopDefaultMode, 0.02, NO); + ASSERT_TRUE(displayLink.paused); + + EXPECT_EQ(entries.size(), size_t(3)); + EXPECT_DOUBLE_EQ(entries[0].timestamp, now + displayLink.nominalOutputRefreshPeriod); + EXPECT_DOUBLE_EQ(entries[0].targetTimestamp, now + 2 * displayLink.nominalOutputRefreshPeriod); + EXPECT_EQ(entries[0].baton, size_t(1)); + EXPECT_DOUBLE_EQ(entries[1].timestamp, now + 2 * displayLink.nominalOutputRefreshPeriod); + EXPECT_DOUBLE_EQ(entries[1].targetTimestamp, now + 3 * displayLink.nominalOutputRefreshPeriod); + EXPECT_EQ(entries[1].baton, size_t(2)); + EXPECT_DOUBLE_EQ(entries[2].timestamp, now + 3 * displayLink.nominalOutputRefreshPeriod); + EXPECT_DOUBLE_EQ(entries[2].targetTimestamp, now + 4 * displayLink.nominalOutputRefreshPeriod); + EXPECT_EQ(entries[2].baton, size_t(3)); +} diff --git a/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm b/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm index f93c3ddcc9815..3dfe4591c1233 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm @@ -448,6 +448,7 @@ - (void)viewDidLoad { [self configureTrackingArea]; [self.view setAllowedTouchTypes:NSTouchTypeMaskIndirect]; [self.view setWantsRestingTouches:YES]; + [_engine viewControllerViewDidLoad:self]; } - (void)viewWillAppear { diff --git a/shell/platform/embedder/embedder.h b/shell/platform/embedder/embedder.h index d8adf5e77834c..dce500fe66897 100644 --- a/shell/platform/embedder/embedder.h +++ b/shell/platform/embedder/embedder.h @@ -1736,6 +1736,10 @@ typedef struct { /// Extra information for the backing store that the embedder may /// use during presentation. FlutterBackingStorePresentInfo* backing_store_present_info; + + // Time in nanoseconds at which this frame is scheduled to be presented. 0 if + // not known. See FlutterEngineGetCurrentTime(). + uint64_t presentation_time; } FlutterLayer; typedef bool (*FlutterBackingStoreCreateCallback)( diff --git a/shell/platform/embedder/embedder_external_view_embedder.cc b/shell/platform/embedder/embedder_external_view_embedder.cc index aaf78f6e75c27..b80548d09392e 100644 --- a/shell/platform/embedder/embedder_external_view_embedder.cc +++ b/shell/platform/embedder/embedder_external_view_embedder.cc @@ -479,12 +479,18 @@ void EmbedderExternalViewEmbedder::SubmitFlutterView( } { + auto presentation_time_optional = frame->submit_info().presentation_time; + uint64_t presentation_time = + presentation_time_optional.has_value() + ? presentation_time_optional->ToEpochDelta().ToNanoseconds() + : 0; + // Submit the scribbled layer to the embedder for presentation. // // @warning: Embedder may trample on our OpenGL context here. - EmbedderLayers presented_layers(pending_frame_size_, - pending_device_pixel_ratio_, - pending_surface_transformation_); + EmbedderLayers presented_layers( + pending_frame_size_, pending_device_pixel_ratio_, + pending_surface_transformation_, presentation_time); builder.PushLayers(presented_layers); diff --git a/shell/platform/embedder/embedder_layers.cc b/shell/platform/embedder/embedder_layers.cc index 2b1035908550c..96e5d71f610db 100644 --- a/shell/platform/embedder/embedder_layers.cc +++ b/shell/platform/embedder/embedder_layers.cc @@ -10,10 +10,12 @@ namespace flutter { EmbedderLayers::EmbedderLayers(SkISize frame_size, double device_pixel_ratio, - SkMatrix root_surface_transformation) + SkMatrix root_surface_transformation, + uint64_t presentation_time) : frame_size_(frame_size), device_pixel_ratio_(device_pixel_ratio), - root_surface_transformation_(root_surface_transformation) {} + root_surface_transformation_(root_surface_transformation), + presentation_time_(presentation_time) {} EmbedderLayers::~EmbedderLayers() = default; @@ -62,6 +64,7 @@ void EmbedderLayers::PushBackingStoreLayer( present_info->paint_region = paint_region.get(); regions_referenced_.push_back(std::move(paint_region)); layer.backing_store_present_info = present_info.get(); + layer.presentation_time = presentation_time_; present_info_referenced_.push_back(std::move(present_info)); presented_layers_.push_back(layer); @@ -225,6 +228,8 @@ void EmbedderLayers::PushPlatformViewLayer( layer.size.width = transformed_layer_bounds.width(); layer.size.height = transformed_layer_bounds.height(); + layer.presentation_time = presentation_time_; + presented_layers_.push_back(layer); } diff --git a/shell/platform/embedder/embedder_layers.h b/shell/platform/embedder/embedder_layers.h index e821cb09a7e85..32727ab329f87 100644 --- a/shell/platform/embedder/embedder_layers.h +++ b/shell/platform/embedder/embedder_layers.h @@ -20,7 +20,8 @@ class EmbedderLayers { public: EmbedderLayers(SkISize frame_size, double device_pixel_ratio, - SkMatrix root_surface_transformation); + SkMatrix root_surface_transformation, + uint64_t presentation_time); ~EmbedderLayers(); @@ -48,6 +49,7 @@ class EmbedderLayers { std::vector> regions_referenced_; std::vector>> rects_referenced_; std::vector presented_layers_; + uint64_t presentation_time_; FML_DISALLOW_COPY_AND_ASSIGN(EmbedderLayers); }; From b42069d47965ebfb40cd1b5c5f1db12b358d8b22 Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Fri, 1 Mar 2024 12:20:15 +0100 Subject: [PATCH 2/9] Convert time to and from engine time --- shell/platform/darwin/macos/BUILD.gn | 2 + .../framework/Source/FlutterCompositor.h | 5 ++ .../framework/Source/FlutterCompositor.mm | 4 +- .../macos/framework/Source/FlutterEngine.mm | 11 +++-- .../framework/Source/FlutterTimeConverter.h | 22 +++++++++ .../framework/Source/FlutterTimeConverter.mm | 46 +++++++++++++++++++ 6 files changed, 85 insertions(+), 5 deletions(-) create mode 100644 shell/platform/darwin/macos/framework/Source/FlutterTimeConverter.h create mode 100644 shell/platform/darwin/macos/framework/Source/FlutterTimeConverter.mm diff --git a/shell/platform/darwin/macos/BUILD.gn b/shell/platform/darwin/macos/BUILD.gn index 15aa2d757fe7f..7228264ba4d14 100644 --- a/shell/platform/darwin/macos/BUILD.gn +++ b/shell/platform/darwin/macos/BUILD.gn @@ -103,6 +103,8 @@ source_set("flutter_framework_source") { "framework/Source/FlutterTextureRegistrar.mm", "framework/Source/FlutterThreadSynchronizer.h", "framework/Source/FlutterThreadSynchronizer.mm", + "framework/Source/FlutterTimeConverter.h", + "framework/Source/FlutterTimeConverter.mm", "framework/Source/FlutterVSyncWaiter.h", "framework/Source/FlutterVSyncWaiter.mm", "framework/Source/FlutterView.h", diff --git a/shell/platform/darwin/macos/framework/Source/FlutterCompositor.h b/shell/platform/darwin/macos/framework/Source/FlutterCompositor.h index 6ebcfaab2b186..d3dc0f6bd4e8b 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterCompositor.h +++ b/shell/platform/darwin/macos/framework/Source/FlutterCompositor.h @@ -10,6 +10,7 @@ #include "flutter/fml/macros.h" #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterPlatformViewController.h" +#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterTimeConverter.h" #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterViewProvider.h" #include "flutter/shell/platform/embedder/embedder.h" @@ -30,6 +31,7 @@ class FlutterCompositor { // which are used for presenting and creating backing stores. // It must not be null, and is typically FlutterViewEngineProvider. explicit FlutterCompositor(id view_provider, + FlutterTimeConverter* time_converter, FlutterPlatformViewController* platform_views_controller); ~FlutterCompositor() = default; @@ -68,6 +70,9 @@ class FlutterCompositor { // Where the compositor can query FlutterViews. Must not be null. id const view_provider_; + // Converts between engine time and core animation media time. + FlutterTimeConverter* const time_converter_; + // The controller used to manage creation and deletion of platform views. const FlutterPlatformViewController* platform_view_controller_; diff --git a/shell/platform/darwin/macos/framework/Source/FlutterCompositor.mm b/shell/platform/darwin/macos/framework/Source/FlutterCompositor.mm index 25471599eb707..1b6715cc777c3 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterCompositor.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterCompositor.mm @@ -10,8 +10,10 @@ namespace flutter { FlutterCompositor::FlutterCompositor(id view_provider, + FlutterTimeConverter* time_converter, FlutterPlatformViewController* platform_view_controller) : view_provider_(view_provider), + time_converter_(time_converter), platform_view_controller_(platform_view_controller), mutator_views_([NSMapTable strongToStrongObjectsMapTable]) { FML_CHECK(view_provider != nullptr) << "view_provider cannot be nullptr"; @@ -72,7 +74,7 @@ CFTimeInterval presentation_time = 0; if (layers_count > 0 && layers[0]->presentation_time != 0) { - presentation_time = layers[0]->presentation_time / 1'000'000'000.0; + presentation_time = [time_converter_ engineTimeToCAMediaTime:layers[0]->presentation_time]; } [view.surfaceManager presentSurfaces:surfaces diff --git a/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm b/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm index bc835ee045888..07f714a06e3f3 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm @@ -23,6 +23,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/FlutterTimeConverter.h" #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterVSyncWaiter.h" #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterViewController_Internal.h" #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterViewEngineProvider.h" @@ -739,13 +740,14 @@ - (void)registerViewController:(FlutterViewController*)controller forId:(Flutter - (void)viewControllerViewDidLoad:(FlutterViewController*)viewController { __weak FlutterEngine* weakSelf = self; + FlutterTimeConverter* timeConverter = [[FlutterTimeConverter alloc] initWithEngine:self]; FlutterVSyncWaiter* waiter = [[FlutterVSyncWaiter alloc] initWithDisplayLink:[FlutterDisplayLink displayLinkWithView:viewController.view] block:^(CFTimeInterval timestamp, CFTimeInterval targetTimestamp, uintptr_t baton) { - // CAMediaTime and flutter time are both mach_absolute_time. - uint64_t timeNanos = timestamp * 1000000000; - uint64_t targetTimeNanos = targetTimestamp * 1000000000; + uint64_t timeNanos = [timeConverter CAMediaTimeToEngineTime:timestamp]; + uint64_t targetTimeNanos = + [timeConverter CAMediaTimeToEngineTime:targetTimestamp]; FlutterEngine* engine = weakSelf; if (engine) { // It is a bit unfortunate that embedder requires OnVSync call on @@ -826,7 +828,8 @@ - (FlutterViewController*)viewController { - (FlutterCompositor*)createFlutterCompositor { _macOSCompositor = std::make_unique( - [[FlutterViewEngineProvider alloc] initWithEngine:self], _platformViewController); + [[FlutterViewEngineProvider alloc] initWithEngine:self], + [[FlutterTimeConverter alloc] initWithEngine:self], _platformViewController); _compositor = {}; _compositor.struct_size = sizeof(FlutterCompositor); diff --git a/shell/platform/darwin/macos/framework/Source/FlutterTimeConverter.h b/shell/platform/darwin/macos/framework/Source/FlutterTimeConverter.h new file mode 100644 index 0000000000000..e60382d0d0eb5 --- /dev/null +++ b/shell/platform/darwin/macos/framework/Source/FlutterTimeConverter.h @@ -0,0 +1,22 @@ +// 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_FLUTTER_TIME_CONVERTER_H_ +#define FLUTTER_SHELL_PLATFORM_DARWIN_MACOS_FRAMEWORK_SOURCE_FLUTTER_TIME_CONVERTER_H_ + +#import + +@class FlutterEngine; + +/// Converts between the time representation used by Flutter Engine and CAMediaTime. +@interface FlutterTimeConverter : NSObject + +- (instancetype)initWithEngine:(FlutterEngine*)engine; + +- (uint64_t)CAMediaTimeToEngineTime:(CFTimeInterval)time; +- (CFTimeInterval)engineTimeToCAMediaTime:(uint64_t)time; + +@end + +#endif // FLUTTER_SHELL_PLATFORM_DARWIN_MACOS_FRAMEWORK_SOURCE_FLUTTER_TIME_CONVERTER_H_ diff --git a/shell/platform/darwin/macos/framework/Source/FlutterTimeConverter.mm b/shell/platform/darwin/macos/framework/Source/FlutterTimeConverter.mm new file mode 100644 index 0000000000000..e8f7ef042b8fd --- /dev/null +++ b/shell/platform/darwin/macos/framework/Source/FlutterTimeConverter.mm @@ -0,0 +1,46 @@ +// 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_FLUTTER_TIME_CONVERTER_MM_ +#define FLUTTER_SHELL_PLATFORM_DARWIN_MACOS_FRAMEWORK_SOURCE_FLUTTER_TIME_CONVERTER_MM_ + +#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterTimeConverter.h" +#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterEngine_Internal.h" + +@interface FlutterTimeConverter () { + __weak FlutterEngine* _engine; +} +@end + +@implementation FlutterTimeConverter + +- (instancetype)initWithEngine:(FlutterEngine*)engine { + self = [super init]; + if (self) { + _engine = engine; + } + return self; +} + +- (uint64_t)CAMediaTimeToEngineTime:(CFTimeInterval)time { + FlutterEngine* engine = _engine; + if (!engine) { + return 0; + } + return (time - CACurrentMediaTime()) * NSEC_PER_SEC + engine.embedderAPI.GetCurrentTime(); +} + +- (CFTimeInterval)engineTimeToCAMediaTime:(uint64_t)time { + FlutterEngine* engine = _engine; + if (!engine) { + return 0; + } + return (static_cast(time) - static_cast(engine.embedderAPI.GetCurrentTime())) / + static_cast(NSEC_PER_SEC) + + CACurrentMediaTime(); +} + +@end + +#endif // FLUTTER_SHELL_PLATFORM_DARWIN_MACOS_FRAMEWORK_SOURCE_FLUTTER_TIME_CONVERTER_MM_- From 1b0f028fa39a52d3aabddef02d5cd6a3cb1688c4 Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Thu, 29 Feb 2024 16:06:05 +0100 Subject: [PATCH 3/9] Speculative fix for timeToFirstFrameRasterizedMicros regression --- .../framework/Source/FlutterVSyncWaiter.mm | 38 ++++++++++++------ .../Source/FlutterVSyncWaiterTest.mm | 39 ++++++++++++++----- 2 files changed, 55 insertions(+), 22 deletions(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterVSyncWaiter.mm b/shell/platform/darwin/macos/framework/Source/FlutterVSyncWaiter.mm index cf948a8b11701..eb4d6fb9b3558 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterVSyncWaiter.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterVSyncWaiter.mm @@ -34,11 +34,12 @@ @interface FlutterVSyncWaiter () static const CFTimeInterval kTimerLatencyCompensation = 0.001; @implementation FlutterVSyncWaiter { - std::optional _pending_baton; + std::optional _pendingBaton; FlutterDisplayLink* _displayLink; void (^_block)(CFTimeInterval, CFTimeInterval, uintptr_t); NSRunLoop* _runLoop; CFTimeInterval _lastTargetTimestamp; + BOOL _warmUpFrame; } - (instancetype)initWithDisplayLink:(FlutterDisplayLink*)displayLink @@ -53,6 +54,7 @@ - (instancetype)initWithDisplayLink:(FlutterDisplayLink*)displayLink _displayLink.delegate = self; // Get at least one callback to initialize _lastTargetTimestamp. _displayLink.paused = NO; + _warmUpFrame = YES; } return self; } @@ -71,20 +73,20 @@ - (void)processDisplayLink:(CFTimeInterval)timestamp CFTimeInterval current = CACurrentMediaTime(); CFTimeInterval remaining = std::max(minStart - current - kTimerLatencyCompensation, 0.0); - TRACE_VSYNC("DisplayLinkCallback-Original", _pending_baton.value_or(0)); + TRACE_VSYNC("DisplayLinkCallback-Original", _pendingBaton.value_or(0)); NSTimer* timer = [NSTimer timerWithTimeInterval:remaining repeats:NO block:^(NSTimer* _Nonnull timer) { - if (!_pending_baton.has_value()) { + if (!_pendingBaton.has_value()) { TRACE_VSYNC("DisplayLinkPaused", size_t(0)); _displayLink.paused = YES; return; } - TRACE_VSYNC("DisplayLinkCallback-Delayed", _pending_baton.value_or(0)); - _block(minStart, targetTimestamp, *_pending_baton); - _pending_baton = std::nullopt; + TRACE_VSYNC("DisplayLinkCallback-Delayed", _pendingBaton.value_or(0)); + _block(minStart, targetTimestamp, *_pendingBaton); + _pendingBaton = std::nullopt; }]; [_runLoop addTimer:timer forMode:NSRunLoopCommonModes]; } @@ -106,6 +108,18 @@ - (void)onDisplayLink:(CFTimeInterval)timestamp targetTimestamp:(CFTimeInterval) // Called from UI thread. - (void)waitForVSync:(uintptr_t)baton { + // 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); + }]; + return; + } + // RunLoop is accessed both from main thread and from the display link thread. @synchronized(self) { if (_runLoop == nil) { @@ -114,13 +128,13 @@ - (void)waitForVSync:(uintptr_t)baton { } FML_DCHECK(_runLoop == [NSRunLoop currentRunLoop]); - if (_pending_baton.has_value()) { + if (_pendingBaton.has_value()) { FML_LOG(WARNING) << "Engine requested vsync while another was pending"; - _block(0, 0, *_pending_baton); - _pending_baton = std::nullopt; + _block(0, 0, *_pendingBaton); + _pendingBaton = std::nullopt; } - TRACE_VSYNC("VSyncRequest", _pending_baton.value_or(0)); + TRACE_VSYNC("VSyncRequest", _pendingBaton.value_or(0)); CFTimeInterval tick_interval = _displayLink.nominalOutputRefreshPeriod; if (_displayLink.paused || tick_interval == 0) { @@ -158,12 +172,12 @@ - (void)waitForVSync:(uintptr_t)baton { [_runLoop addTimer:timer forMode:NSRunLoopCommonModes]; _displayLink.paused = NO; } else { - _pending_baton = baton; + _pendingBaton = baton; } } - (void)dealloc { - if (_pending_baton.has_value()) { + if (_pendingBaton.has_value()) { FML_LOG(WARNING) << "Deallocating FlutterVSyncWaiter with a pending vsync"; } [_displayLink invalidate]; diff --git a/shell/platform/darwin/macos/framework/Source/FlutterVSyncWaiterTest.mm b/shell/platform/darwin/macos/framework/Source/FlutterVSyncWaiterTest.mm index 533d3c636a97a..20cddbebfe88f 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterVSyncWaiterTest.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterVSyncWaiterTest.mm @@ -70,16 +70,23 @@ static void BusyWait(CFTimeInterval duration) { __block CFTimeInterval timestamp = 0; __block CFTimeInterval targetTimestamp = 0; __block size_t baton = 0; + const uintptr_t kWarmUpBaton = 0xFFFFFFFF; FlutterVSyncWaiter* waiter = [[FlutterVSyncWaiter alloc] initWithDisplayLink:displayLink block:^(CFTimeInterval _timestamp, CFTimeInterval _targetTimestamp, uintptr_t _baton) { + if (_baton == kWarmUpBaton) { + return; + } timestamp = _timestamp; targetTimestamp = _targetTimestamp; baton = _baton; EXPECT_TRUE(CACurrentMediaTime() >= _timestamp - kTimerLatencyCompensation); CFRunLoopStop(CFRunLoopGetCurrent()); }]; + + [waiter waitForVSync:kWarmUpBaton]; + // Reference vsync to setup phase. CFTimeInterval now = CACurrentMediaTime(); // CVDisplayLink callback is called one and a half frame before the target. @@ -112,6 +119,7 @@ static void BusyWait(CFTimeInterval duration) { TEST(FlutterVSyncWaiterTest, VSyncWorks) { TestDisplayLink* displayLink = [[TestDisplayLink alloc] init]; displayLink.nominalOutputRefreshPeriod = 1.0 / 60.0; + const uintptr_t kWarmUpBaton = 0xFFFFFFFF; struct Entry { CFTimeInterval timestamp; @@ -125,10 +133,15 @@ static void BusyWait(CFTimeInterval duration) { block:^(CFTimeInterval timestamp, CFTimeInterval targetTimestamp, uintptr_t baton) { entries.push_back({timestamp, targetTimestamp, baton}); + if (baton == kWarmUpBaton) { + return; + } EXPECT_TRUE(CACurrentMediaTime() >= timestamp - kTimerLatencyCompensation); CFRunLoopStop(CFRunLoopGetCurrent()); }]; + [waiter waitForVSync:kWarmUpBaton]; + // Reference vsync to setup phase. CFTimeInterval now = CACurrentMediaTime(); // CVDisplayLink callback is called one and a half frame before the target. @@ -157,14 +170,20 @@ static void BusyWait(CFTimeInterval duration) { CFRunLoopRunInMode(kCFRunLoopDefaultMode, 0.02, NO); ASSERT_TRUE(displayLink.paused); - EXPECT_EQ(entries.size(), size_t(3)); - EXPECT_DOUBLE_EQ(entries[0].timestamp, now + displayLink.nominalOutputRefreshPeriod); - EXPECT_DOUBLE_EQ(entries[0].targetTimestamp, now + 2 * displayLink.nominalOutputRefreshPeriod); - EXPECT_EQ(entries[0].baton, size_t(1)); - EXPECT_DOUBLE_EQ(entries[1].timestamp, now + 2 * displayLink.nominalOutputRefreshPeriod); - EXPECT_DOUBLE_EQ(entries[1].targetTimestamp, now + 3 * displayLink.nominalOutputRefreshPeriod); - EXPECT_EQ(entries[1].baton, size_t(2)); - EXPECT_DOUBLE_EQ(entries[2].timestamp, now + 3 * displayLink.nominalOutputRefreshPeriod); - EXPECT_DOUBLE_EQ(entries[2].targetTimestamp, now + 4 * displayLink.nominalOutputRefreshPeriod); - EXPECT_EQ(entries[2].baton, size_t(3)); + EXPECT_EQ(entries.size(), size_t(4)); + + // Warm up frame should be presented as soon as possible. + EXPECT_TRUE(fabs(entries[0].timestamp - now) < 0.001); + EXPECT_TRUE(fabs(entries[0].targetTimestamp - now) < 0.001); + EXPECT_EQ(entries[0].baton, kWarmUpBaton); + + EXPECT_DOUBLE_EQ(entries[1].timestamp, now + displayLink.nominalOutputRefreshPeriod); + EXPECT_DOUBLE_EQ(entries[1].targetTimestamp, now + 2 * displayLink.nominalOutputRefreshPeriod); + EXPECT_EQ(entries[1].baton, size_t(1)); + EXPECT_DOUBLE_EQ(entries[2].timestamp, now + 2 * displayLink.nominalOutputRefreshPeriod); + EXPECT_DOUBLE_EQ(entries[2].targetTimestamp, now + 3 * displayLink.nominalOutputRefreshPeriod); + EXPECT_EQ(entries[2].baton, size_t(2)); + EXPECT_DOUBLE_EQ(entries[3].timestamp, now + 3 * displayLink.nominalOutputRefreshPeriod); + EXPECT_DOUBLE_EQ(entries[3].targetTimestamp, now + 4 * displayLink.nominalOutputRefreshPeriod); + EXPECT_EQ(entries[3].baton, size_t(3)); } From 92c18728e2fc53b88e852e776e4d678b466861c4 Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Fri, 1 Mar 2024 14:33:23 +0100 Subject: [PATCH 4/9] Fix FlutterEngineTest.Compositor flakyness --- .../darwin/macos/framework/Source/FlutterThreadSynchronizer.mm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.mm b/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.mm index 9834261bff9a5..3eb332fac5663 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.mm @@ -88,9 +88,9 @@ - (void)drain { - (void)blockUntilFrameAvailable { std::unique_lock lock(_mutex); + [self drain]; _beginResizeWaiting = YES; - while (![self someViewsHaveFrame] && !_shuttingDown) { _condBlockBeginResize.wait(lock); [self drain]; From 352038a3d3f0446be1bb47e348f066a40c02c772 Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Fri, 1 Mar 2024 16:28:33 +0100 Subject: [PATCH 5/9] Do not block raster thread when delaying present --- .../framework/Source/FlutterCompositor.h | 11 +++-- .../framework/Source/FlutterCompositor.mm | 44 +++++++++++------ .../framework/Source/FlutterMutatorView.h | 30 +++++++++++- .../framework/Source/FlutterMutatorView.mm | 47 ++++++++++++------- .../Source/FlutterMutatorViewTest.mm | 26 +++------- .../framework/Source/FlutterSurfaceManager.mm | 46 ++++++++++-------- 6 files changed, 129 insertions(+), 75 deletions(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterCompositor.h b/shell/platform/darwin/macos/framework/Source/FlutterCompositor.h index d3dc0f6bd4e8b..aad527a3785df 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterCompositor.h +++ b/shell/platform/darwin/macos/framework/Source/FlutterCompositor.h @@ -18,6 +18,10 @@ namespace flutter { +class PlatformViewLayer; + +typedef std::pair PlatformViewLayerWithIndex; + // FlutterCompositor creates and manages the backing stores used for // rendering Flutter content and presents Flutter content and Platform views. // Platform views are not yet supported. @@ -57,15 +61,14 @@ class FlutterCompositor { private: void PresentPlatformViews(FlutterView* default_base_view, - const FlutterLayer** layers, - size_t layers_count); + const std::vector& platform_views_layers); // Presents the platform view layer represented by `layer`. `layer_index` is // used to position the layer in the z-axis. If the layer does not have a // superview, it will become subview of `default_base_view`. FlutterMutatorView* PresentPlatformView(FlutterView* default_base_view, - const FlutterLayer* layer, - size_t layer_position); + const PlatformViewLayer& layer, + size_t index); // Where the compositor can query FlutterViews. Must not be null. id const view_provider_; diff --git a/shell/platform/darwin/macos/framework/Source/FlutterCompositor.mm b/shell/platform/darwin/macos/framework/Source/FlutterCompositor.mm index 1b6715cc777c3..890a4c0cfca3b 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterCompositor.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterCompositor.mm @@ -9,6 +9,19 @@ namespace flutter { +namespace { +std::vector CopyPlatformViewLayers(const FlutterLayer** layers, + size_t layer_count) { + std::vector platform_views; + for (size_t i = 0; i < layer_count; i++) { + if (layers[i]->type == kFlutterLayerContentTypePlatformView) { + platform_views.push_back(std::make_pair(PlatformViewLayer(layers[i]), i)); + } + } + return platform_views; +} +} // namespace + FlutterCompositor::FlutterCompositor(id view_provider, FlutterTimeConverter* time_converter, FlutterPlatformViewController* platform_view_controller) @@ -77,29 +90,32 @@ presentation_time = [time_converter_ engineTimeToCAMediaTime:layers[0]->presentation_time]; } + // Notify block below may be called asynchronously, hence the need to copy + // the layer information instead of passing the original pointers from embedder. + auto platform_views_layers = std::make_shared>( + CopyPlatformViewLayers(layers, layers_count)); + [view.surfaceManager presentSurfaces:surfaces atTime:presentation_time notify:^{ - PresentPlatformViews(view, layers, layers_count); + PresentPlatformViews(view, *platform_views_layers); }]; return true; } -void FlutterCompositor::PresentPlatformViews(FlutterView* default_base_view, - const FlutterLayer** layers, - size_t layers_count) { +void FlutterCompositor::PresentPlatformViews( + FlutterView* default_base_view, + const std::vector& platform_views) { FML_DCHECK([[NSThread currentThread] isMainThread]) << "Must be on the main thread to present platform views"; // Active mutator views for this frame. NSMutableArray* present_mutators = [NSMutableArray array]; - for (size_t i = 0; i < layers_count; i++) { - FlutterLayer* layer = (FlutterLayer*)layers[i]; - if (layer->type == kFlutterLayerContentTypePlatformView) { - [present_mutators addObject:PresentPlatformView(default_base_view, layer, i)]; - } + for (const auto& platform_view : platform_views) { + [present_mutators addObject:PresentPlatformView(default_base_view, platform_view.first, + platform_view.second)]; } NSMutableArray* obsolete_mutators = @@ -115,12 +131,12 @@ } FlutterMutatorView* FlutterCompositor::PresentPlatformView(FlutterView* default_base_view, - const FlutterLayer* layer, - size_t layer_position) { + const PlatformViewLayer& layer, + size_t index) { FML_DCHECK([[NSThread currentThread] isMainThread]) << "Must be on the main thread to present platform views"; - int64_t platform_view_id = layer->platform_view->identifier; + int64_t platform_view_id = layer.identifier(); NSView* platform_view = [platform_view_controller_ platformViewWithID:platform_view_id]; FML_DCHECK(platform_view) << "Platform view not found for id: " << platform_view_id; @@ -133,8 +149,8 @@ [default_base_view addSubview:container]; } - container.layer.zPosition = layer_position; - [container applyFlutterLayer:layer]; + container.layer.zPosition = index; + [container applyFlutterLayer:&layer]; return container; } diff --git a/shell/platform/darwin/macos/framework/Source/FlutterMutatorView.h b/shell/platform/darwin/macos/framework/Source/FlutterMutatorView.h index 73367ef0fe799..d970a66e055b3 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterMutatorView.h +++ b/shell/platform/darwin/macos/framework/Source/FlutterMutatorView.h @@ -6,9 +6,37 @@ #define FLUTTER_SHELL_PLATFORM_DARWIN_MACOS_FRAMEWORK_SOURCE_FLUTTERMUTATORVIEW_H_ #import +#include #include "flutter/shell/platform/embedder/embedder.h" +namespace flutter { + +/// Represents a platform view layer, including all mutations. +class PlatformViewLayer { + public: + /// Creates platform view from provided FlutterLayer, which must be + /// of type kFlutterLayerContentTypePlatformView. + explicit PlatformViewLayer(const FlutterLayer* _Nonnull layer); + + PlatformViewLayer(FlutterPlatformViewIdentifier identifier, + const std::vector& mutations, + FlutterPoint offset, + FlutterSize size); + + FlutterPlatformViewIdentifier identifier() const { return identifier_; } + const std::vector& mutations() const { return mutations_; } + FlutterPoint offset() const { return offset_; } + FlutterSize size() const { return size_; } + + private: + FlutterPlatformViewIdentifier identifier_; + std::vector mutations_; + FlutterPoint offset_; + FlutterSize size_; +}; +} // namespace flutter + /// FlutterMutatorView contains platform view and is responsible for applying /// FlutterLayer mutations to it. @interface FlutterMutatorView : NSView @@ -22,7 +50,7 @@ /// Applies mutations from FlutterLayer to the platform view. This may involve /// creating or removing intermediate subviews depending on current state and /// requested mutations. -- (void)applyFlutterLayer:(nonnull const FlutterLayer*)layer; +- (void)applyFlutterLayer:(nonnull const flutter::PlatformViewLayer*)layer; @end diff --git a/shell/platform/darwin/macos/framework/Source/FlutterMutatorView.mm b/shell/platform/darwin/macos/framework/Source/FlutterMutatorView.mm index 0c5c4af7b3e3a..9e5a3e5fe084e 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterMutatorView.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterMutatorView.mm @@ -12,6 +12,24 @@ #import "flutter/shell/platform/darwin/macos/framework/Source/NSView+ClipsToBounds.h" +namespace flutter { +PlatformViewLayer::PlatformViewLayer(const FlutterLayer* layer) { + FML_CHECK(layer->type == kFlutterLayerContentTypePlatformView); + const auto* platform_view = layer->platform_view; + identifier_ = platform_view->identifier; + for (size_t i = 0; i < platform_view->mutations_count; i++) { + mutations_.push_back(*platform_view->mutations[i]); + } + offset_ = layer->offset; + size_ = layer->size; +} +PlatformViewLayer::PlatformViewLayer(FlutterPlatformViewIdentifier identifier, + const std::vector& mutations, + FlutterPoint offset, + FlutterSize size) + : identifier_(identifier), mutations_(mutations), offset_(offset), size_(size) {} +} // namespace flutter + @interface FlutterMutatorView () { // Each of these views clips to a CGPathRef. These views, if present, // are nested (first is child of FlutterMutatorView and last is parent of @@ -230,19 +248,16 @@ CGPathRef PathFromRoundedRect(const FlutterRoundedRect& roundedRect) { /// The transforms sent from the engine include a transform from logical to physical coordinates. /// Since Cocoa deals only in logical points, this function prepends a scale transform that scales /// back from physical to logical coordinates to compensate. -MutationVector MutationsForPlatformView(const FlutterPlatformView* view, float scale) { - MutationVector mutations; - mutations.reserve(view->mutations_count + 1); - mutations.push_back({ - .type = kFlutterPlatformViewMutationTypeTransformation, - .transformation{ - .scaleX = 1.0 / scale, - .scaleY = 1.0 / scale, - }, - }); - for (size_t i = 0; i < view->mutations_count; ++i) { - mutations.push_back(*view->mutations[i]); - } +MutationVector MutationsForPlatformView(const MutationVector& mutationsIn, float scale) { + MutationVector mutations(mutationsIn); + + mutations.insert(mutations.begin(), { + .type = kFlutterPlatformViewMutationTypeTransformation, + .transformation{ + .scaleX = 1.0 / scale, + .scaleY = 1.0 / scale, + }, + }); return mutations; } @@ -484,18 +499,18 @@ - (void)updatePlatformViewWithBounds:(CGRect)untransformedBounds /// Whenever possible view will be clipped using layer bounds. /// If clipping to path is needed, CAShapeLayer(s) will be used as mask. /// Clipping to round rect only clips to path if round corners are intersected. -- (void)applyFlutterLayer:(const FlutterLayer*)layer { +- (void)applyFlutterLayer:(const flutter::PlatformViewLayer*)layer { // Compute the untransformed bounding rect for the platform view in logical pixels. // FlutterLayer.size is in physical pixels but Cocoa uses logical points. CGFloat scale = [self contentsScale]; - MutationVector mutations = MutationsForPlatformView(layer->platform_view, scale); + MutationVector mutations = MutationsForPlatformView(layer->mutations(), scale); CATransform3D finalTransform = CATransformFromMutations(mutations); // Compute the untransformed bounding rect for the platform view in logical pixels. // FlutterLayer.size is in physical pixels but Cocoa uses logical points. CGRect untransformedBoundingRect = - CGRectMake(0, 0, layer->size.width / scale, layer->size.height / scale); + CGRectMake(0, 0, layer->size().width / scale, layer->size().height / scale); CGRect finalBoundingRect = CGRectApplyAffineTransform( untransformedBoundingRect, CATransform3DGetAffineTransform(finalTransform)); self.frame = finalBoundingRect; diff --git a/shell/platform/darwin/macos/framework/Source/FlutterMutatorViewTest.mm b/shell/platform/darwin/macos/framework/Source/FlutterMutatorViewTest.mm index aeb241ed76ee1..6d6b6b21c4024 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterMutatorViewTest.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterMutatorViewTest.mm @@ -21,26 +21,12 @@ @interface FlutterMutatorView (Private) void ApplyFlutterLayer(FlutterMutatorView* view, FlutterSize size, const std::vector& mutations) { - FlutterLayer layer; - layer.struct_size = sizeof(FlutterLayer); - layer.type = kFlutterLayerContentTypePlatformView; - // Offset is ignored by mutator view, the bounding rect is determined by - // width and transform. - layer.offset = FlutterPoint{0, 0}; - layer.size = size; - - FlutterPlatformView flutterPlatformView; - flutterPlatformView.struct_size = sizeof(FlutterPlatformView); - flutterPlatformView.identifier = 0; - - std::vector mutationPointers; - mutationPointers.reserve(mutations.size()); - for (auto& mutation : mutations) { - mutationPointers.push_back(&mutation); - } - flutterPlatformView.mutations = mutationPointers.data(); - flutterPlatformView.mutations_count = mutationPointers.size(); - layer.platform_view = &flutterPlatformView; + flutter::PlatformViewLayer layer(0, // identifier + mutations, + // Offset is ignored by mutator view, the bounding rect is + // determined by width and transform. + FlutterPoint{0, 0}, // offset + size); [view applyFlutterLayer:&layer]; } diff --git a/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.mm b/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.mm index b3e7ef3882e34..73161d85f13cc 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.mm @@ -222,6 +222,19 @@ - (void)presentSurfaces:(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(); + } + }]; + }; + if (presentationTime > 0) { // Enforce frame pacing. It seems that the target timestamp of CVDisplayLink does not // exactly correspond to core animation deadline. Especially with 120hz, setting the frame @@ -229,32 +242,25 @@ - (void)presentSurfaces:(NSArray*)surfaces // Empirically setting the content in the second half of frame interval seems to work // well for both 60hz and 120hz. // - // The easiest way to ensure that the content is not set too early is to delay raster thread. - // At this point raster thread should be idle (the next frame vsync has not been signalled yet). - // This will show on a timeline as "FlutterCompositionPresentLayers" but should not cause jank - // because the waiting interval is calculated relative to presentation time. + // This schedules a timer on current (raster) thread runloop. Raster thread at + // this point should be idle (the next frame vsync has not been signalled yet). // - // Alternative to blocking raster thread would be to copy all presentation info provided by - // embedder and schedule a presentation timer. This would require additional coordination with - // FlutterThreadSynchronizer. + // Alternative could be simply blocking the raster thread, but that would show + // as a average_frame_rasterizer_time_millis regresson. CFTimeInterval minPresentationTime = (presentationTime + _lastPresentationTime) / 2.0; CFTimeInterval now = CACurrentMediaTime(); if (now < minPresentationTime) { - [NSThread sleepForTimeInterval:minPresentationTime - now]; + // [NSThread sleepForTimeInterval:minPresentationTime - now]; + NSTimer* timer = [NSTimer timerWithTimeInterval:minPresentationTime - now + repeats:NO + block:^(NSTimer* timer) { + presentBlock(); + }]; + [[NSRunLoop currentRunLoop] addTimer:timer forMode:NSDefaultRunLoopMode]; + return; } } - _lastPresentationTime = presentationTime; - - // Get the actual dimensions of the frame (relevant for thread synchronizer). - CGSize size = GetRequiredFrameSize(surfaces); - - [_delegate onPresent:size - withBlock:^{ - [self commit:surfaces]; - if (notify != nil) { - notify(); - } - }]; + presentBlock(); } @end From 94512277aa256e9c7b7d4a146ae047c317a3e633 Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Fri, 1 Mar 2024 16:29:34 +0100 Subject: [PATCH 6/9] Fix guards --- .../darwin/macos/framework/Source/FlutterTimeConverter.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterTimeConverter.h b/shell/platform/darwin/macos/framework/Source/FlutterTimeConverter.h index e60382d0d0eb5..c7e94b109920a 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterTimeConverter.h +++ b/shell/platform/darwin/macos/framework/Source/FlutterTimeConverter.h @@ -2,8 +2,8 @@ // 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_FLUTTER_TIME_CONVERTER_H_ -#define FLUTTER_SHELL_PLATFORM_DARWIN_MACOS_FRAMEWORK_SOURCE_FLUTTER_TIME_CONVERTER_H_ +#ifndef FLUTTER_SHELL_PLATFORM_DARWIN_MACOS_FRAMEWORK_SOURCE_FLUTTERTIMECONVERTER_H_ +#define FLUTTER_SHELL_PLATFORM_DARWIN_MACOS_FRAMEWORK_SOURCE_FLUTTERTIMECONVERTER_H_ #import @@ -19,4 +19,4 @@ @end -#endif // FLUTTER_SHELL_PLATFORM_DARWIN_MACOS_FRAMEWORK_SOURCE_FLUTTER_TIME_CONVERTER_H_ +#endif // FLUTTER_SHELL_PLATFORM_DARWIN_MACOS_FRAMEWORK_SOURCE_FLUTTERTIMECONVERTER_H_ From b1bfc9697147a319e3480037bf3b0b632dafbad8 Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Fri, 1 Mar 2024 16:54:31 +0100 Subject: [PATCH 7/9] Add licenses --- ci/licenses_golden/licenses_flutter | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index a181c2522deb8..d9eaf158bdce9 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -36642,6 +36642,8 @@ ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterTex 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 ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterVSyncWaiterTest.mm + ../../../flutter/LICENSE ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterView.h + ../../../flutter/LICENSE @@ -39511,6 +39513,8 @@ FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterTextu 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 FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterVSyncWaiter.h FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterVSyncWaiter.mm From 7dbb15686104da110d08a05dbd773d620174b602 Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Fri, 1 Mar 2024 17:31:55 +0100 Subject: [PATCH 8/9] Remove leftover code --- .../darwin/macos/framework/Source/FlutterSurfaceManager.mm | 1 - 1 file changed, 1 deletion(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.mm b/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.mm index 73161d85f13cc..bed88bf581082 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.mm @@ -250,7 +250,6 @@ - (void)presentSurfaces:(NSArray*)surfaces CFTimeInterval minPresentationTime = (presentationTime + _lastPresentationTime) / 2.0; CFTimeInterval now = CACurrentMediaTime(); if (now < minPresentationTime) { - // [NSThread sleepForTimeInterval:minPresentationTime - now]; NSTimer* timer = [NSTimer timerWithTimeInterval:minPresentationTime - now repeats:NO block:^(NSTimer* timer) { From e086d7accfbf66876c899fbcd7e81688a8b2af5a Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Mon, 4 Mar 2024 21:14:54 +0100 Subject: [PATCH 9/9] Add issue link --- shell/platform/darwin/macos/framework/Source/FlutterEngine.mm | 1 + 1 file changed, 1 insertion(+) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm b/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm index 07f714a06e3f3..c8c1b98344dc4 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm @@ -1101,6 +1101,7 @@ - (void)engineCallbackOnPreEngineRestart { - (void)onVSync:(uintptr_t)baton { @synchronized(_vsyncWaiters) { // TODO(knopp): Use vsync waiter for correct view. + // https://github.com/flutter/flutter/issues/142845 FlutterVSyncWaiter* waiter = [_vsyncWaiters objectForKey:@(kFlutterImplicitViewId)]; [waiter waitForVSync:baton]; }