From 376d3cddebd64b1a95cddec981fa78d206adbaa5 Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Fri, 25 Nov 2022 17:45:33 +0100 Subject: [PATCH 01/34] [macos] Refactor rendering process --- ci/licenses_golden/licenses_flutter | 12 +- shell/platform/darwin/macos/BUILD.gn | 12 +- .../framework/Source/FlutterCompositor.h | 68 +---- .../framework/Source/FlutterCompositor.mm | 163 +++-------- .../framework/Source/FlutterCompositorTest.mm | 84 ++++-- .../macos/framework/Source/FlutterEngine.mm | 23 +- .../framework/Source/FlutterEngineTest.mm | 43 ++- .../framework/Source/FlutterIOSurfaceHolder.h | 24 -- .../Source/FlutterIOSurfaceHolder.mm | 47 --- .../Source/FlutterPlatformViewController.mm | 2 + .../macos/framework/Source/FlutterRenderer.h | 7 +- .../macos/framework/Source/FlutterRenderer.mm | 40 +-- .../framework/Source/FlutterRendererTest.mm | 40 ++- .../FlutterResizableBackingStoreProvider.h | 33 --- .../FlutterResizableBackingStoreProvider.mm | 54 ---- .../Source/FlutterResizeSynchronizer.h | 90 ------ .../Source/FlutterResizeSynchronizer.mm | 153 ---------- .../macos/framework/Source/FlutterSurface.h | 17 ++ .../macos/framework/Source/FlutterSurface.mm | 94 ++++++ .../framework/Source/FlutterSurfaceManager.h | 65 ++-- .../framework/Source/FlutterSurfaceManager.mm | 277 +++++++++++------- .../Source/FlutterSurfaceManagerTest.mm | 200 +++++++++++-- .../Source/FlutterSurfaceManager_Internal.h | 41 +++ .../Source/FlutterSurface_Internal.h | 18 ++ .../Source/FlutterThreadSynchronizer.h | 36 +++ .../Source/FlutterThreadSynchronizer.mm | 132 +++++++++ .../macos/framework/Source/FlutterView.h | 33 ++- .../macos/framework/Source/FlutterView.mm | 38 ++- 28 files changed, 958 insertions(+), 888 deletions(-) delete mode 100644 shell/platform/darwin/macos/framework/Source/FlutterIOSurfaceHolder.h delete mode 100644 shell/platform/darwin/macos/framework/Source/FlutterIOSurfaceHolder.mm delete mode 100644 shell/platform/darwin/macos/framework/Source/FlutterResizableBackingStoreProvider.h delete mode 100644 shell/platform/darwin/macos/framework/Source/FlutterResizableBackingStoreProvider.mm delete mode 100644 shell/platform/darwin/macos/framework/Source/FlutterResizeSynchronizer.h delete mode 100644 shell/platform/darwin/macos/framework/Source/FlutterResizeSynchronizer.mm create mode 100644 shell/platform/darwin/macos/framework/Source/FlutterSurface.h create mode 100644 shell/platform/darwin/macos/framework/Source/FlutterSurface.mm create mode 100644 shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager_Internal.h create mode 100644 shell/platform/darwin/macos/framework/Source/FlutterSurface_Internal.h create mode 100644 shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.h create mode 100644 shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.mm diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index 8e517e61ac41c..b288db7e2c57a 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -2717,8 +2717,6 @@ FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterEngin FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterEngine_Internal.h FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterExternalTexture.h FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterExternalTexture.mm -FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterIOSurfaceHolder.h -FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterIOSurfaceHolder.mm FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterKeyPrimaryResponder.h FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterKeyboardManager.h FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterKeyboardManager.mm @@ -2739,13 +2737,13 @@ FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterPlatf FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterRenderer.h FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterRenderer.mm FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterRendererTest.mm -FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterResizableBackingStoreProvider.h -FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterResizableBackingStoreProvider.mm -FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterResizeSynchronizer.h -FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterResizeSynchronizer.mm +FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterSurface.h +FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterSurface.mm FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.mm FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManagerTest.mm +FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager_Internal.h +FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterSurface_Internal.h FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterTextInputPlugin.h FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterTextInputPlugin.mm FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterTextInputPluginTest.mm @@ -2754,6 +2752,8 @@ FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterTextI FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterTextInputSemanticsObjectTest.mm FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterTextureRegistrar.h FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterTextureRegistrar.mm +FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.h +FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.mm FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/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 935a3c3591978..3607af9d4e73f 100644 --- a/shell/platform/darwin/macos/BUILD.gn +++ b/shell/platform/darwin/macos/BUILD.gn @@ -71,8 +71,6 @@ source_set("flutter_framework_source") { "framework/Source/FlutterEngine_Internal.h", "framework/Source/FlutterExternalTexture.h", "framework/Source/FlutterExternalTexture.mm", - "framework/Source/FlutterIOSurfaceHolder.h", - "framework/Source/FlutterIOSurfaceHolder.mm", "framework/Source/FlutterKeyPrimaryResponder.h", "framework/Source/FlutterKeyboardManager.h", "framework/Source/FlutterKeyboardManager.mm", @@ -88,18 +86,20 @@ source_set("flutter_framework_source") { "framework/Source/FlutterPlatformViewController.mm", "framework/Source/FlutterRenderer.h", "framework/Source/FlutterRenderer.mm", - "framework/Source/FlutterResizableBackingStoreProvider.h", - "framework/Source/FlutterResizableBackingStoreProvider.mm", - "framework/Source/FlutterResizeSynchronizer.h", - "framework/Source/FlutterResizeSynchronizer.mm", + "framework/Source/FlutterSurface.h", + "framework/Source/FlutterSurface.mm", "framework/Source/FlutterSurfaceManager.h", "framework/Source/FlutterSurfaceManager.mm", + "framework/Source/FlutterSurfaceManager_Internal.h", + "framework/Source/FlutterSurface_Internal.h", "framework/Source/FlutterTextInputPlugin.h", "framework/Source/FlutterTextInputPlugin.mm", "framework/Source/FlutterTextInputSemanticsObject.h", "framework/Source/FlutterTextInputSemanticsObject.mm", "framework/Source/FlutterTextureRegistrar.h", "framework/Source/FlutterTextureRegistrar.mm", + "framework/Source/FlutterThreadSynchronizer.h", + "framework/Source/FlutterThreadSynchronizer.mm", "framework/Source/FlutterView.h", "framework/Source/FlutterView.mm", "framework/Source/FlutterViewController.mm", diff --git a/shell/platform/darwin/macos/framework/Source/FlutterCompositor.h b/shell/platform/darwin/macos/framework/Source/FlutterCompositor.h index 66cf7b8939d37..d0f4d6445816d 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterCompositor.h +++ b/shell/platform/darwin/macos/framework/Source/FlutterCompositor.h @@ -9,11 +9,12 @@ #include #include "flutter/fml/macros.h" -#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterPlatformViewController.h" -#include "flutter/shell/platform/darwin/macos/framework/Source/FlutterView.h" -#include "flutter/shell/platform/darwin/macos/framework/Source/FlutterViewProvider.h" #include "flutter/shell/platform/embedder/embedder.h" +#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterPlatformViewController.h" +#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterView.h" +#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterViewProvider.h" + namespace flutter { // FlutterCompositor creates and manages the backing stores used for @@ -28,8 +29,7 @@ class FlutterCompositor { // It must not be null, and is typically FlutterViewEngineProvider. explicit FlutterCompositor( id view_provider, - FlutterPlatformViewController* platform_views_controller, - id mtl_device); + FlutterPlatformViewController* platform_views_controller); ~FlutterCompositor() = default; @@ -48,57 +48,12 @@ class FlutterCompositor { bool CreateBackingStore(const FlutterBackingStoreConfig* config, FlutterBackingStore* backing_store_out); - // Releases the memory for any resources that were allocated for the - // specified backing store. - bool CollectBackingStore(const FlutterBackingStore* backing_store); - // Presents the FlutterLayers by updating the FlutterView specified by // `view_id` using the layer content. Sets frame_started_ to false. bool Present(uint64_t view_id, const FlutterLayer** layers, size_t layers_count); - // Callback triggered at the end of the Present function. has_flutter_content - // is true when Flutter content was rendered, otherwise false. - using PresentCallback = std::function; - - // Registers a callback to be triggered at the end of the Present function. - // If a callback was previously registered, it will be replaced. - void SetPresentCallback(const PresentCallback& present_callback); - - // The status of the frame being composited. - // Started: A new frame has begun and we have cleared the old layer tree and - // are now creating backingstore(s) for the embedder to use. - // Presenting: the embedder has finished rendering into the provided - // backingstore(s) and we are creating the layer tree for the system - // compositor to present with. - // Ended: The frame has been presented and we are no longer processing it. - typedef enum { kStarted, kPresenting, kEnded } FrameStatus; - - protected: - // Returns the view associated with the view ID. - // - // Returns nil if the ID is invalid. - FlutterView* GetView(uint64_t view_id); - - // Gets and sets the FrameStatus for the current frame. - void SetFrameStatus(FrameStatus frame_status); - FrameStatus GetFrameStatus(); - - // Clears the previous CALayers and updates the frame status to frame started. - void StartFrame(); - - // Calls the present callback and ensures the frame status is updated - // to frame ended, returning whether the present was successful or not. - bool EndFrame(bool has_flutter_content); - - // Creates a CALayer object which is backed by the supplied IOSurface, and - // adds it to the root CALayer for the given view. - void InsertCALayerForIOSurface( - FlutterView* view, - const IOSurfaceRef& io_surface, - CATransform3D transform = CATransform3DIdentity); - private: // 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 @@ -107,25 +62,12 @@ class FlutterCompositor { const FlutterLayer* layer, size_t layer_position); - // A list of the active CALayer objects for the frame that need to be removed. - std::list active_ca_layers_; - // Where the compositor can query FlutterViews. Must not be null. id const view_provider_; // The controller used to manage creation and deletion of platform views. const FlutterPlatformViewController* platform_view_controller_; - // The Metal device used to draw graphics. - const id mtl_device_; - - // Callback set by the embedder to be called when the layer tree has been - // correctly set up for this frame. - PresentCallback present_callback_; - - // Current frame status. - FrameStatus frame_status_ = kEnded; - FML_DISALLOW_COPY_AND_ASSIGN(FlutterCompositor); }; diff --git a/shell/platform/darwin/macos/framework/Source/FlutterCompositor.mm b/shell/platform/darwin/macos/framework/Source/FlutterCompositor.mm index ccdfaec180c2a..3ed6307f631ae 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterCompositor.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterCompositor.mm @@ -5,16 +5,12 @@ #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterCompositor.h" #include "flutter/fml/logging.h" -#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterIOSurfaceHolder.h" namespace flutter { FlutterCompositor::FlutterCompositor(id view_provider, - FlutterPlatformViewController* platform_view_controller, - id mtl_device) - : view_provider_(view_provider), - platform_view_controller_(platform_view_controller), - mtl_device_(mtl_device) { + FlutterPlatformViewController* platform_view_controller) + : view_provider_(view_provider), platform_view_controller_(platform_view_controller) { FML_CHECK(view_provider != nullptr) << "view_provider cannot be nullptr"; } @@ -23,103 +19,62 @@ // TODO(dkwingsmt): This class only supports single-view for now. As more // classes are gradually converted to multi-view, it should get the view ID // from somewhere. - FlutterView* view = GetView(kFlutterDefaultViewId); + FlutterView* view = [view_provider_ getView:kFlutterDefaultViewId]; if (!view) { return false; } CGSize size = CGSizeMake(config->size.width, config->size.height); - - backing_store_out->metal.struct_size = sizeof(FlutterMetalBackingStore); - backing_store_out->metal.texture.struct_size = sizeof(FlutterMetalTexture); - - if (GetFrameStatus() != FrameStatus::kStarted) { - StartFrame(); - // If the backing store is for the first layer, return the MTLTexture for the - // FlutterView. - FlutterRenderBackingStore* backingStore = [view backingStoreForSize:size]; - backing_store_out->metal.texture.texture = - (__bridge FlutterMetalTextureHandle)backingStore.texture; - } else { - FlutterIOSurfaceHolder* io_surface_holder = [[FlutterIOSurfaceHolder alloc] init]; - [io_surface_holder recreateIOSurfaceWithSize:size]; - auto texture_descriptor = - [MTLTextureDescriptor texture2DDescriptorWithPixelFormat:MTLPixelFormatBGRA8Unorm - width:size.width - height:size.height - mipmapped:NO]; - texture_descriptor.usage = - MTLTextureUsageShaderRead | MTLTextureUsageRenderTarget | MTLTextureUsageShaderWrite; - - backing_store_out->metal.texture.texture = (__bridge_retained FlutterMetalTextureHandle) - [mtl_device_ newTextureWithDescriptor:texture_descriptor - iosurface:[io_surface_holder ioSurface] - plane:0]; - - backing_store_out->metal.texture.user_data = (__bridge_retained void*)io_surface_holder; - } - + FlutterSurface* surface = [view.surfaceManager surfaceForSize:size]; + memset(backing_store_out, 0, sizeof(FlutterBackingStore)); + backing_store_out->struct_size = sizeof(FlutterBackingStore); backing_store_out->type = kFlutterBackingStoreTypeMetal; - backing_store_out->metal.texture.destruction_callback = [](void* user_data) { - if (user_data != nullptr) { - CFRelease(user_data); - } - }; - - return true; -} - -bool FlutterCompositor::CollectBackingStore(const FlutterBackingStore* backing_store) { - // If we allocated this MTLTexture ourselves, user_data is not null, and we will need - // to release it manually. - if (backing_store->metal.texture.user_data != nullptr && - backing_store->metal.texture.texture != nullptr) { - CFRelease(backing_store->metal.texture.texture); - } + backing_store_out->metal.struct_size = sizeof(FlutterMetalBackingStore); + backing_store_out->metal.texture = surface.asFlutterMetalTexture; return true; } bool FlutterCompositor::Present(uint64_t view_id, const FlutterLayer** layers, size_t layers_count) { - FlutterView* view = GetView(view_id); + FlutterView* view = [view_provider_ getView:view_id]; if (!view) { return false; } - SetFrameStatus(FrameStatus::kPresenting); - - bool has_flutter_content = false; - for (size_t i = 0; i < layers_count; ++i) { - const auto* layer = layers[i]; - FlutterBackingStore* backing_store = const_cast(layer->backing_store); - - switch (layer->type) { - case kFlutterLayerContentTypeBackingStore: { - if (backing_store->metal.texture.user_data) { - FlutterIOSurfaceHolder* io_surface_holder = - (__bridge FlutterIOSurfaceHolder*)backing_store->metal.texture.user_data; - IOSurfaceRef io_surface = [io_surface_holder ioSurface]; - InsertCALayerForIOSurface(view, io_surface); - } - has_flutter_content = true; - break; - } - case kFlutterLayerContentTypePlatformView: { - PresentPlatformView(view, layer, i); - break; + NSMutableArray* surfaces = [NSMutableArray array]; + for (size_t i = 0; i < layers_count; i++) { + FlutterLayer* layer = (FlutterLayer*)layers[i]; + if (layer->type == kFlutterLayerContentTypeBackingStore) { + FlutterSurface* surface = + [view.surfaceManager lookupSurface:&layer->backing_store->metal.texture]; + if (surface) { + FlutterSurfacePresentInfo* info = [[FlutterSurfacePresentInfo alloc] init]; + info.surface = surface; + info.offset = CGPointMake(layer->offset.x, layer->offset.y); + info.zIndex = i; + [surfaces addObject:info]; } - }; + } } - return EndFrame(has_flutter_content); + [view.surfaceManager present:surfaces + notify:^{ + for (size_t i = 0; i < layers_count; i++) { + FlutterLayer* layer = (FlutterLayer*)layers[i]; + if (layer->type == kFlutterLayerContentTypePlatformView) { + PresentPlatformView(view, layer, i); + } + } + [platform_view_controller_ disposePlatformViews]; + }]; + + return true; } void FlutterCompositor::PresentPlatformView(FlutterView* default_base_view, const FlutterLayer* layer, size_t layer_position) { - // TODO (https://github.com/flutter/flutter/issues/96668) - // once the issue is fixed, this check will pass. FML_DCHECK([[NSThread currentThread] isMainThread]) << "Must be on the main thread to present platform views"; @@ -128,60 +83,14 @@ FML_DCHECK(platform_view) << "Platform view not found for id: " << platform_view_id; - CGFloat scale = [[NSScreen mainScreen] backingScaleFactor]; + CGFloat scale = platform_view.layer.contentsScale; platform_view.frame = CGRectMake(layer->offset.x / scale, layer->offset.y / scale, layer->size.width / scale, layer->size.height / scale); if (platform_view.superview == nil) { [default_base_view addSubview:platform_view]; + [default_base_view.layer addSublayer:platform_view.layer]; } platform_view.layer.zPosition = layer_position; } -void FlutterCompositor::SetPresentCallback( - const FlutterCompositor::PresentCallback& present_callback) { - present_callback_ = present_callback; -} - -void FlutterCompositor::StartFrame() { - // First remove all CALayers from the superlayer. - for (auto layer : active_ca_layers_) { - [layer removeFromSuperlayer]; - } - - // Reset active layers. - active_ca_layers_.clear(); - SetFrameStatus(FrameStatus::kStarted); -} - -bool FlutterCompositor::EndFrame(bool has_flutter_content) { - bool status = present_callback_(has_flutter_content); - SetFrameStatus(FrameStatus::kEnded); - return status; -} - -FlutterView* FlutterCompositor::GetView(uint64_t view_id) { - return [view_provider_ getView:view_id]; -} - -void FlutterCompositor::SetFrameStatus(FlutterCompositor::FrameStatus frame_status) { - frame_status_ = frame_status; -} - -FlutterCompositor::FrameStatus FlutterCompositor::GetFrameStatus() { - return frame_status_; -} - -void FlutterCompositor::InsertCALayerForIOSurface(FlutterView* view, - const IOSurfaceRef& io_surface, - CATransform3D transform) { - // FlutterCompositor manages the lifecycle of CALayers. - CALayer* content_layer = [[CALayer alloc] init]; - content_layer.transform = transform; - content_layer.frame = view.layer.bounds; - [content_layer setContents:(__bridge id)io_surface]; - [view.layer addSublayer:content_layer]; - - active_ca_layers_.push_back(content_layer); -} - } // namespace flutter diff --git a/shell/platform/darwin/macos/framework/Source/FlutterCompositorTest.mm b/shell/platform/darwin/macos/framework/Source/FlutterCompositorTest.mm index 576af2bbac152..509902334b587 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterCompositorTest.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterCompositorTest.mm @@ -3,6 +3,7 @@ // found in the LICENSE file. #import +#import #import #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterCompositor.h" @@ -41,13 +42,17 @@ - (nullable FlutterView*)getView:(uint64_t)viewId { namespace flutter::testing { namespace { -id MockViewProvider() { +typedef void (^PresentBlock)(NSArray*); + +id MockViewProvider(PresentBlock onPresent = nil) { FlutterView* viewMock = OCMClassMock([FlutterView class]); - FlutterRenderBackingStore* backingStoreMock = OCMClassMock([FlutterRenderBackingStore class]); + FlutterSurfaceManager* surfaceManagerMock = OCMClassMock([FlutterSurfaceManager class]); + FlutterSurface* surfaceMock = OCMClassMock([FlutterSurface class]); __block id textureMock = OCMProtocolMock(@protocol(MTLTexture)); - OCMStub([backingStoreMock texture]).andReturn(textureMock); - OCMStub([viewMock backingStoreForSize:CGSize{}]) + OCMStub([viewMock surfaceManager]).andReturn(surfaceManagerMock); + + OCMStub([surfaceManagerMock surfaceForSize:CGSize{}]) .ignoringNonObjectArgs() .andDo(^(NSInvocation* invocation) { CGSize size; @@ -55,31 +60,39 @@ - (nullable FlutterView*)getView:(uint64_t)viewId { OCMStub([textureMock width]).andReturn(size.width); OCMStub([textureMock height]).andReturn(size.height); }) - .andReturn(backingStoreMock); + .andReturn(surfaceMock); - return [[FlutterViewMockProvider alloc] initWithDefaultView:viewMock]; -} -} // namespace + FlutterMetalTexture texture = { + .struct_size = sizeof(FlutterMetalTexture), + .texture_id = 1, + .texture = (__bridge void*)textureMock, + .user_data = nullptr, + .destruction_callback = nullptr, + }; -TEST(FlutterCompositorTest, TestPresent) { - std::unique_ptr macos_compositor = - std::make_unique(MockViewProvider(), /*platform_view_controller*/ nullptr, - /*mtl_device*/ nullptr); + OCMStub([surfaceManagerMock lookupSurface:&texture]) + .ignoringNonObjectArgs() + .andReturn(surfaceMock); + + OCMStub([surfaceManagerMock present:[OCMArg any] notify:[OCMArg any]]) + .andDo(^(NSInvocation* invocation) { + NSArray* info; + [invocation getArgument:&info atIndex:2]; + if (onPresent != nil) { + onPresent(info); + } + }); - bool flag = false; - macos_compositor->SetPresentCallback([f = &flag](bool has_flutter_content) { - *f = true; - return true; - }); + OCMStub([surfaceMock asFlutterMetalTexture]).andReturn(texture); - ASSERT_TRUE(macos_compositor->Present(0, nil, 0)); - ASSERT_TRUE(flag); + return [[FlutterViewMockProvider alloc] initWithDefaultView:viewMock]; } +} // namespace TEST(FlutterCompositorTest, TestCreate) { std::unique_ptr macos_compositor = - std::make_unique(MockViewProvider(), /*platform_view_controller*/ nullptr, - /*mtl_device*/ nullptr); + std::make_unique(MockViewProvider(), + /*platform_view_controller*/ nullptr); FlutterBackingStore backing_store; FlutterBackingStoreConfig config; @@ -95,10 +108,16 @@ - (nullable FlutterView*)getView:(uint64_t)viewId { ASSERT_EQ(texture.height, 600ul); } -TEST(FlutterCompositorTest, TestCompositing) { +TEST(FlutterCompositorTest, TestPresent) { + __block NSArray* presentedSurfaces = nil; + + auto onPresent = ^(NSArray* info) { + presentedSurfaces = info; + }; + std::unique_ptr macos_compositor = - std::make_unique(MockViewProvider(), /*platform_view_controller*/ nullptr, - /*mtl_device*/ nullptr); + std::make_unique(MockViewProvider(onPresent), + /*platform_view_controller*/ nullptr); FlutterBackingStore backing_store; FlutterBackingStoreConfig config; @@ -107,11 +126,18 @@ - (nullable FlutterView*)getView:(uint64_t)viewId { config.size.height = 600; macos_compositor->CreateBackingStore(&config, &backing_store); - ASSERT_EQ(backing_store.type, kFlutterBackingStoreTypeMetal); - ASSERT_NE(backing_store.metal.texture.texture, nil); - id texture = (__bridge id)backing_store.metal.texture.texture; - ASSERT_EQ(texture.width, 800u); - ASSERT_EQ(texture.height, 600u); + FlutterLayer layers[] = {{ + .struct_size = sizeof(FlutterLayer), + .type = kFlutterLayerContentTypeBackingStore, + .backing_store = &backing_store, + .offset = {0, 0}, + .size = {800, 600}, + }}; + const FlutterLayer* layers_ptr = layers; + + macos_compositor->Present(kFlutterDefaultViewId, &layers_ptr, 1); + + ASSERT_EQ(presentedSurfaces.count, 1ul); } } // namespace flutter::testing diff --git a/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm b/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm index 19ed29aa1171c..870ca4802f3d9 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm @@ -432,22 +432,8 @@ - (FlutterCompositor*)createFlutterCompositor { return nil; } - __weak FlutterEngine* weakSelf = self; - - _macOSCompositor = std::make_unique( - _viewProvider, _platformViewController, _renderer.device); - _macOSCompositor->SetPresentCallback([weakSelf](bool has_flutter_content) { - // TODO(dkwingsmt): The compositor only supports single-view for now. As - // more classes are gradually converted to multi-view, it should get the - // view ID from somewhere. - uint64_t viewId = kFlutterDefaultViewId; - if (has_flutter_content) { - return [weakSelf.renderer present:viewId] == YES; - } else { - [weakSelf.renderer presentWithoutContent:viewId]; - return true; - } - }); + _macOSCompositor = + std::make_unique(_viewProvider, _platformViewController); _compositor = {}; _compositor.struct_size = sizeof(FlutterCompositor); @@ -463,10 +449,7 @@ - (FlutterCompositor*)createFlutterCompositor { _compositor.collect_backing_store_callback = [](const FlutterBackingStore* backing_store, // void* user_data // - ) { - return reinterpret_cast(user_data)->CollectBackingStore( - backing_store); - }; + ) { return true; }; _compositor.present_layers_callback = [](const FlutterLayer** layers, // size_t layers_count, // diff --git a/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm b/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm index 143b05b7ced8e..a1149386ff19d 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm @@ -30,6 +30,20 @@ @interface FlutterEngine (Test) @property(nonatomic, readonly, nullable) flutter::FlutterCompositor* macOSCompositor; @end +@interface TestPlatformViewFactory : NSObject +@end + +@implementation TestPlatformViewFactory +- (nonnull NSView*)createWithViewIdentifier:(int64_t)viewId arguments:(nullable id)args { + if (viewId == 42) { + return [[NSView alloc] init]; + } else { + return nil; + } +} + +@end + namespace flutter::testing { TEST_F(FlutterEngineTest, CanLaunch) { @@ -447,8 +461,7 @@ @interface FlutterEngine (Test) ASSERT_TRUE(latch_called); } -// TODO(iskakaushik): Enable after https://github.com/flutter/flutter/issues/96668 is fixed. -TEST(FlutterEngine, DISABLED_Compositor) { +TEST(FlutterEngine, Compositor) { NSString* fixtures = @(flutter::testing::GetFixturesPath()); FlutterDartProject* project = [[FlutterDartProject alloc] initWithAssetsPath:fixtures @@ -462,26 +475,28 @@ @interface FlutterEngine (Test) EXPECT_TRUE([engine runWithEntrypoint:@"canCompositePlatformViews"]); - // Latch to ensure the entire layer tree has been generated and presented. - fml::AutoResetWaitableEvent latch; - auto compositor = engine.macOSCompositor; - compositor->SetPresentCallback([&](bool has_flutter_content) { - latch.Signal(); - return true; - }); - latch.Wait(); + [engine.platformViewController registerViewFactory:[[TestPlatformViewFactory alloc] init] + withId:@"factory_id"]; + [engine.platformViewController + handleMethodCall:[FlutterMethodCall methodCallWithMethodName:@"create" + arguments:@{ + @"id" : @(42), + @"viewType" : @"factory_id", + }] + result:^(id result){ + }]; + + [viewController.flutterView.threadSynchronizer blockUntilFrameAvailable]; CALayer* rootLayer = viewController.flutterView.layer; // There are three layers total - the root layer and two sublayers. - // This test will need to be updated when PlatformViews are supported, as - // there are two PlatformView layers in this test. - EXPECT_EQ(rootLayer.sublayers.count, 2u); + EXPECT_EQ(rootLayer.sublayers.count, 3u); // TODO(gw280): add support for screenshot tests in this test harness [engine shutDownEngine]; -} +} // namespace flutter::testing TEST(FlutterEngine, DartEntrypointArguments) { NSString* fixtures = @(flutter::testing::GetFixturesPath()); diff --git a/shell/platform/darwin/macos/framework/Source/FlutterIOSurfaceHolder.h b/shell/platform/darwin/macos/framework/Source/FlutterIOSurfaceHolder.h deleted file mode 100644 index e3a1f8a7113dd..0000000000000 --- a/shell/platform/darwin/macos/framework/Source/FlutterIOSurfaceHolder.h +++ /dev/null @@ -1,24 +0,0 @@ -// Copyright 2013 The Flutter Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#import - -/** - * FlutterIOSurfaceHolder maintains an IOSurface - * and provides an interface to bind the IOSurface to a texture. - */ -@interface FlutterIOSurfaceHolder : NSObject - -/** - * Releases the current IOSurface if one exists - * and creates a new IOSurface with the specified size. - */ -- (void)recreateIOSurfaceWithSize:(CGSize)size; - -/** - * Returns a reference to the underlying IOSurface. - */ -- (const IOSurfaceRef&)ioSurface; - -@end diff --git a/shell/platform/darwin/macos/framework/Source/FlutterIOSurfaceHolder.mm b/shell/platform/darwin/macos/framework/Source/FlutterIOSurfaceHolder.mm deleted file mode 100644 index d8553507442f9..0000000000000 --- a/shell/platform/darwin/macos/framework/Source/FlutterIOSurfaceHolder.mm +++ /dev/null @@ -1,47 +0,0 @@ -// Copyright 2013 The Flutter Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterIOSurfaceHolder.h" - -@interface FlutterIOSurfaceHolder () { - IOSurfaceRef _ioSurface; -} -@end - -@implementation FlutterIOSurfaceHolder - -- (void)recreateIOSurfaceWithSize:(CGSize)size { - if (_ioSurface) { - CFRelease(_ioSurface); - } - - unsigned pixelFormat = 'BGRA'; - unsigned bytesPerElement = 4; - - size_t bytesPerRow = IOSurfaceAlignProperty(kIOSurfaceBytesPerRow, size.width * bytesPerElement); - size_t totalBytes = IOSurfaceAlignProperty(kIOSurfaceAllocSize, size.height * bytesPerRow); - NSDictionary* options = @{ - (id)kIOSurfaceWidth : @(size.width), - (id)kIOSurfaceHeight : @(size.height), - (id)kIOSurfacePixelFormat : @(pixelFormat), - (id)kIOSurfaceBytesPerElement : @(bytesPerElement), - (id)kIOSurfaceBytesPerRow : @(bytesPerRow), - (id)kIOSurfaceAllocSize : @(totalBytes), - }; - - _ioSurface = IOSurfaceCreate((CFDictionaryRef)options); - IOSurfaceSetValue(_ioSurface, CFSTR("IOSurfaceColorSpace"), kCGColorSpaceSRGB); -} - -- (const IOSurfaceRef&)ioSurface { - return _ioSurface; -} - -- (void)dealloc { - if (_ioSurface) { - CFRelease(_ioSurface); - } -} - -@end diff --git a/shell/platform/darwin/macos/framework/Source/FlutterPlatformViewController.mm b/shell/platform/darwin/macos/framework/Source/FlutterPlatformViewController.mm index b15b1ca5ebd42..60defbff7b740 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterPlatformViewController.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterPlatformViewController.mm @@ -53,6 +53,8 @@ - (void)onCreateWithViewID:(int64_t)viewId } NSView* platform_view = [factory createWithViewIdentifier:viewId arguments:nil]; + // Force view to be layer-backed. + [platform_view setWantsLayer:YES]; _platformViews[viewId] = platform_view; result(nil); } diff --git a/shell/platform/darwin/macos/framework/Source/FlutterRenderer.h b/shell/platform/darwin/macos/framework/Source/FlutterRenderer.h index d9cbe1da5abb0..483a858032bc8 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterRenderer.h +++ b/shell/platform/darwin/macos/framework/Source/FlutterRenderer.h @@ -38,12 +38,7 @@ /** * Called by the engine when the given view's buffers should be swapped. */ -- (BOOL)present:(uint64_t)viewId; - -/** - * Tells the renderer that there is no Flutter content available for this frame. - */ -- (void)presentWithoutContent:(uint64_t)viewId; +- (BOOL)present:(uint64_t)viewId texture:(nonnull const FlutterMetalTexture*)texture; /** * Creates a Metal texture for the given view with the given size. diff --git a/shell/platform/darwin/macos/framework/Source/FlutterRenderer.mm b/shell/platform/darwin/macos/framework/Source/FlutterRenderer.mm index 996e613dba926..a82c43a6a7c23 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterRenderer.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterRenderer.mm @@ -28,7 +28,7 @@ static bool OnPresentDrawableOfDefaultView(FlutterEngine* engine, // operates on the default view. To support multi-view, we need a new callback // that also receives a view ID. uint64_t viewId = kFlutterDefaultViewId; - return [engine.renderer present:viewId]; + return [engine.renderer present:viewId texture:texture]; } static bool OnAcquireExternalTexture(FlutterEngine* engine, @@ -95,30 +95,34 @@ - (FlutterMetalTexture)createTextureForView:(uint64_t)viewId size:(CGSize)size { // FlutterMetalTexture has texture `null`, therefore is discarded. return FlutterMetalTexture{}; } - FlutterRenderBackingStore* backingStore = [view backingStoreForSize:size]; - id texture = backingStore.texture; - FlutterMetalTexture embedderTexture; - embedderTexture.struct_size = sizeof(FlutterMetalTexture); - embedderTexture.texture = (__bridge void*)texture; - embedderTexture.texture_id = reinterpret_cast(texture); - return embedderTexture; + FlutterSurface* surface = [view.surfaceManager surfaceForSize:size]; + FlutterMetalTexture texture = surface.asFlutterMetalTexture; + // This is here because the non-compositor path completely ignores + // destruction and user data callback. It is ugly, but the surface manager + // will keep the surface retained until present. + if (texture.destruction_callback != nullptr) { + texture.destruction_callback(texture.user_data); + } + texture.destruction_callback = nullptr; + texture.user_data = nullptr; + return texture; } -- (BOOL)present:(uint64_t)viewId { +- (BOOL)present:(uint64_t)viewId texture:(const FlutterMetalTexture*)texture { FlutterView* view = [_viewProvider getView:viewId]; if (view == nil) { return NO; } - [view present]; - return YES; -} - -- (void)presentWithoutContent:(uint64_t)viewId { - FlutterView* view = [_viewProvider getView:viewId]; - if (view == nil) { - return; + FlutterSurface* surface = [view.surfaceManager lookupSurface:texture]; + if (surface == nil) { + return NO; } - [view presentWithoutContent]; + FlutterSurfacePresentInfo* info = [[FlutterSurfacePresentInfo alloc] init]; + info.offset = CGPointMake(0, 0); + info.zIndex = 0; + info.surface = surface; + [view.surfaceManager present:@[ info ] notify:nil]; + return YES; } #pragma mark - FlutterTextureRegistrar methods. diff --git a/shell/platform/darwin/macos/framework/Source/FlutterRendererTest.mm b/shell/platform/darwin/macos/framework/Source/FlutterRendererTest.mm index 6cd57e33d48f9..55025d838c9a6 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterRendererTest.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterRendererTest.mm @@ -37,17 +37,41 @@ void SetEngineDefaultView(FlutterEngine* engine, id flutterView) { TEST(FlutterRenderer, PresentDelegatesToFlutterView) { FlutterEngine* engine = CreateTestEngine(); FlutterRenderer* renderer = [[FlutterRenderer alloc] initWithFlutterEngine:engine]; - id mockFlutterView = OCMClassMock([FlutterView class]); - SetEngineDefaultView(engine, mockFlutterView); - [(FlutterView*)[mockFlutterView expect] present]; - [renderer present:kFlutterDefaultViewId]; + + id viewMock = OCMClassMock([FlutterView class]); + SetEngineDefaultView(engine, viewMock); + + id surfaceManagerMock = OCMClassMock([FlutterSurfaceManager class]); + OCMStub([viewMock surfaceManager]).andReturn(surfaceManagerMock); + + id surfaceMock = OCMClassMock([FlutterSurface class]); + + FlutterMetalTexture texture = {}; + + OCMStub([surfaceManagerMock lookupSurface:&texture]) + .ignoringNonObjectArgs() + .andReturn(surfaceMock); + + [[surfaceManagerMock expect] present:[OCMArg checkWithBlock:^(id obj) { + NSArray* array = (NSArray*)obj; + return array.count == 1 ? YES : NO; + }] + notify:nil]; + + [renderer present:kFlutterDefaultViewId texture:&texture]; + [surfaceManagerMock verify]; } TEST(FlutterRenderer, TextureReturnedByFlutterView) { FlutterEngine* engine = CreateTestEngine(); FlutterRenderer* renderer = [[FlutterRenderer alloc] initWithFlutterEngine:engine]; - id mockFlutterView = OCMClassMock([FlutterView class]); - SetEngineDefaultView(engine, mockFlutterView); + + id viewMock = OCMClassMock([FlutterView class]); + SetEngineDefaultView(engine, viewMock); + + id surfaceManagerMock = OCMClassMock([FlutterSurfaceManager class]); + OCMStub([viewMock surfaceManager]).andReturn(surfaceManagerMock); + FlutterFrameInfo frameInfo; frameInfo.struct_size = sizeof(FlutterFrameInfo); FlutterUIntSize dimensions; @@ -55,8 +79,10 @@ void SetEngineDefaultView(FlutterEngine* engine, id flutterView) { dimensions.height = 200; frameInfo.size = dimensions; CGSize size = CGSizeMake(dimensions.width, dimensions.height); - [[mockFlutterView expect] backingStoreForSize:size]; + + [[surfaceManagerMock expect] surfaceForSize:size]; [renderer createTextureForView:kFlutterDefaultViewId size:size]; + [surfaceManagerMock verify]; } } // namespace flutter::testing diff --git a/shell/platform/darwin/macos/framework/Source/FlutterResizableBackingStoreProvider.h b/shell/platform/darwin/macos/framework/Source/FlutterResizableBackingStoreProvider.h deleted file mode 100644 index 00a93ac213078..0000000000000 --- a/shell/platform/darwin/macos/framework/Source/FlutterResizableBackingStoreProvider.h +++ /dev/null @@ -1,33 +0,0 @@ -// Copyright 2013 The Flutter Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#import -#import -#import - -#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterBackingStore.h" -#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterResizeSynchronizer.h" - -/** - * Provides resizable buffers backed by a MTLTexture. - */ -@interface FlutterResizableBackingStoreProvider : NSObject - -/** - * Creates a resizable backing store provider for the given CAMetalLayer. - */ -- (nonnull instancetype)initWithDevice:(nonnull id)device - commandQueue:(nonnull id)commandQueue - layer:(nonnull CALayer*)layer; -/** - * Notify of the required backing store size updates. Called during window resize. - */ -- (void)onBackingStoreResized:(CGSize)size; - -/** - * Returns the FlutterBackingStore corresponding to the latest size. - */ -- (nonnull FlutterRenderBackingStore*)backingStore; - -@end diff --git a/shell/platform/darwin/macos/framework/Source/FlutterResizableBackingStoreProvider.mm b/shell/platform/darwin/macos/framework/Source/FlutterResizableBackingStoreProvider.mm deleted file mode 100644 index 436d41af68f15..0000000000000 --- a/shell/platform/darwin/macos/framework/Source/FlutterResizableBackingStoreProvider.mm +++ /dev/null @@ -1,54 +0,0 @@ -// Copyright 2013 The Flutter Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterResizableBackingStoreProvider.h" - -#import - -#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h" - -@implementation FlutterResizableBackingStoreProvider { - id _device; - id _commandQueue; - FlutterSurfaceManager* _surfaceManager; -} - -- (instancetype)initWithDevice:(id)device - commandQueue:(id)commandQueue - layer:(CALayer*)layer { - self = [super init]; - if (self) { - _device = device; - _commandQueue = commandQueue; - _surfaceManager = [[FlutterSurfaceManager alloc] initWithDevice:device - commandQueue:commandQueue - layer:layer]; - } - return self; -} - -- (void)onBackingStoreResized:(CGSize)size { - [_surfaceManager ensureSurfaceSize:size]; -} - -- (FlutterRenderBackingStore*)backingStore { - return [_surfaceManager renderBuffer]; -} - -- (void)resizeSynchronizerFlush:(nonnull FlutterResizeSynchronizer*)synchronizer { - id commandBuffer = [_commandQueue commandBuffer]; - [commandBuffer commit]; - [commandBuffer waitUntilScheduled]; -} - -- (void)resizeSynchronizerCommit:(nonnull FlutterResizeSynchronizer*)synchronizer { - [CATransaction begin]; - [CATransaction setDisableActions:YES]; - - [_surfaceManager swapBuffers]; - - [CATransaction commit]; -} - -@end diff --git a/shell/platform/darwin/macos/framework/Source/FlutterResizeSynchronizer.h b/shell/platform/darwin/macos/framework/Source/FlutterResizeSynchronizer.h deleted file mode 100644 index 556856ed3684d..0000000000000 --- a/shell/platform/darwin/macos/framework/Source/FlutterResizeSynchronizer.h +++ /dev/null @@ -1,90 +0,0 @@ -// Copyright 2013 The Flutter Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#import - -@class FlutterResizeSynchronizer; - -/** - * Implemented by FlutterView. - */ -@protocol FlutterResizeSynchronizerDelegate - -/** - * Invoked on raster thread; Delegate should flush the graphics context. - */ -- (void)resizeSynchronizerFlush:(nonnull FlutterResizeSynchronizer*)synchronizer; - -/** - * Invoked on platform thread; Delegate should flip the surfaces. - */ -- (void)resizeSynchronizerCommit:(nonnull FlutterResizeSynchronizer*)synchronizer; - -@end - -/** - * Encapsulates the logic for blocking platform thread during window resize as - * well as synchronizing the raster and platform thread during commit (presenting frame). - * - * Flow during window resize - * - * 1. Platform thread calls [synchronizer beginResize:notify:] - * This will hold the platform thread until the raster thread is ready to display contents. - * 2. Raster thread calls [synchronizer shouldEnsureSurfaceForSize:] with target size - * This will return false for any size other than target size - * 3. Raster thread calls [synchronizer requestCommit] - * Any commit calls before shouldEnsureSurfaceForSize: is called with the right - * size are simply ignored; There's no point rasterizing and displaying frames - * with wrong size. - * Both delegate methods (flush/commit) will be invoked before beginResize returns - * - * Flow during regular operation (no resizing) - * - * 1. Raster thread calls [synchronizer requestCommit] - * This will invoke [delegate flush:] on raster thread and - * [delegate commit:] on platform thread. The requestCommit call will be blocked - * until this is done. This is necessary to ensure that rasterizer won't start - * rasterizing next frame before the FlutterSurfaceManager flipped the surface, - * which must be performed on platform thread. - */ -@interface FlutterResizeSynchronizer : NSObject - -- (nullable instancetype)initWithDelegate:(nonnull id)delegate; - -/** - * Blocks the platform thread until - * - shouldEnsureSurfaceForSize is called with proper size and - * - requestCommit is called - * All requestCommit calls before `shouldEnsureSurfaceForSize` is called with - * expected size are ignored; - * The notify block is invoked immediately after synchronizer mutex is acquired. - */ -- (void)beginResize:(CGSize)size notify:(nonnull dispatch_block_t)notify; - -/** - * Returns whether the view should ensure surfaces with given size; - * This will be false during resizing for any size other than size specified - * during beginResize. - */ -- (BOOL)shouldEnsureSurfaceForSize:(CGSize)size; - -/** - * Called from rasterizer thread, will block until delegate resizeSynchronizerCommit: - * method is called (on platform thread). - */ -- (void)requestCommit; - -/** - * Called from view to notify the synchronizer that there are no Flutter frames - * coming. Synchronizer must unblock main thread and not block until another - * frame is available. - */ -- (void)noFlutterContent; - -/** - * Called when shutting down. Unblocks everything and prevents any further synchronization. - */ -- (void)shutdown; - -@end diff --git a/shell/platform/darwin/macos/framework/Source/FlutterResizeSynchronizer.mm b/shell/platform/darwin/macos/framework/Source/FlutterResizeSynchronizer.mm deleted file mode 100644 index 7681aa3385779..0000000000000 --- a/shell/platform/darwin/macos/framework/Source/FlutterResizeSynchronizer.mm +++ /dev/null @@ -1,153 +0,0 @@ -// Copyright 2013 The Flutter Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterResizeSynchronizer.h" - -#include - -@interface FlutterResizeSynchronizer () { - // Counter to detect stale callbacks. - uint32_t _cookie; - - std::mutex _mutex; - - // Used to block [beginResize:]. - std::condition_variable _condBlockBeginResize; - // Used to block [requestCommit]. - std::condition_variable _condBlockRequestCommit; - - // Whether a frame was received; the synchronizer doesn't block platform thread during resize - // until it knows that framework is running and producing frames - BOOL _hasFlutterContent; - - // If NO, requestCommit calls are ignored until shouldEnsureSurfaceForSize is called with - // proper size. - BOOL _acceptingCommit; - - // Waiting for resize to finish. - BOOL _waiting; - - // RequestCommit was called and [delegate commit:] must be performed on platform thread. - BOOL _pendingCommit; - - // Target size for resizing. - CGSize _newSize; - - // if YES prevents all synchronization - BOOL _shuttingDown; - - __weak id _delegate; -} -@end - -@implementation FlutterResizeSynchronizer - -- (instancetype)initWithDelegate:(id)delegate { - if (self = [super init]) { - _acceptingCommit = YES; - _delegate = delegate; - } - return self; -} - -- (void)beginResize:(CGSize)size notify:(dispatch_block_t)notify { - std::unique_lock lock(_mutex); - if (!_delegate) { - return; - } - - if (!_hasFlutterContent || _shuttingDown) { - // No blocking until framework produces at least one frame - notify(); - return; - } - - ++_cookie; - - // from now on, ignore all incoming commits until the block below gets - // scheduled on raster thread - _acceptingCommit = NO; - - // let pending commits finish to unblock the raster thread - _pendingCommit = NO; - _condBlockBeginResize.notify_all(); - - // let the engine send resize notification - notify(); - - _newSize = size; - - _waiting = YES; - - _condBlockRequestCommit.wait(lock, [&] { return _pendingCommit || _shuttingDown; }); - - [_delegate resizeSynchronizerFlush:self]; - [_delegate resizeSynchronizerCommit:self]; - _pendingCommit = NO; - _condBlockBeginResize.notify_all(); - - _waiting = NO; -} - -- (BOOL)shouldEnsureSurfaceForSize:(CGSize)size { - std::unique_lock lock(_mutex); - - if (!_hasFlutterContent) { - return YES; - } - - if (!_acceptingCommit) { - if (CGSizeEqualToSize(_newSize, size)) { - _acceptingCommit = YES; - } - } - return _acceptingCommit; -} - -- (void)requestCommit { - std::unique_lock lock(_mutex); - - if (!_acceptingCommit || _shuttingDown) { - return; - } - - _hasFlutterContent = YES; - - _pendingCommit = YES; - if (_waiting) { // BeginResize is in progress, interrupt it and schedule commit call - _condBlockRequestCommit.notify_all(); - _condBlockBeginResize.wait(lock, [&]() { return !_pendingCommit || _shuttingDown; }); - } else { - // No resize, schedule commit on platform thread and wait until either done - // or interrupted by incoming BeginResize - [_delegate resizeSynchronizerFlush:self]; - dispatch_async(dispatch_get_main_queue(), [self, cookie = _cookie] { - std::unique_lock lock(_mutex); - if (cookie == _cookie) { - if (_delegate) { - [_delegate resizeSynchronizerCommit:self]; - } - _pendingCommit = NO; - _condBlockBeginResize.notify_all(); - } - }); - _condBlockBeginResize.wait(lock, [&]() { return !_pendingCommit || _shuttingDown; }); - } -} - -- (void)noFlutterContent { - std::unique_lock lock(_mutex); - _hasFlutterContent = NO; - _acceptingCommit = YES; - _condBlockBeginResize.notify_all(); -} - -- (void)shutdown { - std::unique_lock lock(_mutex); - _shuttingDown = YES; - _condBlockBeginResize.notify_all(); - _condBlockRequestCommit.notify_all(); -} - -@end diff --git a/shell/platform/darwin/macos/framework/Source/FlutterSurface.h b/shell/platform/darwin/macos/framework/Source/FlutterSurface.h new file mode 100644 index 0000000000000..4570b3303d4cc --- /dev/null +++ b/shell/platform/darwin/macos/framework/Source/FlutterSurface.h @@ -0,0 +1,17 @@ +// 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 + +#import "flutter/shell/platform/embedder/embedder.h" + +/** + * Opaque surface type. + * Can be represented as FlutterMetalTexture to cross the embedder API boundary. + */ +@interface FlutterSurface : NSObject + +- (FlutterMetalTexture)asFlutterMetalTexture; + +@end diff --git a/shell/platform/darwin/macos/framework/Source/FlutterSurface.mm b/shell/platform/darwin/macos/framework/Source/FlutterSurface.mm new file mode 100644 index 0000000000000..e9016b543d301 --- /dev/null +++ b/shell/platform/darwin/macos/framework/Source/FlutterSurface.mm @@ -0,0 +1,94 @@ +// 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/FlutterSurface_Internal.h" + +#import + +@interface FlutterSurface () { + CGSize _size; + IOSurfaceRef _ioSurface; + id _texture; +} +@end + +@implementation FlutterSurface + +- (IOSurfaceRef)ioSurface { + return _ioSurface; +} + +- (CGSize)size { + return _size; +} + +- (int64_t)textureId { + return reinterpret_cast(_texture); +} + +- (instancetype)initWithSize:(CGSize)size device:(id)device { + if (self = [super init]) { + self->_size = size; + self->_ioSurface = [FlutterSurface createIOSurfaceWithSize:size]; + self->_texture = [FlutterSurface createTextureForIOSurface:_ioSurface size:size device:device]; + } + return self; +} + +static void ReleaseSurface(void* surface) { + if (surface != nullptr) { + CFBridgingRelease(surface); + } +} + +- (FlutterMetalTexture)asFlutterMetalTexture { + FlutterMetalTexture res; + memset(&res, 0, sizeof(FlutterMetalTexture)); + res.struct_size = sizeof(FlutterMetalTexture); + res.texture = (__bridge void*)_texture; + res.texture_id = self.textureId; + res.user_data = (void*)CFBridgingRetain(self); + res.destruction_callback = ReleaseSurface; + return res; +} + +- (void)dealloc { + CFRelease(_ioSurface); +} + ++ (IOSurfaceRef)createIOSurfaceWithSize:(CGSize)size { + unsigned pixelFormat = 'BGRA'; + unsigned bytesPerElement = 4; + + size_t bytesPerRow = IOSurfaceAlignProperty(kIOSurfaceBytesPerRow, size.width * bytesPerElement); + size_t totalBytes = IOSurfaceAlignProperty(kIOSurfaceAllocSize, size.height * bytesPerRow); + NSDictionary* options = @{ + (id)kIOSurfaceWidth : @(size.width), + (id)kIOSurfaceHeight : @(size.height), + (id)kIOSurfacePixelFormat : @(pixelFormat), + (id)kIOSurfaceBytesPerElement : @(bytesPerElement), + (id)kIOSurfaceBytesPerRow : @(bytesPerRow), + (id)kIOSurfaceAllocSize : @(totalBytes), + }; + + IOSurfaceRef res = IOSurfaceCreate((CFDictionaryRef)options); + IOSurfaceSetValue(res, CFSTR("IOSurfaceColorSpace"), kCGColorSpaceSRGB); + return res; +} + ++ (id)createTextureForIOSurface:(IOSurfaceRef)surface + size:(CGSize)size + device:(id)device { + MTLTextureDescriptor* textureDescriptor = + [MTLTextureDescriptor texture2DDescriptorWithPixelFormat:MTLPixelFormatBGRA8Unorm + width:size.width + height:size.height + mipmapped:NO]; + textureDescriptor.usage = + MTLTextureUsageShaderRead | MTLTextureUsageRenderTarget | MTLTextureUsageShaderWrite; + // plane = 0 for BGRA. + return [device newTextureWithDescriptor:textureDescriptor iosurface:surface plane:0]; +} + +@end diff --git a/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h b/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h index 2f6a0974e4164..0a7a1b3202cec 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h +++ b/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h @@ -3,42 +3,73 @@ // found in the LICENSE file. #import -#import +#import -#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterBackingStore.h" -#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterIOSurfaceHolder.h" +#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterSurface.h" /** - * Manages render surfaces and corresponding backing stores used by the engine. - * - * The backing store when rendering with on Metal is a Metal texture. There are two IOSurfaces - * created during initialization, FlutterSurfaceManager manages the lifecycle of these. + * Surface with additional properties needed for presenting. + */ +@interface FlutterSurfacePresentInfo : NSObject + +@property(readwrite, strong, nonatomic, nonnull) FlutterSurface* surface; +@property(readwrite, nonatomic) CGPoint offset; +@property(readwrite, nonatomic) size_t zIndex; + +@end + +@protocol FlutterSurfaceManagerDelegate + +/* + * Schedules the block on platform thread and waits until the block is executed. + * Provided `frameSize` is used to unblock platform thread if it waits for + * certain frame size during resizing. + */ +- (void)onPresent:(CGSize)frameSize withBlock:(nonnull dispatch_block_t)block; + +@end + +/** + * Owned by `FlutterView`. + * Responsible for providing surfaces for Flutter to render into and subsequent + * presentation of these surfaces. Manages core animation sublayers. */ @interface FlutterSurfaceManager : NSObject /** * Initializes and returns a surface manager that renders to a child layer (referred to as the - * content layer) of the containing layer and applies the transform to the contents of the content - * layer. + * content layer) of the containing layer. */ - (nullable instancetype)initWithDevice:(nonnull id)device commandQueue:(nonnull id)commandQueue - layer:(nonnull CALayer*)containingLayer; + layer:(nonnull CALayer*)containingLayer + delegate:(nonnull id)delegate; /** - * Updates the backing store size of the managed IOSurfaces the specified size. If the surfaces are - * already of this size, this is a no-op. + * Returns back buffer surface for given size that Flutter can render content to. + * Will return cached surface if one is available, or create new one otherwise. + * + * Must be called on raster thread. */ -- (void)ensureSurfaceSize:(CGSize)size; +- (nonnull FlutterSurface*)surfaceForSize:(CGSize)size; /** - * Swaps the front and the back buffer. + * Looks up surface for given metal texture. Can only be called for surfaces + * obtained through `surfaceForSize:` until `present:` is called. */ -- (void)swapBuffers; +- (nullable FlutterSurface*)lookupSurface:(nonnull const FlutterMetalTexture*)texture; /** - * Returns the backing store for the back buffer. + * Sets the provided surfaces as contents of FlutterView. Will create, update and + * remove sublayers as needed. + * + * Must be called on raster thread. This will schedule a commit on platform + * thread and block raster thread until the commit is done. + * `notify` block will be invoked on platform thread and can be used to perform + * additional work, such as mutating platform views. It is guaranteed be called in + * same CATransaction. */ -- (nonnull FlutterRenderBackingStore*)renderBuffer; +- (void)present:(nonnull NSArray*)surfaces + 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 d3d612fd294c8..69c13cbe12bf2 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.mm @@ -2,106 +2,63 @@ // 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/FlutterSurfaceManager.h" +#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager_Internal.h" +#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterSurface_Internal.h" #import - #include -#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterIOSurfaceHolder.h" - -enum { - kFlutterSurfaceManagerFrontBuffer = 0, - kFlutterSurfaceManagerBackBuffer = 1, - kFlutterSurfaceManagerBufferCount, -}; - -// BackBuffer will be released after kIdleDelay if there is no activity. +// Cached back buffers will be released after kIdleDelay if there is no activity. static const double kIdleDelay = 1.0; -@interface FlutterSurfaceManager () - -/** - * Cancels any previously-scheduled onIdle requests. - */ -- (void)cancelIdle; - -/** - * Creates a backing textures for the specified surface with the specified size. - */ -- (id)createTextureForSurface:(FlutterIOSurfaceHolder*)surface size:(CGSize)size; - -@end - -@implementation FlutterSurfaceManager { - CALayer* _containingLayer; // provided (parent layer) - CALayer* _contentLayer; - CATransform3D _contentTransform; - - CGSize _surfaceSize; - FlutterIOSurfaceHolder* _ioSurfaces[kFlutterSurfaceManagerBufferCount]; - BOOL _frameInProgress; - - id _device; - id _commandQueue; - id _textures[kFlutterSurfaceManagerBufferCount]; +@interface FlutterBackBufferCache () { + NSMutableArray* _surfaces; } -- (nullable instancetype)initWithDevice:(nonnull id)device - commandQueue:(nonnull id)commandQueue - layer:(nonnull CALayer*)containingLayer { - self = [super init]; - if (self) { - _containingLayer = containingLayer; - _contentTransform = CATransform3DIdentity; - _contentLayer = [[CALayer alloc] init]; - [_containingLayer addSublayer:_contentLayer]; +@end - _ioSurfaces[0] = [[FlutterIOSurfaceHolder alloc] init]; - _ioSurfaces[1] = [[FlutterIOSurfaceHolder alloc] init]; +@implementation FlutterBackBufferCache - _device = device; - _commandQueue = commandQueue; +- (instancetype)init { + if (self = [super init]) { + self->_surfaces = [[NSMutableArray alloc] init]; } return self; } -- (void)ensureSurfaceSize:(CGSize)size { - if (CGSizeEqualToSize(size, _surfaceSize)) { - return; - } - _surfaceSize = size; - for (int i = 0; i < kFlutterSurfaceManagerBufferCount; ++i) { - if (_ioSurfaces[i] != nil) { - [_ioSurfaces[i] recreateIOSurfaceWithSize:size]; - _textures[i] = [self createTextureForSurface:_ioSurfaces[i] size:size]; +- (nullable FlutterSurface*)removeSurfaceForSize:(CGSize)size { + @synchronized(self) { + for (FlutterSurface* surface in _surfaces) { + if (CGSizeEqualToSize(surface.size, size)) { + // By default ARC doesn't retain enumartion iteration variables. + FlutterSurface* res = surface; + [_surfaces removeObject:surface]; + return res; + } } + return nil; } } -- (void)swapBuffers { -#ifndef NDEBUG - // swapBuffers should not be called unless a frame was drawn +- (void)replaceWith:(nonnull NSArray*)surfaces { @synchronized(self) { - assert(_frameInProgress); + [_surfaces removeAllObjects]; + [_surfaces addObjectsFromArray:surfaces]; } -#endif - - _contentLayer.frame = _containingLayer.bounds; - _contentLayer.transform = _contentTransform; - IOSurfaceRef contentIOSurface = [_ioSurfaces[kFlutterSurfaceManagerBackBuffer] ioSurface]; - [_contentLayer setContents:(__bridge id)contentIOSurface]; - - std::swap(_ioSurfaces[kFlutterSurfaceManagerBackBuffer], - _ioSurfaces[kFlutterSurfaceManagerFrontBuffer]); - std::swap(_textures[kFlutterSurfaceManagerBackBuffer], - _textures[kFlutterSurfaceManagerFrontBuffer]); // performSelector:withObject:afterDelay needs to be performed on RunLoop thread [self performSelectorOnMainThread:@selector(reschedule) withObject:nil waitUntilDone:NO]; +} + +- (NSUInteger)count { + @synchronized(self) { + return _surfaces.count; + } +} +- (void)onIdle { @synchronized(self) { - _frameInProgress = NO; + [_surfaces removeAllObjects]; } } @@ -110,52 +67,150 @@ - (void)reschedule { [self performSelector:@selector(onIdle) withObject:nil afterDelay:kIdleDelay]; } -- (void)onIdle { - @synchronized(self) { - if (!_frameInProgress) { - // Release the back buffer and notify delegate. The buffer will be restored - // on demand in ensureBackBuffer - _ioSurfaces[kFlutterSurfaceManagerBackBuffer] = nil; - _textures[kFlutterSurfaceManagerBackBuffer] = nil; - } +- (void)dealloc { + [NSObject cancelPreviousPerformRequestsWithTarget:self selector:@selector(onIdle) object:nil]; +} + +@end + +@implementation FlutterSurfacePresentInfo +@end + +@interface FlutterSurfaceManager () { + id _device; + id _commandQueue; + CALayer* _containingLayer; + __weak id _delegate; + + // Available (cached) back buffer surfaces. These will be cleared during + // present and replaced by current frong surfaces. + FlutterBackBufferCache* _backBufferCache; + + // Surfaces that currently obtained through surfaceForSize waiting to be + // presented. This is used to keep the surface alive because the non compositor + // codepath doesn't properly retain the surface. + NSMutableArray* _borrowedSurfaces; + + // Surfaces currently used to back visible layers. + NSMutableArray* _frontSurfaces; + + // Currently visible layers. + NSMutableArray* _layers; +} +@end + +@implementation FlutterSurfaceManager + +- (instancetype)initWithDevice:(id)device + commandQueue:(id)commandQueue + layer:(CALayer*)containingLayer + delegate:(__weak id)delegate { + if (self = [super init]) { + _device = device; + _commandQueue = commandQueue; + _containingLayer = containingLayer; + _delegate = delegate; + + _backBufferCache = [[FlutterBackBufferCache alloc] init]; + _borrowedSurfaces = [NSMutableArray array]; + _frontSurfaces = [NSMutableArray array]; + _layers = [NSMutableArray array]; } + return self; } -- (void)ensureBackBuffer { - @synchronized(self) { - _frameInProgress = YES; - if (_ioSurfaces[kFlutterSurfaceManagerBackBuffer] == nil) { - // Restore previously released backbuffer - _ioSurfaces[kFlutterSurfaceManagerBackBuffer] = [[FlutterIOSurfaceHolder alloc] init]; - [_ioSurfaces[kFlutterSurfaceManagerBackBuffer] recreateIOSurfaceWithSize:_surfaceSize]; - _textures[kFlutterSurfaceManagerBackBuffer] = - [self createTextureForSurface:_ioSurfaces[kFlutterSurfaceManagerBackBuffer] - size:_surfaceSize]; +- (FlutterBackBufferCache*)backBufferCache { + return _backBufferCache; +} + +- (NSArray*)borrowedSurfaces { + return _borrowedSurfaces; +} + +- (NSArray*)frontSurfaces { + return _frontSurfaces; +} + +- (NSArray*)layers { + return _layers; +} + +- (FlutterSurface*)lookupSurface:(nonnull const FlutterMetalTexture*)texture { + for (FlutterSurface* surface in _borrowedSurfaces) { + if (surface.textureId == texture->texture_id) { + return surface; } - }; - [self performSelectorOnMainThread:@selector(cancelIdle) withObject:nil waitUntilDone:NO]; + } + return nil; } -- (void)cancelIdle { - [NSObject cancelPreviousPerformRequestsWithTarget:self selector:@selector(onIdle) object:nil]; +- (FlutterSurface*)surfaceForSize:(CGSize)size { + FlutterSurface* res = [_backBufferCache removeSurfaceForSize:size]; + if (res == nil) { + res = [[FlutterSurface alloc] initWithSize:size device:_device]; + } + [_borrowedSurfaces addObject:res]; + return res; } -- (nonnull FlutterRenderBackingStore*)renderBuffer { - [self ensureBackBuffer]; - id texture = _textures[kFlutterSurfaceManagerBackBuffer]; - return [[FlutterRenderBackingStore alloc] initWithTexture:texture]; +- (void)commit:(NSArray*)surfaces { + assert([NSThread isMainThread]); + + [_borrowedSurfaces removeAllObjects]; + + // Release all unused back buffer surfaces and replace them with front surfaces. + [_backBufferCache replaceWith:_frontSurfaces]; + + // Front surfaces will be replaced by currently presented surfaces. + [_frontSurfaces removeAllObjects]; + for (FlutterSurfacePresentInfo* info in surfaces) { + [_frontSurfaces addObject:info.surface]; + } + + // Remove excess layers. + while (_layers.count > _frontSurfaces.count) { + [_layers.lastObject removeFromSuperlayer]; + [_layers removeLastObject]; + } + + // Add missing layers. + while (_layers.count < _frontSurfaces.count) { + CALayer* layer = [CALayer layer]; + [_containingLayer addSublayer:layer]; + [_layers addObject:layer]; + } + + // Update contents of surfaces. + for (size_t i = 0; i < surfaces.count; ++i) { + FlutterSurfacePresentInfo* info = surfaces[i]; + CALayer* layer = _layers[i]; + CGFloat scale = _containingLayer.contentsScale; + layer.frame = CGRectMake(info.offset.x / scale, info.offset.y / scale, + info.surface.size.width / scale, info.surface.size.height / scale); + layer.contents = (__bridge id)info.surface.ioSurface; + layer.zPosition = info.zIndex; + } } -- (id)createTextureForSurface:(FlutterIOSurfaceHolder*)surface size:(CGSize)size { - MTLTextureDescriptor* textureDescriptor = - [MTLTextureDescriptor texture2DDescriptorWithPixelFormat:MTLPixelFormatBGRA8Unorm - width:size.width - height:size.height - mipmapped:NO]; - textureDescriptor.usage = - MTLTextureUsageShaderRead | MTLTextureUsageRenderTarget | MTLTextureUsageShaderWrite; - // plane = 0 for BGRA. - return [_device newTextureWithDescriptor:textureDescriptor iosurface:[surface ioSurface] plane:0]; +- (void)present:(NSArray*)surfaces notify:(dispatch_block_t)notify { + id commandBuffer = [_commandQueue commandBuffer]; + [commandBuffer commit]; + [commandBuffer waitUntilScheduled]; + + // Get the actual dimensions of the frame (relevant for thread synchronizer). + CGSize size = CGSizeZero; + for (FlutterSurfacePresentInfo* info in surfaces) { + size = CGSizeMake(std::max(size.width, info.offset.x + info.surface.size.width), + std::max(size.height, info.offset.y + info.surface.size.height)); + } + + [_delegate onPresent:size + withBlock:^{ + [self commit:surfaces]; + if (notify != nil) { + notify(); + } + }]; } @end diff --git a/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManagerTest.mm b/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManagerTest.mm index b9e1742abd547..d0184e596e8b9 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManagerTest.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManagerTest.mm @@ -3,14 +3,18 @@ // found in the LICENSE file. #import +#import #import -#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h" +#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager_Internal.h" +#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterSurface_Internal.h" + #include "flutter/testing/testing.h" #include "gtest/gtest.h" -@interface TestView : NSView +@interface TestView : NSView +@property(readwrite, nonatomic) CGSize presentedFrameSize; - (nonnull instancetype)init; @end @@ -25,38 +29,182 @@ - (instancetype)init { return self; } +- (void)onPresent:(CGSize)frameSize withBlock:(nonnull dispatch_block_t)block { + self.presentedFrameSize = frameSize; + block(); +} + @end namespace flutter::testing { -static FlutterSurfaceManager* CreateSurfaceManager() { +static FlutterSurfaceManager* CreateSurfaceManager(TestView* testView) { id device = MTLCreateSystemDefaultDevice(); id commandQueue = [device newCommandQueue]; - TestView* metalView = [[TestView alloc] init]; - CALayer* layer = reinterpret_cast(metalView.layer); + CALayer* layer = reinterpret_cast(testView.layer); return [[FlutterSurfaceManager alloc] initWithDevice:device commandQueue:commandQueue - layer:layer]; -} - -TEST(FlutterSurfaceManager, EnsureSizeUpdatesSize) { - FlutterSurfaceManager* surfaceManager = CreateSurfaceManager(); - CGSize size = CGSizeMake(100, 50); - [surfaceManager ensureSurfaceSize:size]; - id texture = [surfaceManager renderBuffer].texture; - CGSize textureSize = CGSizeMake(texture.width, texture.height); - ASSERT_TRUE(CGSizeEqualToSize(size, textureSize)); -} - -TEST(FlutterSurfaceManager, EnsureSizeUpdatesSizeForBackBuffer) { - FlutterSurfaceManager* surfaceManager = CreateSurfaceManager(); - CGSize size = CGSizeMake(100, 50); - [surfaceManager ensureSurfaceSize:size]; - [surfaceManager renderBuffer]; // make sure we have back buffer - [surfaceManager swapBuffers]; - id texture = [surfaceManager renderBuffer].texture; - CGSize textureSize = CGSizeMake(texture.width, texture.height); - ASSERT_TRUE(CGSizeEqualToSize(size, textureSize)); + layer:layer + delegate:testView]; +} + +static FlutterSurfacePresentInfo* CreatePresentInfo(FlutterSurface* surface, + CGPoint offset = CGPointZero, + size_t index = 0) { + FlutterSurfacePresentInfo* res = [[FlutterSurfacePresentInfo alloc] init]; + res.surface = surface; + res.offset = offset; + res.zIndex = index; + return res; +} + +TEST(FlutterSurfaceManager, MetalTexture) { + TestView* testView = [[TestView alloc] init]; + FlutterSurfaceManager* surfaceManager = CreateSurfaceManager(testView); + + // Get back buffer, lookup should work for borrowed surfaces util present. + auto surface = [surfaceManager surfaceForSize:CGSizeMake(100, 50)]; + auto texture = surface.asFlutterMetalTexture; + id metalTexture = (__bridge id)texture.texture; + EXPECT_EQ(metalTexture.width, 100ul); + EXPECT_EQ(metalTexture.height, 50ul); + texture.destruction_callback(texture.user_data); +} + +TEST(FlutterSurfaceManager, TestLookup) { + TestView* testView = [[TestView alloc] init]; + FlutterSurfaceManager* surfaceManager = CreateSurfaceManager(testView); + + // Get back buffer, lookup should work for borrowed surfaces util present. + auto surface = [surfaceManager surfaceForSize:CGSizeMake(100, 50)]; + + // SurfaceManager should keep texture alive while borrowed. + auto texture = surface.asFlutterMetalTexture; + texture.destruction_callback(texture.user_data); + + FlutterMetalTexture dummyTexture{.texture_id = 1, .texture = nullptr}; + auto surface1 = [surfaceManager lookupSurface:&dummyTexture]; + EXPECT_EQ(surface1, nil); + + auto surface2 = [surfaceManager lookupSurface:&texture]; + EXPECT_EQ(surface2, surface); + + [surfaceManager present:@[ CreatePresentInfo(surface) ] notify:nil]; + + auto surface3 = [surfaceManager lookupSurface:&texture]; + EXPECT_EQ(surface3, nil); +} + +TEST(FlutterSurfaceManager, BackBufferCacheDoesNotLeak) { + TestView* testView = [[TestView alloc] init]; + FlutterSurfaceManager* surfaceManager = CreateSurfaceManager(testView); + EXPECT_EQ(surfaceManager.backBufferCache.count, 0ul); + + auto surface1 = [surfaceManager surfaceForSize:CGSizeMake(100, 100)]; + [surfaceManager present:@[ CreatePresentInfo(surface1) ] notify:nil]; + + EXPECT_EQ(surfaceManager.backBufferCache.count, 0ul); + + auto surface2 = [surfaceManager surfaceForSize:CGSizeMake(110, 110)]; + [surfaceManager present:@[ CreatePresentInfo(surface2) ] notify:nil]; + + EXPECT_EQ(surfaceManager.backBufferCache.count, 1ul); + + auto surface3 = [surfaceManager surfaceForSize:CGSizeMake(120, 120)]; + [surfaceManager present:@[ CreatePresentInfo(surface3) ] notify:nil]; + + // Cache should be cleaned during present and only contain the last visible + // surface(s). + EXPECT_EQ(surfaceManager.backBufferCache.count, 1ul); + auto surfaceFromCache = [surfaceManager surfaceForSize:CGSizeMake(110, 110)]; + EXPECT_EQ(surfaceFromCache, surface2); + + [surfaceManager present:@[] notify:nil]; + EXPECT_EQ(surfaceManager.backBufferCache.count, 1ul); + + [surfaceManager present:@[] notify:nil]; + EXPECT_EQ(surfaceManager.backBufferCache.count, 0ul); +} + +TEST(FlutterSurfaceManager, SurfaceCycle) { + TestView* testView = [[TestView alloc] init]; + FlutterSurfaceManager* surfaceManager = CreateSurfaceManager(testView); + + EXPECT_EQ(surfaceManager.borrowedSurfaces.count, 0ul); + EXPECT_EQ(surfaceManager.frontSurfaces.count, 0ul); + + // Get first surface and present it. + + auto surface1 = [surfaceManager surfaceForSize:CGSizeMake(100, 100)]; + EXPECT_TRUE(CGSizeEqualToSize(surface1.size, CGSizeMake(100, 100))); + + EXPECT_EQ(surfaceManager.backBufferCache.count, 0ul); + EXPECT_EQ(surfaceManager.borrowedSurfaces.count, 1ul); + EXPECT_EQ(surfaceManager.frontSurfaces.count, 0ul); + + [surfaceManager present:@[ CreatePresentInfo(surface1) ] notify:nil]; + + EXPECT_EQ(surfaceManager.backBufferCache.count, 0ul); + EXPECT_EQ(surfaceManager.borrowedSurfaces.count, 0ul); + EXPECT_EQ(surfaceManager.frontSurfaces.count, 1ul); + EXPECT_EQ(testView.layer.sublayers.count, 1ul); + + // Get second surface and present it. + + auto surface2 = [surfaceManager surfaceForSize:CGSizeMake(100, 100)]; + EXPECT_TRUE(CGSizeEqualToSize(surface2.size, CGSizeMake(100, 100))); + + EXPECT_EQ(surfaceManager.backBufferCache.count, 0ul); + EXPECT_EQ(surfaceManager.borrowedSurfaces.count, 1ul); + + [surfaceManager present:@[ CreatePresentInfo(surface2) ] notify:nil]; + + // Check that current front surface returns to cache. + EXPECT_EQ(surfaceManager.backBufferCache.count, 1ul); + EXPECT_EQ(surfaceManager.borrowedSurfaces.count, 0ul); + EXPECT_EQ(surfaceManager.frontSurfaces.count, 1ul); + EXPECT_EQ(testView.layer.sublayers.count, 1ull); + + // Check that surface is properly reused. + auto surface3 = [surfaceManager surfaceForSize:CGSizeMake(100, 100)]; + EXPECT_EQ(surface3, surface1); +} + +TEST(FlutterSurfaceManager, LayerManagement) { + TestView* testView = [[TestView alloc] init]; + FlutterSurfaceManager* surfaceManager = CreateSurfaceManager(testView); + + 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]; + + 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:@[ + CreatePresentInfo(surface2_1, CGPointMake(20, 10), 1), + CreatePresentInfo(surface2_2, CGPointMake(40, 50), 2) + ] + notify:nil]; + + EXPECT_EQ(testView.layer.sublayers.count, 2ul); + EXPECT_EQ([testView.layer.sublayers objectAtIndex:0].zPosition, 1.0); + EXPECT_EQ([testView.layer.sublayers objectAtIndex:1].zPosition, 2.0); + EXPECT_TRUE(CGSizeEqualToSize(testView.presentedFrameSize, CGSizeMake(70, 70))); + + auto surface3_1 = [surfaceManager surfaceForSize:CGSizeMake(50, 30)]; + [surfaceManager present:@[ CreatePresentInfo(surface3_1, CGPointMake(20, 10)) ] 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]; + EXPECT_EQ(testView.layer.sublayers.count, 0ul); + EXPECT_TRUE(CGSizeEqualToSize(testView.presentedFrameSize, CGSizeMake(0, 0))); } } // namespace flutter::testing diff --git a/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager_Internal.h b/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager_Internal.h new file mode 100644 index 0000000000000..fda05cbf20623 --- /dev/null +++ b/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager_Internal.h @@ -0,0 +1,41 @@ +// 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/FlutterSurfaceManager.h" + +#import + +/** + * Responsible for caching back buffers to prevent unnecessary IOSurface allocations. + */ +@interface FlutterBackBufferCache : NSObject + +/** + * Removes surface with given size from cache (if available) and returns it. + */ +- (nullable FlutterSurface*)removeSurfaceForSize:(CGSize)size; + +/** + * Removes all cached surface replacing them with new ones. + */ +- (void)replaceWith:(nonnull NSArray*)surfaces; + +/** + * Returns number of surfaces currently in cache. Used for tests. + */ +- (NSUInteger)count; + +@end + +/** + * Interface to internal properties used for testing. + */ +@interface FlutterSurfaceManager () + +@property(readonly, nonatomic, nonnull) FlutterBackBufferCache* backBufferCache; +@property(readonly, nonatomic, nonnull) NSArray* borrowedSurfaces; +@property(readonly, nonatomic, nonnull) NSArray* frontSurfaces; +@property(readonly, nonatomic, nonnull) NSArray* layers; + +@end diff --git a/shell/platform/darwin/macos/framework/Source/FlutterSurface_Internal.h b/shell/platform/darwin/macos/framework/Source/FlutterSurface_Internal.h new file mode 100644 index 0000000000000..8367b9a44432f --- /dev/null +++ b/shell/platform/darwin/macos/framework/Source/FlutterSurface_Internal.h @@ -0,0 +1,18 @@ +// 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/FlutterSurface.h" + +/** + * Internal FlutterSurface interface used by FlutterSurfaceManager. + */ +@interface FlutterSurface () + +- (instancetype)initWithSize:(CGSize)size device:(id)device; + +@property(readonly, nonatomic) IOSurfaceRef ioSurface; +@property(readonly, nonatomic) CGSize size; +@property(readonly, nonatomic) int64_t textureId; + +@end diff --git a/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.h b/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.h new file mode 100644 index 0000000000000..8eca9611f0f48 --- /dev/null +++ b/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.h @@ -0,0 +1,36 @@ +#import + +/** + * Takes care of synchronization between raster and platform thread. + */ +@interface FlutterThreadSynchronizer : NSObject + +/** + * Blocks current thread until there is frame available. + * Used in FlutterEngineTest. + */ +- (void)blockUntilFrameAvailable; + +/** + * Called from platform thread. Blocks until commit with given size (or empty) + * is requested. + */ +- (void)beginResize:(CGSize)size notify:(nonnull dispatch_block_t)notify; + +/** + * Called from raster thread. Schedules the given block on platform thread + * and blocks until it is performed. + * + * If platform thread is blocked in `beginResize:` for given size (or size is empty), + * unblocks platform thread. + * + * The notify block is guaranteed to be called within a core animation transaction. + */ +- (void)performCommit:(CGSize)size notify:(nonnull dispatch_block_t)notify; + +/** + * Called when shutting down. Unblocks everything and prevents any further synchronization. + */ +- (void)shutdown; + +@end diff --git a/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.mm b/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.mm new file mode 100644 index 0000000000000..98b64ef1efb9b --- /dev/null +++ b/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.mm @@ -0,0 +1,132 @@ +#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.h" + +#import +#include + +@interface FlutterThreadSynchronizer () { + std::mutex _mutex; + BOOL _shuttingDown; + CGSize _contentSize; + std::vector _scheduledBlocks; + + BOOL _beginResizeWaiting; + + // Used to block [beginResize:]. + std::condition_variable _condBlockBeginResize; +} + +@end + +namespace { +/// Single use event (can only be signalled once) +class Event { + public: + void Signal() { + assert(!signaled_); + std::scoped_lock locker(mutex_); + signaled_ = true; + cv_.notify_one(); + } + + void Wait() { + std::unique_lock locker(mutex_); + while (!signaled_) { + cv_.wait(locker); + } + } + + private: + std::condition_variable cv_; + std::mutex mutex_; + bool signaled_ = false; +}; +} // namespace + +@implementation FlutterThreadSynchronizer + +- (void)drain { + assert([NSThread isMainThread]); + + [CATransaction begin]; + [CATransaction setDisableActions:YES]; + for (dispatch_block_t block : _scheduledBlocks) { + block(); + } + [CATransaction commit]; + _scheduledBlocks.clear(); +} + +- (void)blockUntilFrameAvailable { + std::unique_lock lock(_mutex); + + _beginResizeWaiting = YES; + + while (CGSizeEqualToSize(_contentSize, CGSizeZero) && !_shuttingDown) { + _condBlockBeginResize.wait(lock); + [self drain]; + } + + _beginResizeWaiting = NO; +} + +- (void)beginResize:(CGSize)size notify:(nonnull dispatch_block_t)notify { + std::unique_lock lock(_mutex); + + if (CGSizeEqualToSize(_contentSize, CGSizeZero) || _shuttingDown) { + // No blocking until framework produces at least one frame + notify(); + return; + } + + [CATransaction begin]; + [CATransaction setDisableActions:YES]; + + [self drain]; + + notify(); + + _contentSize = CGSizeMake(-1, -1); + + _beginResizeWaiting = YES; + + while (!CGSizeEqualToSize(_contentSize, size) && // + !CGSizeEqualToSize(_contentSize, CGSizeZero) && !_shuttingDown) { + _condBlockBeginResize.wait(lock); + [self drain]; + } + + _beginResizeWaiting = NO; + + [CATransaction commit]; +} + +- (void)performCommit:(CGSize)size notify:(nonnull dispatch_block_t)notify { + Event event; + { + std::unique_lock lock(_mutex); + Event& e = event; + _scheduledBlocks.push_back(^{ + notify(); + _contentSize = size; + e.Signal(); + }); + if (_beginResizeWaiting) { + _condBlockBeginResize.notify_all(); + } else { + dispatch_async(dispatch_get_main_queue(), ^{ + std::unique_lock lock(_mutex); + [self drain]; + }); + } + } + event.Wait(); +} + +- (void)shutdown { + std::unique_lock lock(_mutex); + _shuttingDown = YES; + _condBlockBeginResize.notify_all(); + [self drain]; +} + +@end diff --git a/shell/platform/darwin/macos/framework/Source/FlutterView.h b/shell/platform/darwin/macos/framework/Source/FlutterView.h index 2da2f48f34963..c17c5ce89ddce 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterView.h +++ b/shell/platform/darwin/macos/framework/Source/FlutterView.h @@ -3,9 +3,11 @@ // found in the LICENSE file. #import -#include -#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterResizableBackingStoreProvider.h" +#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h" +#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.h" + +#include /** * The view ID for APIs that don't support multi-view. @@ -49,21 +51,10 @@ constexpr uint64_t kFlutterDefaultViewId = 0; - (nonnull instancetype)init NS_UNAVAILABLE; /** - * Flushes the graphics context and flips the surfaces. Expected to be called on raster thread. - */ -- (void)present; - -/** - * Called when there is no Flutter content available to render. This must be passed to resize - * synchronizer. + * Returns SurfaceManager for this view. SurfaceManager is responsible for + * providing and presenting render surfaces. */ -- (void)presentWithoutContent; - -/** - * Ensures that a backing store with requested size exists and returns the descriptor. Expected to - * be called on raster thread. - */ -- (nonnull FlutterRenderBackingStore*)backingStoreForSize:(CGSize)size; +- (nonnull FlutterSurfaceManager*)surfaceManager; /** * Must be called when shutting down. Unblocks raster thread and prevents any further @@ -81,3 +72,13 @@ constexpr uint64_t kFlutterDefaultViewId = 0; - (void)setBackgroundColor:(nonnull NSColor*)color; @end + +@interface FlutterView (FlutterViewPrivate) + +/** + * Returns FlutterThreadSynchronizer for this view. + * Used for FlutterEngineTest. + */ +- (nonnull FlutterThreadSynchronizer*)threadSynchronizer; + +@end diff --git a/shell/platform/darwin/macos/framework/Source/FlutterView.mm b/shell/platform/darwin/macos/framework/Source/FlutterView.mm index 8a92b9f5fbb59..9991918153dfa 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterView.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterView.mm @@ -4,15 +4,15 @@ #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterView.h" -#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterResizeSynchronizer.h" #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h" +#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.h" #import -@interface FlutterView () { +@interface FlutterView () { __weak id _reshapeListener; - FlutterResizeSynchronizer* _resizeSynchronizer; - FlutterResizableBackingStoreProvider* _resizableBackingStoreProvider; + FlutterThreadSynchronizer* _threadSynchronizer; + FlutterSurfaceManager* _surfaceManager; } @end @@ -28,34 +28,30 @@ - (instancetype)initWithMTLDevice:(id)device [self setBackgroundColor:[NSColor blackColor]]; [self setLayerContentsRedrawPolicy:NSViewLayerContentsRedrawDuringViewResize]; _reshapeListener = reshapeListener; - _resizableBackingStoreProvider = - [[FlutterResizableBackingStoreProvider alloc] initWithDevice:device - commandQueue:commandQueue - layer:self.layer]; - _resizeSynchronizer = - [[FlutterResizeSynchronizer alloc] initWithDelegate:_resizableBackingStoreProvider]; + _threadSynchronizer = [[FlutterThreadSynchronizer alloc] init]; + _surfaceManager = [[FlutterSurfaceManager alloc] initWithDevice:device + commandQueue:commandQueue + layer:self.layer + delegate:self]; } return self; } -- (FlutterRenderBackingStore*)backingStoreForSize:(CGSize)size { - if ([_resizeSynchronizer shouldEnsureSurfaceForSize:size]) { - [_resizableBackingStoreProvider onBackingStoreResized:size]; - } - return [_resizableBackingStoreProvider backingStore]; +- (void)onPresent:(CGSize)frameSize withBlock:(dispatch_block_t)block { + [_threadSynchronizer performCommit:frameSize notify:block]; } -- (void)present { - [_resizeSynchronizer requestCommit]; +- (FlutterSurfaceManager*)surfaceManager { + return _surfaceManager; } -- (void)presentWithoutContent { - [_resizeSynchronizer noFlutterContent]; +- (FlutterThreadSynchronizer*)threadSynchronizer { + return _threadSynchronizer; } - (void)reshaped { CGSize scaledSize = [self convertSizeToBacking:self.bounds.size]; - [_resizeSynchronizer beginResize:scaledSize + [_threadSynchronizer beginResize:scaledSize notify:^{ [_reshapeListener viewDidReshape:self]; }]; @@ -111,7 +107,7 @@ - (void)viewDidChangeBackingProperties { } - (void)shutdown { - [_resizeSynchronizer shutdown]; + [_threadSynchronizer shutdown]; } #pragma mark - NSAccessibility overrides From a4a545ba646ad7a0039c72d5452b34da04a0d33a Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Mon, 28 Nov 2022 17:14:53 +0100 Subject: [PATCH 02/34] fix comments --- .../platform/darwin/macos/framework/Source/FlutterRenderer.mm | 4 ++-- .../darwin/macos/framework/Source/FlutterSurfaceManager.mm | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterRenderer.mm b/shell/platform/darwin/macos/framework/Source/FlutterRenderer.mm index a82c43a6a7c23..f8ff85e6f612d 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterRenderer.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterRenderer.mm @@ -98,8 +98,8 @@ - (FlutterMetalTexture)createTextureForView:(uint64_t)viewId size:(CGSize)size { FlutterSurface* surface = [view.surfaceManager surfaceForSize:size]; FlutterMetalTexture texture = surface.asFlutterMetalTexture; // This is here because the non-compositor path completely ignores - // destruction and user data callback. It is ugly, but the surface manager - // will keep the surface retained until present. + // destruction and callback. It is ugly, but the surface manager + // will keep the borrowed surface retained until present: is called. if (texture.destruction_callback != nullptr) { texture.destruction_callback(texture.user_data); } diff --git a/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.mm b/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.mm index 69c13cbe12bf2..ac9190fa82a17 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.mm @@ -30,7 +30,7 @@ - (nullable FlutterSurface*)removeSurfaceForSize:(CGSize)size { @synchronized(self) { for (FlutterSurface* surface in _surfaces) { if (CGSizeEqualToSize(surface.size, size)) { - // By default ARC doesn't retain enumartion iteration variables. + // By default ARC doesn't retain enumeration iteration variables. FlutterSurface* res = surface; [_surfaces removeObject:surface]; return res; From 2657127871e23b9b367971e5dda2e9c0fc0d1bfb Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Wed, 30 Nov 2022 12:59:47 +0100 Subject: [PATCH 03/34] Put includes and imports together --- .../platform/darwin/macos/framework/Source/FlutterCompositor.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterCompositor.h b/shell/platform/darwin/macos/framework/Source/FlutterCompositor.h index d0f4d6445816d..99983e8ddb9bf 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterCompositor.h +++ b/shell/platform/darwin/macos/framework/Source/FlutterCompositor.h @@ -9,11 +9,10 @@ #include #include "flutter/fml/macros.h" -#include "flutter/shell/platform/embedder/embedder.h" - #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterPlatformViewController.h" #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterView.h" #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterViewProvider.h" +#include "flutter/shell/platform/embedder/embedder.h" namespace flutter { From 5d9a08c1cfe34e0061c6442f74fa3efa5e87fc2f Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Thu, 1 Dec 2022 12:25:53 +0100 Subject: [PATCH 04/34] Update shell/platform/darwin/macos/framework/Source/FlutterCompositor.mm Co-authored-by: Chris Bracken --- .../platform/darwin/macos/framework/Source/FlutterCompositor.mm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterCompositor.mm b/shell/platform/darwin/macos/framework/Source/FlutterCompositor.mm index 3ed6307f631ae..a9e4b633514dd 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterCompositor.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterCompositor.mm @@ -44,7 +44,7 @@ NSMutableArray* surfaces = [NSMutableArray array]; for (size_t i = 0; i < layers_count; i++) { - FlutterLayer* layer = (FlutterLayer*)layers[i]; + FlutterLayer* layer = static_cast(layers[i]); if (layer->type == kFlutterLayerContentTypeBackingStore) { FlutterSurface* surface = [view.surfaceManager lookupSurface:&layer->backing_store->metal.texture]; From c6b54b90f63c5534c26e6dfc310c377664962a49 Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Thu, 1 Dec 2022 12:26:02 +0100 Subject: [PATCH 05/34] Update shell/platform/darwin/macos/framework/Source/FlutterCompositor.mm Co-authored-by: Chris Bracken --- .../platform/darwin/macos/framework/Source/FlutterCompositor.mm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterCompositor.mm b/shell/platform/darwin/macos/framework/Source/FlutterCompositor.mm index a9e4b633514dd..aa6a40d2e9e8e 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterCompositor.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterCompositor.mm @@ -61,7 +61,7 @@ [view.surfaceManager present:surfaces notify:^{ for (size_t i = 0; i < layers_count; i++) { - FlutterLayer* layer = (FlutterLayer*)layers[i]; + FlutterLayer* layer = static_cast(layers[i]); if (layer->type == kFlutterLayerContentTypePlatformView) { PresentPlatformView(view, layer, i); } From e184cea39b3048e99006a02a6364b2cce4910627 Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Thu, 1 Dec 2022 12:26:17 +0100 Subject: [PATCH 06/34] Update shell/platform/darwin/macos/framework/Source/FlutterCompositor.mm Co-authored-by: Chris Bracken --- .../darwin/macos/framework/Source/FlutterCompositor.mm | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterCompositor.mm b/shell/platform/darwin/macos/framework/Source/FlutterCompositor.mm index aa6a40d2e9e8e..0326425cb510c 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterCompositor.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterCompositor.mm @@ -62,8 +62,12 @@ notify:^{ for (size_t i = 0; i < layers_count; i++) { FlutterLayer* layer = static_cast(layers[i]); - if (layer->type == kFlutterLayerContentTypePlatformView) { + switch (layer->type) { + case kFlutterLayerContentTypeBackingStore: + break; + case kFlutterLayerContentTypePlatformView: PresentPlatformView(view, layer, i); + break; } } [platform_view_controller_ disposePlatformViews]; From 5ef610c2ed709b894fbcfa0fbe6db1a6899196b9 Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Thu, 1 Dec 2022 12:57:53 +0100 Subject: [PATCH 07/34] Update shell/platform/darwin/macos/framework/Source/FlutterPlatformViewController.mm Co-authored-by: Chris Bracken --- .../macos/framework/Source/FlutterPlatformViewController.mm | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterPlatformViewController.mm b/shell/platform/darwin/macos/framework/Source/FlutterPlatformViewController.mm index 60defbff7b740..ac01ef57bc1cf 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterPlatformViewController.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterPlatformViewController.mm @@ -53,7 +53,8 @@ - (void)onCreateWithViewID:(int64_t)viewId } NSView* platform_view = [factory createWithViewIdentifier:viewId arguments:nil]; - // Force view to be layer-backed. + // Flutter compositing requires CALayer-backed platform views. + // Force the platform view to be backed by a CALayer. [platform_view setWantsLayer:YES]; _platformViews[viewId] = platform_view; result(nil); From 96ef01c33926128889aee751af54e42a9f4a8f03 Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Thu, 1 Dec 2022 12:59:59 +0100 Subject: [PATCH 08/34] Update shell/platform/darwin/macos/framework/Source/FlutterRenderer.mm Co-authored-by: Chris Bracken --- shell/platform/darwin/macos/framework/Source/FlutterRenderer.mm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterRenderer.mm b/shell/platform/darwin/macos/framework/Source/FlutterRenderer.mm index f8ff85e6f612d..32e1086a97a00 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterRenderer.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterRenderer.mm @@ -118,7 +118,7 @@ - (BOOL)present:(uint64_t)viewId texture:(const FlutterMetalTexture*)texture { return NO; } FlutterSurfacePresentInfo* info = [[FlutterSurfacePresentInfo alloc] init]; - info.offset = CGPointMake(0, 0); + info.offset = CGPointZero; info.zIndex = 0; info.surface = surface; [view.surfaceManager present:@[ info ] notify:nil]; From 3fa4f7b61c3f7709b4f8c27a335558a629011877 Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Thu, 1 Dec 2022 12:42:56 +0100 Subject: [PATCH 09/34] Remove cast --- .../macos/framework/Source/FlutterCompositor.mm | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterCompositor.mm b/shell/platform/darwin/macos/framework/Source/FlutterCompositor.mm index 0326425cb510c..bf29d48326a5a 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterCompositor.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterCompositor.mm @@ -44,7 +44,7 @@ NSMutableArray* surfaces = [NSMutableArray array]; for (size_t i = 0; i < layers_count; i++) { - FlutterLayer* layer = static_cast(layers[i]); + const FlutterLayer* layer = layers[i]; if (layer->type == kFlutterLayerContentTypeBackingStore) { FlutterSurface* surface = [view.surfaceManager lookupSurface:&layer->backing_store->metal.texture]; @@ -61,13 +61,13 @@ [view.surfaceManager present:surfaces notify:^{ for (size_t i = 0; i < layers_count; i++) { - FlutterLayer* layer = static_cast(layers[i]); + const FlutterLayer* layer = layers[i]; switch (layer->type) { - case kFlutterLayerContentTypeBackingStore: - break; - case kFlutterLayerContentTypePlatformView: - PresentPlatformView(view, layer, i); - break; + case kFlutterLayerContentTypeBackingStore: + break; + case kFlutterLayerContentTypePlatformView: + PresentPlatformView(view, layer, i); + break; } } [platform_view_controller_ disposePlatformViews]; From 0676efe0a28ef5109d7bcd863dcd64caac526223 Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Thu, 1 Dec 2022 12:48:47 +0100 Subject: [PATCH 10/34] Do not manually add platform view layer to FlutterView --- .../darwin/macos/framework/Source/FlutterCompositor.mm | 1 - .../darwin/macos/framework/Source/FlutterEngineTest.mm | 5 +++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterCompositor.mm b/shell/platform/darwin/macos/framework/Source/FlutterCompositor.mm index bf29d48326a5a..0e6062db84e2e 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterCompositor.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterCompositor.mm @@ -92,7 +92,6 @@ layer->size.width / scale, layer->size.height / scale); if (platform_view.superview == nil) { [default_base_view addSubview:platform_view]; - [default_base_view.layer addSublayer:platform_view.layer]; } platform_view.layer.zPosition = layer_position; } diff --git a/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm b/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm index a1149386ff19d..fa4fff77fb962 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm @@ -490,8 +490,9 @@ - (nonnull NSView*)createWithViewIdentifier:(int64_t)viewId arguments:(nullable CALayer* rootLayer = viewController.flutterView.layer; - // There are three layers total - the root layer and two sublayers. - EXPECT_EQ(rootLayer.sublayers.count, 3u); + // There are two layers with Flutter contents and one view + EXPECT_EQ(rootLayer.sublayers.count, 2u); + EXPECT_EQ(viewController.flutterView.subviews.count, 1u); // TODO(gw280): add support for screenshot tests in this test harness From 33f11b678d1f58c26810dc73cdfc2765ffbed5d7 Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Thu, 1 Dec 2022 12:52:03 +0100 Subject: [PATCH 11/34] Remove unnecesary _Internal header files --- ci/licenses_golden/licenses_flutter | 2 - shell/platform/darwin/macos/BUILD.gn | 2 - .../macos/framework/Source/FlutterSurface.h | 13 ++++++ .../macos/framework/Source/FlutterSurface.mm | 2 +- .../framework/Source/FlutterSurfaceManager.h | 34 +++++++++++++++ .../framework/Source/FlutterSurfaceManager.mm | 4 +- .../Source/FlutterSurfaceManagerTest.mm | 4 +- .../Source/FlutterSurfaceManager_Internal.h | 41 ------------------- .../Source/FlutterSurface_Internal.h | 18 -------- 9 files changed, 52 insertions(+), 68 deletions(-) delete mode 100644 shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager_Internal.h delete mode 100644 shell/platform/darwin/macos/framework/Source/FlutterSurface_Internal.h diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index b288db7e2c57a..e4c6e82e0e05f 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -2742,8 +2742,6 @@ FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterSurfa FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.mm FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManagerTest.mm -FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager_Internal.h -FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterSurface_Internal.h FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterTextInputPlugin.h FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterTextInputPlugin.mm FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterTextInputPluginTest.mm diff --git a/shell/platform/darwin/macos/BUILD.gn b/shell/platform/darwin/macos/BUILD.gn index 3607af9d4e73f..6dc6b79a98146 100644 --- a/shell/platform/darwin/macos/BUILD.gn +++ b/shell/platform/darwin/macos/BUILD.gn @@ -90,8 +90,6 @@ source_set("flutter_framework_source") { "framework/Source/FlutterSurface.mm", "framework/Source/FlutterSurfaceManager.h", "framework/Source/FlutterSurfaceManager.mm", - "framework/Source/FlutterSurfaceManager_Internal.h", - "framework/Source/FlutterSurface_Internal.h", "framework/Source/FlutterTextInputPlugin.h", "framework/Source/FlutterTextInputPlugin.mm", "framework/Source/FlutterTextInputSemanticsObject.h", diff --git a/shell/platform/darwin/macos/framework/Source/FlutterSurface.h b/shell/platform/darwin/macos/framework/Source/FlutterSurface.h index 4570b3303d4cc..8c5c92f0b3815 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterSurface.h +++ b/shell/platform/darwin/macos/framework/Source/FlutterSurface.h @@ -15,3 +15,16 @@ - (FlutterMetalTexture)asFlutterMetalTexture; @end + +/** + * Internal FlutterSurface interface used by FlutterSurfaceManager. + */ +@interface FlutterSurface (Private) + +- (instancetype)initWithSize:(CGSize)size device:(id)device; + +@property(readonly, nonatomic) IOSurfaceRef ioSurface; +@property(readonly, nonatomic) CGSize size; +@property(readonly, nonatomic) int64_t textureId; + +@end diff --git a/shell/platform/darwin/macos/framework/Source/FlutterSurface.mm b/shell/platform/darwin/macos/framework/Source/FlutterSurface.mm index e9016b543d301..8980778d9ac11 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterSurface.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterSurface.mm @@ -2,7 +2,7 @@ // 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/FlutterSurface_Internal.h" +#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterSurface.h" #import diff --git a/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h b/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h index 0a7a1b3202cec..02e7522d7c990 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h +++ b/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h @@ -73,3 +73,37 @@ notify:(nullable dispatch_block_t)notify; @end + +/** + * Responsible for caching back buffers to prevent unnecessary IOSurface allocations. + */ +@interface FlutterBackBufferCache : NSObject + +/** + * Removes surface with given size from cache (if available) and returns it. + */ +- (nullable FlutterSurface*)removeSurfaceForSize:(CGSize)size; + +/** + * Removes all cached surface replacing them with new ones. + */ +- (void)replaceWith:(nonnull NSArray*)surfaces; + +/** + * Returns number of surfaces currently in cache. Used for tests. + */ +- (NSUInteger)count; + +@end + +/** + * Interface to internal properties used for testing. + */ +@interface FlutterSurfaceManager (Private) + +@property(readonly, nonatomic, nonnull) FlutterBackBufferCache* backBufferCache; +@property(readonly, nonatomic, nonnull) NSArray* borrowedSurfaces; +@property(readonly, nonatomic, nonnull) NSArray* frontSurfaces; +@property(readonly, nonatomic, nonnull) NSArray* layers; + +@end diff --git a/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.mm b/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.mm index ac9190fa82a17..e813a0b90de1f 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.mm @@ -2,8 +2,8 @@ // 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/FlutterSurfaceManager_Internal.h" -#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterSurface_Internal.h" +#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h" +#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterSurface.h" #import #include diff --git a/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManagerTest.mm b/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManagerTest.mm index d0184e596e8b9..727e36528d75a 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManagerTest.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManagerTest.mm @@ -6,8 +6,8 @@ #import #import -#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager_Internal.h" -#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterSurface_Internal.h" +#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h" +#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterSurface.h" #include "flutter/testing/testing.h" #include "gtest/gtest.h" diff --git a/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager_Internal.h b/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager_Internal.h deleted file mode 100644 index fda05cbf20623..0000000000000 --- a/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager_Internal.h +++ /dev/null @@ -1,41 +0,0 @@ -// Copyright 2013 The Flutter Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h" - -#import - -/** - * Responsible for caching back buffers to prevent unnecessary IOSurface allocations. - */ -@interface FlutterBackBufferCache : NSObject - -/** - * Removes surface with given size from cache (if available) and returns it. - */ -- (nullable FlutterSurface*)removeSurfaceForSize:(CGSize)size; - -/** - * Removes all cached surface replacing them with new ones. - */ -- (void)replaceWith:(nonnull NSArray*)surfaces; - -/** - * Returns number of surfaces currently in cache. Used for tests. - */ -- (NSUInteger)count; - -@end - -/** - * Interface to internal properties used for testing. - */ -@interface FlutterSurfaceManager () - -@property(readonly, nonatomic, nonnull) FlutterBackBufferCache* backBufferCache; -@property(readonly, nonatomic, nonnull) NSArray* borrowedSurfaces; -@property(readonly, nonatomic, nonnull) NSArray* frontSurfaces; -@property(readonly, nonatomic, nonnull) NSArray* layers; - -@end diff --git a/shell/platform/darwin/macos/framework/Source/FlutterSurface_Internal.h b/shell/platform/darwin/macos/framework/Source/FlutterSurface_Internal.h deleted file mode 100644 index 8367b9a44432f..0000000000000 --- a/shell/platform/darwin/macos/framework/Source/FlutterSurface_Internal.h +++ /dev/null @@ -1,18 +0,0 @@ -// Copyright 2013 The Flutter Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterSurface.h" - -/** - * Internal FlutterSurface interface used by FlutterSurfaceManager. - */ -@interface FlutterSurface () - -- (instancetype)initWithSize:(CGSize)size device:(id)device; - -@property(readonly, nonatomic) IOSurfaceRef ioSurface; -@property(readonly, nonatomic) CGSize size; -@property(readonly, nonatomic) int64_t textureId; - -@end From e8e6bc7ac52823369d8c6369486f4de406fc85bb Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Thu, 1 Dec 2022 12:53:45 +0100 Subject: [PATCH 12/34] Make surfaceManager a readonly property --- shell/platform/darwin/macos/framework/Source/FlutterView.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterView.h b/shell/platform/darwin/macos/framework/Source/FlutterView.h index c17c5ce89ddce..ba70833b46b76 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterView.h +++ b/shell/platform/darwin/macos/framework/Source/FlutterView.h @@ -54,7 +54,7 @@ constexpr uint64_t kFlutterDefaultViewId = 0; * Returns SurfaceManager for this view. SurfaceManager is responsible for * providing and presenting render surfaces. */ -- (nonnull FlutterSurfaceManager*)surfaceManager; +@property(readonly, nonatomic, nonnull) FlutterSurfaceManager* surfaceManager; /** * Must be called when shutting down. Unblocks raster thread and prevents any further From cb5507042b7a93fafba4ee3cc11a69181b62d1a2 Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Thu, 1 Dec 2022 12:54:50 +0100 Subject: [PATCH 13/34] Add comment --- shell/platform/darwin/macos/framework/Source/FlutterSurface.h | 1 + 1 file changed, 1 insertion(+) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterSurface.h b/shell/platform/darwin/macos/framework/Source/FlutterSurface.h index 8c5c92f0b3815..8f9604db78f48 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterSurface.h +++ b/shell/platform/darwin/macos/framework/Source/FlutterSurface.h @@ -18,6 +18,7 @@ /** * Internal FlutterSurface interface used by FlutterSurfaceManager. + * Wraps an IOSurface framebuffer and metadata related to the surface. */ @interface FlutterSurface (Private) From 18d0ce0c764490114c20b355a29edc7254488480 Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Thu, 1 Dec 2022 12:55:26 +0100 Subject: [PATCH 14/34] Style nit: Rewrite as a noun phrase. --- .../darwin/macos/framework/Source/FlutterSurfaceManager.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h b/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h index 02e7522d7c990..7d0d1d761eb36 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h +++ b/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h @@ -75,7 +75,7 @@ @end /** - * Responsible for caching back buffers to prevent unnecessary IOSurface allocations. + * Cache of back buffers to prevent unnecessary IOSurface allocations. */ @interface FlutterBackBufferCache : NSObject From c272e1fbc77842822117ab5076f9226bd324242c Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Thu, 1 Dec 2022 12:56:55 +0100 Subject: [PATCH 15/34] Naming nit for consistency with removeSurfaceForSize: --- .../darwin/macos/framework/Source/FlutterSurfaceManager.h | 2 +- .../darwin/macos/framework/Source/FlutterSurfaceManager.mm | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h b/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h index 7d0d1d761eb36..b2ad5958b43d3 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h +++ b/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h @@ -87,7 +87,7 @@ /** * Removes all cached surface replacing them with new ones. */ -- (void)replaceWith:(nonnull NSArray*)surfaces; +- (void)replaceSurfaces:(nonnull NSArray*)surfaces; /** * Returns number of surfaces currently in cache. Used for tests. diff --git a/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.mm b/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.mm index e813a0b90de1f..ab29ccd206e06 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.mm @@ -40,7 +40,7 @@ - (nullable FlutterSurface*)removeSurfaceForSize:(CGSize)size { } } -- (void)replaceWith:(nonnull NSArray*)surfaces { +- (void)replaceSurfaces:(nonnull NSArray*)surfaces { @synchronized(self) { [_surfaces removeAllObjects]; [_surfaces addObjectsFromArray:surfaces]; @@ -159,7 +159,7 @@ - (void)commit:(NSArray*)surfaces { [_borrowedSurfaces removeAllObjects]; // Release all unused back buffer surfaces and replace them with front surfaces. - [_backBufferCache replaceWith:_frontSurfaces]; + [_backBufferCache replaceSurfaces:_frontSurfaces]; // Front surfaces will be replaced by currently presented surfaces. [_frontSurfaces removeAllObjects]; From 1804415e157fd592a8af330dbe9fae34d241c556 Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Thu, 1 Dec 2022 12:57:41 +0100 Subject: [PATCH 16/34] Write as ternary conditional. --- .../darwin/macos/framework/Source/FlutterEngineTest.mm | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm b/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm index fa4fff77fb962..ad7535c83f157 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm @@ -35,11 +35,7 @@ @interface TestPlatformViewFactory : NSObject @implementation TestPlatformViewFactory - (nonnull NSView*)createWithViewIdentifier:(int64_t)viewId arguments:(nullable id)args { - if (viewId == 42) { - return [[NSView alloc] init]; - } else { - return nil; - } + return viewId == 42 ? [[NSView alloc] init] : nil; } @end From 0508b3632bc50f2ddcdc17bb25bb97a65743954d Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Thu, 1 Dec 2022 12:58:52 +0100 Subject: [PATCH 17/34] Fix plural in comment. --- .../darwin/macos/framework/Source/FlutterSurfaceManager.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h b/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h index b2ad5958b43d3..591996859935d 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h +++ b/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h @@ -85,7 +85,7 @@ - (nullable FlutterSurface*)removeSurfaceForSize:(CGSize)size; /** - * Removes all cached surface replacing them with new ones. + * Removes all cached surfaces replacing them with new ones. */ - (void)replaceSurfaces:(nonnull NSArray*)surfaces; From bc3147beb984b72c5e799062fca00bb3251cd9d9 Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Thu, 1 Dec 2022 13:14:15 +0100 Subject: [PATCH 18/34] Offset and index are already set to 0. --- shell/platform/darwin/macos/framework/Source/FlutterRenderer.mm | 2 -- 1 file changed, 2 deletions(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterRenderer.mm b/shell/platform/darwin/macos/framework/Source/FlutterRenderer.mm index 32e1086a97a00..dd021a6474537 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterRenderer.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterRenderer.mm @@ -118,8 +118,6 @@ - (BOOL)present:(uint64_t)viewId texture:(const FlutterMetalTexture*)texture { return NO; } FlutterSurfacePresentInfo* info = [[FlutterSurfacePresentInfo alloc] init]; - info.offset = CGPointZero; - info.zIndex = 0; info.surface = surface; [view.surfaceManager present:@[ info ] notify:nil]; return YES; From fa645a09e9f9162772dd85a5d48a13f36754fa8a Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Mon, 5 Dec 2022 17:53:00 +0100 Subject: [PATCH 19/34] Remove FlutterSurfaceManager lookupSurface And the associate bookkeeping of borrowed (lent) surfaces. --- .../framework/Source/FlutterCompositor.mm | 3 ++- .../framework/Source/FlutterCompositorTest.mm | 6 +---- .../macos/framework/Source/FlutterRenderer.mm | 10 +-------- .../framework/Source/FlutterRendererTest.mm | 8 +++---- .../macos/framework/Source/FlutterSurface.h | 6 +++-- .../macos/framework/Source/FlutterSurface.mm | 4 ++++ .../framework/Source/FlutterSurfaceManager.h | 7 ------ .../framework/Source/FlutterSurfaceManager.mm | 22 ------------------- .../Source/FlutterSurfaceManagerTest.mm | 18 ++++----------- 9 files changed, 19 insertions(+), 65 deletions(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterCompositor.mm b/shell/platform/darwin/macos/framework/Source/FlutterCompositor.mm index 0e6062db84e2e..d71f5accafab4 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterCompositor.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterCompositor.mm @@ -47,7 +47,8 @@ const FlutterLayer* layer = layers[i]; if (layer->type == kFlutterLayerContentTypeBackingStore) { FlutterSurface* surface = - [view.surfaceManager lookupSurface:&layer->backing_store->metal.texture]; + [FlutterSurface fromFlutterMetalTexture:&layer->backing_store->metal.texture]; + if (surface) { FlutterSurfacePresentInfo* info = [[FlutterSurfacePresentInfo alloc] init]; info.surface = surface; diff --git a/shell/platform/darwin/macos/framework/Source/FlutterCompositorTest.mm b/shell/platform/darwin/macos/framework/Source/FlutterCompositorTest.mm index 509902334b587..bd6bf4d57b58b 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterCompositorTest.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterCompositorTest.mm @@ -66,14 +66,10 @@ - (nullable FlutterView*)getView:(uint64_t)viewId { .struct_size = sizeof(FlutterMetalTexture), .texture_id = 1, .texture = (__bridge void*)textureMock, - .user_data = nullptr, + .user_data = (__bridge void*)surfaceMock, .destruction_callback = nullptr, }; - OCMStub([surfaceManagerMock lookupSurface:&texture]) - .ignoringNonObjectArgs() - .andReturn(surfaceMock); - OCMStub([surfaceManagerMock present:[OCMArg any] notify:[OCMArg any]]) .andDo(^(NSInvocation* invocation) { NSArray* info; diff --git a/shell/platform/darwin/macos/framework/Source/FlutterRenderer.mm b/shell/platform/darwin/macos/framework/Source/FlutterRenderer.mm index dd021a6474537..6e96f2e1b9713 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterRenderer.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterRenderer.mm @@ -97,14 +97,6 @@ - (FlutterMetalTexture)createTextureForView:(uint64_t)viewId size:(CGSize)size { } FlutterSurface* surface = [view.surfaceManager surfaceForSize:size]; FlutterMetalTexture texture = surface.asFlutterMetalTexture; - // This is here because the non-compositor path completely ignores - // destruction and callback. It is ugly, but the surface manager - // will keep the borrowed surface retained until present: is called. - if (texture.destruction_callback != nullptr) { - texture.destruction_callback(texture.user_data); - } - texture.destruction_callback = nullptr; - texture.user_data = nullptr; return texture; } @@ -113,7 +105,7 @@ - (BOOL)present:(uint64_t)viewId texture:(const FlutterMetalTexture*)texture { if (view == nil) { return NO; } - FlutterSurface* surface = [view.surfaceManager lookupSurface:texture]; + FlutterSurface* surface = [FlutterSurface fromFlutterMetalTexture:texture]; if (surface == nil) { return NO; } diff --git a/shell/platform/darwin/macos/framework/Source/FlutterRendererTest.mm b/shell/platform/darwin/macos/framework/Source/FlutterRendererTest.mm index 55025d838c9a6..01146e1464c02 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterRendererTest.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterRendererTest.mm @@ -46,11 +46,9 @@ void SetEngineDefaultView(FlutterEngine* engine, id flutterView) { id surfaceMock = OCMClassMock([FlutterSurface class]); - FlutterMetalTexture texture = {}; - - OCMStub([surfaceManagerMock lookupSurface:&texture]) - .ignoringNonObjectArgs() - .andReturn(surfaceMock); + FlutterMetalTexture texture = { + .user_data = (__bridge void*)surfaceMock, + }; [[surfaceManagerMock expect] present:[OCMArg checkWithBlock:^(id obj) { NSArray* array = (NSArray*)obj; diff --git a/shell/platform/darwin/macos/framework/Source/FlutterSurface.h b/shell/platform/darwin/macos/framework/Source/FlutterSurface.h index 8f9604db78f48..697bd29dee81c 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterSurface.h +++ b/shell/platform/darwin/macos/framework/Source/FlutterSurface.h @@ -14,6 +14,8 @@ - (FlutterMetalTexture)asFlutterMetalTexture; ++ (nullable FlutterSurface*)fromFlutterMetalTexture:(nonnull const FlutterMetalTexture*)texture; + @end /** @@ -22,9 +24,9 @@ */ @interface FlutterSurface (Private) -- (instancetype)initWithSize:(CGSize)size device:(id)device; +- (nonnull instancetype)initWithSize:(CGSize)size device:(nonnull id)device; -@property(readonly, nonatomic) IOSurfaceRef ioSurface; +@property(readonly, nonatomic, nonnull) IOSurfaceRef ioSurface; @property(readonly, nonatomic) CGSize size; @property(readonly, nonatomic) int64_t textureId; diff --git a/shell/platform/darwin/macos/framework/Source/FlutterSurface.mm b/shell/platform/darwin/macos/framework/Source/FlutterSurface.mm index 8980778d9ac11..4b65f2553c315 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterSurface.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterSurface.mm @@ -53,6 +53,10 @@ - (FlutterMetalTexture)asFlutterMetalTexture { return res; } ++ (FlutterSurface*)fromFlutterMetalTexture:(const FlutterMetalTexture*)texture { + return (__bridge FlutterSurface*)texture->user_data; +} + - (void)dealloc { CFRelease(_ioSurface); } diff --git a/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h b/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h index 591996859935d..a8c6241a5b382 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h +++ b/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h @@ -53,12 +53,6 @@ */ - (nonnull FlutterSurface*)surfaceForSize:(CGSize)size; -/** - * Looks up surface for given metal texture. Can only be called for surfaces - * obtained through `surfaceForSize:` until `present:` is called. - */ -- (nullable FlutterSurface*)lookupSurface:(nonnull const FlutterMetalTexture*)texture; - /** * Sets the provided surfaces as contents of FlutterView. Will create, update and * remove sublayers as needed. @@ -102,7 +96,6 @@ @interface FlutterSurfaceManager (Private) @property(readonly, nonatomic, nonnull) FlutterBackBufferCache* backBufferCache; -@property(readonly, nonatomic, nonnull) NSArray* borrowedSurfaces; @property(readonly, nonatomic, nonnull) NSArray* frontSurfaces; @property(readonly, nonatomic, nonnull) NSArray* layers; diff --git a/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.mm b/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.mm index ab29ccd206e06..5607e63d4d9d2 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.mm @@ -86,11 +86,6 @@ @interface FlutterSurfaceManager () { // present and replaced by current frong surfaces. FlutterBackBufferCache* _backBufferCache; - // Surfaces that currently obtained through surfaceForSize waiting to be - // presented. This is used to keep the surface alive because the non compositor - // codepath doesn't properly retain the surface. - NSMutableArray* _borrowedSurfaces; - // Surfaces currently used to back visible layers. NSMutableArray* _frontSurfaces; @@ -112,7 +107,6 @@ - (instancetype)initWithDevice:(id)device _delegate = delegate; _backBufferCache = [[FlutterBackBufferCache alloc] init]; - _borrowedSurfaces = [NSMutableArray array]; _frontSurfaces = [NSMutableArray array]; _layers = [NSMutableArray array]; } @@ -123,10 +117,6 @@ - (FlutterBackBufferCache*)backBufferCache { return _backBufferCache; } -- (NSArray*)borrowedSurfaces { - return _borrowedSurfaces; -} - - (NSArray*)frontSurfaces { return _frontSurfaces; } @@ -135,29 +125,17 @@ - (NSArray*)layers { return _layers; } -- (FlutterSurface*)lookupSurface:(nonnull const FlutterMetalTexture*)texture { - for (FlutterSurface* surface in _borrowedSurfaces) { - if (surface.textureId == texture->texture_id) { - return surface; - } - } - return nil; -} - - (FlutterSurface*)surfaceForSize:(CGSize)size { FlutterSurface* res = [_backBufferCache removeSurfaceForSize:size]; if (res == nil) { res = [[FlutterSurface alloc] initWithSize:size device:_device]; } - [_borrowedSurfaces addObject:res]; return res; } - (void)commit:(NSArray*)surfaces { assert([NSThread isMainThread]); - [_borrowedSurfaces removeAllObjects]; - // Release all unused back buffer surfaces and replace them with front surfaces. [_backBufferCache replaceSurfaces:_frontSurfaces]; diff --git a/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManagerTest.mm b/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManagerTest.mm index 727e36528d75a..76393adb7b0e1 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManagerTest.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManagerTest.mm @@ -6,8 +6,8 @@ #import #import -#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h" #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterSurface.h" +#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h" #include "flutter/testing/testing.h" #include "gtest/gtest.h" @@ -82,17 +82,12 @@ - (void)onPresent:(CGSize)frameSize withBlock:(nonnull dispatch_block_t)block { auto texture = surface.asFlutterMetalTexture; texture.destruction_callback(texture.user_data); - FlutterMetalTexture dummyTexture{.texture_id = 1, .texture = nullptr}; - auto surface1 = [surfaceManager lookupSurface:&dummyTexture]; + FlutterMetalTexture dummyTexture{.texture_id = 1, .texture = nullptr, .user_data = nullptr}; + auto surface1 = [FlutterSurface fromFlutterMetalTexture:&dummyTexture]; EXPECT_EQ(surface1, nil); - auto surface2 = [surfaceManager lookupSurface:&texture]; + auto surface2 = [FlutterSurface fromFlutterMetalTexture:&texture]; EXPECT_EQ(surface2, surface); - - [surfaceManager present:@[ CreatePresentInfo(surface) ] notify:nil]; - - auto surface3 = [surfaceManager lookupSurface:&texture]; - EXPECT_EQ(surface3, nil); } TEST(FlutterSurfaceManager, BackBufferCacheDoesNotLeak) { @@ -130,7 +125,6 @@ - (void)onPresent:(CGSize)frameSize withBlock:(nonnull dispatch_block_t)block { TestView* testView = [[TestView alloc] init]; FlutterSurfaceManager* surfaceManager = CreateSurfaceManager(testView); - EXPECT_EQ(surfaceManager.borrowedSurfaces.count, 0ul); EXPECT_EQ(surfaceManager.frontSurfaces.count, 0ul); // Get first surface and present it. @@ -139,13 +133,11 @@ - (void)onPresent:(CGSize)frameSize withBlock:(nonnull dispatch_block_t)block { EXPECT_TRUE(CGSizeEqualToSize(surface1.size, CGSizeMake(100, 100))); EXPECT_EQ(surfaceManager.backBufferCache.count, 0ul); - EXPECT_EQ(surfaceManager.borrowedSurfaces.count, 1ul); EXPECT_EQ(surfaceManager.frontSurfaces.count, 0ul); [surfaceManager present:@[ CreatePresentInfo(surface1) ] notify:nil]; EXPECT_EQ(surfaceManager.backBufferCache.count, 0ul); - EXPECT_EQ(surfaceManager.borrowedSurfaces.count, 0ul); EXPECT_EQ(surfaceManager.frontSurfaces.count, 1ul); EXPECT_EQ(testView.layer.sublayers.count, 1ul); @@ -155,13 +147,11 @@ - (void)onPresent:(CGSize)frameSize withBlock:(nonnull dispatch_block_t)block { EXPECT_TRUE(CGSizeEqualToSize(surface2.size, CGSizeMake(100, 100))); EXPECT_EQ(surfaceManager.backBufferCache.count, 0ul); - EXPECT_EQ(surfaceManager.borrowedSurfaces.count, 1ul); [surfaceManager present:@[ CreatePresentInfo(surface2) ] notify:nil]; // Check that current front surface returns to cache. EXPECT_EQ(surfaceManager.backBufferCache.count, 1ul); - EXPECT_EQ(surfaceManager.borrowedSurfaces.count, 0ul); EXPECT_EQ(surfaceManager.frontSurfaces.count, 1ul); EXPECT_EQ(testView.layer.sublayers.count, 1ull); From 1dce078be00b19cf1a8a7d095d49f70ecfaf6010 Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Tue, 13 Dec 2022 19:28:26 +0100 Subject: [PATCH 20/34] Update shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.mm Co-authored-by: Chris Bracken --- .../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 98b64ef1efb9b..f8f1468c7da31 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.mm @@ -18,7 +18,7 @@ @interface FlutterThreadSynchronizer () { @end namespace { -/// Single use event (can only be signalled once) +/// Single use event (can only be signalled once). class Event { public: void Signal() { From 5971c822811084114a1213f27aa5f9a6e913e9c7 Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Tue, 13 Dec 2022 19:28:56 +0100 Subject: [PATCH 21/34] Update shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.mm Co-authored-by: Chris Bracken --- .../darwin/macos/framework/Source/FlutterSurfaceManager.mm | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.mm b/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.mm index 5607e63d4d9d2..4a78aad236ca7 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.mm @@ -92,6 +92,12 @@ @interface FlutterSurfaceManager () { // Currently visible layers. NSMutableArray* _layers; } + +/** + * Updates underlying CALayers with the contents of the surfaces to present. + */ +- (void)commit:(NSArray*)surfaces + @end @implementation FlutterSurfaceManager From 9902324c3fc3c92b245957e1df63647e21d49e14 Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Tue, 13 Dec 2022 19:29:49 +0100 Subject: [PATCH 22/34] Update shell/platform/darwin/macos/framework/Source/FlutterSurfaceManagerTest.mm Co-authored-by: Chris Bracken --- .../darwin/macos/framework/Source/FlutterSurfaceManagerTest.mm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManagerTest.mm b/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManagerTest.mm index 76393adb7b0e1..0a5e72648e024 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManagerTest.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManagerTest.mm @@ -58,7 +58,7 @@ - (void)onPresent:(CGSize)frameSize withBlock:(nonnull dispatch_block_t)block { return res; } -TEST(FlutterSurfaceManager, MetalTexture) { +TEST(FlutterSurfaceManager, MetalTextureSizeMatchesSurfaceSize) { TestView* testView = [[TestView alloc] init]; FlutterSurfaceManager* surfaceManager = CreateSurfaceManager(testView); From e077a6c46831e92f104da3449d08fbbe875fcf28 Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Tue, 13 Dec 2022 19:31:46 +0100 Subject: [PATCH 23/34] Update shell/platform/darwin/macos/framework/Source/FlutterRenderer.mm Co-authored-by: Chris Bracken --- .../platform/darwin/macos/framework/Source/FlutterRenderer.mm | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterRenderer.mm b/shell/platform/darwin/macos/framework/Source/FlutterRenderer.mm index 6e96f2e1b9713..007a3cce157a5 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterRenderer.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterRenderer.mm @@ -95,9 +95,7 @@ - (FlutterMetalTexture)createTextureForView:(uint64_t)viewId size:(CGSize)size { // FlutterMetalTexture has texture `null`, therefore is discarded. return FlutterMetalTexture{}; } - FlutterSurface* surface = [view.surfaceManager surfaceForSize:size]; - FlutterMetalTexture texture = surface.asFlutterMetalTexture; - return texture; + return [view.surfaceManager surfaceForSize:size].asFlutterMetalTexture; } - (BOOL)present:(uint64_t)viewId texture:(const FlutterMetalTexture*)texture { From cb7c4b7425c83d085eff413d831b781f30eafe60 Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Tue, 13 Dec 2022 19:32:21 +0100 Subject: [PATCH 24/34] Update shell/platform/darwin/macos/framework/Source/FlutterSurfaceManagerTest.mm Co-authored-by: Chris Bracken --- .../darwin/macos/framework/Source/FlutterSurfaceManagerTest.mm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManagerTest.mm b/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManagerTest.mm index 0a5e72648e024..1666cd92dede3 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManagerTest.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManagerTest.mm @@ -71,7 +71,7 @@ - (void)onPresent:(CGSize)frameSize withBlock:(nonnull dispatch_block_t)block { texture.destruction_callback(texture.user_data); } -TEST(FlutterSurfaceManager, TestLookup) { +TEST(FlutterSurfaceManager, TestSurfaceLookupFromTexture) { TestView* testView = [[TestView alloc] init]; FlutterSurfaceManager* surfaceManager = CreateSurfaceManager(testView); From a13d260f54a7830bedaaff7f89e7506f97495323 Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Tue, 13 Dec 2022 19:32:47 +0100 Subject: [PATCH 25/34] Update shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h Co-authored-by: Chris Bracken --- .../macos/framework/Source/FlutterSurfaceManager.h | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h b/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h index a8c6241a5b382..a270da7e33eaa 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h +++ b/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h @@ -57,11 +57,10 @@ * Sets the provided surfaces as contents of FlutterView. Will create, update and * remove sublayers as needed. * - * Must be called on raster thread. This will schedule a commit on platform - * thread and block raster thread until the commit is done. - * `notify` block will be invoked on platform thread and can be used to perform - * additional work, such as mutating platform views. It is guaranteed be called in - * same CATransaction. + * Must be called on raster thread. This will schedule a commit on the platform thread and block the + * raster thread until the commit is done. The `notify` block will be invoked on the platform thread + * 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; From e25e5028383c7bd416b1aa679b0a71038f3bd5d3 Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Tue, 13 Dec 2022 19:33:00 +0100 Subject: [PATCH 26/34] Update shell/platform/darwin/macos/framework/Source/FlutterSurfaceManagerTest.mm Co-authored-by: Chris Bracken --- .../darwin/macos/framework/Source/FlutterSurfaceManagerTest.mm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManagerTest.mm b/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManagerTest.mm index 1666cd92dede3..058913ae2e6f8 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManagerTest.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManagerTest.mm @@ -121,7 +121,7 @@ - (void)onPresent:(CGSize)frameSize withBlock:(nonnull dispatch_block_t)block { EXPECT_EQ(surfaceManager.backBufferCache.count, 0ul); } -TEST(FlutterSurfaceManager, SurfaceCycle) { +TEST(FlutterSurfaceManager, SurfacesAreRecycled) { TestView* testView = [[TestView alloc] init]; FlutterSurfaceManager* surfaceManager = CreateSurfaceManager(testView); From 117b15c32ce8d17c54c650b683469ba54b7d5696 Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Tue, 13 Dec 2022 19:33:12 +0100 Subject: [PATCH 27/34] Update shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h Co-authored-by: Chris Bracken --- .../darwin/macos/framework/Source/FlutterSurfaceManager.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h b/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h index a270da7e33eaa..ecc2847e834b7 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h +++ b/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h @@ -21,9 +21,9 @@ @protocol FlutterSurfaceManagerDelegate /* - * Schedules the block on platform thread and waits until the block is executed. - * Provided `frameSize` is used to unblock platform thread if it waits for - * certain frame size during resizing. + * Schedules the block on the platform thread and blocks until the block is executed. + * Provided `frameSize` is used to unblock the platform thread if it waits for + * a certain frame size during resizing. */ - (void)onPresent:(CGSize)frameSize withBlock:(nonnull dispatch_block_t)block; From 6bab84a789e8fb4117ba1651d45a376cddb5e3c4 Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Tue, 13 Dec 2022 19:33:31 +0100 Subject: [PATCH 28/34] Update shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h Co-authored-by: Chris Bracken --- .../darwin/macos/framework/Source/FlutterSurfaceManager.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h b/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h index ecc2847e834b7..a1d13b9c49997 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h +++ b/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h @@ -46,8 +46,8 @@ delegate:(nonnull id)delegate; /** - * Returns back buffer surface for given size that Flutter can render content to. - * Will return cached surface if one is available, or create new one otherwise. + * Returns a back buffer surface of the given size to which Flutter can render content. + * A cached surface will be returned if available; otherwise a new one will be created. * * Must be called on raster thread. */ From 10275f78d8bee197ec88e603c1fe32c26505658c Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Tue, 13 Dec 2022 19:33:54 +0100 Subject: [PATCH 29/34] Update shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h Co-authored-by: Chris Bracken --- .../darwin/macos/framework/Source/FlutterSurfaceManager.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h b/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h index a1d13b9c49997..da1e2bcc9b988 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h +++ b/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h @@ -30,9 +30,10 @@ @end /** + * FlutterSurfaceManager is responsible for providing and presenting Core Animation render + * surfaces and managing sublayers. + * * Owned by `FlutterView`. - * Responsible for providing surfaces for Flutter to render into and subsequent - * presentation of these surfaces. Manages core animation sublayers. */ @interface FlutterSurfaceManager : NSObject From 849a2735b6f7c4e5beb08477debb37d2261a8297 Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Tue, 13 Dec 2022 19:38:54 +0100 Subject: [PATCH 30/34] Fix build error --- .../darwin/macos/framework/Source/FlutterSurfaceManager.mm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.mm b/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.mm index 4a78aad236ca7..9f52900ae2dc1 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.mm @@ -96,7 +96,7 @@ @interface FlutterSurfaceManager () { /** * Updates underlying CALayers with the contents of the surfaces to present. */ -- (void)commit:(NSArray*)surfaces +- (void)commit:(NSArray*)surfaces; @end From 71649f3487141d2d6020e3cde9bb91cdf5b9687f Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Tue, 13 Dec 2022 19:56:39 +0100 Subject: [PATCH 31/34] Address nits --- .../framework/Source/FlutterSurfaceManager.mm | 142 +++++++++--------- 1 file changed, 70 insertions(+), 72 deletions(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.mm b/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.mm index 9f52900ae2dc1..29223f8180e3e 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.mm @@ -8,71 +8,6 @@ #import #include -// Cached back buffers will be released after kIdleDelay if there is no activity. -static const double kIdleDelay = 1.0; - -@interface FlutterBackBufferCache () { - NSMutableArray* _surfaces; -} - -@end - -@implementation FlutterBackBufferCache - -- (instancetype)init { - if (self = [super init]) { - self->_surfaces = [[NSMutableArray alloc] init]; - } - return self; -} - -- (nullable FlutterSurface*)removeSurfaceForSize:(CGSize)size { - @synchronized(self) { - for (FlutterSurface* surface in _surfaces) { - if (CGSizeEqualToSize(surface.size, size)) { - // By default ARC doesn't retain enumeration iteration variables. - FlutterSurface* res = surface; - [_surfaces removeObject:surface]; - return res; - } - } - return nil; - } -} - -- (void)replaceSurfaces:(nonnull NSArray*)surfaces { - @synchronized(self) { - [_surfaces removeAllObjects]; - [_surfaces addObjectsFromArray:surfaces]; - } - - // performSelector:withObject:afterDelay needs to be performed on RunLoop thread - [self performSelectorOnMainThread:@selector(reschedule) withObject:nil waitUntilDone:NO]; -} - -- (NSUInteger)count { - @synchronized(self) { - return _surfaces.count; - } -} - -- (void)onIdle { - @synchronized(self) { - [_surfaces removeAllObjects]; - } -} - -- (void)reschedule { - [NSObject cancelPreviousPerformRequestsWithTarget:self selector:@selector(onIdle) object:nil]; - [self performSelector:@selector(onIdle) withObject:nil afterDelay:kIdleDelay]; -} - -- (void)dealloc { - [NSObject cancelPreviousPerformRequestsWithTarget:self selector:@selector(onIdle) object:nil]; -} - -@end - @implementation FlutterSurfacePresentInfo @end @@ -132,11 +67,11 @@ - (NSArray*)layers { } - (FlutterSurface*)surfaceForSize:(CGSize)size { - FlutterSurface* res = [_backBufferCache removeSurfaceForSize:size]; - if (res == nil) { - res = [[FlutterSurface alloc] initWithSize:size device:_device]; + FlutterSurface* surface = [_backBufferCache removeSurfaceForSize:size]; + if (surface == nil) { + surface = [[FlutterSurface alloc] initWithSize:size device:_device]; } - return res; + return surface; } - (void)commit:(NSArray*)surfaces { @@ -151,13 +86,11 @@ - (void)commit:(NSArray*)surfaces { [_frontSurfaces addObject:info.surface]; } - // Remove excess layers. + // Add or remove layers to match the count of surfaces to present. while (_layers.count > _frontSurfaces.count) { [_layers.lastObject removeFromSuperlayer]; [_layers removeLastObject]; } - - // Add missing layers. while (_layers.count < _frontSurfaces.count) { CALayer* layer = [CALayer layer]; [_containingLayer addSublayer:layer]; @@ -198,3 +131,68 @@ - (void)present:(NSArray*)surfaces notify:(dispatch_ } @end + +// Cached back buffers will be released after kIdleDelay if there is no activity. +static const double kIdleDelay = 1.0; + +@interface FlutterBackBufferCache () { + NSMutableArray* _surfaces; +} + +@end + +@implementation FlutterBackBufferCache + +- (instancetype)init { + if (self = [super init]) { + self->_surfaces = [[NSMutableArray alloc] init]; + } + return self; +} + +- (nullable FlutterSurface*)removeSurfaceForSize:(CGSize)size { + @synchronized(self) { + for (FlutterSurface* surface in _surfaces) { + if (CGSizeEqualToSize(surface.size, size)) { + // By default ARC doesn't retain enumeration iteration variables. + FlutterSurface* res = surface; + [_surfaces removeObject:surface]; + return res; + } + } + return nil; + } +} + +- (void)replaceSurfaces:(nonnull NSArray*)surfaces { + @synchronized(self) { + [_surfaces removeAllObjects]; + [_surfaces addObjectsFromArray:surfaces]; + } + + // performSelector:withObject:afterDelay needs to be performed on RunLoop thread + [self performSelectorOnMainThread:@selector(reschedule) withObject:nil waitUntilDone:NO]; +} + +- (NSUInteger)count { + @synchronized(self) { + return _surfaces.count; + } +} + +- (void)onIdle { + @synchronized(self) { + [_surfaces removeAllObjects]; + } +} + +- (void)reschedule { + [NSObject cancelPreviousPerformRequestsWithTarget:self selector:@selector(onIdle) object:nil]; + [self performSelector:@selector(onIdle) withObject:nil afterDelay:kIdleDelay]; +} + +- (void)dealloc { + [NSObject cancelPreviousPerformRequestsWithTarget:self selector:@selector(onIdle) object:nil]; +} + +@end From 77b5209608c8f21349e2b4efa3336cc99fdc7386 Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Tue, 13 Dec 2022 20:09:33 +0100 Subject: [PATCH 32/34] Replace EVent with fml::AutoResetWaitableEvent --- .../Source/FlutterThreadSynchronizer.mm | 30 ++----------------- 1 file changed, 3 insertions(+), 27 deletions(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.mm b/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.mm index f8f1468c7da31..12ff75885f505 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.mm @@ -1,4 +1,5 @@ #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.h" +#import "fml/synchronization/waitable_event.h" #import #include @@ -17,31 +18,6 @@ @interface FlutterThreadSynchronizer () { @end -namespace { -/// Single use event (can only be signalled once). -class Event { - public: - void Signal() { - assert(!signaled_); - std::scoped_lock locker(mutex_); - signaled_ = true; - cv_.notify_one(); - } - - void Wait() { - std::unique_lock locker(mutex_); - while (!signaled_) { - cv_.wait(locker); - } - } - - private: - std::condition_variable cv_; - std::mutex mutex_; - bool signaled_ = false; -}; -} // namespace - @implementation FlutterThreadSynchronizer - (void)drain { @@ -101,10 +77,10 @@ - (void)beginResize:(CGSize)size notify:(nonnull dispatch_block_t)notify { } - (void)performCommit:(CGSize)size notify:(nonnull dispatch_block_t)notify { - Event event; + fml::AutoResetWaitableEvent event; { std::unique_lock lock(_mutex); - Event& e = event; + fml::AutoResetWaitableEvent& e = event; _scheduledBlocks.push_back(^{ notify(); _contentSize = size; From 768aca824cdfcd15783e921cedec3adccac8866e Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Tue, 13 Dec 2022 20:09:43 +0100 Subject: [PATCH 33/34] Remove unecessary CATransaction --- .../macos/framework/Source/FlutterThreadSynchronizer.mm | 5 ----- 1 file changed, 5 deletions(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.mm b/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.mm index 12ff75885f505..10f9ae53e5cce 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.mm @@ -54,9 +54,6 @@ - (void)beginResize:(CGSize)size notify:(nonnull dispatch_block_t)notify { return; } - [CATransaction begin]; - [CATransaction setDisableActions:YES]; - [self drain]; notify(); @@ -72,8 +69,6 @@ - (void)beginResize:(CGSize)size notify:(nonnull dispatch_block_t)notify { } _beginResizeWaiting = NO; - - [CATransaction commit]; } - (void)performCommit:(CGSize)size notify:(nonnull dispatch_block_t)notify { From f981e60e0bbd1c6a938e810668f73f96c44f58fa Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Tue, 13 Dec 2022 20:24:15 +0100 Subject: [PATCH 34/34] Add GetRequiredFrameSize --- .../framework/Source/FlutterSurfaceManager.mm | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.mm b/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.mm index 29223f8180e3e..50d7be11117da 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.mm @@ -109,17 +109,22 @@ - (void)commit:(NSArray*)surfaces { } } +static CGSize GetRequiredFrameSize(NSArray* surfaces) { + CGSize size = CGSizeZero; + for (FlutterSurfacePresentInfo* info in surfaces) { + size = CGSizeMake(std::max(size.width, info.offset.x + info.surface.size.width), + std::max(size.height, info.offset.y + info.surface.size.height)); + } + return size; +} + - (void)present:(NSArray*)surfaces notify:(dispatch_block_t)notify { id commandBuffer = [_commandQueue commandBuffer]; [commandBuffer commit]; [commandBuffer waitUntilScheduled]; // Get the actual dimensions of the frame (relevant for thread synchronizer). - CGSize size = CGSizeZero; - for (FlutterSurfacePresentInfo* info in surfaces) { - size = CGSizeMake(std::max(size.width, info.offset.x + info.surface.size.width), - std::max(size.height, info.offset.y + info.surface.size.height)); - } + CGSize size = GetRequiredFrameSize(surfaces); [_delegate onPresent:size withBlock:^{