Skip to content

Commit

Permalink
Eliminate ShellTestPlatformView::BackendType::kDefaultBackendType (fl…
Browse files Browse the repository at this point in the history
…utter/engine#56744)

`kDefaultBackendType` is intended to make life easier for authors of tests, but in any switch statement where it's used (currently just a single location), we rely on ordering it first and `#ifdef`ing out all backends that aren't available.

Instead, we define a static function that returns the default that callers can invoke instead. This avoids relinace on case ordering and fallthrough.

In flutter/engine#56722 we split backends out into separate translation units, and ideally should remove the `#ifdef`s, which means we can't rely on this trick anymore.

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
  • Loading branch information
cbracken authored Nov 22, 2024
1 parent 932d854 commit 5883c1a
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 18 deletions.
4 changes: 2 additions & 2 deletions engine/src/flutter/shell/common/animator_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,8 @@ TEST_F(ShellTest, VSyncTargetTime) {
flutter::PlatformData(), task_runners, settings,
[vsync_clock, &create_vsync_waiter](Shell& shell) {
return ShellTestPlatformView::Create(
shell, shell.GetTaskRunners(), vsync_clock, create_vsync_waiter,
ShellTestPlatformView::BackendType::kDefaultBackend, nullptr,
ShellTestPlatformView::DefaultBackendType(), shell,
shell.GetTaskRunners(), vsync_clock, create_vsync_waiter, nullptr,
shell.GetIsGpuDisabledSyncSwitch());
},
[](Shell& shell) { return std::make_unique<Rasterizer>(shell); });
Expand Down
24 changes: 14 additions & 10 deletions engine/src/flutter/shell/common/shell_test_platform_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,40 +20,44 @@ namespace flutter {
namespace testing {

std::unique_ptr<ShellTestPlatformView> ShellTestPlatformView::Create(
BackendType backend,
PlatformView::Delegate& delegate,
const TaskRunners& task_runners,
const std::shared_ptr<ShellTestVsyncClock>& vsync_clock,
const CreateVsyncWaiter& create_vsync_waiter,
BackendType backend,
const std::shared_ptr<ShellTestExternalViewEmbedder>&
shell_test_external_view_embedder,
const std::shared_ptr<const fml::SyncSwitch>& is_gpu_disabled_sync_switch) {
// TODO(gw280): https://github.com/flutter/flutter/issues/50298
// Make this fully runtime configurable
switch (backend) {
case BackendType::kDefaultBackend:
#ifdef SHELL_ENABLE_GL
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
#ifdef SHELL_ENABLE_VULKAN
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
#ifdef SHELL_ENABLE_METAL
case BackendType::kMetalBackend:
#ifdef SHELL_ENABLE_METAL
return std::make_unique<ShellTestPlatformViewMetal>(
delegate, task_runners, vsync_clock, create_vsync_waiter,
shell_test_external_view_embedder, is_gpu_disabled_sync_switch);
#endif // SHELL_ENABLE_METAL

default:
FML_LOG(FATAL) << "No backends supported for ShellTestPlatformView";
#else
FML_LOG(FATAL) << "Metal not enabled in this build";
return nullptr;
#endif // SHELL_ENABLE_METAL
}
}

Expand All @@ -76,11 +80,11 @@ std::unique_ptr<PlatformView> ShellTestPlatformViewBuilder::operator()(
}
};
return ShellTestPlatformView::Create(
config_.rendering_backend, //
shell, //
task_runners, //
vsync_clock, //
create_vsync_waiter, //
config_.rendering_backend, //
config_.shell_test_external_view_embedder, //
shell.GetIsGpuDisabledSyncSwitch() //
);
Expand Down
20 changes: 17 additions & 3 deletions engine/src/flutter/shell/common/shell_test_platform_view.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
#ifndef FLUTTER_SHELL_COMMON_SHELL_TEST_PLATFORM_VIEW_H_
#define FLUTTER_SHELL_COMMON_SHELL_TEST_PLATFORM_VIEW_H_

#include <exception>

#include "flutter/shell/common/platform_view.h"
#include "flutter/shell/common/shell_test_external_view_embedder.h"
#include "flutter/shell/common/vsync_waiters_test.h"
Expand All @@ -15,18 +17,30 @@ namespace testing {
class ShellTestPlatformView : public PlatformView {
public:
enum class BackendType {
kDefaultBackend = 0,
kGLBackend,
kVulkanBackend,
kMetalBackend,
};

static BackendType DefaultBackendType() {
#if defined(SHELL_ENABLE_GL)
return BackendType::kGLBackend;
#elif defined(SHELL_ENABLE_METAL)
return BackendType::kMetalBackend;
#elif defined(SHELL_ENABLE_VULKAN)
return BackendType::kVulkanBackend;
#else
FML_LOG(FATAL) << "No backend is enabled in this build.";
std::terminate();
#endif
}

static std::unique_ptr<ShellTestPlatformView> Create(
BackendType backend,
PlatformView::Delegate& delegate,
const TaskRunners& task_runners,
const std::shared_ptr<ShellTestVsyncClock>& vsync_clock,
const CreateVsyncWaiter& create_vsync_waiter,
BackendType backend,
const std::shared_ptr<ShellTestExternalViewEmbedder>&
shell_test_external_view_embedder,
const std::shared_ptr<const fml::SyncSwitch>&
Expand All @@ -50,7 +64,7 @@ class ShellTestPlatformViewBuilder {
std::shared_ptr<ShellTestExternalViewEmbedder>
shell_test_external_view_embedder = nullptr;
ShellTestPlatformView::BackendType rendering_backend =
ShellTestPlatformView::BackendType::kDefaultBackend;
ShellTestPlatformView::DefaultBackendType();
};

explicit ShellTestPlatformViewBuilder(Config config);
Expand Down
6 changes: 3 additions & 3 deletions engine/src/flutter/shell/common/shell_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -500,13 +500,13 @@ TEST_F(ShellTest,
// vsync mechanism. We should have better DI in the tests.
const auto vsync_clock = std::make_shared<ShellTestVsyncClock>();
return ShellTestPlatformView::Create(
shell, shell.GetTaskRunners(), vsync_clock,
ShellTestPlatformView::DefaultBackendType(), shell,
shell.GetTaskRunners(), vsync_clock,
[task_runners = shell.GetTaskRunners()]() {
return static_cast<std::unique_ptr<VsyncWaiter>>(
std::make_unique<VsyncWaiterFallback>(task_runners));
},
ShellTestPlatformView::BackendType::kDefaultBackend, nullptr,
shell.GetIsGpuDisabledSyncSwitch());
nullptr, shell.GetIsGpuDisabledSyncSwitch());
},
[](Shell& shell) { return std::make_unique<Rasterizer>(shell); });
ASSERT_TRUE(ValidateShell(shell.get()));
Expand Down

0 comments on commit 5883c1a

Please sign in to comment.