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

Commit 3e96ceb

Browse files
authored
Move renderer config to EmbedderTestContext (#56699)
Moves all backend-specific renderer configuration out of `EmbedderConfigBuilder` and into the backend-specific subclasses of `EmbedderTestContext`. `EmbedderTestContext` is already backend-specific and as of recent patches, also houses compositor configuration, making it the natural home of this code. As a result, we no longer need backend-specific methods such as `SetSoftwareRendererConfig`, `SetMetalRendererConfig`, `SetOpenGLRendererConfig`, `SetVulkanRendererConfig`. Nor do we need manual backend initialisation in `EmbedderConfigBuilder`. Nor does that initialisation any longer require relying on internal backend-specific code within `EmbedderTestContext`, since we now do that initialisation in the `EmbedderTestContext` constructor. Since the bulk of the work previously done by this method now occurs in the `EmbedderTestContext` constructor, the only work remaining in these methods is surface creation. Further, since this is all now implemented in backend-specific `EmbedderTestContext` subclasses, they have all been renamed to a single method: `SetSurface`. Previously, all of these methods took a surface_size parameter, with two exceptions: * `SetVulkanRendererConfig` also took an optional `FlutterVulkanInstanceProcAddressCallback` parameter. This has been extracted to a separate method `SetVulkanInstanceProcAddressCallback` on `EmbedderTestContextVulkan`. * `SetSoftwareRendererConfig` defaulted the parameter to a size of (1, 1). For consistency, this is no longer defaulted, and all call sites have been updated, consistent with other backends. Lastly, one nice benefit is that because the render config is initialised in the `EmbedderTestContext` constructor, there's no longer a requirement to call `Set*RendererConfig` prior modifying any specific properties on the config, nor is it problematic to call the (replacement) `SetSurface` method after modifying the config. Where the renderer config was being customised in embedder unit tests, I've pushed that customisation up to the top of the test where the rest of the test context is configured. This eliminates nearly all remaining `#ifdef SHELL_ENABLE_$BACKEND` blocks in the EmbedderTest infrastructure. Issue: flutter/flutter#158998 [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
1 parent 8ad96e0 commit 3e96ceb

21 files changed

+705
-927
lines changed

shell/platform/embedder/BUILD.gn

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -315,7 +315,6 @@ if (enable_unittests) {
315315

316316
if (test_enable_gl) {
317317
sources += [
318-
"tests/embedder_config_builder_gl.cc",
319318
"tests/embedder_test_backingstore_producer_gl.cc",
320319
"tests/embedder_test_backingstore_producer_gl.h",
321320
"tests/embedder_test_compositor_gl.cc",
@@ -333,7 +332,6 @@ if (enable_unittests) {
333332

334333
if (test_enable_metal) {
335334
sources += [
336-
"tests/embedder_config_builder_metal.mm",
337335
"tests/embedder_test_backingstore_producer_metal.h",
338336
"tests/embedder_test_backingstore_producer_metal.mm",
339337
"tests/embedder_test_compositor_metal.h",
@@ -348,7 +346,6 @@ if (enable_unittests) {
348346

349347
if (test_enable_vulkan) {
350348
sources += [
351-
"tests/embedder_config_builder_vulkan.cc",
352349
"tests/embedder_test_backingstore_producer_vulkan.cc",
353350
"tests/embedder_test_backingstore_producer_vulkan.h",
354351
"tests/embedder_test_compositor_vulkan.cc",

shell/platform/embedder/tests/embedder_a11y_unittests.cc

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ TEST_F(EmbedderTest, CannotProvideMultipleSemanticsCallbacks) {
3535
{
3636
EmbedderConfigBuilder builder(
3737
GetEmbedderContext(EmbedderTestContextType::kSoftwareContext));
38-
builder.SetSoftwareRendererConfig();
38+
builder.SetSurface(SkISize::Make(1, 1));
3939
builder.GetProjectArgs().update_semantics_callback =
4040
[](const FlutterSemanticsUpdate* update, void* user_data) {};
4141
builder.GetProjectArgs().update_semantics_callback2 =
@@ -48,7 +48,7 @@ TEST_F(EmbedderTest, CannotProvideMultipleSemanticsCallbacks) {
4848
{
4949
EmbedderConfigBuilder builder(
5050
GetEmbedderContext(EmbedderTestContextType::kSoftwareContext));
51-
builder.SetSoftwareRendererConfig();
51+
builder.SetSurface(SkISize::Make(1, 1));
5252
builder.GetProjectArgs().update_semantics_callback2 =
5353
[](const FlutterSemanticsUpdate2* update, void* user_data) {};
5454
builder.GetProjectArgs().update_semantics_node_callback =
@@ -63,7 +63,7 @@ TEST_F(EmbedderTest, CannotProvideMultipleSemanticsCallbacks) {
6363
{
6464
EmbedderConfigBuilder builder(
6565
GetEmbedderContext(EmbedderTestContextType::kSoftwareContext));
66-
builder.SetSoftwareRendererConfig();
66+
builder.SetSurface(SkISize::Make(1, 1));
6767
builder.GetProjectArgs().update_semantics_callback =
6868
[](const FlutterSemanticsUpdate* update, void* user_data) {};
6969
builder.GetProjectArgs().update_semantics_node_callback =
@@ -78,7 +78,7 @@ TEST_F(EmbedderTest, CannotProvideMultipleSemanticsCallbacks) {
7878
{
7979
EmbedderConfigBuilder builder(
8080
GetEmbedderContext(EmbedderTestContextType::kSoftwareContext));
81-
builder.SetSoftwareRendererConfig();
81+
builder.SetSurface(SkISize::Make(1, 1));
8282
builder.GetProjectArgs().update_semantics_callback2 =
8383
[](const FlutterSemanticsUpdate2* update, void* user_data) {};
8484
builder.GetProjectArgs().update_semantics_callback =
@@ -171,7 +171,7 @@ TEST_F(EmbedderA11yTest, A11yTreeIsConsistentUsingV3Callbacks) {
171171
});
172172

173173
EmbedderConfigBuilder builder(context);
174-
builder.SetSoftwareRendererConfig();
174+
builder.SetSurface(SkISize::Make(1, 1));
175175
builder.SetDartEntrypoint("a11y_main");
176176

177177
auto engine = builder.LaunchEngine();
@@ -375,7 +375,7 @@ TEST_F(EmbedderA11yTest, A11yStringAttributes) {
375375
});
376376

377377
EmbedderConfigBuilder builder(context);
378-
builder.SetSoftwareRendererConfig();
378+
builder.SetSurface(SkISize::Make(1, 1));
379379
builder.SetDartEntrypoint("a11y_string_attributes");
380380

381381
auto engine = builder.LaunchEngine();
@@ -468,7 +468,7 @@ TEST_F(EmbedderA11yTest, A11yTreeIsConsistentUsingV2Callbacks) {
468468
});
469469

470470
EmbedderConfigBuilder builder(context);
471-
builder.SetSoftwareRendererConfig();
471+
builder.SetSurface(SkISize::Make(1, 1));
472472
builder.SetDartEntrypoint("a11y_main");
473473

474474
auto engine = builder.LaunchEngine();
@@ -662,7 +662,7 @@ TEST_F(EmbedderA11yTest, A11yTreeIsConsistentUsingV1Callbacks) {
662662
});
663663

664664
EmbedderConfigBuilder builder(context);
665-
builder.SetSoftwareRendererConfig();
665+
builder.SetSurface(SkISize::Make(1, 1));
666666
builder.SetDartEntrypoint("a11y_main");
667667

668668
auto engine = builder.LaunchEngine();

shell/platform/embedder/tests/embedder_config_builder.cc

Lines changed: 6 additions & 104 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
#include "flutter/runtime/dart_vm.h"
99
#include "flutter/shell/platform/embedder/embedder.h"
1010
#include "tests/embedder_test_context.h"
11-
#include "third_party/skia/include/core/SkBitmap.h"
1211
#include "third_party/skia/include/core/SkImage.h"
1312

1413
namespace flutter::testing {
@@ -27,28 +26,6 @@ EmbedderConfigBuilder::EmbedderConfigBuilder(
2726

2827
custom_task_runners_.struct_size = sizeof(FlutterCustomTaskRunners);
2928

30-
InitializeGLRendererConfig();
31-
InitializeMetalRendererConfig();
32-
InitializeVulkanRendererConfig();
33-
34-
software_renderer_config_.struct_size = sizeof(FlutterSoftwareRendererConfig);
35-
software_renderer_config_.surface_present_callback =
36-
[](void* context, const void* allocation, size_t row_bytes,
37-
size_t height) {
38-
auto image_info =
39-
SkImageInfo::MakeN32Premul(SkISize::Make(row_bytes / 4, height));
40-
SkBitmap bitmap;
41-
if (!bitmap.installPixels(image_info, const_cast<void*>(allocation),
42-
row_bytes)) {
43-
FML_LOG(ERROR) << "Could not copy pixels for the software "
44-
"composition from the engine.";
45-
return false;
46-
}
47-
bitmap.setImmutable();
48-
return reinterpret_cast<EmbedderTestContextSoftware*>(context)->Present(
49-
SkImages::RasterFromBitmap(bitmap));
50-
};
51-
5229
// The first argument is always the executable name. Don't make tests have to
5330
// do this manually.
5431
AddCommandLineArgument("embedder_unittest");
@@ -79,30 +56,6 @@ FlutterProjectArgs& EmbedderConfigBuilder::GetProjectArgs() {
7956
return project_args_;
8057
}
8158

82-
void EmbedderConfigBuilder::SetSoftwareRendererConfig(SkISize surface_size) {
83-
renderer_config_.type = FlutterRendererType::kSoftware;
84-
renderer_config_.software = software_renderer_config_;
85-
context_.SetupSurface(surface_size);
86-
}
87-
88-
void EmbedderConfigBuilder::SetRendererConfig(EmbedderTestContextType type,
89-
SkISize surface_size) {
90-
switch (type) {
91-
case EmbedderTestContextType::kOpenGLContext:
92-
SetOpenGLRendererConfig(surface_size);
93-
break;
94-
case EmbedderTestContextType::kMetalContext:
95-
SetMetalRendererConfig(surface_size);
96-
break;
97-
case EmbedderTestContextType::kVulkanContext:
98-
SetVulkanRendererConfig(surface_size);
99-
break;
100-
case EmbedderTestContextType::kSoftwareContext:
101-
SetSoftwareRendererConfig(surface_size);
102-
break;
103-
}
104-
}
105-
10659
void EmbedderConfigBuilder::SetAssetsPath() {
10760
project_args_.assets_path = context_.GetAssetsPath().c_str();
10861
}
@@ -217,10 +170,6 @@ void EmbedderConfigBuilder::SetupVsyncCallback() {
217170
};
218171
}
219172

220-
FlutterRendererConfig& EmbedderConfigBuilder::GetRendererConfig() {
221-
return renderer_config_;
222-
}
223-
224173
void EmbedderConfigBuilder::SetRenderTaskRunner(
225174
const FlutterTaskRunnerDescription* runner) {
226175
if (runner == nullptr) {
@@ -336,11 +285,12 @@ UniqueEngine EmbedderConfigBuilder::SetupEngine(bool run) const {
336285
project_args.dart_entrypoint_argc = 0;
337286
}
338287

339-
auto result =
340-
run ? FlutterEngineRun(FLUTTER_ENGINE_VERSION, &renderer_config_,
341-
&project_args, &context_, &engine)
342-
: FlutterEngineInitialize(FLUTTER_ENGINE_VERSION, &renderer_config_,
343-
&project_args, &context_, &engine);
288+
auto result = run ? FlutterEngineRun(FLUTTER_ENGINE_VERSION,
289+
&context_.GetRendererConfig(),
290+
&project_args, &context_, &engine)
291+
: FlutterEngineInitialize(
292+
FLUTTER_ENGINE_VERSION, &context_.GetRendererConfig(),
293+
&project_args, &context_, &engine);
344294

345295
if (result != kSuccess) {
346296
return {};
@@ -349,52 +299,4 @@ UniqueEngine EmbedderConfigBuilder::SetupEngine(bool run) const {
349299
return UniqueEngine{engine};
350300
}
351301

352-
#ifndef SHELL_ENABLE_GL
353-
// OpenGL fallback implementations.
354-
// See: flutter/shell/platform/embedder/tests/embedder_config_builder_gl.cc.
355-
356-
void EmbedderConfigBuilder::InitializeGLRendererConfig() {
357-
// no-op.
358-
}
359-
360-
void EmbedderConfigBuilder::SetOpenGLFBOCallBack() {
361-
FML_LOG(FATAL) << "OpenGL is not enabled in this build.";
362-
}
363-
364-
void EmbedderConfigBuilder::SetOpenGLPresentCallBack() {
365-
FML_LOG(FATAL) << "OpenGL is not enabled in this build.";
366-
}
367-
368-
void EmbedderConfigBuilder::SetOpenGLRendererConfig(SkISize surface_size) {
369-
FML_LOG(FATAL) << "OpenGL is not enabled in this build.";
370-
}
371-
#endif
372-
#ifndef SHELL_ENABLE_METAL
373-
// Metal fallback implementations.
374-
// See: flutter/shell/platform/embedder/tests/embedder_config_builder_metal.mm.
375-
376-
void EmbedderConfigBuilder::InitializeMetalRendererConfig() {
377-
// no-op.
378-
}
379-
380-
void EmbedderConfigBuilder::SetMetalRendererConfig(SkISize surface_size) {
381-
FML_LOG(FATAL) << "Metal is not enabled in this build.";
382-
}
383-
#endif
384-
#ifndef SHELL_ENABLE_VULKAN
385-
// Vulkan fallback implementations.
386-
// See: flutter/shell/platform/embedder/tests/embedder_config_builder_vulkan.cc.
387-
388-
void EmbedderConfigBuilder::InitializeVulkanRendererConfig() {
389-
// no-op.
390-
}
391-
392-
void EmbedderConfigBuilder::SetVulkanRendererConfig(
393-
SkISize surface_size,
394-
std::optional<FlutterVulkanInstanceProcAddressCallback>
395-
instance_proc_address_callback) {
396-
FML_LOG(FATAL) << "Vulkan is not enabled in this build.";
397-
}
398-
#endif
399-
400302
} // namespace flutter::testing

shell/platform/embedder/tests/embedder_config_builder.h

Lines changed: 1 addition & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -44,31 +44,6 @@ class EmbedderConfigBuilder {
4444

4545
FlutterProjectArgs& GetProjectArgs();
4646

47-
void SetRendererConfig(EmbedderTestContextType type, SkISize surface_size);
48-
49-
void SetSoftwareRendererConfig(SkISize surface_size = SkISize::Make(1, 1));
50-
51-
void SetOpenGLRendererConfig(SkISize surface_size);
52-
53-
void SetMetalRendererConfig(SkISize surface_size);
54-
55-
void SetVulkanRendererConfig(
56-
SkISize surface_size,
57-
std::optional<FlutterVulkanInstanceProcAddressCallback>
58-
instance_proc_address_callback = {});
59-
60-
// Used to explicitly set an `open_gl.fbo_callback`. Using this method will
61-
// cause your test to fail since the ctor for this class sets
62-
// `open_gl.fbo_callback_with_frame_info`. This method exists as a utility to
63-
// explicitly test this behavior.
64-
void SetOpenGLFBOCallBack();
65-
66-
// Used to explicitly set an `open_gl.present`. Using this method will cause
67-
// your test to fail since the ctor for this class sets
68-
// `open_gl.present_with_info`. This method exists as a utility to explicitly
69-
// test this behavior.
70-
void SetOpenGLPresentCallBack();
71-
7247
void SetAssetsPath();
7348

7449
void SetSnapshots();
@@ -109,7 +84,7 @@ class EmbedderConfigBuilder {
10984

11085
FlutterCompositor& GetCompositor();
11186

112-
FlutterRendererConfig& GetRendererConfig();
87+
void SetSurface(SkISize surface_size) { context_.SetSurface(surface_size); }
11388

11489
void SetRenderTargetType(
11590
EmbedderTestBackingStoreProducer::RenderTargetType type,
@@ -125,23 +100,8 @@ class EmbedderConfigBuilder {
125100
void SetupVsyncCallback();
126101

127102
private:
128-
void InitializeGLRendererConfig();
129-
void InitializeVulkanRendererConfig();
130-
void InitializeMetalRendererConfig();
131-
132103
EmbedderTestContext& context_;
133104
FlutterProjectArgs project_args_ = {};
134-
FlutterRendererConfig renderer_config_ = {};
135-
FlutterSoftwareRendererConfig software_renderer_config_ = {};
136-
#ifdef SHELL_ENABLE_GL
137-
FlutterOpenGLRendererConfig opengl_renderer_config_ = {};
138-
#endif
139-
#ifdef SHELL_ENABLE_METAL
140-
FlutterMetalRendererConfig metal_renderer_config_ = {};
141-
#endif
142-
#ifdef SHELL_ENABLE_VULKAN
143-
FlutterVulkanRendererConfig vulkan_renderer_config_ = {};
144-
#endif
145105
std::string dart_entrypoint_;
146106
FlutterCustomTaskRunners custom_task_runners_ = {};
147107
FlutterCompositor compositor_ = {};

shell/platform/embedder/tests/embedder_config_builder_gl.cc

Lines changed: 0 additions & 81 deletions
This file was deleted.

0 commit comments

Comments
 (0)