Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Impeller] Fix GL depth state. #51040

Merged
merged 5 commits into from
Feb 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions impeller/renderer/backend/gles/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ impeller_component("gles_unittests") {
"test/mock_gles.cc",
"test/mock_gles.h",
"test/mock_gles_unittests.cc",
"test/proc_table_gles_unittests.cc",
"test/specialization_constants_unittests.cc",
]
deps = [
Expand Down
24 changes: 17 additions & 7 deletions impeller/renderer/backend/gles/proc_table_gles.cc
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,11 @@ ProcTableGLES::Resolver WrappedResolver(
};
}

ProcTableGLES::ProcTableGLES(Resolver resolver) {
ProcTableGLES::ProcTableGLES( // NOLINT(google-readability-function-size)
Resolver resolver) {
// The reason this constructor has anywhere near enough code to tip off
// `google-readability-function-size` is the proc macros, so ignore the lint.

if (!resolver) {
return;
}
Expand All @@ -96,6 +100,18 @@ ProcTableGLES::ProcTableGLES(Resolver resolver) {

FOR_EACH_IMPELLER_PROC(IMPELLER_PROC);

description_ = std::make_unique<DescriptionGLES>(*this);

if (!description_->IsValid()) {
return;
}

if (description_->IsES()) {
FOR_EACH_IMPELLER_ES_ONLY_PROC(IMPELLER_PROC);
} else {
FOR_EACH_IMPELLER_DESKTOP_ONLY_PROC(IMPELLER_PROC);
}

#undef IMPELLER_PROC

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

#undef IMPELLER_PROC

description_ = std::make_unique<DescriptionGLES>(*this);

if (!description_->IsValid()) {
return;
}

if (!description_->HasDebugExtension()) {
PushDebugGroupKHR.Reset();
PopDebugGroupKHR.Reset();
Expand Down
20 changes: 18 additions & 2 deletions impeller/renderer/backend/gles/proc_table_gles.h
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,6 @@ struct GLProc {
PROC(CheckFramebufferStatus); \
PROC(Clear); \
PROC(ClearColor); \
PROC(ClearDepthf); \
PROC(ClearStencil); \
PROC(ColorMask); \
PROC(CompileShader); \
Expand All @@ -128,7 +127,6 @@ struct GLProc {
PROC(DeleteTextures); \
PROC(DepthFunc); \
PROC(DepthMask); \
PROC(DepthRangef); \
PROC(DetachShader); \
PROC(Disable); \
PROC(DisableVertexAttribArray); \
Expand Down Expand Up @@ -186,6 +184,22 @@ struct GLProc {
PROC(GetShaderSource); \
PROC(ReadPixels);

// Calls specific to OpenGLES.
void(glClearDepthf)(GLfloat depth);
void(glDepthRangef)(GLfloat n, GLfloat f);

#define FOR_EACH_IMPELLER_ES_ONLY_PROC(PROC) \
PROC(ClearDepthf); \
PROC(DepthRangef);

// Calls specific to desktop GL.
void(glClearDepth)(GLdouble depth);
void(glDepthRange)(GLdouble n, GLdouble f);

#define FOR_EACH_IMPELLER_DESKTOP_ONLY_PROC(PROC) \
PROC(ClearDepth); \
PROC(DepthRange);

#define FOR_EACH_IMPELLER_GLES3_PROC(PROC) PROC(BlitFramebuffer);

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

FOR_EACH_IMPELLER_PROC(IMPELLER_PROC);
FOR_EACH_IMPELLER_ES_ONLY_PROC(IMPELLER_PROC);
FOR_EACH_IMPELLER_DESKTOP_ONLY_PROC(IMPELLER_PROC);
FOR_EACH_IMPELLER_GLES3_PROC(IMPELLER_PROC);
FOR_EACH_IMPELLER_EXT_PROC(IMPELLER_PROC);

Expand Down
25 changes: 13 additions & 12 deletions impeller/renderer/backend/gles/render_pass_gles.cc
Original file line number Diff line number Diff line change
Expand Up @@ -214,12 +214,11 @@ struct RenderPassData {
pass_data.clear_color.alpha // alpha
);
if (pass_data.depth_attachment) {
// TODO(bdero): Desktop GL for Apple requires glClearDepth. glClearDepthf
// throws GL_INVALID_OPERATION.
// https://github.com/flutter/flutter/issues/136322
#if !FML_OS_MACOSX
gl.ClearDepthf(pass_data.clear_depth);
#endif
Copy link
Member Author

@bdero bdero Feb 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was one of the issues for OpenGLES playground runs on MacOS. OpenGL defaults the depth clear value to 1. But my new depth clipping scheme relies on clearing to 0.

And so not being able to modify the depth clear value meant nothing could draw in subpasses that had a depth attachment.

if (gl.DepthRangef.IsAvailable()) {
gl.ClearDepthf(pass_data.clear_depth);
} else {
gl.ClearDepth(pass_data.clear_depth);
}
}
if (pass_data.stencil_attachment) {
gl.ClearStencil(pass_data.clear_stencil);
Expand All @@ -242,6 +241,9 @@ struct RenderPassData {
gl.Disable(GL_CULL_FACE);
gl.Disable(GL_BLEND);
gl.ColorMask(GL_TRUE, GL_TRUE, GL_TRUE, GL_TRUE);
gl.DepthMask(GL_TRUE);
gl.StencilMaskSeparate(GL_FRONT, 0xFFFFFFFF);
gl.StencilMaskSeparate(GL_BACK, 0xFFFFFFFF);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When depth buffer clipping + StC is enabled, the depth mask toggles very frequently, so we need to make sure it's on before clearing.

We've yet to utilize the stencil mask in the 2D renderer, which is why we haven't noticed this problem up until now.


gl.Clear(clear_bits);

Expand Down Expand Up @@ -315,12 +317,11 @@ struct RenderPassData {
viewport.rect.GetHeight() // height
);
if (pass_data.depth_attachment) {
// TODO(bdero): Desktop GL for Apple requires glDepthRange. glDepthRangef
// throws GL_INVALID_OPERATION.
// https://github.com/flutter/flutter/issues/136322
#if !FML_OS_MACOSX
gl.DepthRangef(viewport.depth_range.z_near, viewport.depth_range.z_far);
#endif
if (gl.DepthRangef.IsAvailable()) {
gl.DepthRangef(viewport.depth_range.z_near, viewport.depth_range.z_far);
} else {
gl.DepthRange(viewport.depth_range.z_near, viewport.depth_range.z_far);
}
}

//--------------------------------------------------------------------------
Expand Down
22 changes: 15 additions & 7 deletions impeller/renderer/backend/gles/test/mock_gles.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,12 @@ static std::mutex g_test_lock;

static std::weak_ptr<MockGLES> g_mock_gles;

static ProcTableGLES::Resolver g_resolver;

static std::vector<const unsigned char*> g_extensions;

static const unsigned char* g_version;

// Has friend visibility into MockGLES to record calls.
void RecordGLCall(const char* name) {
if (auto mock_gles = g_mock_gles.lock()) {
Expand All @@ -38,7 +42,7 @@ struct CheckSameSignature<Ret(Args...), Ret(Args...)> : std::true_type {};
void doNothing() {}

auto const kMockVendor = (unsigned char*)"MockGLES";
auto const kMockVersion = (unsigned char*)"3.0";
const auto kMockShadingLanguageVersion = (unsigned char*)"GLSL ES 1.0";
auto const kExtensions = std::vector<const unsigned char*>{
(unsigned char*)"GL_KHR_debug" //
};
Expand All @@ -48,9 +52,9 @@ const unsigned char* mockGetString(GLenum name) {
case GL_VENDOR:
return kMockVendor;
case GL_VERSION:
return kMockVersion;
return g_version;
case GL_SHADING_LANGUAGE_VERSION:
return kMockVersion;
return kMockShadingLanguageVersion;
default:
return (unsigned char*)"";
}
Expand Down Expand Up @@ -160,17 +164,20 @@ static_assert(CheckSameSignature<decltype(mockDeleteQueriesEXT), //
decltype(glDeleteQueriesEXT)>::value);

std::shared_ptr<MockGLES> MockGLES::Init(
const std::optional<std::vector<const unsigned char*>>& extensions) {
const std::optional<std::vector<const unsigned char*>>& extensions,
const char* version_string,
ProcTableGLES::Resolver resolver) {
// If we cannot obtain a lock, MockGLES is already being used elsewhere.
FML_CHECK(g_test_lock.try_lock())
<< "MockGLES is already being used by another test.";
g_version = (unsigned char*)version_string;
g_extensions = extensions.value_or(kExtensions);
auto mock_gles = std::shared_ptr<MockGLES>(new MockGLES());
auto mock_gles = std::shared_ptr<MockGLES>(new MockGLES(std::move(resolver)));
g_mock_gles = mock_gles;
return mock_gles;
}

const ProcTableGLES::Resolver kMockResolver = [](const char* name) {
const ProcTableGLES::Resolver kMockResolverGLES = [](const char* name) {
if (strcmp(name, "glPopDebugGroupKHR") == 0) {
return reinterpret_cast<void*>(&mockPopDebugGroupKHR);
} else if (strcmp(name, "glPushDebugGroupKHR") == 0) {
Expand Down Expand Up @@ -200,7 +207,8 @@ const ProcTableGLES::Resolver kMockResolver = [](const char* name) {
}
};

MockGLES::MockGLES() : proc_table_(kMockResolver) {}
MockGLES::MockGLES(ProcTableGLES::Resolver resolver)
: proc_table_(std::move(resolver)) {}

MockGLES::~MockGLES() {
g_test_lock.unlock();
Expand Down
10 changes: 7 additions & 3 deletions impeller/renderer/backend/gles/test/mock_gles.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
namespace impeller {
namespace testing {

extern const ProcTableGLES::Resolver kMockResolverGLES;

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

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

MockGLES();
explicit MockGLES(ProcTableGLES::Resolver resolver = kMockResolverGLES);

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

const ProcTableGLES proc_table_;
ProcTableGLES proc_table_;
std::vector<std::string> captured_calls_;

MockGLES(const MockGLES&) = delete;
Expand Down
37 changes: 37 additions & 0 deletions impeller/renderer/backend/gles/test/proc_table_gles_unittests.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// Copyright 2013 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include <optional>

#include "flutter/testing/testing.h" // IWYU pragma: keep
#include "gtest/gtest.h"
#include "impeller/renderer/backend/gles/proc_table_gles.h"
#include "impeller/renderer/backend/gles/test/mock_gles.h"

namespace impeller {
namespace testing {

#define EXPECT_AVAILABLE(proc_ivar) \
EXPECT_TRUE(mock_gles->GetProcTable().proc_ivar.IsAvailable());
#define EXPECT_UNAVAILABLE(proc_ivar) \
EXPECT_FALSE(mock_gles->GetProcTable().proc_ivar.IsAvailable());

TEST(ProcTableGLES, ResolvesCorrectClearDepthProcOnES) {
auto mock_gles = MockGLES::Init(std::nullopt, "OpenGL ES 3.0");
EXPECT_TRUE(mock_gles->GetProcTable().GetDescription()->IsES());

FOR_EACH_IMPELLER_ES_ONLY_PROC(EXPECT_AVAILABLE);
FOR_EACH_IMPELLER_DESKTOP_ONLY_PROC(EXPECT_UNAVAILABLE);
}

TEST(ProcTableGLES, ResolvesCorrectClearDepthProcOnDesktopGL) {
auto mock_gles = MockGLES::Init(std::nullopt, "OpenGL 4.0");
EXPECT_FALSE(mock_gles->GetProcTable().GetDescription()->IsES());

FOR_EACH_IMPELLER_DESKTOP_ONLY_PROC(EXPECT_AVAILABLE);
FOR_EACH_IMPELLER_ES_ONLY_PROC(EXPECT_UNAVAILABLE);
}

} // namespace testing
} // namespace impeller
1 change: 1 addition & 0 deletions impeller/renderer/render_target.cc
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,7 @@ RenderTarget RenderTarget::CreateOffscreen(
stencil_attachment_config.value());
} else {
target.SetStencilAttachment(std::nullopt);
target.SetDepthAttachment(std::nullopt);
}

return target;
Expand Down