Visitar URL original
[windows] Add OpenGL compositor tests against ANGLE at D3D11 feature … · suryaKommana2662/flutter@3530929 · GitHub
Skip to content

Commit 3530929

Browse files
b-luksuryaKommana2662
b-luk
authored and
suryaKommana2662
committed
[windows] Add OpenGL compositor tests against ANGLE at D3D11 feature level 10_0 (flutter#194114)
Adds a parameter (only used in tests) to `egl::Manager` to cap ANGLE's D3D11 feature level. Adds `compositor_opengl_angle_unittests.cc`, based on [`compositor_opengl_unittests.cc`](https://github.com/flutter/flutter/blob/97db29fcaa2aa757c8d86f06187b5268bf4eed4a/engine/src/flutter/shell/platform/windows/compositor_opengl_unittests.cc). Unlike the existing tests, the new tests exercise the compositor with real ANGLE instead of mocks. The new tests exercise the compositor's backing store creation and presenting. The tests run both at uncapped feature level and capped to 10_0. This emulates the case where flutter#191978 resulted in a black window. Verified locally that when flutter#192917 is reverted, the new tests fail when running at feature level 10_0 and pass when running without a feature level cap. Part of flutter#192899: - This PR exercises the OpenGL compositor and swapchain presenting codepath on 10_0, verifying that the backing store framebuffer is valid (GL_FRAMEBUFFER_COMPLETE) with no GL errors. This catches issues that occur at the compositor level, like flutter#191978. - This would not catch issues where the rendering behavior of Impeller is broken due to taking different code paths on 10_0 (e.g. flutter#192918 resulting in different BackdropFilter behavior because offscreen MSAA is unavailable with 10_0). - I intend to follow up with a PR that extends our existing [`engine_integration_golden_test.dart`](https://github.com/flutter/flutter/blob/38785ef8b19e2a1ee1303167074ec98f95f7cb5b/dev/integration_tests/engine_integration_golden_test/integration_test/engine_integration_golden_test.dart) devicelab golden test, which currently [runs on Windows](https://github.com/flutter/flutter/blob/38785ef8b19e2a1ee1303167074ec98f95f7cb5b/dev/devicelab/lib/tasks/integration_tests.dart#L242-L248), to also run with feature level 10_0. These golden tests would catch issues where our Impeller rendering is broken. Goldens do not catch compositor level issues because the golden functionality reads its images from Impeller, skipping the compositor portion of the rendering pipeline. - This PR, together with the anticipated follow up PR, will give us basic smoke testing coverage on 10_0 of the rendering pipeline from Flutter app code, through Impeller rendering, to presenting on the swapchain. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [AI contribution guidelines] and understand my responsibilities, or I am not using AI tools. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant in-code documentation (doc comments with `///`). - [x] If this PR introduces a new feature or capability, I created and linked a website documentation issue or PR in [flutter/website] (or verified none is needed). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. If this change needs to override an active code freeze, provide a comment explaining why. The code freeze workflow can be overridden by code reviewers. See pinned issues for any active code freezes with guidance. **Note**: The Flutter team is currently trialing the use of [Gemini Code Assist for GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code). Comments from the `gemini-code-assist` bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [AI contribution guidelines]: https://github.com/flutter/flutter/blob/main/CONTRIBUTING.ai.md [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [flutter/website]: https://github.com/flutter/website [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
1 parent 8991e16 commit 3530929

6 files changed

Lines changed: 294 additions & 14 deletions

File tree

‎engine/src/flutter/shell/platform/windows/BUILD.gn‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,12 +224,14 @@ executable("flutter_windows_unittests") {
224224
sources = [
225225
"accessibility_bridge_windows_unittests.cc",
226226
"accessibility_plugin_unittests.cc",
227+
"compositor_opengl_angle_unittests.cc",
227228
"compositor_opengl_unittests.cc",
228229
"compositor_software_unittests.cc",
229230
"cursor_handler_unittests.cc",
230231
"direct_manipulation_unittests.cc",
231232
"display_manager_unittests.cc",
232233
"dpi_utils_unittests.cc",
234+
"egl/manager_unittests.cc",
233235
"flutter_project_bundle_unittests.cc",
234236
"flutter_window_unittests.cc",
235237
"flutter_windows_engine_unittests.cc",

‎engine/src/flutter/shell/platform/windows/compositor_opengl_angle_unittests.cc‎

Lines changed: 177 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,177 @@
1+
// Copyright 2013 The Flutter Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
#include <memory>
6+
#include <optional>
7+
8+
#include "flutter/impeller/renderer/backend/gles/gles.h"
9+
#include "flutter/shell/platform/windows/compositor_opengl.h"
10+
#include "flutter/shell/platform/windows/egl/manager.h"
11+
#include "flutter/shell/platform/windows/flutter_window.h"
12+
#include "flutter/shell/platform/windows/flutter_windows_view.h"
13+
#include "flutter/shell/platform/windows/testing/engine_modifier.h"
14+
#include "flutter/shell/platform/windows/testing/flutter_windows_engine_builder.h"
15+
#include "flutter/shell/platform/windows/testing/windows_test.h"
16+
#include "gtest/gtest.h"
17+
18+
namespace flutter {
19+
namespace testing {
20+
21+
namespace {
22+
23+
// Resolves OpenGL procedures through EGL, as the engine does.
24+
const impeller::ProcTableGLES::Resolver kEGLResolver =
25+
[](const char* name) -> void* {
26+
return reinterpret_cast<void*>(::eglGetProcAddress(name));
27+
};
28+
29+
// Verifies that |framebuffer| is complete on the current context.
30+
void ExpectFramebufferComplete(GLuint framebuffer) {
31+
impeller::ProcTableGLES gl{kEGLResolver};
32+
ASSERT_TRUE(gl.IsValid());
33+
34+
gl.BindFramebuffer(GL_FRAMEBUFFER, framebuffer);
35+
EXPECT_EQ(gl.CheckFramebufferStatus(GL_FRAMEBUFFER),
36+
static_cast<GLenum>(GL_FRAMEBUFFER_COMPLETE));
37+
}
38+
39+
// Verifies that no GL error is pending on the current context. GL reports
40+
// failed calls this way rather than through return values.
41+
void ExpectNoGLError() {
42+
auto get_error =
43+
reinterpret_cast<PFNGLGETERRORPROC>(kEGLResolver("glGetError"));
44+
ASSERT_NE(get_error, nullptr);
45+
EXPECT_EQ(get_error(), static_cast<GLenum>(GL_NO_ERROR));
46+
}
47+
48+
struct TestParams {
49+
// The test's name suffix.
50+
const char* name;
51+
// Whether the compositor is configured for Impeller rather than Skia.
52+
bool enable_impeller;
53+
// Caps the D3D11 feature level ANGLE may use. At 10_0 it only creates OpenGL
54+
// ES 2.0 contexts, like the GPUs where
55+
// https://github.com/flutter/flutter/issues/191978 left the window black.
56+
std::optional<egl::D3DFeatureLevel> max_feature_level;
57+
};
58+
59+
// Tests the compositor against ANGLE, unlike CompositorOpenGLTest in
60+
// compositor_opengl_unittests.cc, which uses mocked EGL and OpenGL.
61+
class CompositorOpenGLANGLETest
62+
: public WindowsTest,
63+
public ::testing::WithParamInterface<TestParams> {
64+
public:
65+
CompositorOpenGLANGLETest() = default;
66+
virtual ~CompositorOpenGLANGLETest() = default;
67+
68+
protected:
69+
void TearDown() override {
70+
// The compositor leaves the render context current on this thread.
71+
// Release it before the engine destroys it, or ANGLE crashes the next test
72+
// that reuses the display.
73+
if (engine_ && engine_->egl_manager() &&
74+
engine_->egl_manager()->render_context()) {
75+
engine_->egl_manager()->render_context()->ClearCurrent();
76+
}
77+
WindowsTest::TearDown();
78+
}
79+
80+
FlutterWindowsEngine* engine() { return engine_.get(); }
81+
FlutterWindowsView* view() { return view_.get(); }
82+
83+
void UseHeadlessEngine() {
84+
auto egl_manager = egl::Manager::Create(egl::GpuPreference::NoPreference,
85+
/*allow_inverted_surface=*/false,
86+
GetParam().max_feature_level);
87+
EXPECT_NE(egl_manager, nullptr);
88+
89+
FlutterWindowsEngineBuilder builder{GetContext()};
90+
91+
engine_ = builder.Build();
92+
EngineModifier modifier{engine_.get()};
93+
modifier.SetEGLManager(std::move(egl_manager));
94+
}
95+
96+
// Adds a view with a real window and surface to the engine.
97+
void UseEngineWithView() {
98+
UseHeadlessEngine();
99+
100+
auto window = std::make_unique<FlutterWindow>(
101+
320, 240, engine_->display_manager(), engine_->windows_proc_table());
102+
view_ = std::make_unique<FlutterWindowsView>(kImplicitViewId, engine_.get(),
103+
std::move(window), false,
104+
BoxConstraints());
105+
view_->CreateRenderSurface();
106+
}
107+
108+
private:
109+
std::unique_ptr<FlutterWindowsEngine> engine_;
110+
std::unique_ptr<FlutterWindowsView> view_;
111+
112+
FML_DISALLOW_COPY_AND_ASSIGN(CompositorOpenGLANGLETest);
113+
};
114+
115+
} // namespace
116+
117+
INSTANTIATE_TEST_SUITE_P(
118+
RendererAndFeatureLevel,
119+
CompositorOpenGLANGLETest,
120+
::testing::Values(
121+
TestParams{.name = "SkiaFeatureLevelDefault",
122+
.enable_impeller = false,
123+
.max_feature_level = std::nullopt},
124+
TestParams{.name = "ImpellerFeatureLevelDefault",
125+
.enable_impeller = true,
126+
.max_feature_level = std::nullopt},
127+
TestParams{.name = "SkiaFeatureLevel10_0",
128+
.enable_impeller = false,
129+
.max_feature_level = egl::D3DFeatureLevel{10, 0}},
130+
TestParams{.name = "ImpellerFeatureLevel10_0",
131+
.enable_impeller = true,
132+
.max_feature_level = egl::D3DFeatureLevel{10, 0}}),
133+
[](const ::testing::TestParamInfo<TestParams>& info) {
134+
return std::string(info.param.name);
135+
});
136+
137+
TEST_P(CompositorOpenGLANGLETest, CreateBackingStore) {
138+
UseHeadlessEngine();
139+
140+
auto compositor =
141+
CompositorOpenGL{engine(), kEGLResolver, GetParam().enable_impeller};
142+
FlutterBackingStoreConfig config = {};
143+
config.size = {320, 240};
144+
FlutterBackingStore backing_store = {};
145+
146+
ASSERT_TRUE(compositor.CreateBackingStore(config, &backing_store));
147+
ExpectNoGLError();
148+
ExpectFramebufferComplete(backing_store.open_gl.framebuffer.name);
149+
ASSERT_TRUE(compositor.CollectBackingStore(&backing_store));
150+
}
151+
152+
TEST_P(CompositorOpenGLANGLETest, Present) {
153+
UseEngineWithView();
154+
155+
auto compositor =
156+
CompositorOpenGL{engine(), kEGLResolver, GetParam().enable_impeller};
157+
FlutterBackingStoreConfig config = {};
158+
config.size = {320, 240};
159+
FlutterBackingStore backing_store = {};
160+
161+
ASSERT_TRUE(compositor.CreateBackingStore(config, &backing_store));
162+
ExpectNoGLError();
163+
164+
FlutterLayer layer = {};
165+
layer.type = kFlutterLayerContentTypeBackingStore;
166+
layer.backing_store = &backing_store;
167+
layer.size = {320, 240};
168+
const FlutterLayer* layer_ptr = &layer;
169+
170+
EXPECT_TRUE(compositor.Present(view(), &layer_ptr, 1));
171+
ExpectNoGLError();
172+
173+
ASSERT_TRUE(compositor.CollectBackingStore(&backing_store));
174+
}
175+
176+
} // namespace testing
177+
} // namespace flutter

‎engine/src/flutter/shell/platform/windows/compositor_opengl_unittests.cc‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,9 @@ const impeller::ProcTableGLES::Resolver kRecordingResolverGLES3 =
218218
return kMockResolver(name);
219219
};
220220

221+
// Tests the compositor against mocked EGL and OpenGL, unlike
222+
// CompositorOpenGLANGLETest in compositor_opengl_angle_unittests.cc, which uses
223+
// ANGLE.
221224
class CompositorOpenGLTest : public WindowsTest {
222225
public:
223226
CompositorOpenGLTest() = default;

‎engine/src/flutter/shell/platform/windows/egl/manager.cc‎

Lines changed: 42 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -33,24 +33,51 @@ bool HasExtension(std::string_view extensions, std::string_view name) {
3333
return false;
3434
}
3535

36+
// If |max_feature_level| is set, appends attributes that limit ANGLE's D3D11
37+
// device to at most that feature level.
38+
void AppendMaxFeatureLevel(std::optional<D3DFeatureLevel> max_feature_level,
39+
std::vector<EGLint>& attributes) {
40+
if (!max_feature_level.has_value()) {
41+
return;
42+
}
43+
attributes.push_back(EGL_PLATFORM_ANGLE_MAX_VERSION_MAJOR_ANGLE);
44+
attributes.push_back(static_cast<EGLint>(max_feature_level->major));
45+
attributes.push_back(EGL_PLATFORM_ANGLE_MAX_VERSION_MINOR_ANGLE);
46+
attributes.push_back(static_cast<EGLint>(max_feature_level->minor));
47+
48+
// ANGLE caches displays per process, keyed on a fixed set of attributes
49+
// that does not include the feature level cap. Add a key based on the
50+
// feature level to prevent incorrect cache hits for displays that have
51+
// different feature levels.
52+
attributes.push_back(EGL_PLATFORM_ANGLE_DISPLAY_KEY_ANGLE);
53+
attributes.push_back(static_cast<EGLint>(max_feature_level->major * 10 +
54+
max_feature_level->minor));
55+
}
56+
3657
} // namespace
3758

3859
int Manager::instance_count_ = 0;
3960

40-
std::unique_ptr<Manager> Manager::Create(GpuPreference gpu_preference,
41-
bool allow_inverted_surface) {
61+
std::unique_ptr<Manager> Manager::Create(
62+
GpuPreference gpu_preference,
63+
bool allow_inverted_surface,
64+
std::optional<D3DFeatureLevel> max_feature_level) {
4265
std::unique_ptr<Manager> manager;
43-
manager.reset(new Manager(gpu_preference, allow_inverted_surface));
66+
manager.reset(
67+
new Manager(gpu_preference, allow_inverted_surface, max_feature_level));
4468
if (!manager->IsValid()) {
4569
return nullptr;
4670
}
4771
return std::move(manager);
4872
}
4973

50-
Manager::Manager(GpuPreference gpu_preference, bool allow_inverted_surface) {
74+
Manager::Manager(GpuPreference gpu_preference,
75+
bool allow_inverted_surface,
76+
std::optional<D3DFeatureLevel> max_feature_level) {
5177
++instance_count_;
5278

53-
if (!InitializeDisplay(gpu_preference, allow_inverted_surface)) {
79+
if (!InitializeDisplay(gpu_preference, allow_inverted_surface,
80+
max_feature_level)) {
5481
return;
5582
}
5683

@@ -70,8 +97,10 @@ Manager::~Manager() {
7097
--instance_count_;
7198
}
7299

73-
bool Manager::InitializeDisplay(GpuPreference gpu_preference,
74-
bool allow_inverted_surface) {
100+
bool Manager::InitializeDisplay(
101+
GpuPreference gpu_preference,
102+
bool allow_inverted_surface,
103+
std::optional<D3DFeatureLevel> max_feature_level) {
75104
// If the request for a low power GPU is provided,
76105
// we will attempt to select GPU explicitly, via ANGLE extension
77106
// that allows to specify the GPU to use via LUID.
@@ -132,6 +161,8 @@ bool Manager::InitializeDisplay(GpuPreference gpu_preference,
132161
attributes.push_back(static_cast<EGLint>(luid->LowPart));
133162
}
134163

164+
AppendMaxFeatureLevel(max_feature_level, attributes);
165+
135166
attributes.push_back(EGL_NONE);
136167
return attributes;
137168
};
@@ -161,13 +192,14 @@ bool Manager::InitializeDisplay(GpuPreference gpu_preference,
161192

162193
// These attributes request D3D11 WARP (software rendering fallback) in case
163194
// hardware-backed D3D11 is unavailable.
164-
const EGLint d3d11_warp_display_attributes[] = {
195+
std::vector<EGLint> d3d11_warp_display_attributes = {
165196
EGL_PLATFORM_ANGLE_TYPE_ANGLE,
166197
EGL_PLATFORM_ANGLE_TYPE_D3D11_ANGLE,
167198
EGL_PLATFORM_ANGLE_ENABLE_AUTOMATIC_TRIM_ANGLE,
168199
EGL_TRUE,
169-
EGL_NONE,
170200
};
201+
AppendMaxFeatureLevel(max_feature_level, d3d11_warp_display_attributes);
202+
d3d11_warp_display_attributes.push_back(EGL_NONE);
171203

172204
std::vector<const EGLint*> display_attributes_configs;
173205

@@ -178,7 +210,7 @@ bool Manager::InitializeDisplay(GpuPreference gpu_preference,
178210
}
179211
display_attributes_configs.push_back(d3d11_display_attributes.data());
180212
display_attributes_configs.push_back(d3d11_fl_9_3_display_attributes);
181-
display_attributes_configs.push_back(d3d11_warp_display_attributes);
213+
display_attributes_configs.push_back(d3d11_warp_display_attributes.data());
182214

183215
PFNEGLGETPLATFORMDISPLAYEXTPROC egl_get_platform_display_EXT =
184216
reinterpret_cast<PFNEGLGETPLATFORMDISPLAYEXTPROC>(

‎engine/src/flutter/shell/platform/windows/egl/manager.h‎

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,12 @@ enum class GpuPreference {
3535
HighPerformancePreference,
3636
};
3737

38+
// A Direct3D feature level, such as 10_0 or 11_1.
39+
struct D3DFeatureLevel {
40+
int major;
41+
int minor;
42+
};
43+
3844
// A manager for initializing ANGLE correctly and using it to create and
3945
// destroy surfaces
4046
class Manager {
@@ -45,8 +51,15 @@ class Manager {
4551
// EGL_ANGLE_surface_orientation, window surfaces are created with an
4652
// inverted Y axis so that the default framebuffer shares Impeller's
4753
// top-left origin. See |surface_origin_is_top_left|.
48-
static std::unique_ptr<Manager> Create(GpuPreference gpu_preference,
49-
bool allow_inverted_surface);
54+
//
55+
// If |max_feature_level| is set, ANGLE's D3D11 renderer is limited to at
56+
// most that feature level, even if the hardware supports a higher one. This
57+
// is intended for testing: capping to 10_0 makes ANGLE expose only an
58+
// OpenGL ES 2.0 context, emulating older GPUs on any machine.
59+
static std::unique_ptr<Manager> Create(
60+
GpuPreference gpu_preference,
61+
bool allow_inverted_surface,
62+
std::optional<D3DFeatureLevel> max_feature_level = std::nullopt);
5063

5164
virtual ~Manager();
5265

@@ -105,7 +118,9 @@ class Manager {
105118
protected:
106119
// Creates a new surface manager retaining reference to the passed-in target
107120
// for the lifetime of the manager.
108-
Manager(GpuPreference gpu_preference, bool allow_inverted_surface);
121+
Manager(GpuPreference gpu_preference,
122+
bool allow_inverted_surface,
123+
std::optional<D3DFeatureLevel> max_feature_level = std::nullopt);
109124

110125
private:
111126
// Number of active instances of Manager
@@ -117,7 +132,8 @@ class Manager {
117132

118133
// Initialize the EGL display.
119134
bool InitializeDisplay(GpuPreference gpu_preference,
120-
bool allow_inverted_surface);
135+
bool allow_inverted_surface,
136+
std::optional<D3DFeatureLevel> max_feature_level);
121137

122138
// Initialize the EGL configs.
123139
bool InitializeConfig();

0 commit comments

Comments
 (0)