Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit 4001881

Browse files
knoppknopp
authored
Reland: [macOS] Use CVDisplayLink to drive repaint (#51126)
This relands the PR reverted in #51095 Changes since the original PR: - The macOS embedder does not assume particular clock when calling the embedder API, but converts CAMediaTime to engine time and back (`FlutterTimeConverter`) - `FlutterVSyncWaiter` does not wait for displaylink callback during warmup frame. This should prevent `timeToFirstFrameRasterizedMicros` regressions. - When enforcing frame pacing the raster thread is not blocked. This should prevent `average_frame_rasterizer_time_millis` regressions. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide] and the [C++, Objective-C, Java style guides]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I added new tests to check the change I am making or feature I am adding, or the PR is [test-exempt]. See [testing the engine] for instructions on writing and running engine tests. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I signed the [CLA]. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/wiki/Tree-hygiene#overview [Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene [test-exempt]: https://github.com/flutter/flutter/wiki/Tree-hygiene#tests [Flutter Style Guide]: https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style [testing the engine]: https://github.com/flutter/flutter/wiki/Testing-the-engine [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/wiki/Chat
1 parent 9b71d60 commit 4001881

29 files changed

+1349
-102
lines changed

‎ci/licenses_golden/licenses_flutter

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36595,6 +36595,7 @@ ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterCom
3659536595
ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterCompositor.mm + ../../../flutter/LICENSE
3659636596
ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterDartProject.mm + ../../../flutter/LICENSE
3659736597
ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterDartProject_Internal.h + ../../../flutter/LICENSE
36598+
ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterDisplayLinkTest.mm + ../../../flutter/LICENSE
3659836599
ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterEmbedderExternalTextureTest.mm + ../../../flutter/LICENSE
3659936600
ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterEmbedderKeyResponder.h + ../../../flutter/LICENSE
3660036601
ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterEmbedderKeyResponder.mm + ../../../flutter/LICENSE
@@ -36644,7 +36645,10 @@ ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterTex
3664436645
ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.h + ../../../flutter/LICENSE
3664536646
ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.mm + ../../../flutter/LICENSE
3664636647
ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizerTest.mm + ../../../flutter/LICENSE
36648+
ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterTimeConverter.h + ../../../flutter/LICENSE
36649+
ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterTimeConverter.mm + ../../../flutter/LICENSE
3664736650
ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterUmbrellaImportTests.m + ../../../flutter/LICENSE
36651+
ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterVSyncWaiterTest.mm + ../../../flutter/LICENSE
3664836652
ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterView.h + ../../../flutter/LICENSE
3664936653
ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterView.mm + ../../../flutter/LICENSE
3665036654
ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm + ../../../flutter/LICENSE
@@ -39463,6 +39467,9 @@ FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterCompo
3946339467
FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterCompositor.mm
3946439468
FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterDartProject.mm
3946539469
FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterDartProject_Internal.h
39470+
FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterDisplayLink.h
39471+
FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterDisplayLink.mm
39472+
FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterDisplayLinkTest.mm
3946639473
FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterEmbedderExternalTextureTest.mm
3946739474
FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterEmbedderKeyResponder.h
3946839475
FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterEmbedderKeyResponder.mm
@@ -39512,7 +39519,12 @@ FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterTextu
3951239519
FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.h
3951339520
FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.mm
3951439521
FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizerTest.mm
39522+
FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterTimeConverter.h
39523+
FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterTimeConverter.mm
3951539524
FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterUmbrellaImportTests.m
39525+
FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterVSyncWaiter.h
39526+
FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterVSyncWaiter.mm
39527+
FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterVSyncWaiterTest.mm
3951639528
FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterView.h
3951739529
FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterView.mm
3951839530
FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm

‎shell/platform/darwin/macos/BUILD.gn

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,8 @@ source_set("flutter_framework_source") {
6666
"framework/Source/FlutterCompositor.mm",
6767
"framework/Source/FlutterDartProject.mm",
6868
"framework/Source/FlutterDartProject_Internal.h",
69+
"framework/Source/FlutterDisplayLink.h",
70+
"framework/Source/FlutterDisplayLink.mm",
6971
"framework/Source/FlutterEmbedderKeyResponder.h",
7072
"framework/Source/FlutterEmbedderKeyResponder.mm",
7173
"framework/Source/FlutterEngine.mm",
@@ -101,6 +103,10 @@ source_set("flutter_framework_source") {
101103
"framework/Source/FlutterTextureRegistrar.mm",
102104
"framework/Source/FlutterThreadSynchronizer.h",
103105
"framework/Source/FlutterThreadSynchronizer.mm",
106+
"framework/Source/FlutterTimeConverter.h",
107+
"framework/Source/FlutterTimeConverter.mm",
108+
"framework/Source/FlutterVSyncWaiter.h",
109+
"framework/Source/FlutterVSyncWaiter.mm",
104110
"framework/Source/FlutterView.h",
105111
"framework/Source/FlutterView.mm",
106112
"framework/Source/FlutterViewController.mm",
@@ -173,6 +179,7 @@ executable("flutter_desktop_darwin_unittests") {
173179
"framework/Source/FlutterAppDelegateTest.mm",
174180
"framework/Source/FlutterAppLifecycleDelegateTest.mm",
175181
"framework/Source/FlutterChannelKeyResponderTest.mm",
182+
"framework/Source/FlutterDisplayLinkTest.mm",
176183
"framework/Source/FlutterEmbedderExternalTextureTest.mm",
177184
"framework/Source/FlutterEmbedderKeyResponderTest.mm",
178185
"framework/Source/FlutterEngineTest.mm",
@@ -187,6 +194,7 @@ executable("flutter_desktop_darwin_unittests") {
187194
"framework/Source/FlutterTextInputPluginTest.mm",
188195
"framework/Source/FlutterTextInputSemanticsObjectTest.mm",
189196
"framework/Source/FlutterThreadSynchronizerTest.mm",
197+
"framework/Source/FlutterVSyncWaiterTest.mm",
190198
"framework/Source/FlutterViewControllerTest.mm",
191199
"framework/Source/FlutterViewControllerTestUtils.h",
192200
"framework/Source/FlutterViewControllerTestUtils.mm",

‎shell/platform/darwin/macos/framework/Source/FlutterCompositor.h

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,18 @@
1010

1111
#include "flutter/fml/macros.h"
1212
#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterPlatformViewController.h"
13+
#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterTimeConverter.h"
1314
#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterViewProvider.h"
1415
#include "flutter/shell/platform/embedder/embedder.h"
1516

1617
@class FlutterMutatorView;
1718

1819
namespace flutter {
1920

21+
class PlatformViewLayer;
22+
23+
typedef std::pair<PlatformViewLayer, size_t> PlatformViewLayerWithIndex;
24+
2025
// FlutterCompositor creates and manages the backing stores used for
2126
// rendering Flutter content and presents Flutter content and Platform views.
2227
// Platform views are not yet supported.
@@ -30,6 +35,7 @@ class FlutterCompositor {
3035
// which are used for presenting and creating backing stores.
3136
// It must not be null, and is typically FlutterViewEngineProvider.
3237
explicit FlutterCompositor(id<FlutterViewProvider> view_provider,
38+
FlutterTimeConverter* time_converter,
3339
FlutterPlatformViewController* platform_views_controller);
3440

3541
~FlutterCompositor() = default;
@@ -55,19 +61,21 @@ class FlutterCompositor {
5561

5662
private:
5763
void PresentPlatformViews(FlutterView* default_base_view,
58-
const FlutterLayer** layers,
59-
size_t layers_count);
64+
const std::vector<PlatformViewLayerWithIndex>& platform_views_layers);
6065

6166
// Presents the platform view layer represented by `layer`. `layer_index` is
6267
// used to position the layer in the z-axis. If the layer does not have a
6368
// superview, it will become subview of `default_base_view`.
6469
FlutterMutatorView* PresentPlatformView(FlutterView* default_base_view,
65-
const FlutterLayer* layer,
66-
size_t layer_position);
70+
const PlatformViewLayer& layer,
71+
size_t index);
6772

6873
// Where the compositor can query FlutterViews. Must not be null.
6974
id<FlutterViewProvider> const view_provider_;
7075

76+
// Converts between engine time and core animation media time.
77+
FlutterTimeConverter* const time_converter_;
78+
7179
// The controller used to manage creation and deletion of platform views.
7280
const FlutterPlatformViewController* platform_view_controller_;
7381

‎shell/platform/darwin/macos/framework/Source/FlutterCompositor.mm

Lines changed: 42 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,24 @@
99

1010
namespace flutter {
1111

12+
namespace {
13+
std::vector<PlatformViewLayerWithIndex> CopyPlatformViewLayers(const FlutterLayer** layers,
14+
size_t layer_count) {
15+
std::vector<PlatformViewLayerWithIndex> platform_views;
16+
for (size_t i = 0; i < layer_count; i++) {
17+
if (layers[i]->type == kFlutterLayerContentTypePlatformView) {
18+
platform_views.push_back(std::make_pair(PlatformViewLayer(layers[i]), i));
19+
}
20+
}
21+
return platform_views;
22+
}
23+
} // namespace
24+
1225
FlutterCompositor::FlutterCompositor(id<FlutterViewProvider> view_provider,
26+
FlutterTimeConverter* time_converter,
1327
FlutterPlatformViewController* platform_view_controller)
1428
: view_provider_(view_provider),
29+
time_converter_(time_converter),
1530
platform_view_controller_(platform_view_controller),
1631
mutator_views_([NSMapTable strongToStrongObjectsMapTable]) {
1732
FML_CHECK(view_provider != nullptr) << "view_provider cannot be nullptr";
@@ -69,28 +84,38 @@
6984
}
7085
}
7186

72-
[view.surfaceManager present:surfaces
73-
notify:^{
74-
PresentPlatformViews(view, layers, layers_count);
75-
}];
87+
CFTimeInterval presentation_time = 0;
88+
89+
if (layers_count > 0 && layers[0]->presentation_time != 0) {
90+
presentation_time = [time_converter_ engineTimeToCAMediaTime:layers[0]->presentation_time];
91+
}
92+
93+
// Notify block below may be called asynchronously, hence the need to copy
94+
// the layer information instead of passing the original pointers from embedder.
95+
auto platform_views_layers = std::make_shared<std::vector<PlatformViewLayerWithIndex>>(
96+
CopyPlatformViewLayers(layers, layers_count));
97+
98+
[view.surfaceManager presentSurfaces:surfaces
99+
atTime:presentation_time
100+
notify:^{
101+
PresentPlatformViews(view, *platform_views_layers);
102+
}];
76103

77104
return true;
78105
}
79106

80-
void FlutterCompositor::PresentPlatformViews(FlutterView* default_base_view,
81-
const FlutterLayer** layers,
82-
size_t layers_count) {
107+
void FlutterCompositor::PresentPlatformViews(
108+
FlutterView* default_base_view,
109+
const std::vector<PlatformViewLayerWithIndex>& platform_views) {
83110
FML_DCHECK([[NSThread currentThread] isMainThread])
84111
<< "Must be on the main thread to present platform views";
85112

86113
// Active mutator views for this frame.
87114
NSMutableArray<FlutterMutatorView*>* present_mutators = [NSMutableArray array];
88115

89-
for (size_t i = 0; i < layers_count; i++) {
90-
FlutterLayer* layer = (FlutterLayer*)layers[i];
91-
if (layer->type == kFlutterLayerContentTypePlatformView) {
92-
[present_mutators addObject:PresentPlatformView(default_base_view, layer, i)];
93-
}
116+
for (const auto& platform_view : platform_views) {
117+
[present_mutators addObject:PresentPlatformView(default_base_view, platform_view.first,
118+
platform_view.second)];
94119
}
95120

96121
NSMutableArray<FlutterMutatorView*>* obsolete_mutators =
@@ -106,12 +131,12 @@
106131
}
107132

108133
FlutterMutatorView* FlutterCompositor::PresentPlatformView(FlutterView* default_base_view,
109-
const FlutterLayer* layer,
110-
size_t layer_position) {
134+
const PlatformViewLayer& layer,
135+
size_t index) {
111136
FML_DCHECK([[NSThread currentThread] isMainThread])
112137
<< "Must be on the main thread to present platform views";
113138

114-
int64_t platform_view_id = layer->platform_view->identifier;
139+
int64_t platform_view_id = layer.identifier();
115140
NSView* platform_view = [platform_view_controller_ platformViewWithID:platform_view_id];
116141

117142
FML_DCHECK(platform_view) << "Platform view not found for id: " << platform_view_id;
@@ -124,8 +149,8 @@
124149
[default_base_view addSubview:container];
125150
}
126151

127-
container.layer.zPosition = layer_position;
128-
[container applyFlutterLayer:layer];
152+
container.layer.zPosition = index;
153+
[container applyFlutterLayer:&layer];
129154

130155
return container;
131156
}

‎shell/platform/darwin/macos/framework/Source/FlutterDisplayLink.h

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
#ifndef FLUTTER_SHELL_PLATFORM_DARWIN_MACOS_FRAMEWORK_SOURCE_FLUTTERDISPLAYLINK_H_
2+
#define FLUTTER_SHELL_PLATFORM_DARWIN_MACOS_FRAMEWORK_SOURCE_FLUTTERDISPLAYLINK_H_
3+
4+
#import <AppKit/AppKit.h>
5+
6+
@protocol FlutterDisplayLinkDelegate <NSObject>
7+
- (void)onDisplayLink:(CFTimeInterval)timestamp targetTimestamp:(CFTimeInterval)targetTimestamp;
8+
@end
9+
10+
/// Provides notifications of display refresh.
11+
///
12+
/// Internally FlutterDisplayLink will use at most one CVDisplayLink per
13+
/// screen shared for all views belonging to that screen. This is necessary
14+
/// because each CVDisplayLink comes with its own thread.
15+
@interface FlutterDisplayLink : NSObject
16+
17+
/// Creates new instance tied to provided NSView. FlutterDisplayLink
18+
/// will track view display changes transparently to synchronize
19+
/// update with display refresh.
20+
/// This function must be called on the main thread.
21+
+ (instancetype)displayLinkWithView:(NSView*)view;
22+
23+
/// Delegate must be set on main thread. Delegate method will be called on
24+
/// on display link thread.
25+
@property(nonatomic, weak) id<FlutterDisplayLinkDelegate> delegate;
26+
27+
/// Pauses and resumes the display link. May be called from any thread.
28+
@property(readwrite) BOOL paused;
29+
30+
/// Returns the nominal refresh period of the display to which the view
31+
/// currently belongs (in seconds). If view does not belong to any display,
32+
/// returns 0. Can be called from any thread.
33+
@property(readonly) CFTimeInterval nominalOutputRefreshPeriod;
34+
35+
/// Invalidates the display link. Must be called on the main thread.
36+
- (void)invalidate;
37+
38+
@end
39+
40+
#endif // FLUTTER_SHELL_PLATFORM_DARWIN_MACOS_FRAMEWORK_SOURCE_FLUTTERDISPLAYLINK_H_

0 commit comments

Comments
 (0)