Skip to content

Commit 963615c

Browse files
loic-sharmaloic-sharma
authored
Revert "[Windows] Introduce egl::Surface and egl::WindowSurface" (flutter#50104)
Reverts flutter#49983
1 parent 3678745 commit 963615c

16 files changed

+217
-455
lines changed

‎ci/licenses_golden/licenses_flutter

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7242,10 +7242,6 @@ ORIGIN: ../../../flutter/shell/platform/windows/egl/manager.cc + ../../../flutte
72427242
ORIGIN: ../../../flutter/shell/platform/windows/egl/manager.h + ../../../flutter/LICENSE
72437243
ORIGIN: ../../../flutter/shell/platform/windows/egl/proc_table.cc + ../../../flutter/LICENSE
72447244
ORIGIN: ../../../flutter/shell/platform/windows/egl/proc_table.h + ../../../flutter/LICENSE
7245-
ORIGIN: ../../../flutter/shell/platform/windows/egl/surface.cc + ../../../flutter/LICENSE
7246-
ORIGIN: ../../../flutter/shell/platform/windows/egl/surface.h + ../../../flutter/LICENSE
7247-
ORIGIN: ../../../flutter/shell/platform/windows/egl/window_surface.cc + ../../../flutter/LICENSE
7248-
ORIGIN: ../../../flutter/shell/platform/windows/egl/window_surface.h + ../../../flutter/LICENSE
72497245
ORIGIN: ../../../flutter/shell/platform/windows/event_watcher.cc + ../../../flutter/LICENSE
72507246
ORIGIN: ../../../flutter/shell/platform/windows/event_watcher.h + ../../../flutter/LICENSE
72517247
ORIGIN: ../../../flutter/shell/platform/windows/external_texture.h + ../../../flutter/LICENSE
@@ -10121,10 +10117,6 @@ FILE: ../../../flutter/shell/platform/windows/egl/manager.cc
1012110117
FILE: ../../../flutter/shell/platform/windows/egl/manager.h
1012210118
FILE: ../../../flutter/shell/platform/windows/egl/proc_table.cc
1012310119
FILE: ../../../flutter/shell/platform/windows/egl/proc_table.h
10124-
FILE: ../../../flutter/shell/platform/windows/egl/surface.cc
10125-
FILE: ../../../flutter/shell/platform/windows/egl/surface.h
10126-
FILE: ../../../flutter/shell/platform/windows/egl/window_surface.cc
10127-
FILE: ../../../flutter/shell/platform/windows/egl/window_surface.h
1012810120
FILE: ../../../flutter/shell/platform/windows/event_watcher.cc
1012910121
FILE: ../../../flutter/shell/platform/windows/event_watcher.h
1013010122
FILE: ../../../flutter/shell/platform/windows/external_texture.h

‎shell/platform/windows/BUILD.gn

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -59,10 +59,6 @@ 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",
6662
"event_watcher.cc",
6763
"event_watcher.h",
6864
"external_texture.h",
@@ -213,7 +209,6 @@ executable("flutter_windows_unittests") {
213209
"testing/egl/mock_context.h",
214210
"testing/egl/mock_manager.h",
215211
"testing/egl/mock_proc_table.h",
216-
"testing/egl/mock_window_surface.h",
217212
"testing/engine_modifier.h",
218213
"testing/flutter_window_test.cc",
219214
"testing/flutter_window_test.h",

‎shell/platform/windows/compositor_opengl.cc

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -97,11 +97,6 @@ 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-
105100
// Clear the view if there are no layers to present.
106101
if (layers_count == 0) {
107102
// Normally the compositor is initialized when the first backing store is
@@ -133,7 +128,7 @@ bool CompositorOpenGL::Present(const FlutterLayer** layers,
133128
return false;
134129
}
135130

136-
if (!engine_->egl_manager()->surface()->MakeCurrent()) {
131+
if (!engine_->egl_manager()->MakeCurrent()) {
137132
return false;
138133
}
139134

@@ -159,7 +154,7 @@ bool CompositorOpenGL::Present(const FlutterLayer** layers,
159154
GL_NEAREST // filter
160155
);
161156

162-
if (!engine_->egl_manager()->surface()->SwapBuffers()) {
157+
if (!engine_->egl_manager()->SwapBuffers()) {
163158
return false;
164159
}
165160

@@ -170,7 +165,7 @@ bool CompositorOpenGL::Present(const FlutterLayer** layers,
170165
bool CompositorOpenGL::Initialize() {
171166
FML_DCHECK(!is_initialized_);
172167

173-
if (!engine_->egl_manager()->surface()->MakeCurrent()) {
168+
if (!engine_->egl_manager()->MakeCurrent()) {
174169
return false;
175170
}
176171

@@ -191,14 +186,14 @@ bool CompositorOpenGL::ClearSurface() {
191186
// Resize the surface if needed.
192187
engine_->view()->OnEmptyFrameGenerated();
193188

194-
if (!engine_->egl_manager()->surface()->MakeCurrent()) {
189+
if (!engine_->egl_manager()->MakeCurrent()) {
195190
return false;
196191
}
197192

198193
gl_->ClearColor(0.0f, 0.0f, 0.0f, 0.0f);
199194
gl_->Clear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT | GL_STENCIL_BUFFER_BIT);
200195

201-
if (!engine_->egl_manager()->surface()->SwapBuffers()) {
196+
if (!engine_->egl_manager()->SwapBuffers()) {
202197
return false;
203198
}
204199

‎shell/platform/windows/compositor_opengl_unittests.cc

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
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"
1413
#include "flutter/shell/platform/windows/testing/engine_modifier.h"
1514
#include "flutter/shell/platform/windows/testing/flutter_windows_engine_builder.h"
1615
#include "flutter/shell/platform/windows/testing/mock_window_binding_handler.h"
@@ -66,14 +65,10 @@ class CompositorOpenGLTest : public WindowsTest {
6665
protected:
6766
FlutterWindowsEngine* engine() { return engine_.get(); }
6867
egl::MockManager* egl_manager() { return egl_manager_; }
69-
egl::MockWindowSurface* surface() { return surface_.get(); }
7068

7169
void UseHeadlessEngine() {
7270
auto egl_manager = std::make_unique<egl::MockManager>();
7371
egl_manager_ = egl_manager.get();
74-
surface_ = std::make_unique<egl::MockWindowSurface>();
75-
76-
EXPECT_CALL(*egl_manager_, surface).WillRepeatedly(Return(surface_.get()));
7772

7873
FlutterWindowsEngineBuilder builder{GetContext()};
7974

@@ -97,7 +92,6 @@ class CompositorOpenGLTest : public WindowsTest {
9792
private:
9893
std::unique_ptr<FlutterWindowsEngine> engine_;
9994
std::unique_ptr<FlutterWindowsView> view_;
100-
std::unique_ptr<egl::MockWindowSurface> surface_;
10195
egl::MockManager* egl_manager_;
10296

10397
FML_DISALLOW_COPY_AND_ASSIGN(CompositorOpenGLTest);
@@ -113,7 +107,7 @@ TEST_F(CompositorOpenGLTest, CreateBackingStore) {
113107
FlutterBackingStoreConfig config = {};
114108
FlutterBackingStore backing_store = {};
115109

116-
EXPECT_CALL(*surface(), MakeCurrent).WillOnce(Return(true));
110+
EXPECT_CALL(*egl_manager(), MakeCurrent).WillOnce(Return(true));
117111
ASSERT_TRUE(compositor.CreateBackingStore(config, &backing_store));
118112
ASSERT_TRUE(compositor.CollectBackingStore(&backing_store));
119113
}
@@ -126,7 +120,7 @@ TEST_F(CompositorOpenGLTest, InitializationFailure) {
126120
FlutterBackingStoreConfig config = {};
127121
FlutterBackingStore backing_store = {};
128122

129-
EXPECT_CALL(*surface(), MakeCurrent).WillOnce(Return(false));
123+
EXPECT_CALL(*egl_manager(), MakeCurrent).WillOnce(Return(false));
130124
EXPECT_FALSE(compositor.CreateBackingStore(config, &backing_store));
131125
}
132126

@@ -138,16 +132,16 @@ TEST_F(CompositorOpenGLTest, Present) {
138132
FlutterBackingStoreConfig config = {};
139133
FlutterBackingStore backing_store = {};
140134

141-
EXPECT_CALL(*surface(), MakeCurrent).WillOnce(Return(true));
135+
EXPECT_CALL(*egl_manager(), MakeCurrent).WillOnce(Return(true));
142136
ASSERT_TRUE(compositor.CreateBackingStore(config, &backing_store));
143137

144138
FlutterLayer layer = {};
145139
layer.type = kFlutterLayerContentTypeBackingStore;
146140
layer.backing_store = &backing_store;
147141
const FlutterLayer* layer_ptr = &layer;
148142

149-
EXPECT_CALL(*surface(), MakeCurrent).WillOnce(Return(true));
150-
EXPECT_CALL(*surface(), SwapBuffers).WillOnce(Return(true));
143+
EXPECT_CALL(*egl_manager(), MakeCurrent).WillOnce(Return(true));
144+
EXPECT_CALL(*egl_manager(), SwapBuffers).WillOnce(Return(true));
151145
EXPECT_TRUE(compositor.Present(&layer_ptr, 1));
152146

153147
ASSERT_TRUE(compositor.CollectBackingStore(&backing_store));
@@ -160,8 +154,10 @@ TEST_F(CompositorOpenGLTest, PresentEmpty) {
160154

161155
// The context will be bound twice: first to initialize the compositor, second
162156
// to clear the surface.
163-
EXPECT_CALL(*surface(), MakeCurrent).Times(2).WillRepeatedly(Return(true));
164-
EXPECT_CALL(*surface(), SwapBuffers).WillOnce(Return(true));
157+
EXPECT_CALL(*egl_manager(), MakeCurrent)
158+
.Times(2)
159+
.WillRepeatedly(Return(true));
160+
EXPECT_CALL(*egl_manager(), SwapBuffers).WillOnce(Return(true));
165161
EXPECT_TRUE(compositor.Present(nullptr, 0));
166162
}
167163

@@ -173,7 +169,7 @@ TEST_F(CompositorOpenGLTest, HeadlessPresentIgnored) {
173169
FlutterBackingStoreConfig config = {};
174170
FlutterBackingStore backing_store = {};
175171

176-
EXPECT_CALL(*surface(), MakeCurrent).WillOnce(Return(true));
172+
EXPECT_CALL(*egl_manager(), MakeCurrent).WillOnce(Return(true));
177173
ASSERT_TRUE(compositor.CreateBackingStore(config, &backing_store));
178174

179175
FlutterLayer layer = {};

‎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 error logging and mocking.
17+
// This enables automatic eror logging and mocking.
1818
//
1919
// Flutter Windows uses this to create render and resource contexts.
2020
class Context {

‎shell/platform/windows/egl/manager.cc

Lines changed: 74 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -263,78 +263,116 @@ bool Manager::IsValid() const {
263263
return is_valid_;
264264
}
265265

266-
bool Manager::CreateWindowSurface(HWND hwnd, size_t width, size_t height) {
267-
FML_DCHECK(surface_ == nullptr || !surface_->IsValid());
268-
266+
bool Manager::CreateSurface(HWND hwnd, EGLint width, EGLint height) {
269267
if (!hwnd || !is_valid_) {
270268
return false;
271269
}
272270

271+
EGLSurface surface = EGL_NO_SURFACE;
272+
273273
// Disable ANGLE's automatic surface resizing and provide an explicit size.
274274
// The surface will need to be destroyed and re-created if the HWND is
275275
// resized.
276-
const EGLint surface_attributes[] = {EGL_FIXED_SIZE_ANGLE,
277-
EGL_TRUE,
278-
EGL_WIDTH,
279-
static_cast<EGLint>(width),
280-
EGL_HEIGHT,
281-
static_cast<EGLint>(height),
282-
EGL_NONE};
283-
284-
auto const surface = ::eglCreateWindowSurface(
285-
display_, config_, static_cast<EGLNativeWindowType>(hwnd),
286-
surface_attributes);
276+
const EGLint surface_attributes[] = {
277+
EGL_FIXED_SIZE_ANGLE, EGL_TRUE, EGL_WIDTH, width,
278+
EGL_HEIGHT, height, EGL_NONE};
279+
280+
surface = ::eglCreateWindowSurface(display_, config_,
281+
static_cast<EGLNativeWindowType>(hwnd),
282+
surface_attributes);
287283
if (surface == EGL_NO_SURFACE) {
288284
LogEGLError("Surface creation failed.");
289285
return false;
290286
}
291287

292-
surface_ = std::make_unique<WindowSurface>(
293-
display_, render_context_->GetHandle(), surface, width, height);
288+
surface_width_ = width;
289+
surface_height_ = height;
290+
surface_ = surface;
294291
return true;
295292
}
296293

297-
void Manager::ResizeWindowSurface(HWND hwnd, size_t width, size_t height) {
298-
FML_CHECK(surface_ != nullptr);
299-
300-
auto const existing_width = surface_->width();
301-
auto const existing_height = surface_->height();
302-
auto const existing_vsync = surface_->vsync_enabled();
303-
294+
void Manager::ResizeSurface(HWND hwnd,
295+
EGLint width,
296+
EGLint height,
297+
bool vsync_enabled) {
298+
EGLint existing_width, existing_height;
299+
GetSurfaceDimensions(&existing_width, &existing_height);
304300
if (width != existing_width || height != existing_height) {
301+
surface_width_ = width;
302+
surface_height_ = height;
303+
305304
// TODO: Destroying the surface and re-creating it is expensive.
306305
// Ideally this would use ANGLE's automatic surface sizing instead.
307306
// See: https://github.com/flutter/flutter/issues/79427
308-
if (!surface_->Destroy()) {
309-
FML_LOG(ERROR) << "Manager::ResizeSurface failed to destroy surface";
310-
return;
311-
}
312-
313-
if (!CreateWindowSurface(hwnd, width, height)) {
307+
render_context_->ClearCurrent();
308+
DestroySurface();
309+
if (!CreateSurface(hwnd, width, height)) {
314310
FML_LOG(ERROR) << "Manager::ResizeSurface failed to create surface";
315-
return;
316311
}
312+
}
317313

318-
if (!surface_->SetVSyncEnabled(existing_vsync)) {
319-
// Surfaces block until the v-blank by default.
320-
// Failing to update the vsync might result in unnecessary blocking.
321-
// This regresses performance but not correctness.
322-
FML_LOG(ERROR) << "Manager::ResizeSurface failed to set vsync";
323-
}
314+
SetVSyncEnabled(vsync_enabled);
315+
}
316+
317+
void Manager::GetSurfaceDimensions(EGLint* width, EGLint* height) {
318+
if (surface_ == EGL_NO_SURFACE || !is_valid_) {
319+
*width = 0;
320+
*height = 0;
321+
return;
322+
}
323+
324+
// This avoids eglQuerySurface as ideally surfaces would be automatically
325+
// sized by ANGLE to avoid expensive surface destroy & re-create. With
326+
// automatic sizing, ANGLE could resize the surface before Flutter asks it to,
327+
// which would break resize redraw synchronization.
328+
*width = surface_width_;
329+
*height = surface_height_;
330+
}
331+
332+
void Manager::DestroySurface() {
333+
if (display_ != EGL_NO_DISPLAY && surface_ != EGL_NO_SURFACE) {
334+
::eglDestroySurface(display_, surface_);
324335
}
336+
surface_ = EGL_NO_SURFACE;
325337
}
326338

327339
bool Manager::HasContextCurrent() {
328340
return ::eglGetCurrentContext() != EGL_NO_CONTEXT;
329341
}
330342

343+
bool Manager::MakeCurrent() {
344+
return (::eglMakeCurrent(display_, surface_, surface_,
345+
render_context_->GetHandle()) == EGL_TRUE);
346+
}
347+
348+
bool Manager::SwapBuffers() {
349+
return (::eglSwapBuffers(display_, surface_));
350+
}
351+
331352
EGLSurface Manager::CreateSurfaceFromHandle(EGLenum handle_type,
332353
EGLClientBuffer handle,
333354
const EGLint* attributes) const {
334355
return ::eglCreatePbufferFromClientBuffer(display_, handle_type, handle,
335356
config_, attributes);
336357
}
337358

359+
void Manager::SetVSyncEnabled(bool enabled) {
360+
if (!MakeCurrent()) {
361+
LogEGLError("Unable to make surface current to update the swap interval");
362+
return;
363+
}
364+
365+
// OpenGL swap intervals can be used to prevent screen tearing.
366+
// If enabled, the raster thread blocks until the v-blank.
367+
// This is unnecessary if DWM composition is enabled.
368+
// See: https://www.khronos.org/opengl/wiki/Swap_Interval
369+
// See: https://learn.microsoft.com/windows/win32/dwm/composition-ovw
370+
if (::eglSwapInterval(display_, enabled ? 1 : 0) != EGL_TRUE) {
371+
LogEGLError("Unable to update the swap interval");
372+
return;
373+
}
374+
}
375+
338376
bool Manager::GetDevice(ID3D11Device** device) {
339377
if (!resolved_device_) {
340378
if (!InitializeDevice()) {
@@ -354,9 +392,5 @@ Context* Manager::resource_context() const {
354392
return resource_context_.get();
355393
}
356394

357-
WindowSurface* Manager::surface() const {
358-
return surface_.get();
359-
}
360-
361395
} // namespace egl
362396
} // namespace flutter

0 commit comments

Comments
 (0)