Skip to content

Commit

Permalink
Extract backend-specific code in ShellTestPlatformView (flutter#56722)
Browse files Browse the repository at this point in the history 
Moves code specific to each graphics backend into the (existing) translation unit associated with that backend.

Previously, we could not include any Objective-C types in `shell_test_platform_view_metal.h`, since that file was included into `shell_test_platform_view.cc`, which is pure C++. To work around this, we had encapsulated Objective-C Metal types in a `DarwinContextMetal` struct hidden in the implementation file with a pointer to it forward-declared in the header.

We now use Metal types directly in the header, without the workarounds.

Issue: flutter/flutter#158998
Issue: flutter/flutter#137801

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
  • Loading branch information
@cbracken
cbracken authored Nov 22, 2024
1 parent 0dd1cba commit 28e519d
Show file tree
Hide file tree
Showing 7 changed files with 157 additions and 102 deletions.
87 changes: 51 additions & 36 deletions shell/common/shell_test_platform_view.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,11 @@

#include "flutter/shell/common/shell_test_platform_view.h"

#ifdef SHELL_ENABLE_GL
#include "flutter/shell/common/shell_test_platform_view_gl.h"
#endif // SHELL_ENABLE_GL
#ifdef SHELL_ENABLE_VULKAN
#include "flutter/shell/common/shell_test_platform_view_vulkan.h"
#endif // SHELL_ENABLE_VULKAN
#ifdef SHELL_ENABLE_METAL
#include "flutter/shell/common/shell_test_platform_view_metal.h"
#endif // SHELL_ENABLE_METAL
#include <memory>

#include "flutter/shell/common/vsync_waiter_fallback.h"

namespace flutter {
namespace testing {
namespace flutter::testing {

std::unique_ptr<ShellTestPlatformView> ShellTestPlatformView::Create(
BackendType backend,
Expand All @@ -32,35 +23,60 @@ std::unique_ptr<ShellTestPlatformView> ShellTestPlatformView::Create(
// Make this fully runtime configurable
switch (backend) {
case BackendType::kGLBackend:
#ifdef SHELL_ENABLE_GL
return std::make_unique<ShellTestPlatformViewGL>(
delegate, task_runners, vsync_clock, create_vsync_waiter,
shell_test_external_view_embedder);
#else
FML_LOG(FATAL) << "OpenGL not enabled in this build";
return nullptr;
#endif // SHELL_ENABLE_GL
case BackendType::kVulkanBackend:
#ifdef SHELL_ENABLE_VULKAN
return std::make_unique<ShellTestPlatformViewVulkan>(
delegate, task_runners, vsync_clock, create_vsync_waiter,
shell_test_external_view_embedder);
#else
FML_LOG(FATAL) << "Vulkan not enabled in this build";
return nullptr;
#endif // SHELL_ENABLE_VULKAN
return CreateGL(delegate, task_runners, vsync_clock, create_vsync_waiter,
shell_test_external_view_embedder,
is_gpu_disabled_sync_switch);
case BackendType::kMetalBackend:
#ifdef SHELL_ENABLE_METAL
return std::make_unique<ShellTestPlatformViewMetal>(
return CreateMetal(delegate, task_runners, vsync_clock,
create_vsync_waiter, shell_test_external_view_embedder,
is_gpu_disabled_sync_switch);
case BackendType::kVulkanBackend:
return CreateVulkan(
delegate, task_runners, vsync_clock, create_vsync_waiter,
shell_test_external_view_embedder, is_gpu_disabled_sync_switch);
#else
FML_LOG(FATAL) << "Metal not enabled in this build";
return nullptr;
#endif // SHELL_ENABLE_METAL
}
}

#ifndef SHELL_ENABLE_GL
std::unique_ptr<ShellTestPlatformView> ShellTestPlatformView::CreateGL(
PlatformView::Delegate& delegate,
const TaskRunners& task_runners,
const std::shared_ptr<ShellTestVsyncClock>& vsync_clock,
const CreateVsyncWaiter& create_vsync_waiter,
const std::shared_ptr<ShellTestExternalViewEmbedder>&
shell_test_external_view_embedder,
const std::shared_ptr<const fml::SyncSwitch>& is_gpu_disabled_sync_switch) {
FML_LOG(FATAL) << "OpenGL backend not enabled in this build";
return nullptr;
}
#endif // SHELL_ENABLE_GL
#ifndef SHELL_ENABLE_METAL
std::unique_ptr<ShellTestPlatformView> ShellTestPlatformView::CreateMetal(
PlatformView::Delegate& delegate,
const TaskRunners& task_runners,
const std::shared_ptr<ShellTestVsyncClock>& vsync_clock,
const CreateVsyncWaiter& create_vsync_waiter,
const std::shared_ptr<ShellTestExternalViewEmbedder>&
shell_test_external_view_embedder,
const std::shared_ptr<const fml::SyncSwitch>& is_gpu_disabled_sync_switch) {
FML_LOG(FATAL) << "Metal backend not enabled in this build";
return nullptr;
}
#endif // SHELL_ENABLE_METAL
#ifndef SHELL_ENABLE_VULKAN
std::unique_ptr<ShellTestPlatformView> ShellTestPlatformView::CreateVulkan(
PlatformView::Delegate& delegate,
const TaskRunners& task_runners,
const std::shared_ptr<ShellTestVsyncClock>& vsync_clock,
const CreateVsyncWaiter& create_vsync_waiter,
const std::shared_ptr<ShellTestExternalViewEmbedder>&
shell_test_external_view_embedder,
const std::shared_ptr<const fml::SyncSwitch>& is_gpu_disabled_sync_switch) {
FML_LOG(FATAL) << "Vulkan backend not enabled in this build";
return nullptr;
}
#endif // SHELL_ENABLE_VULKAN

ShellTestPlatformViewBuilder::ShellTestPlatformViewBuilder(Config config)
: config_(std::move(config)) {}

Expand Down Expand Up @@ -90,5 +106,4 @@ std::unique_ptr<PlatformView> ShellTestPlatformViewBuilder::operator()(
);
}

} // namespace testing
} // namespace flutter
} // namespace flutter::testing
34 changes: 30 additions & 4 deletions shell/common/shell_test_platform_view.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,7 @@
#include "flutter/shell/common/shell_test_external_view_embedder.h"
#include "flutter/shell/common/vsync_waiters_test.h"

namespace flutter {
namespace testing {
namespace flutter::testing {

class ShellTestPlatformView : public PlatformView {
public:
Expand Down Expand Up @@ -53,6 +52,34 @@ class ShellTestPlatformView : public PlatformView {
const TaskRunners& task_runners)
: PlatformView(delegate, task_runners) {}

static std::unique_ptr<ShellTestPlatformView> CreateGL(
PlatformView::Delegate& delegate,
const TaskRunners& task_runners,
const std::shared_ptr<ShellTestVsyncClock>& vsync_clock,
const CreateVsyncWaiter& create_vsync_waiter,
const std::shared_ptr<ShellTestExternalViewEmbedder>&
shell_test_external_view_embedder,
const std::shared_ptr<const fml::SyncSwitch>&
is_gpu_disabled_sync_switch);
static std::unique_ptr<ShellTestPlatformView> CreateMetal(
PlatformView::Delegate& delegate,
const TaskRunners& task_runners,
const std::shared_ptr<ShellTestVsyncClock>& vsync_clock,
const CreateVsyncWaiter& create_vsync_waiter,
const std::shared_ptr<ShellTestExternalViewEmbedder>&
shell_test_external_view_embedder,
const std::shared_ptr<const fml::SyncSwitch>&
is_gpu_disabled_sync_switch);
static std::unique_ptr<ShellTestPlatformView> CreateVulkan(
PlatformView::Delegate& delegate,
const TaskRunners& task_runners,
const std::shared_ptr<ShellTestVsyncClock>& vsync_clock,
const CreateVsyncWaiter& create_vsync_waiter,
const std::shared_ptr<ShellTestExternalViewEmbedder>&
shell_test_external_view_embedder,
const std::shared_ptr<const fml::SyncSwitch>&
is_gpu_disabled_sync_switch);

FML_DISALLOW_COPY_AND_ASSIGN(ShellTestPlatformView);
};

Expand All @@ -77,7 +104,6 @@ class ShellTestPlatformViewBuilder {
Config config_;
};

} // namespace testing
} // namespace flutter
} // namespace flutter::testing

#endif // FLUTTER_SHELL_COMMON_SHELL_TEST_PLATFORM_VIEW_H_
13 changes: 13 additions & 0 deletions shell/common/shell_test_platform_view_gl.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,19 @@ static std::vector<std::shared_ptr<fml::Mapping>> ShaderLibraryMappings() {
};
}

std::unique_ptr<ShellTestPlatformView> ShellTestPlatformView::CreateGL(
PlatformView::Delegate& delegate,
const TaskRunners& task_runners,
const std::shared_ptr<ShellTestVsyncClock>& vsync_clock,
const CreateVsyncWaiter& create_vsync_waiter,
const std::shared_ptr<ShellTestExternalViewEmbedder>&
shell_test_external_view_embedder,
const std::shared_ptr<const fml::SyncSwitch>& is_gpu_disabled_sync_switch) {
return std::make_unique<ShellTestPlatformViewGL>(
delegate, task_runners, vsync_clock, create_vsync_waiter,
shell_test_external_view_embedder);
}

ShellTestPlatformViewGL::ShellTestPlatformViewGL(
PlatformView::Delegate& delegate,
const TaskRunners& task_runners,
Expand Down
19 changes: 11 additions & 8 deletions shell/common/shell_test_platform_view_metal.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,16 @@
#ifndef FLUTTER_SHELL_COMMON_SHELL_TEST_PLATFORM_VIEW_METAL_H_
#define FLUTTER_SHELL_COMMON_SHELL_TEST_PLATFORM_VIEW_METAL_H_

#include "flutter/fml/macros.h"
#include "flutter/shell/common/shell_test_platform_view.h"
#include "flutter/shell/gpu/gpu_surface_metal_delegate.h"

namespace flutter {
namespace testing {
#import <Metal/Metal.h>

#include "flutter/fml/macros.h"
#include "flutter/shell/gpu/gpu_surface_metal_delegate.h"
#include "flutter/shell/platform/darwin/graphics/FlutterDarwinContextMetalImpeller.h"
#include "flutter/shell/platform/darwin/graphics/FlutterDarwinContextMetalSkia.h"

struct DarwinContextMetal;
namespace flutter::testing {

class ShellTestPlatformViewMetal final : public ShellTestPlatformView,
public GPUSurfaceMetalDelegate {
Expand All @@ -30,7 +32,9 @@ class ShellTestPlatformViewMetal final : public ShellTestPlatformView,
virtual ~ShellTestPlatformViewMetal() override;

private:
const std::unique_ptr<DarwinContextMetal> metal_context_;
FlutterDarwinContextMetalSkia* skia_context_;
FlutterDarwinContextMetalImpeller* impeller_context_;
id<MTLTexture> offscreen_texture_;
const CreateVsyncWaiter create_vsync_waiter_;
const std::shared_ptr<ShellTestVsyncClock> vsync_clock_;
const std::shared_ptr<ShellTestExternalViewEmbedder>
Expand Down Expand Up @@ -70,7 +74,6 @@ class ShellTestPlatformViewMetal final : public ShellTestPlatformView,
FML_DISALLOW_COPY_AND_ASSIGN(ShellTestPlatformViewMetal);
};

} // namespace testing
} // namespace flutter
} // namespace flutter::testing

#endif // FLUTTER_SHELL_COMMON_SHELL_TEST_PLATFORM_VIEW_METAL_H_
81 changes: 35 additions & 46 deletions shell/common/shell_test_platform_view_metal.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,52 +4,25 @@

#include "flutter/shell/common/shell_test_platform_view_metal.h"

#import <Metal/Metal.h>

#include <utility>

#include "flutter/shell/gpu/gpu_surface_metal_impeller.h"
#include "flutter/shell/gpu/gpu_surface_metal_skia.h"
#include "flutter/shell/platform/darwin/graphics/FlutterDarwinContextMetalImpeller.h"
#include "flutter/shell/platform/darwin/graphics/FlutterDarwinContextMetalSkia.h"

FLUTTER_ASSERT_ARC

namespace flutter {
namespace testing {

// This is out of the header so that shell_test_platform_view_metal.h can be included in
// non-Objective-C TUs.
struct DarwinContextMetal {
FlutterDarwinContextMetalSkia* skia_context;
FlutterDarwinContextMetalImpeller* impeller_context;
id<MTLTexture> offscreen_texture;
};
namespace flutter::testing {

static std::unique_ptr<DarwinContextMetal> CreateDarwinContext(
bool impeller,
std::unique_ptr<ShellTestPlatformView> ShellTestPlatformView::CreateMetal(
PlatformView::Delegate& delegate,
const TaskRunners& task_runners,
const std::shared_ptr<ShellTestVsyncClock>& vsync_clock,
const CreateVsyncWaiter& create_vsync_waiter,
const std::shared_ptr<ShellTestExternalViewEmbedder>& shell_test_external_view_embedder,
const std::shared_ptr<const fml::SyncSwitch>& is_gpu_disabled_sync_switch) {
FlutterDarwinContextMetalSkia* skia_context = nil;
FlutterDarwinContextMetalImpeller* impeller_context = nil;
id<MTLDevice> device = nil;
if (impeller) {
impeller_context = [[FlutterDarwinContextMetalImpeller alloc] init:is_gpu_disabled_sync_switch];
FML_CHECK(impeller_context.context);
device = impeller_context.context->GetMTLDevice();
} else {
skia_context = [[FlutterDarwinContextMetalSkia alloc] initWithDefaultMTLDevice];
FML_CHECK(skia_context.mainContext);
device = skia_context.device;
}
auto descriptor =
[MTLTextureDescriptor texture2DDescriptorWithPixelFormat:MTLPixelFormatBGRA8Unorm
width:800
height:600
mipmapped:NO];
descriptor.usage = MTLTextureUsageRenderTarget | MTLTextureUsageShaderRead;
id<MTLTexture> offscreen_texture = [device newTextureWithDescriptor:descriptor];
return std::make_unique<DarwinContextMetal>(
DarwinContextMetal{skia_context, impeller_context, offscreen_texture});
return std::make_unique<ShellTestPlatformViewMetal>(
delegate, task_runners, vsync_clock, create_vsync_waiter, shell_test_external_view_embedder,
is_gpu_disabled_sync_switch);
}

ShellTestPlatformViewMetal::ShellTestPlatformViewMetal(
Expand All @@ -61,11 +34,28 @@
const std::shared_ptr<const fml::SyncSwitch>& is_gpu_disabled_sync_switch)
: ShellTestPlatformView(delegate, task_runners),
GPUSurfaceMetalDelegate(MTLRenderTargetType::kMTLTexture),
metal_context_(
CreateDarwinContext(GetSettings().enable_impeller, is_gpu_disabled_sync_switch)),
create_vsync_waiter_(std::move(create_vsync_waiter)),
vsync_clock_(std::move(vsync_clock)),
shell_test_external_view_embedder_(std::move(shell_test_external_view_embedder)) {}
shell_test_external_view_embedder_(std::move(shell_test_external_view_embedder)) {
id<MTLDevice> device = nil;
if (GetSettings().enable_impeller) {
impeller_context_ =
[[FlutterDarwinContextMetalImpeller alloc] init:is_gpu_disabled_sync_switch];
FML_CHECK(impeller_context_.context);
device = impeller_context_.context->GetMTLDevice();
} else {
skia_context_ = [[FlutterDarwinContextMetalSkia alloc] initWithDefaultMTLDevice];
FML_CHECK(skia_context_.mainContext);
device = skia_context_.device;
}
auto descriptor =
[MTLTextureDescriptor texture2DDescriptorWithPixelFormat:MTLPixelFormatBGRA8Unorm
width:800
height:600
mipmapped:NO];
descriptor.usage = MTLTextureUsageRenderTarget | MTLTextureUsageShaderRead;
offscreen_texture_ = [device newTextureWithDescriptor:descriptor];
}

ShellTestPlatformViewMetal::~ShellTestPlatformViewMetal() = default;

Expand Down Expand Up @@ -93,16 +83,16 @@
// |PlatformView|
std::unique_ptr<Surface> ShellTestPlatformViewMetal::CreateRenderingSurface() {
if (GetSettings().enable_impeller) {
auto context = metal_context_->impeller_context.context;
auto context = impeller_context_.context;
return std::make_unique<GPUSurfaceMetalImpeller>(
this, std::make_shared<impeller::AiksContext>(context, nullptr));
}
return std::make_unique<GPUSurfaceMetalSkia>(this, metal_context_->skia_context.mainContext);
return std::make_unique<GPUSurfaceMetalSkia>(this, skia_context_.mainContext);
}

// |PlatformView|
std::shared_ptr<impeller::Context> ShellTestPlatformViewMetal::GetImpellerContext() const {
return metal_context_->impeller_context.context;
return impeller_context_.context;
}

// |GPUSurfaceMetalDelegate|
Expand All @@ -123,7 +113,7 @@
GPUMTLTextureInfo ShellTestPlatformViewMetal::GetMTLTexture(const SkISize& frame_info) const {
return {
.texture_id = 0,
.texture = (__bridge GPUMTLTextureHandle)metal_context_->offscreen_texture,
.texture = (__bridge GPUMTLTextureHandle)offscreen_texture_,
};
}

Expand All @@ -133,5 +123,4 @@
return true;
}

} // namespace testing
} // namespace flutter
} // namespace flutter::testing
19 changes: 15 additions & 4 deletions shell/common/shell_test_platform_view_vulkan.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,20 @@
#include "flutter/vulkan/swiftshader_path.h"
#endif

namespace flutter {
namespace testing {
namespace flutter::testing {

std::unique_ptr<ShellTestPlatformView> ShellTestPlatformView::CreateVulkan(
PlatformView::Delegate& delegate,
const TaskRunners& task_runners,
const std::shared_ptr<ShellTestVsyncClock>& vsync_clock,
const CreateVsyncWaiter& create_vsync_waiter,
const std::shared_ptr<ShellTestExternalViewEmbedder>&
shell_test_external_view_embedder,
const std::shared_ptr<const fml::SyncSwitch>& is_gpu_disabled_sync_switch) {
return std::make_unique<ShellTestPlatformViewVulkan>(
delegate, task_runners, vsync_clock, create_vsync_waiter,
shell_test_external_view_embedder);
}

ShellTestPlatformViewVulkan::ShellTestPlatformViewVulkan(
PlatformView::Delegate& delegate,
Expand Down Expand Up @@ -232,5 +244,4 @@ SkMatrix ShellTestPlatformViewVulkan::OffScreenSurface::GetRootTransformation()
return matrix;
}

} // namespace testing
} // namespace flutter
} // namespace flutter::testing
Loading

0 comments on commit 28e519d

Please sign in to comment.