From d0522a278e1a4e0ab333af5adde757255f4fceb7 Mon Sep 17 00:00:00 2001 From: Chris Bracken Date: Wed, 16 Nov 2022 19:57:03 -0800 Subject: [PATCH] [macOS] Merge FlutterSurfaceManager and impls Previously, FlutterSurfaceManager was a protocol with two concrete implementations: FlutterGLSurfaceManager and FlutterMetalSurfaceManager. Most of the implementation was in a shared superclass, FlutterIOSurfaceManager, which called into the OpenGL or Metal-specific subclass when backend-specific operations (such as allocating textures) was required. It did so via a delegate pattern, wherein the subclasses both implemented the FlutterIOSurfaceManagerDelegate protocol that exposed the backend-specific functionality. Now that only the Metal implementation remains, the delegate code can be inlined into the calling functions, and the class hierarchy can be squashed into a single concrete implementation class, FlutterSurfaceManager, similar to how it was originally implemented in https://github.com/flutter/engine/pull/21525 before we had two backends. Issue: https://github.com/flutter/flutter/issues/108304 Issue: https://github.com/flutter/flutter/issues/114445 --- ci/licenses_golden/licenses_flutter | 1 + shell/platform/darwin/macos/BUILD.gn | 2 +- .../FlutterResizableBackingStoreProvider.mm | 8 +- .../framework/Source/FlutterSurfaceManager.h | 84 ++++-------------- .../framework/Source/FlutterSurfaceManager.mm | 87 ++++++++----------- ...erTest.mm => FlutterSurfaceManagerTest.mm} | 16 ++-- 6 files changed, 65 insertions(+), 133 deletions(-) rename shell/platform/darwin/macos/framework/Source/{FlutterMetalSurfaceManagerTest.mm => FlutterSurfaceManagerTest.mm} (75%) diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index b9363219a93d3..345bb68f916d1 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -2641,6 +2641,7 @@ FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterResiz FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterResizeSynchronizer.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/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 a7b62727ff526..7e75603d6c6f7 100644 --- a/shell/platform/darwin/macos/BUILD.gn +++ b/shell/platform/darwin/macos/BUILD.gn @@ -178,9 +178,9 @@ executable("flutter_desktop_darwin_unittests") { "framework/Source/FlutterKeyboardManagerUnittests.mm", "framework/Source/FlutterMenuPluginTest.mm", "framework/Source/FlutterMetalRendererTest.mm", - "framework/Source/FlutterMetalSurfaceManagerTest.mm", "framework/Source/FlutterPlatformNodeDelegateMacTest.mm", "framework/Source/FlutterPlatformViewControllerTest.mm", + "framework/Source/FlutterSurfaceManagerTest.mm", "framework/Source/FlutterTextInputPluginTest.mm", "framework/Source/FlutterTextInputSemanticsObjectTest.mm", "framework/Source/FlutterViewControllerTest.mm", diff --git a/shell/platform/darwin/macos/framework/Source/FlutterResizableBackingStoreProvider.mm b/shell/platform/darwin/macos/framework/Source/FlutterResizableBackingStoreProvider.mm index 620c8dbf8cddb..fc07530c375a0 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterResizableBackingStoreProvider.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterResizableBackingStoreProvider.mm @@ -11,7 +11,7 @@ @implementation FlutterMetalResizableBackingStoreProvider { id _device; id _commandQueue; - id _surfaceManager; + FlutterSurfaceManager* _surfaceManager; } - (instancetype)initWithDevice:(id)device @@ -21,9 +21,9 @@ - (instancetype)initWithDevice:(id)device if (self) { _device = device; _commandQueue = commandQueue; - _surfaceManager = [[FlutterMetalSurfaceManager alloc] initWithDevice:device - commandQueue:commandQueue - layer:layer]; + _surfaceManager = [[FlutterSurfaceManager alloc] initWithDevice:device + commandQueue:commandQueue + layer:layer]; } return self; } diff --git a/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h b/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h index cc5ab327823f4..2f6a0974e4164 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h +++ b/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.h @@ -9,88 +9,36 @@ #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterIOSurfaceHolder.h" /** - * Manages the render surfaces and their corresponding backing stores. - */ -@protocol FlutterSurfaceManager - -/** - * Updates the backing store size of the managed IOSurfaces to `size`. If the surfaces are already - * of the same size, this is a no-op. - */ -- (void)ensureSurfaceSize:(CGSize)size; - -/** - * Swaps the front and the back buffer. - */ -- (void)swapBuffers; - -/** - * Returns the backing store for the back buffer. - */ -- (nonnull FlutterRenderBackingStore*)renderBuffer; - -@end - -/** - * Methods for managing the IOSurfaces held by FlutterIOSurfaceManager. - */ -@protocol FlutterIOSurfaceManagerDelegate - -/** - * Tells the delegate that the front and back IOSurfaces are swapped. - */ -- (void)onSwapBuffers; - -/** - * Tells the delegate that the IOSurfaces have been resized. `bufferIndex` is to indicate the front - * vs back buffer. `size` is the new size of the IOSurface. - */ -- (void)onUpdateSurface:(nonnull FlutterIOSurfaceHolder*)surface - bufferIndex:(size_t)index - size:(CGSize)size; - -/** - * Tells the delegate that IOSurface with given index has been released. Delegate should free - * all resources associated with the surface - */ -- (void)onSurfaceReleased:(size_t)index; - -@end - -/** - * Manages IOSurfaces for the FlutterEngine to render to. + * Manages render surfaces and corresponding backing stores used by the engine. * - * The backing store when rendering with on Metal its a Metal texture. There are two IOSurfaces + * 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. */ -@interface FlutterIOSurfaceManager : NSObject - -/** - * The object that acts as the delegate for the FlutterIOSurfaceManager. See: - * FlutterIOSurfaceManagerDelegate. - */ -@property(nullable, nonatomic, weak) id delegate; +@interface FlutterSurfaceManager : NSObject /** - * Initializes and returns an IOSurface manager that renders to a child layer (referred to as the + * 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. */ -- (nullable instancetype)initWithLayer:(nonnull CALayer*)containingLayer - contentTransform:(CATransform3D)transform; +- (nullable instancetype)initWithDevice:(nonnull id)device + commandQueue:(nonnull id)commandQueue + layer:(nonnull CALayer*)containingLayer; -@end +/** + * 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. + */ +- (void)ensureSurfaceSize:(CGSize)size; /** - * FlutterSurfaceManager implementation where the IOSurfaces managed are backed by a Metal textures. + * Swaps the front and the back buffer. */ -@interface FlutterMetalSurfaceManager : FlutterIOSurfaceManager +- (void)swapBuffers; /** - * Creates two IOSurfaces backed by Metal textures. + * Returns the backing store for the back buffer. */ -- (nullable instancetype)initWithDevice:(nonnull id)device - commandQueue:(nonnull id)commandQueue - layer:(nonnull CALayer*)containingLayer; +- (nonnull FlutterRenderBackingStore*)renderBuffer; @end diff --git a/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.mm b/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.mm index 938c9cf08ac00..06d0e5f234877 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.mm @@ -19,7 +19,21 @@ // BackBuffer will be released after kIdleDelay if there is no activity. static const double kIdleDelay = 1.0; -@implementation FlutterIOSurfaceManager { +@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; @@ -27,18 +41,27 @@ @implementation FlutterIOSurfaceManager { CGSize _surfaceSize; FlutterIOSurfaceHolder* _ioSurfaces[kFlutterSurfaceManagerBufferCount]; BOOL _frameInProgress; + + id _device; + id _commandQueue; + id _textures[kFlutterSurfaceManagerBufferCount]; } -- (instancetype)initWithLayer:(CALayer*)containingLayer contentTransform:(CATransform3D)transform { +- (nullable instancetype)initWithDevice:(nonnull id)device + commandQueue:(nonnull id)commandQueue + layer:(nonnull CALayer*)containingLayer { self = [super init]; if (self) { _containingLayer = containingLayer; - _contentTransform = transform; + _contentTransform = CATransform3DIdentity; _contentLayer = [[CALayer alloc] init]; [_containingLayer addSublayer:_contentLayer]; _ioSurfaces[0] = [[FlutterIOSurfaceHolder alloc] init]; _ioSurfaces[1] = [[FlutterIOSurfaceHolder alloc] init]; + + _device = device; + _commandQueue = commandQueue; } return self; } @@ -51,7 +74,7 @@ - (void)ensureSurfaceSize:(CGSize)size { for (int i = 0; i < kFlutterSurfaceManagerBufferCount; ++i) { if (_ioSurfaces[i] != nil) { [_ioSurfaces[i] recreateIOSurfaceWithSize:size]; - [_delegate onUpdateSurface:_ioSurfaces[i] bufferIndex:i size:size]; + _textures[i] = [self createTextureForSurface:_ioSurfaces[i] size:size]; } } } @@ -71,7 +94,8 @@ - (void)swapBuffers { std::swap(_ioSurfaces[kFlutterSurfaceManagerBackBuffer], _ioSurfaces[kFlutterSurfaceManagerFrontBuffer]); - [_delegate onSwapBuffers]; + std::swap(_textures[kFlutterSurfaceManagerBackBuffer], + _textures[kFlutterSurfaceManagerFrontBuffer]); // performSelector:withObject:afterDelay needs to be performed on RunLoop thread [self performSelectorOnMainThread:@selector(reschedule) withObject:nil waitUntilDone:NO]; @@ -92,7 +116,7 @@ - (void)onIdle { // Release the back buffer and notify delegate. The buffer will be restored // on demand in ensureBackBuffer _ioSurfaces[kFlutterSurfaceManagerBackBuffer] = nil; - [self.delegate onSurfaceReleased:kFlutterSurfaceManagerBackBuffer]; + _textures[kFlutterSurfaceManagerBackBuffer] = nil; } } } @@ -104,9 +128,9 @@ - (void)ensureBackBuffer { // Restore previously released backbuffer _ioSurfaces[kFlutterSurfaceManagerBackBuffer] = [[FlutterIOSurfaceHolder alloc] init]; [_ioSurfaces[kFlutterSurfaceManagerBackBuffer] recreateIOSurfaceWithSize:_surfaceSize]; - [_delegate onUpdateSurface:_ioSurfaces[kFlutterSurfaceManagerBackBuffer] - bufferIndex:kFlutterSurfaceManagerBackBuffer - size:_surfaceSize]; + _textures[kFlutterSurfaceManagerBackBuffer] = + [self createTextureForSurface:_ioSurfaces[kFlutterSurfaceManagerBackBuffer] + size:_surfaceSize]; } }; [self performSelectorOnMainThread:@selector(cancelIdle) withObject:nil waitUntilDone:NO]; @@ -117,47 +141,12 @@ - (void)cancelIdle { } - (nonnull FlutterRenderBackingStore*)renderBuffer { - @throw([NSException exceptionWithName:@"Sub-classes FlutterIOSurfaceManager of" - " must override renderBuffer." - reason:nil - userInfo:nil]); -} - -@end - -@implementation FlutterMetalSurfaceManager { - id _device; - id _commandQueue; - - id _textures[kFlutterSurfaceManagerBufferCount]; -} - -- (nullable instancetype)initWithDevice:(nonnull id)device - commandQueue:(nonnull id)commandQueue - layer:(nonnull CALayer*)containingLayer { - self = [super initWithLayer:containingLayer contentTransform:CATransform3DIdentity]; - if (self) { - super.delegate = self; - _device = device; - _commandQueue = commandQueue; - } - return self; -} - -- (FlutterRenderBackingStore*)renderBuffer { [self ensureBackBuffer]; id texture = _textures[kFlutterSurfaceManagerBackBuffer]; return [[FlutterMetalRenderBackingStore alloc] initWithTexture:texture]; } -- (void)onSwapBuffers { - std::swap(_textures[kFlutterSurfaceManagerBackBuffer], - _textures[kFlutterSurfaceManagerFrontBuffer]); -} - -- (void)onUpdateSurface:(FlutterIOSurfaceHolder*)surface - bufferIndex:(size_t)index - size:(CGSize)size { +- (id)createTextureForSurface:(FlutterIOSurfaceHolder*)surface size:(CGSize)size { MTLTextureDescriptor* textureDescriptor = [MTLTextureDescriptor texture2DDescriptorWithPixelFormat:MTLPixelFormatBGRA8Unorm width:size.width @@ -166,13 +155,7 @@ - (void)onUpdateSurface:(FlutterIOSurfaceHolder*)surface textureDescriptor.usage = MTLTextureUsageShaderRead | MTLTextureUsageRenderTarget | MTLTextureUsageShaderWrite; // plane = 0 for BGRA. - _textures[index] = [_device newTextureWithDescriptor:textureDescriptor - iosurface:[surface ioSurface] - plane:0]; -} - -- (void)onSurfaceReleased:(size_t)index { - _textures[index] = nil; + return [_device newTextureWithDescriptor:textureDescriptor iosurface:[surface ioSurface] plane:0]; } @end diff --git a/shell/platform/darwin/macos/framework/Source/FlutterMetalSurfaceManagerTest.mm b/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManagerTest.mm similarity index 75% rename from shell/platform/darwin/macos/framework/Source/FlutterMetalSurfaceManagerTest.mm rename to shell/platform/darwin/macos/framework/Source/FlutterSurfaceManagerTest.mm index 77760776383b7..fdcd62f9d7e34 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterMetalSurfaceManagerTest.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManagerTest.mm @@ -29,18 +29,18 @@ - (instancetype)init { namespace flutter::testing { -static FlutterMetalSurfaceManager* CreateSurfaceManager() { +static FlutterSurfaceManager* CreateSurfaceManager() { id device = MTLCreateSystemDefaultDevice(); id commandQueue = [device newCommandQueue]; TestMetalView* metalView = [[TestMetalView alloc] init]; CALayer* layer = reinterpret_cast(metalView.layer); - return [[FlutterMetalSurfaceManager alloc] initWithDevice:device - commandQueue:commandQueue - layer:layer]; + return [[FlutterSurfaceManager alloc] initWithDevice:device + commandQueue:commandQueue + layer:layer]; } -TEST(FlutterMetalSurfaceManager, EnsureSizeUpdatesSize) { - FlutterMetalSurfaceManager* surfaceManager = CreateSurfaceManager(); +TEST(FlutterSurfaceManager, EnsureSizeUpdatesSize) { + FlutterSurfaceManager* surfaceManager = CreateSurfaceManager(); CGSize size = CGSizeMake(100, 50); [surfaceManager ensureSurfaceSize:size]; id texture = @@ -49,8 +49,8 @@ - (instancetype)init { ASSERT_TRUE(CGSizeEqualToSize(size, textureSize)); } -TEST(FlutterMetalSurfaceManager, EnsureSizeUpdatesSizeForBackBuffer) { - FlutterMetalSurfaceManager* surfaceManager = CreateSurfaceManager(); +TEST(FlutterSurfaceManager, EnsureSizeUpdatesSizeForBackBuffer) { + FlutterSurfaceManager* surfaceManager = CreateSurfaceManager(); CGSize size = CGSizeMake(100, 50); [surfaceManager ensureSurfaceSize:size]; [surfaceManager renderBuffer]; // make sure we have back buffer