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

Commit a718337

Browse files
authored
Reland "[Windows] Introduce egl::Surface and egl::WindowSurface" (#50148)
# Original pull request description This introduces the `egl::Surface` and `egl::WindowSurface` types to abstract a raw `EGLSurface`. This also removes some - but not all - EGL surface logic out from `EGLManager`. Subsequent pull requests will be necessary to: 1. Move ownership of the `egl::WindowSurface` from `egl::Manager` to `FlutterWindowsView` 2. Refactor external texture's off-screen EGL surface to use `egl::Surface` Part of flutter/flutter#141996 # Reland #49983 was reverted as it introduced a crash if the render surface fails to be created even though EGL was initialized successfully. This pull request is split into the following commits: 1. c0b11be is the original pull request unchanged 2. 1dc7813 is the fix: it checks a surface is valid before using it. This also adds several tests to prevent this kind of regression. [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
1 parent 39415c3 commit a718337

18 files changed

+655
-232
lines changed

ci/licenses_golden/licenses_flutter

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7252,6 +7252,10 @@ ORIGIN: ../../../flutter/shell/platform/windows/egl/manager.cc + ../../../flutte
72527252
ORIGIN: ../../../flutter/shell/platform/windows/egl/manager.h + ../../../flutter/LICENSE
72537253
ORIGIN: ../../../flutter/shell/platform/windows/egl/proc_table.cc + ../../../flutter/LICENSE
72547254
ORIGIN: ../../../flutter/shell/platform/windows/egl/proc_table.h + ../../../flutter/LICENSE
7255+
ORIGIN: ../../../flutter/shell/platform/windows/egl/surface.cc + ../../../flutter/LICENSE
7256+
ORIGIN: ../../../flutter/shell/platform/windows/egl/surface.h + ../../../flutter/LICENSE
7257+
ORIGIN: ../../../flutter/shell/platform/windows/egl/window_surface.cc + ../../../flutter/LICENSE
7258+
ORIGIN: ../../../flutter/shell/platform/windows/egl/window_surface.h + ../../../flutter/LICENSE
72557259
ORIGIN: ../../../flutter/shell/platform/windows/event_watcher.cc + ../../../flutter/LICENSE
72567260
ORIGIN: ../../../flutter/shell/platform/windows/event_watcher.h + ../../../flutter/LICENSE
72577261
ORIGIN: ../../../flutter/shell/platform/windows/external_texture.h + ../../../flutter/LICENSE
@@ -10137,6 +10141,10 @@ FILE: ../../../flutter/shell/platform/windows/egl/manager.cc
1013710141
FILE: ../../../flutter/shell/platform/windows/egl/manager.h
1013810142
FILE: ../../../flutter/shell/platform/windows/egl/proc_table.cc
1013910143
FILE: ../../../flutter/shell/platform/windows/egl/proc_table.h
10144+
FILE: ../../../flutter/shell/platform/windows/egl/surface.cc
10145+
FILE: ../../../flutter/shell/platform/windows/egl/surface.h
10146+
FILE: ../../../flutter/shell/platform/windows/egl/window_surface.cc
10147+
FILE: ../../../flutter/shell/platform/windows/egl/window_surface.h
1014010148
FILE: ../../../flutter/shell/platform/windows/event_watcher.cc
1014110149
FILE: ../../../flutter/shell/platform/windows/event_watcher.h
1014210150
FILE: ../../../flutter/shell/platform/windows/external_texture.h

shell/platform/windows/BUILD.gn

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,10 @@ source_set("flutter_windows_source") {
5959
"egl/manager.h",
6060
"egl/proc_table.cc",
6161
"egl/proc_table.h",
62+
"egl/surface.cc",
63+
"egl/surface.h",
64+
"egl/window_surface.cc",
65+
"egl/window_surface.h",
6266
"event_watcher.cc",
6367
"event_watcher.h",
6468
"external_texture.h",
@@ -209,6 +213,7 @@ executable("flutter_windows_unittests") {
209213
"testing/egl/mock_context.h",
210214
"testing/egl/mock_manager.h",
211215
"testing/egl/mock_proc_table.h",
216+
"testing/egl/mock_window_surface.h",
212217
"testing/engine_modifier.h",
213218
"testing/flutter_window_test.cc",
214219
"testing/flutter_window_test.h",

shell/platform/windows/compositor_opengl.cc

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,11 @@ bool CompositorOpenGL::Present(const FlutterLayer** layers,
9797
return false;
9898
}
9999

100+
if (!engine_->egl_manager()->surface() ||
101+
!engine_->egl_manager()->surface()->IsValid()) {
102+
return false;
103+
}
104+
100105
// Clear the view if there are no layers to present.
101106
if (layers_count == 0) {
102107
// Normally the compositor is initialized when the first backing store is
@@ -128,7 +133,7 @@ bool CompositorOpenGL::Present(const FlutterLayer** layers,
128133
return false;
129134
}
130135

131-
if (!engine_->egl_manager()->MakeCurrent()) {
136+
if (!engine_->egl_manager()->surface()->MakeCurrent()) {
132137
return false;
133138
}
134139

@@ -154,7 +159,7 @@ bool CompositorOpenGL::Present(const FlutterLayer** layers,
154159
GL_NEAREST // filter
155160
);
156161

157-
if (!engine_->egl_manager()->SwapBuffers()) {
162+
if (!engine_->egl_manager()->surface()->SwapBuffers()) {
158163
return false;
159164
}
160165

@@ -165,7 +170,17 @@ bool CompositorOpenGL::Present(const FlutterLayer** layers,
165170
bool CompositorOpenGL::Initialize() {
166171
FML_DCHECK(!is_initialized_);
167172

168-
if (!engine_->egl_manager()->MakeCurrent()) {
173+
egl::Manager* manager = engine_->egl_manager();
174+
if (!manager) {
175+
return false;
176+
}
177+
178+
egl::Surface* surface = manager->surface();
179+
if (!surface || !surface->IsValid()) {
180+
return false;
181+
}
182+
183+
if (!surface->MakeCurrent()) {
169184
return false;
170185
}
171186

@@ -186,14 +201,14 @@ bool CompositorOpenGL::ClearSurface() {
186201
// Resize the surface if needed.
187202
engine_->view()->OnEmptyFrameGenerated();
188203

189-
if (!engine_->egl_manager()->MakeCurrent()) {
204+
if (!engine_->egl_manager()->surface()->MakeCurrent()) {
190205
return false;
191206
}
192207

193208
gl_->ClearColor(0.0f, 0.0f, 0.0f, 0.0f);
194209
gl_->Clear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT | GL_STENCIL_BUFFER_BIT);
195210

196-
if (!engine_->egl_manager()->SwapBuffers()) {
211+
if (!engine_->egl_manager()->surface()->SwapBuffers()) {
197212
return false;
198213
}
199214

shell/platform/windows/compositor_opengl_unittests.cc

Lines changed: 45 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#include "flutter/shell/platform/windows/egl/manager.h"
1111
#include "flutter/shell/platform/windows/flutter_windows_view.h"
1212
#include "flutter/shell/platform/windows/testing/egl/mock_manager.h"
13+
#include "flutter/shell/platform/windows/testing/egl/mock_window_surface.h"
1314
#include "flutter/shell/platform/windows/testing/engine_modifier.h"
1415
#include "flutter/shell/platform/windows/testing/flutter_windows_engine_builder.h"
1516
#include "flutter/shell/platform/windows/testing/mock_window_binding_handler.h"
@@ -65,20 +66,29 @@ class CompositorOpenGLTest : public WindowsTest {
6566
protected:
6667
FlutterWindowsEngine* engine() { return engine_.get(); }
6768
egl::MockManager* egl_manager() { return egl_manager_; }
69+
egl::MockWindowSurface* surface() { return surface_.get(); }
6870

69-
void UseHeadlessEngine() {
71+
void UseHeadlessEngine(bool add_surface = true) {
7072
auto egl_manager = std::make_unique<egl::MockManager>();
7173
egl_manager_ = egl_manager.get();
7274

75+
if (add_surface) {
76+
surface_ = std::make_unique<egl::MockWindowSurface>();
77+
EXPECT_CALL(*egl_manager_, surface)
78+
.WillRepeatedly(Return(surface_.get()));
79+
} else {
80+
EXPECT_CALL(*egl_manager_, surface).WillRepeatedly(Return(nullptr));
81+
}
82+
7383
FlutterWindowsEngineBuilder builder{GetContext()};
7484

7585
engine_ = builder.Build();
7686
EngineModifier modifier(engine_.get());
7787
modifier.SetEGLManager(std::move(egl_manager));
7888
}
7989

80-
void UseEngineWithView() {
81-
UseHeadlessEngine();
90+
void UseEngineWithView(bool add_surface = true) {
91+
UseHeadlessEngine(add_surface);
8292

8393
auto window = std::make_unique<MockWindowBindingHandler>();
8494
EXPECT_CALL(*window.get(), SetView).Times(1);
@@ -92,6 +102,7 @@ class CompositorOpenGLTest : public WindowsTest {
92102
private:
93103
std::unique_ptr<FlutterWindowsEngine> engine_;
94104
std::unique_ptr<FlutterWindowsView> view_;
105+
std::unique_ptr<egl::MockWindowSurface> surface_;
95106
egl::MockManager* egl_manager_;
96107

97108
FML_DISALLOW_COPY_AND_ASSIGN(CompositorOpenGLTest);
@@ -107,7 +118,8 @@ TEST_F(CompositorOpenGLTest, CreateBackingStore) {
107118
FlutterBackingStoreConfig config = {};
108119
FlutterBackingStore backing_store = {};
109120

110-
EXPECT_CALL(*egl_manager(), MakeCurrent).WillOnce(Return(true));
121+
EXPECT_CALL(*surface(), IsValid).WillOnce(Return(true));
122+
EXPECT_CALL(*surface(), MakeCurrent).WillOnce(Return(true));
111123
ASSERT_TRUE(compositor.CreateBackingStore(config, &backing_store));
112124
ASSERT_TRUE(compositor.CollectBackingStore(&backing_store));
113125
}
@@ -120,7 +132,8 @@ TEST_F(CompositorOpenGLTest, InitializationFailure) {
120132
FlutterBackingStoreConfig config = {};
121133
FlutterBackingStore backing_store = {};
122134

123-
EXPECT_CALL(*egl_manager(), MakeCurrent).WillOnce(Return(false));
135+
EXPECT_CALL(*surface(), IsValid).WillOnce(Return(true));
136+
EXPECT_CALL(*surface(), MakeCurrent).WillOnce(Return(false));
124137
EXPECT_FALSE(compositor.CreateBackingStore(config, &backing_store));
125138
}
126139

@@ -132,16 +145,17 @@ TEST_F(CompositorOpenGLTest, Present) {
132145
FlutterBackingStoreConfig config = {};
133146
FlutterBackingStore backing_store = {};
134147

135-
EXPECT_CALL(*egl_manager(), MakeCurrent).WillOnce(Return(true));
148+
EXPECT_CALL(*surface(), IsValid).WillRepeatedly(Return(true));
149+
EXPECT_CALL(*surface(), MakeCurrent).WillOnce(Return(true));
136150
ASSERT_TRUE(compositor.CreateBackingStore(config, &backing_store));
137151

138152
FlutterLayer layer = {};
139153
layer.type = kFlutterLayerContentTypeBackingStore;
140154
layer.backing_store = &backing_store;
141155
const FlutterLayer* layer_ptr = &layer;
142156

143-
EXPECT_CALL(*egl_manager(), MakeCurrent).WillOnce(Return(true));
144-
EXPECT_CALL(*egl_manager(), SwapBuffers).WillOnce(Return(true));
157+
EXPECT_CALL(*surface(), MakeCurrent).WillOnce(Return(true));
158+
EXPECT_CALL(*surface(), SwapBuffers).WillOnce(Return(true));
145159
EXPECT_TRUE(compositor.Present(&layer_ptr, 1));
146160

147161
ASSERT_TRUE(compositor.CollectBackingStore(&backing_store));
@@ -154,10 +168,9 @@ TEST_F(CompositorOpenGLTest, PresentEmpty) {
154168

155169
// The context will be bound twice: first to initialize the compositor, second
156170
// to clear the surface.
157-
EXPECT_CALL(*egl_manager(), MakeCurrent)
158-
.Times(2)
159-
.WillRepeatedly(Return(true));
160-
EXPECT_CALL(*egl_manager(), SwapBuffers).WillOnce(Return(true));
171+
EXPECT_CALL(*surface(), IsValid).WillRepeatedly(Return(true));
172+
EXPECT_CALL(*surface(), MakeCurrent).Times(2).WillRepeatedly(Return(true));
173+
EXPECT_CALL(*surface(), SwapBuffers).WillOnce(Return(true));
161174
EXPECT_TRUE(compositor.Present(nullptr, 0));
162175
}
163176

@@ -169,7 +182,8 @@ TEST_F(CompositorOpenGLTest, HeadlessPresentIgnored) {
169182
FlutterBackingStoreConfig config = {};
170183
FlutterBackingStore backing_store = {};
171184

172-
EXPECT_CALL(*egl_manager(), MakeCurrent).WillOnce(Return(true));
185+
EXPECT_CALL(*surface(), IsValid).WillOnce(Return(true));
186+
EXPECT_CALL(*surface(), MakeCurrent).WillOnce(Return(true));
173187
ASSERT_TRUE(compositor.CreateBackingStore(config, &backing_store));
174188

175189
FlutterLayer layer = {};
@@ -182,5 +196,23 @@ TEST_F(CompositorOpenGLTest, HeadlessPresentIgnored) {
182196
ASSERT_TRUE(compositor.CollectBackingStore(&backing_store));
183197
}
184198

199+
TEST_F(CompositorOpenGLTest, NoSurfaceIgnored) {
200+
UseEngineWithView(/*add_surface = */ false);
201+
202+
auto compositor = CompositorOpenGL{engine(), kMockResolver};
203+
204+
FlutterBackingStoreConfig config = {};
205+
FlutterBackingStore backing_store = {};
206+
207+
ASSERT_FALSE(compositor.CreateBackingStore(config, &backing_store));
208+
209+
FlutterLayer layer = {};
210+
layer.type = kFlutterLayerContentTypeBackingStore;
211+
layer.backing_store = nullptr;
212+
const FlutterLayer* layer_ptr = &layer;
213+
214+
EXPECT_FALSE(compositor.Present(&layer_ptr, 1));
215+
}
216+
185217
} // namespace testing
186218
} // namespace flutter

shell/platform/windows/egl/context.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ namespace egl {
1414

1515
// An EGL context to interact with OpenGL.
1616
//
17-
// This enables automatic eror logging and mocking.
17+
// This enables automatic error logging and mocking.
1818
//
1919
// Flutter Windows uses this to create render and resource contexts.
2020
class Context {

0 commit comments

Comments
 (0)