Skip to content

Commit

Permalink
Reverts "Reland: [macOS] Use CVDisplayLink to drive repaint (#51126)" (…
Browse files Browse the repository at this point in the history 
…#51192)

Reverts #51126
Initiated by: cbracken
Reason for reverting: looks like this regressed Skia benchmarks: https://flutter-flutter-perf.skia.org/e/?keys=Xbf8dce9c233bce34d20e2280e3f1d8db&selected=commit%3D39653%26name%3D%2Carch%3Dintel%2Cbranch%3Dmaster%2Cconfig%3Ddefault%2Cdevice_type%3Dnone%2Cdevice_version%3Dnone%2Chost_type%3Dmac%2Csub_result%3DtimeToFirstFrameRasterizedMicros%2Ctest%3Dflutter_gallery_macos__start_up%2C&xbaroffset=39569

The regression s
Original PR Author: knopp

Reviewed By: {cbracken}

This change reverts the following previous change:
Original Description:
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.

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
  • Loading branch information
@auto-submit
auto-submit[bot] authored Mar 5, 2024
1 parent 6250b9f commit 78cecee
Show file tree
Hide file tree
Showing 29 changed files with 102 additions and 1,349 deletions.
12 changes: 0 additions & 12 deletions ci/licenses_golden/licenses_flutter
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36597,7 +36597,6 @@ 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
Expand Down Expand Up @@ -36647,10 +36646,7 @@ 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
ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterView.mm + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm + ../../../flutter/LICENSE
Expand Down Expand Up @@ -39471,9 +39467,6 @@ 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
Expand Down Expand Up @@ -39523,12 +39516,7 @@ 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
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
Expand Down
8 changes: 0 additions & 8 deletions shell/platform/darwin/macos/BUILD.gn
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,6 @@ 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",
Expand Down Expand Up @@ -103,10 +101,6 @@ 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",
"framework/Source/FlutterView.mm",
"framework/Source/FlutterViewController.mm",
Expand Down Expand Up @@ -179,7 +173,6 @@ 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",
Expand All @@ -194,7 +187,6 @@ 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",
Expand Down
16 changes: 4 additions & 12 deletions shell/platform/darwin/macos/framework/Source/FlutterCompositor.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,13 @@

#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"

@class FlutterMutatorView;

namespace flutter {

class PlatformViewLayer;

typedef std::pair<PlatformViewLayer, size_t> 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.
Expand All @@ -35,7 +30,6 @@ class FlutterCompositor {
// which are used for presenting and creating backing stores.
// It must not be null, and is typically FlutterViewEngineProvider.
explicit FlutterCompositor(id<FlutterViewProvider> view_provider,
FlutterTimeConverter* time_converter,
FlutterPlatformViewController* platform_views_controller);

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

private:
void PresentPlatformViews(FlutterView* default_base_view,
const std::vector<PlatformViewLayerWithIndex>& platform_views_layers);
const FlutterLayer** layers,
size_t layers_count);

// 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 PlatformViewLayer& layer,
size_t index);
const FlutterLayer* layer,
size_t layer_position);

// Where the compositor can query FlutterViews. Must not be null.
id<FlutterViewProvider> 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_;

Expand Down
59 changes: 17 additions & 42 deletions shell/platform/darwin/macos/framework/Source/FlutterCompositor.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,24 +9,9 @@

namespace flutter {

namespace {
std::vector<PlatformViewLayerWithIndex> CopyPlatformViewLayers(const FlutterLayer** layers,
size_t layer_count) {
std::vector<PlatformViewLayerWithIndex> 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<FlutterViewProvider> 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";
Expand Down Expand Up @@ -84,38 +69,28 @@
}
}

CFTimeInterval presentation_time = 0;

if (layers_count > 0 && layers[0]->presentation_time != 0) {
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<std::vector<PlatformViewLayerWithIndex>>(
CopyPlatformViewLayers(layers, layers_count));

[view.surfaceManager presentSurfaces:surfaces
atTime:presentation_time
notify:^{
PresentPlatformViews(view, *platform_views_layers);
}];
[view.surfaceManager present:surfaces
notify:^{
PresentPlatformViews(view, layers, layers_count);
}];

return true;
}

void FlutterCompositor::PresentPlatformViews(
FlutterView* default_base_view,
const std::vector<PlatformViewLayerWithIndex>& platform_views) {
void FlutterCompositor::PresentPlatformViews(FlutterView* default_base_view,
const FlutterLayer** layers,
size_t layers_count) {
FML_DCHECK([[NSThread currentThread] isMainThread])
<< "Must be on the main thread to present platform views";

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

for (const auto& platform_view : platform_views) {
[present_mutators addObject:PresentPlatformView(default_base_view, platform_view.first,
platform_view.second)];
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)];
}
}

NSMutableArray<FlutterMutatorView*>* obsolete_mutators =
Expand All @@ -131,12 +106,12 @@
}

FlutterMutatorView* FlutterCompositor::PresentPlatformView(FlutterView* default_base_view,
const PlatformViewLayer& layer,
size_t index) {
const FlutterLayer* layer,
size_t layer_position) {
FML_DCHECK([[NSThread currentThread] isMainThread])
<< "Must be on the main thread to present platform views";

int64_t platform_view_id = layer.identifier();
int64_t platform_view_id = layer->platform_view->identifier;
NSView* platform_view = [platform_view_controller_ platformViewWithID:platform_view_id];

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

container.layer.zPosition = index;
[container applyFlutterLayer:&layer];
container.layer.zPosition = layer_position;
[container applyFlutterLayer:layer];

return container;
}
Expand Down
40 changes: 0 additions & 40 deletions shell/platform/darwin/macos/framework/Source/FlutterDisplayLink.h
View file

This file was deleted.

Loading

0 comments on commit 78cecee

Please sign in to comment.