Skip to content

Commit 6de390b

Browse files
authored
[Impeller] Fix GL depth state. (flutter#51040)
This patch fixes problems with GL depth (in service of flutter/engine#50856): * The depth + stencil masks weren't being set prior to clearing. * When the driver identifies as desktop GL rather than ES, use `glClearDepth` and `glDepthRange` rather than `glClearDepthf` and `glDepthRangef`.
1 parent 2c949da commit 6de390b

8 files changed

Lines changed: 109 additions & 31 deletions

File tree

impeller/renderer/backend/gles/BUILD.gn

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ impeller_component("gles_unittests") {
2020
"test/mock_gles.cc",
2121
"test/mock_gles.h",
2222
"test/mock_gles_unittests.cc",
23+
"test/proc_table_gles_unittests.cc",
2324
"test/specialization_constants_unittests.cc",
2425
]
2526
deps = [

impeller/renderer/backend/gles/proc_table_gles.cc

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,11 @@ ProcTableGLES::Resolver WrappedResolver(
7171
};
7272
}
7373

74-
ProcTableGLES::ProcTableGLES(Resolver resolver) {
74+
ProcTableGLES::ProcTableGLES( // NOLINT(google-readability-function-size)
75+
Resolver resolver) {
76+
// The reason this constructor has anywhere near enough code to tip off
77+
// `google-readability-function-size` is the proc macros, so ignore the lint.
78+
7579
if (!resolver) {
7680
return;
7781
}
@@ -96,6 +100,18 @@ ProcTableGLES::ProcTableGLES(Resolver resolver) {
96100

97101
FOR_EACH_IMPELLER_PROC(IMPELLER_PROC);
98102

103+
description_ = std::make_unique<DescriptionGLES>(*this);
104+
105+
if (!description_->IsValid()) {
106+
return;
107+
}
108+
109+
if (description_->IsES()) {
110+
FOR_EACH_IMPELLER_ES_ONLY_PROC(IMPELLER_PROC);
111+
} else {
112+
FOR_EACH_IMPELLER_DESKTOP_ONLY_PROC(IMPELLER_PROC);
113+
}
114+
99115
#undef IMPELLER_PROC
100116

101117
#define IMPELLER_PROC(proc_ivar) \
@@ -109,12 +125,6 @@ ProcTableGLES::ProcTableGLES(Resolver resolver) {
109125

110126
#undef IMPELLER_PROC
111127

112-
description_ = std::make_unique<DescriptionGLES>(*this);
113-
114-
if (!description_->IsValid()) {
115-
return;
116-
}
117-
118128
if (!description_->HasDebugExtension()) {
119129
PushDebugGroupKHR.Reset();
120130
PopDebugGroupKHR.Reset();

impeller/renderer/backend/gles/proc_table_gles.h

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,6 @@ struct GLProc {
113113
PROC(CheckFramebufferStatus); \
114114
PROC(Clear); \
115115
PROC(ClearColor); \
116-
PROC(ClearDepthf); \
117116
PROC(ClearStencil); \
118117
PROC(ColorMask); \
119118
PROC(CompileShader); \
@@ -128,7 +127,6 @@ struct GLProc {
128127
PROC(DeleteTextures); \
129128
PROC(DepthFunc); \
130129
PROC(DepthMask); \
131-
PROC(DepthRangef); \
132130
PROC(DetachShader); \
133131
PROC(Disable); \
134132
PROC(DisableVertexAttribArray); \
@@ -186,6 +184,22 @@ struct GLProc {
186184
PROC(GetShaderSource); \
187185
PROC(ReadPixels);
188186

187+
// Calls specific to OpenGLES.
188+
void(glClearDepthf)(GLfloat depth);
189+
void(glDepthRangef)(GLfloat n, GLfloat f);
190+
191+
#define FOR_EACH_IMPELLER_ES_ONLY_PROC(PROC) \
192+
PROC(ClearDepthf); \
193+
PROC(DepthRangef);
194+
195+
// Calls specific to desktop GL.
196+
void(glClearDepth)(GLdouble depth);
197+
void(glDepthRange)(GLdouble n, GLdouble f);
198+
199+
#define FOR_EACH_IMPELLER_DESKTOP_ONLY_PROC(PROC) \
200+
PROC(ClearDepth); \
201+
PROC(DepthRange);
202+
189203
#define FOR_EACH_IMPELLER_GLES3_PROC(PROC) PROC(BlitFramebuffer);
190204

191205
#define FOR_EACH_IMPELLER_EXT_PROC(PROC) \
@@ -224,6 +238,8 @@ class ProcTableGLES {
224238
GLProc<decltype(gl##name)> name = {"gl" #name, nullptr};
225239

226240
FOR_EACH_IMPELLER_PROC(IMPELLER_PROC);
241+
FOR_EACH_IMPELLER_ES_ONLY_PROC(IMPELLER_PROC);
242+
FOR_EACH_IMPELLER_DESKTOP_ONLY_PROC(IMPELLER_PROC);
227243
FOR_EACH_IMPELLER_GLES3_PROC(IMPELLER_PROC);
228244
FOR_EACH_IMPELLER_EXT_PROC(IMPELLER_PROC);
229245

impeller/renderer/backend/gles/render_pass_gles.cc

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -214,12 +214,11 @@ struct RenderPassData {
214214
pass_data.clear_color.alpha // alpha
215215
);
216216
if (pass_data.depth_attachment) {
217-
// TODO(bdero): Desktop GL for Apple requires glClearDepth. glClearDepthf
218-
// throws GL_INVALID_OPERATION.
219-
// https://github.com/flutter/flutter/issues/136322
220-
#if !FML_OS_MACOSX
221-
gl.ClearDepthf(pass_data.clear_depth);
222-
#endif
217+
if (gl.DepthRangef.IsAvailable()) {
218+
gl.ClearDepthf(pass_data.clear_depth);
219+
} else {
220+
gl.ClearDepth(pass_data.clear_depth);
221+
}
223222
}
224223
if (pass_data.stencil_attachment) {
225224
gl.ClearStencil(pass_data.clear_stencil);
@@ -242,6 +241,9 @@ struct RenderPassData {
242241
gl.Disable(GL_CULL_FACE);
243242
gl.Disable(GL_BLEND);
244243
gl.ColorMask(GL_TRUE, GL_TRUE, GL_TRUE, GL_TRUE);
244+
gl.DepthMask(GL_TRUE);
245+
gl.StencilMaskSeparate(GL_FRONT, 0xFFFFFFFF);
246+
gl.StencilMaskSeparate(GL_BACK, 0xFFFFFFFF);
245247

246248
gl.Clear(clear_bits);
247249

@@ -315,12 +317,11 @@ struct RenderPassData {
315317
viewport.rect.GetHeight() // height
316318
);
317319
if (pass_data.depth_attachment) {
318-
// TODO(bdero): Desktop GL for Apple requires glDepthRange. glDepthRangef
319-
// throws GL_INVALID_OPERATION.
320-
// https://github.com/flutter/flutter/issues/136322
321-
#if !FML_OS_MACOSX
322-
gl.DepthRangef(viewport.depth_range.z_near, viewport.depth_range.z_far);
323-
#endif
320+
if (gl.DepthRangef.IsAvailable()) {
321+
gl.DepthRangef(viewport.depth_range.z_near, viewport.depth_range.z_far);
322+
} else {
323+
gl.DepthRange(viewport.depth_range.z_near, viewport.depth_range.z_far);
324+
}
324325
}
325326

326327
//--------------------------------------------------------------------------

impeller/renderer/backend/gles/test/mock_gles.cc

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,12 @@ static std::mutex g_test_lock;
1919

2020
static std::weak_ptr<MockGLES> g_mock_gles;
2121

22+
static ProcTableGLES::Resolver g_resolver;
23+
2224
static std::vector<const unsigned char*> g_extensions;
2325

26+
static const unsigned char* g_version;
27+
2428
// Has friend visibility into MockGLES to record calls.
2529
void RecordGLCall(const char* name) {
2630
if (auto mock_gles = g_mock_gles.lock()) {
@@ -38,7 +42,7 @@ struct CheckSameSignature<Ret(Args...), Ret(Args...)> : std::true_type {};
3842
void doNothing() {}
3943

4044
auto const kMockVendor = (unsigned char*)"MockGLES";
41-
auto const kMockVersion = (unsigned char*)"3.0";
45+
const auto kMockShadingLanguageVersion = (unsigned char*)"GLSL ES 1.0";
4246
auto const kExtensions = std::vector<const unsigned char*>{
4347
(unsigned char*)"GL_KHR_debug" //
4448
};
@@ -48,9 +52,9 @@ const unsigned char* mockGetString(GLenum name) {
4852
case GL_VENDOR:
4953
return kMockVendor;
5054
case GL_VERSION:
51-
return kMockVersion;
55+
return g_version;
5256
case GL_SHADING_LANGUAGE_VERSION:
53-
return kMockVersion;
57+
return kMockShadingLanguageVersion;
5458
default:
5559
return (unsigned char*)"";
5660
}
@@ -160,17 +164,20 @@ static_assert(CheckSameSignature<decltype(mockDeleteQueriesEXT), //
160164
decltype(glDeleteQueriesEXT)>::value);
161165

162166
std::shared_ptr<MockGLES> MockGLES::Init(
163-
const std::optional<std::vector<const unsigned char*>>& extensions) {
167+
const std::optional<std::vector<const unsigned char*>>& extensions,
168+
const char* version_string,
169+
ProcTableGLES::Resolver resolver) {
164170
// If we cannot obtain a lock, MockGLES is already being used elsewhere.
165171
FML_CHECK(g_test_lock.try_lock())
166172
<< "MockGLES is already being used by another test.";
173+
g_version = (unsigned char*)version_string;
167174
g_extensions = extensions.value_or(kExtensions);
168-
auto mock_gles = std::shared_ptr<MockGLES>(new MockGLES());
175+
auto mock_gles = std::shared_ptr<MockGLES>(new MockGLES(std::move(resolver)));
169176
g_mock_gles = mock_gles;
170177
return mock_gles;
171178
}
172179

173-
const ProcTableGLES::Resolver kMockResolver = [](const char* name) {
180+
const ProcTableGLES::Resolver kMockResolverGLES = [](const char* name) {
174181
if (strcmp(name, "glPopDebugGroupKHR") == 0) {
175182
return reinterpret_cast<void*>(&mockPopDebugGroupKHR);
176183
} else if (strcmp(name, "glPushDebugGroupKHR") == 0) {
@@ -200,7 +207,8 @@ const ProcTableGLES::Resolver kMockResolver = [](const char* name) {
200207
}
201208
};
202209

203-
MockGLES::MockGLES() : proc_table_(kMockResolver) {}
210+
MockGLES::MockGLES(ProcTableGLES::Resolver resolver)
211+
: proc_table_(std::move(resolver)) {}
204212

205213
MockGLES::~MockGLES() {
206214
g_test_lock.unlock();

impeller/renderer/backend/gles/test/mock_gles.h

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
namespace impeller {
1414
namespace testing {
1515

16+
extern const ProcTableGLES::Resolver kMockResolverGLES;
17+
1618
/// @brief Provides a mocked version of the |ProcTableGLES| class.
1719
///
1820
/// Typically, Open GLES at runtime will be provided the host's GLES bindings
@@ -30,7 +32,9 @@ class MockGLES final {
3032
/// called once per test.
3133
static std::shared_ptr<MockGLES> Init(
3234
const std::optional<std::vector<const unsigned char*>>& extensions =
33-
std::nullopt);
35+
std::nullopt,
36+
const char* version_string = "OpenGL ES 3.0",
37+
ProcTableGLES::Resolver resolver = kMockResolverGLES);
3438

3539
/// @brief Returns a configured |ProcTableGLES| instance.
3640
const ProcTableGLES& GetProcTable() const { return proc_table_; }
@@ -49,11 +53,11 @@ class MockGLES final {
4953
private:
5054
friend void RecordGLCall(const char* name);
5155

52-
MockGLES();
56+
explicit MockGLES(ProcTableGLES::Resolver resolver = kMockResolverGLES);
5357

5458
void RecordCall(const char* name) { captured_calls_.emplace_back(name); }
5559

56-
const ProcTableGLES proc_table_;
60+
ProcTableGLES proc_table_;
5761
std::vector<std::string> captured_calls_;
5862

5963
MockGLES(const MockGLES&) = delete;
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
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 <optional>
6+
7+
#include "flutter/testing/testing.h" // IWYU pragma: keep
8+
#include "gtest/gtest.h"
9+
#include "impeller/renderer/backend/gles/proc_table_gles.h"
10+
#include "impeller/renderer/backend/gles/test/mock_gles.h"
11+
12+
namespace impeller {
13+
namespace testing {
14+
15+
#define EXPECT_AVAILABLE(proc_ivar) \
16+
EXPECT_TRUE(mock_gles->GetProcTable().proc_ivar.IsAvailable());
17+
#define EXPECT_UNAVAILABLE(proc_ivar) \
18+
EXPECT_FALSE(mock_gles->GetProcTable().proc_ivar.IsAvailable());
19+
20+
TEST(ProcTableGLES, ResolvesCorrectClearDepthProcOnES) {
21+
auto mock_gles = MockGLES::Init(std::nullopt, "OpenGL ES 3.0");
22+
EXPECT_TRUE(mock_gles->GetProcTable().GetDescription()->IsES());
23+
24+
FOR_EACH_IMPELLER_ES_ONLY_PROC(EXPECT_AVAILABLE);
25+
FOR_EACH_IMPELLER_DESKTOP_ONLY_PROC(EXPECT_UNAVAILABLE);
26+
}
27+
28+
TEST(ProcTableGLES, ResolvesCorrectClearDepthProcOnDesktopGL) {
29+
auto mock_gles = MockGLES::Init(std::nullopt, "OpenGL 4.0");
30+
EXPECT_FALSE(mock_gles->GetProcTable().GetDescription()->IsES());
31+
32+
FOR_EACH_IMPELLER_DESKTOP_ONLY_PROC(EXPECT_AVAILABLE);
33+
FOR_EACH_IMPELLER_ES_ONLY_PROC(EXPECT_UNAVAILABLE);
34+
}
35+
36+
} // namespace testing
37+
} // namespace impeller

impeller/renderer/render_target.cc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -260,6 +260,7 @@ RenderTarget RenderTarget::CreateOffscreen(
260260
stencil_attachment_config.value());
261261
} else {
262262
target.SetStencilAttachment(std::nullopt);
263+
target.SetDepthAttachment(std::nullopt);
263264
}
264265

265266
return target;

0 commit comments

Comments
 (0)