Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Move renderer config to EmbedderTestContext #56699

Merged
auto-submit merged 1 commit into from
Nov 19, 2024
Merged

Move renderer config to EmbedderTestContext #56699

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions shell/platform/embedder/BUILD.gn
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,6 @@ if (enable_unittests) {

if (test_enable_gl) {
sources += [
"tests/embedder_config_builder_gl.cc",
"tests/embedder_test_backingstore_producer_gl.cc",
"tests/embedder_test_backingstore_producer_gl.h",
"tests/embedder_test_compositor_gl.cc",
Expand All @@ -333,7 +332,6 @@ if (enable_unittests) {

if (test_enable_metal) {
sources += [
"tests/embedder_config_builder_metal.mm",
"tests/embedder_test_backingstore_producer_metal.h",
"tests/embedder_test_backingstore_producer_metal.mm",
"tests/embedder_test_compositor_metal.h",
Expand All @@ -348,7 +346,6 @@ if (enable_unittests) {

if (test_enable_vulkan) {
sources += [
"tests/embedder_config_builder_vulkan.cc",
"tests/embedder_test_backingstore_producer_vulkan.cc",
"tests/embedder_test_backingstore_producer_vulkan.h",
"tests/embedder_test_compositor_vulkan.cc",
Expand Down
16 changes: 8 additions & 8 deletions shell/platform/embedder/tests/embedder_a11y_unittests.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ TEST_F(EmbedderTest, CannotProvideMultipleSemanticsCallbacks) {
{
EmbedderConfigBuilder builder(
GetEmbedderContext(EmbedderTestContextType::kSoftwareContext));
builder.SetSoftwareRendererConfig();
builder.SetSurface(SkISize::Make(1, 1));
Copy link
Member Author

@cbracken cbracken Nov 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here and below (1, 1) was the default parameter value... but we only ever defaulted it for the software renderer, now we're consistent across the board and make people announce what they want so readers don't need to check the method declaration.

builder.GetProjectArgs().update_semantics_callback =
[](const FlutterSemanticsUpdate* update, void* user_data) {};
builder.GetProjectArgs().update_semantics_callback2 =
Expand All @@ -48,7 +48,7 @@ TEST_F(EmbedderTest, CannotProvideMultipleSemanticsCallbacks) {
{
EmbedderConfigBuilder builder(
GetEmbedderContext(EmbedderTestContextType::kSoftwareContext));
builder.SetSoftwareRendererConfig();
builder.SetSurface(SkISize::Make(1, 1));
builder.GetProjectArgs().update_semantics_callback2 =
[](const FlutterSemanticsUpdate2* update, void* user_data) {};
builder.GetProjectArgs().update_semantics_node_callback =
Expand All @@ -63,7 +63,7 @@ TEST_F(EmbedderTest, CannotProvideMultipleSemanticsCallbacks) {
{
EmbedderConfigBuilder builder(
GetEmbedderContext(EmbedderTestContextType::kSoftwareContext));
builder.SetSoftwareRendererConfig();
builder.SetSurface(SkISize::Make(1, 1));
builder.GetProjectArgs().update_semantics_callback =
[](const FlutterSemanticsUpdate* update, void* user_data) {};
builder.GetProjectArgs().update_semantics_node_callback =
Expand All @@ -78,7 +78,7 @@ TEST_F(EmbedderTest, CannotProvideMultipleSemanticsCallbacks) {
{
EmbedderConfigBuilder builder(
GetEmbedderContext(EmbedderTestContextType::kSoftwareContext));
builder.SetSoftwareRendererConfig();
builder.SetSurface(SkISize::Make(1, 1));
builder.GetProjectArgs().update_semantics_callback2 =
[](const FlutterSemanticsUpdate2* update, void* user_data) {};
builder.GetProjectArgs().update_semantics_callback =
Expand Down Expand Up @@ -171,7 +171,7 @@ TEST_F(EmbedderA11yTest, A11yTreeIsConsistentUsingV3Callbacks) {
});

EmbedderConfigBuilder builder(context);
builder.SetSoftwareRendererConfig();
builder.SetSurface(SkISize::Make(1, 1));
builder.SetDartEntrypoint("a11y_main");

auto engine = builder.LaunchEngine();
Expand Down Expand Up @@ -375,7 +375,7 @@ TEST_F(EmbedderA11yTest, A11yStringAttributes) {
});

EmbedderConfigBuilder builder(context);
builder.SetSoftwareRendererConfig();
builder.SetSurface(SkISize::Make(1, 1));
builder.SetDartEntrypoint("a11y_string_attributes");

auto engine = builder.LaunchEngine();
Expand Down Expand Up @@ -468,7 +468,7 @@ TEST_F(EmbedderA11yTest, A11yTreeIsConsistentUsingV2Callbacks) {
});

EmbedderConfigBuilder builder(context);
builder.SetSoftwareRendererConfig();
builder.SetSurface(SkISize::Make(1, 1));
builder.SetDartEntrypoint("a11y_main");

auto engine = builder.LaunchEngine();
Expand Down Expand Up @@ -662,7 +662,7 @@ TEST_F(EmbedderA11yTest, A11yTreeIsConsistentUsingV1Callbacks) {
});

EmbedderConfigBuilder builder(context);
builder.SetSoftwareRendererConfig();
builder.SetSurface(SkISize::Make(1, 1));
builder.SetDartEntrypoint("a11y_main");

auto engine = builder.LaunchEngine();
Expand Down
110 changes: 6 additions & 104 deletions shell/platform/embedder/tests/embedder_config_builder.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
#include "flutter/runtime/dart_vm.h"
#include "flutter/shell/platform/embedder/embedder.h"
#include "tests/embedder_test_context.h"
#include "third_party/skia/include/core/SkBitmap.h"
#include "third_party/skia/include/core/SkImage.h"

namespace flutter::testing {
Expand All @@ -27,28 +26,6 @@ EmbedderConfigBuilder::EmbedderConfigBuilder(

custom_task_runners_.struct_size = sizeof(FlutterCustomTaskRunners);

InitializeGLRendererConfig();
InitializeMetalRendererConfig();
InitializeVulkanRendererConfig();
Copy link
Member Author

@cbracken cbracken Nov 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The initialise methods above (and the similar software renderer config initialisation below) moved to ctors of EmbedderTestContextMetal etc.


software_renderer_config_.struct_size = sizeof(FlutterSoftwareRendererConfig);
software_renderer_config_.surface_present_callback =
[](void* context, const void* allocation, size_t row_bytes,
size_t height) {
auto image_info =
SkImageInfo::MakeN32Premul(SkISize::Make(row_bytes / 4, height));
SkBitmap bitmap;
if (!bitmap.installPixels(image_info, const_cast<void*>(allocation),
row_bytes)) {
FML_LOG(ERROR) << "Could not copy pixels for the software "
"composition from the engine.";
return false;
}
bitmap.setImmutable();
return reinterpret_cast<EmbedderTestContextSoftware*>(context)->Present(
SkImages::RasterFromBitmap(bitmap));
};

// The first argument is always the executable name. Don't make tests have to
// do this manually.
AddCommandLineArgument("embedder_unittest");
Expand Down Expand Up @@ -79,30 +56,6 @@ FlutterProjectArgs& EmbedderConfigBuilder::GetProjectArgs() {
return project_args_;
}

void EmbedderConfigBuilder::SetSoftwareRendererConfig(SkISize surface_size) {
renderer_config_.type = FlutterRendererType::kSoftware;
renderer_config_.software = software_renderer_config_;
context_.SetupSurface(surface_size);
}

void EmbedderConfigBuilder::SetRendererConfig(EmbedderTestContextType type,
SkISize surface_size) {
switch (type) {
case EmbedderTestContextType::kOpenGLContext:
SetOpenGLRendererConfig(surface_size);
break;
case EmbedderTestContextType::kMetalContext:
SetMetalRendererConfig(surface_size);
break;
case EmbedderTestContextType::kVulkanContext:
SetVulkanRendererConfig(surface_size);
break;
case EmbedderTestContextType::kSoftwareContext:
SetSoftwareRendererConfig(surface_size);
break;
}
}
Copy link
Member Author

@cbracken cbracken Nov 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for for the type argument anymore since the EmbedderConfigBuilder already took an EmbedderTestContext of that type in its constructor, so we knew the type all along. Renamed all the referenced methods to just EmbedderTestContext{GL,Metal,Sofware,Vulkan}::SetRendererConfig except turns out that now that the initialisation is done in the ctor, all they actually do is create the surface of the right type with the specified size, so renamed them to SetSurface(surface_size).


void EmbedderConfigBuilder::SetAssetsPath() {
project_args_.assets_path = context_.GetAssetsPath().c_str();
}
Expand Down Expand Up @@ -217,10 +170,6 @@ void EmbedderConfigBuilder::SetupVsyncCallback() {
};
}

FlutterRendererConfig& EmbedderConfigBuilder::GetRendererConfig() {
return renderer_config_;
}
Copy link
Member Author

@cbracken cbracken Nov 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unloved and unused now.


void EmbedderConfigBuilder::SetRenderTaskRunner(
const FlutterTaskRunnerDescription* runner) {
if (runner == nullptr) {
Expand Down Expand Up @@ -336,11 +285,12 @@ UniqueEngine EmbedderConfigBuilder::SetupEngine(bool run) const {
project_args.dart_entrypoint_argc = 0;
}

auto result =
run ? FlutterEngineRun(FLUTTER_ENGINE_VERSION, &renderer_config_,
&project_args, &context_, &engine)
: FlutterEngineInitialize(FLUTTER_ENGINE_VERSION, &renderer_config_,
&project_args, &context_, &engine);
auto result = run ? FlutterEngineRun(FLUTTER_ENGINE_VERSION,
&context_.GetRendererConfig(),
&project_args, &context_, &engine)
: FlutterEngineInitialize(
FLUTTER_ENGINE_VERSION, &context_.GetRendererConfig(),
&project_args, &context_, &engine);

if (result != kSuccess) {
return {};
Expand All @@ -349,52 +299,4 @@ UniqueEngine EmbedderConfigBuilder::SetupEngine(bool run) const {
return UniqueEngine{engine};
}

#ifndef SHELL_ENABLE_GL
Copy link
Member Author

@cbracken cbracken Nov 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything below here are just methods on the appropriate EmbedderTestContext subclass. People who need GL-specific methods can get them from the GL subclass, etc.

// OpenGL fallback implementations.
// See: flutter/shell/platform/embedder/tests/embedder_config_builder_gl.cc.

void EmbedderConfigBuilder::InitializeGLRendererConfig() {
// no-op.
}

void EmbedderConfigBuilder::SetOpenGLFBOCallBack() {
FML_LOG(FATAL) << "OpenGL is not enabled in this build.";
}

void EmbedderConfigBuilder::SetOpenGLPresentCallBack() {
FML_LOG(FATAL) << "OpenGL is not enabled in this build.";
}

void EmbedderConfigBuilder::SetOpenGLRendererConfig(SkISize surface_size) {
FML_LOG(FATAL) << "OpenGL is not enabled in this build.";
}
#endif
#ifndef SHELL_ENABLE_METAL
// Metal fallback implementations.
// See: flutter/shell/platform/embedder/tests/embedder_config_builder_metal.mm.

void EmbedderConfigBuilder::InitializeMetalRendererConfig() {
// no-op.
}

void EmbedderConfigBuilder::SetMetalRendererConfig(SkISize surface_size) {
FML_LOG(FATAL) << "Metal is not enabled in this build.";
}
#endif
#ifndef SHELL_ENABLE_VULKAN
// Vulkan fallback implementations.
// See: flutter/shell/platform/embedder/tests/embedder_config_builder_vulkan.cc.

void EmbedderConfigBuilder::InitializeVulkanRendererConfig() {
// no-op.
}

void EmbedderConfigBuilder::SetVulkanRendererConfig(
SkISize surface_size,
std::optional<FlutterVulkanInstanceProcAddressCallback>
instance_proc_address_callback) {
FML_LOG(FATAL) << "Vulkan is not enabled in this build.";
}
#endif

} // namespace flutter::testing
42 changes: 1 addition & 41 deletions shell/platform/embedder/tests/embedder_config_builder.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,31 +44,6 @@ class EmbedderConfigBuilder {

FlutterProjectArgs& GetProjectArgs();

void SetRendererConfig(EmbedderTestContextType type, SkISize surface_size);

void SetSoftwareRendererConfig(SkISize surface_size = SkISize::Make(1, 1));

void SetOpenGLRendererConfig(SkISize surface_size);

void SetMetalRendererConfig(SkISize surface_size);

void SetVulkanRendererConfig(
SkISize surface_size,
std::optional<FlutterVulkanInstanceProcAddressCallback>
instance_proc_address_callback = {});

// Used to explicitly set an `open_gl.fbo_callback`. Using this method will
// cause your test to fail since the ctor for this class sets
// `open_gl.fbo_callback_with_frame_info`. This method exists as a utility to
// explicitly test this behavior.
void SetOpenGLFBOCallBack();

// Used to explicitly set an `open_gl.present`. Using this method will cause
// your test to fail since the ctor for this class sets
// `open_gl.present_with_info`. This method exists as a utility to explicitly
// test this behavior.
void SetOpenGLPresentCallBack();

void SetAssetsPath();

void SetSnapshots();
Expand Down Expand Up @@ -109,7 +84,7 @@ class EmbedderConfigBuilder {

FlutterCompositor& GetCompositor();

FlutterRendererConfig& GetRendererConfig();
void SetSurface(SkISize surface_size) { context_.SetSurface(surface_size); }

void SetRenderTargetType(
EmbedderTestBackingStoreProducer::RenderTargetType type,
Expand All @@ -125,23 +100,8 @@ class EmbedderConfigBuilder {
void SetupVsyncCallback();

private:
void InitializeGLRendererConfig();
void InitializeVulkanRendererConfig();
void InitializeMetalRendererConfig();

EmbedderTestContext& context_;
FlutterProjectArgs project_args_ = {};
FlutterRendererConfig renderer_config_ = {};
FlutterSoftwareRendererConfig software_renderer_config_ = {};
#ifdef SHELL_ENABLE_GL
FlutterOpenGLRendererConfig opengl_renderer_config_ = {};
#endif
#ifdef SHELL_ENABLE_METAL
FlutterMetalRendererConfig metal_renderer_config_ = {};
#endif
#ifdef SHELL_ENABLE_VULKAN
FlutterVulkanRendererConfig vulkan_renderer_config_ = {};
#endif
std::string dart_entrypoint_;
FlutterCustomTaskRunners custom_task_runners_ = {};
FlutterCompositor compositor_ = {};
Expand Down
81 changes: 0 additions & 81 deletions shell/platform/embedder/tests/embedder_config_builder_gl.cc
View file
Open in desktop

This file was deleted.

Loading