Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from 8 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 @@ -25,6 +25,7 @@ impeller_component("gles_unittests") {
"test/reactor_unittests.cc",
"test/specialization_constants_unittests.cc",
"test/surface_gles_unittests.cc",
"test/texture_gles_unittests.cc",
]
deps = [
":gles",
Expand Down
5 changes: 5 additions & 0 deletions impeller/renderer/backend/gles/command_buffer_gles.cc
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,11 @@ void CommandBufferGLES::OnWaitUntilCompleted() {
reactor_->GetProcTable().Finish();
}

// |CommandBuffer|
void CommandBufferGLES::OnWaitUntilScheduled() {
reactor_->GetProcTable().Flush();
}

// |CommandBuffer|
std::shared_ptr<RenderPass> CommandBufferGLES::OnCreateRenderPass(
RenderTarget target) {
Expand Down
3 changes: 3 additions & 0 deletions impeller/renderer/backend/gles/command_buffer_gles.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ class CommandBufferGLES final : public CommandBuffer {
// |CommandBuffer|
void OnWaitUntilCompleted() override;

// |CommandBuffer|
void OnWaitUntilScheduled() override;

// |CommandBuffer|
std::shared_ptr<RenderPass> OnCreateRenderPass(RenderTarget target) override;

Expand Down
13 changes: 13 additions & 0 deletions impeller/renderer/backend/gles/context_gles.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@
#include "impeller/base/validation.h"
#include "impeller/renderer/backend/gles/command_buffer_gles.h"
#include "impeller/renderer/backend/gles/gpu_tracer_gles.h"
#include "impeller/renderer/backend/gles/handle_gles.h"
#include "impeller/renderer/backend/gles/render_pass_gles.h"
#include "impeller/renderer/backend/gles/texture_gles.h"
#include "impeller/renderer/command_queue.h"

namespace impeller {
Expand Down Expand Up @@ -157,4 +159,15 @@ void ContextGLES::ResetThreadLocalState() const {
});
}

// |Context|
bool ContextGLES::AddTrackingFence(
const std::shared_ptr<Texture>& texture) const {
if (!reactor_->GetProcTable().FenceSync.IsAvailable()) {
return false;
}
HandleGLES fence = reactor_->CreateHandle(HandleType::kFence);
TextureGLES::Cast(*texture).SetFence(fence);
return true;
}

} // namespace impeller
3 changes: 3 additions & 0 deletions impeller/renderer/backend/gles/context_gles.h
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,9 @@ class ContextGLES final : public Context,
// |Context|
void Shutdown() override;

// |Context|
bool AddTrackingFence(const std::shared_ptr<Texture>& texture) const override;

// |Context|
void ResetThreadLocalState() const override;

Expand Down
2 changes: 2 additions & 0 deletions impeller/renderer/backend/gles/handle_gles.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ std::string HandleTypeToString(HandleType type) {
return "RenderBuffer";
case HandleType::kFrameBuffer:
return "Framebuffer";
case HandleType::kFence:
return "Fence";
}
FML_UNREACHABLE();
}
Expand Down
1 change: 1 addition & 0 deletions impeller/renderer/backend/gles/handle_gles.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ enum class HandleType {
kProgram,
kRenderBuffer,
kFrameBuffer,
kFence,
};

std::string HandleTypeToString(HandleType type);
Expand Down
4 changes: 4 additions & 0 deletions impeller/renderer/backend/gles/proc_table_gles.cc
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,8 @@ static std::optional<GLenum> ToDebugIdentifier(DebugResourceType type) {
return GL_RENDERBUFFER;
case DebugResourceType::kFrameBuffer:
return GL_FRAMEBUFFER;
case DebugResourceType::kFence:
return GL_SYNC_FENCE;
}
FML_UNREACHABLE();
}
Expand All @@ -346,6 +348,8 @@ static bool ResourceIsLive(const ProcTableGLES& gl,
return gl.IsRenderbuffer(name);
case DebugResourceType::kFrameBuffer:
return gl.IsFramebuffer(name);
case DebugResourceType::kFence:
return true;
Copy link
Member

Choose a reason for hiding this comment

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

GLES3 has a glIsSync API that can be used here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is only used for debug labeling which I don't believe can be done to the sync fence APIs

}
FML_UNREACHABLE();
}
Expand Down
7 changes: 6 additions & 1 deletion impeller/renderer/backend/gles/proc_table_gles.h
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,11 @@ void(glDepthRange)(GLdouble n, GLdouble f);
PROC(ClearDepth); \
PROC(DepthRange);

#define FOR_EACH_IMPELLER_GLES3_PROC(PROC) PROC(BlitFramebuffer);
#define FOR_EACH_IMPELLER_GLES3_PROC(PROC) \
PROC(FenceSync); \
PROC(DeleteSync); \
PROC(WaitSync); \
PROC(BlitFramebuffer);

#define FOR_EACH_IMPELLER_EXT_PROC(PROC) \
PROC(DebugMessageControlKHR); \
Expand All @@ -262,6 +266,7 @@ enum class DebugResourceType {
kShader,
kRenderBuffer,
kFrameBuffer,
kFence,
};

class ProcTableGLES {
Expand Down
75 changes: 55 additions & 20 deletions impeller/renderer/backend/gles/reactor_gles.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,51 +13,57 @@

namespace impeller {

static std::optional<GLuint> CreateGLHandle(const ProcTableGLES& gl,
HandleType type) {
GLuint handle = GL_NONE;
static std::optional<GLHandle> CreateGLHandle(const ProcTableGLES& gl,
HandleType type) {
GLHandle handle = GLHandle{.handle = GL_NONE};
switch (type) {
case HandleType::kUnknown:
return std::nullopt;
case HandleType::kTexture:
gl.GenTextures(1u, &handle);
gl.GenTextures(1u, &handle.handle);
return handle;
case HandleType::kBuffer:
gl.GenBuffers(1u, &handle);
gl.GenBuffers(1u, &handle.handle);
return handle;
case HandleType::kProgram:
return gl.CreateProgram();
return GLHandle{.handle = gl.CreateProgram()};
case HandleType::kRenderBuffer:
gl.GenRenderbuffers(1u, &handle);
gl.GenRenderbuffers(1u, &handle.handle);
return handle;
case HandleType::kFrameBuffer:
gl.GenFramebuffers(1u, &handle);
gl.GenFramebuffers(1u, &handle.handle);
return handle;
case HandleType::kFence:
return GLHandle{.sync = gl.FenceSync(GL_SYNC_GPU_COMMANDS_COMPLETE, 0)};
break;
Copy link
Member

Choose a reason for hiding this comment

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

The break can be deleted

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

}
return std::nullopt;
}

static bool CollectGLHandle(const ProcTableGLES& gl,
HandleType type,
GLuint handle) {
GLHandle handle) {
switch (type) {
case HandleType::kUnknown:
return false;
case HandleType::kTexture:
gl.DeleteTextures(1u, &handle);
gl.DeleteTextures(1u, &handle.handle);
return true;
case HandleType::kBuffer:
gl.DeleteBuffers(1u, &handle);
gl.DeleteBuffers(1u, &handle.handle);
return true;
case HandleType::kProgram:
gl.DeleteProgram(handle);
gl.DeleteProgram(handle.handle);
return true;
case HandleType::kRenderBuffer:
gl.DeleteRenderbuffers(1u, &handle);
gl.DeleteRenderbuffers(1u, &handle.handle);
return true;
case HandleType::kFrameBuffer:
gl.DeleteFramebuffers(1u, &handle);
gl.DeleteFramebuffers(1u, &handle.handle);
return true;
case HandleType::kFence:
gl.DeleteSync(handle.sync);
break;
}
return false;
}
Expand Down Expand Up @@ -117,18 +123,44 @@ const ProcTableGLES& ReactorGLES::GetProcTable() const {
}

std::optional<GLuint> ReactorGLES::GetGLHandle(const HandleGLES& handle) const {
if (handle.type == HandleType::kFence) {
Copy link
Member

Choose a reason for hiding this comment

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

The locking and the lookup in the handles_ map can be extracted out into a separate method in order to reduce the duplication between ReactorGLES::GetGLHandle and GetGLFence.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

return std::nullopt;
}
ReaderLock handles_lock(handles_mutex_);
if (auto found = handles_.find(handle); found != handles_.end()) {
if (found->second.pending_collection) {
VALIDATION_LOG
<< "Attempted to acquire a handle that was pending collection.";
return std::nullopt;
}
std::optional<GLHandle> name = found->second.name;
if (!name.has_value()) {
VALIDATION_LOG << "Attempt to acquire a handle outside of an operation.";
return std::nullopt;
}
return name->handle;
}
VALIDATION_LOG << "Attempted to acquire an invalid GL handle.";
return std::nullopt;
}

std::optional<GLsync> ReactorGLES::GetGLFence(const HandleGLES& handle) const {
if (handle.type != HandleType::kFence) {
return std::nullopt;
}
ReaderLock handles_lock(handles_mutex_);
if (auto found = handles_.find(handle); found != handles_.end()) {
if (found->second.pending_collection) {
VALIDATION_LOG
<< "Attempted to acquire a handle that was pending collection.";
return std::nullopt;
}
if (!found->second.name.has_value()) {
std::optional<GLHandle> name = found->second.name;
if (!name.has_value()) {
VALIDATION_LOG << "Attempt to acquire a handle outside of an operation.";
return std::nullopt;
}
return found->second.name;
return name->sync;
}
VALIDATION_LOG << "Attempted to acquire an invalid GL handle.";
return std::nullopt;
Expand Down Expand Up @@ -171,9 +203,9 @@ HandleGLES ReactorGLES::CreateHandle(HandleType type, GLuint external_handle) {
}
WriterLock handles_lock(handles_mutex_);

std::optional<GLuint> gl_handle;
std::optional<GLHandle> gl_handle;
if (external_handle != GL_NONE) {
gl_handle = external_handle;
gl_handle = GLHandle{.handle = external_handle};
} else if (CanReactOnCurrentThread()) {
gl_handle = CreateGLHandle(GetProcTable(), type);
}
Expand Down Expand Up @@ -215,6 +247,8 @@ static DebugResourceType ToDebugResourceType(HandleType type) {
return DebugResourceType::kRenderBuffer;
case HandleType::kFrameBuffer:
return DebugResourceType::kFrameBuffer;
case HandleType::kFence:
return DebugResourceType::kFence;
}
FML_UNREACHABLE();
}
Expand Down Expand Up @@ -253,9 +287,10 @@ bool ReactorGLES::ConsolidateHandles() {
handle.second.name = gl_handle;
}
// Set pending debug labels.
if (handle.second.pending_debug_label.has_value()) {
if (handle.second.pending_debug_label.has_value() &&
handle.first.type != HandleType::kFence) {
if (gl.SetDebugLabel(ToDebugResourceType(handle.first.type),
handle.second.name.value(),
handle.second.name.value().handle,
handle.second.pending_debug_label.value())) {
handle.second.pending_debug_label = std::nullopt;
}
Expand Down
14 changes: 12 additions & 2 deletions impeller/renderer/backend/gles/reactor_gles.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,14 @@

namespace impeller {

/// @brief Storage for either a GL handle or sync fence.
struct GLHandle {
union {
GLuint handle;
GLsync sync;
};
};

//------------------------------------------------------------------------------
/// @brief The reactor attempts to make thread-safe usage of OpenGL ES
/// easier to reason about.
Expand Down Expand Up @@ -158,6 +166,8 @@ class ReactorGLES {
///
std::optional<GLuint> GetGLHandle(const HandleGLES& handle) const;

std::optional<GLsync> GetGLFence(const HandleGLES& handle) const;

//----------------------------------------------------------------------------
/// @brief Create a reactor handle.
///
Expand Down Expand Up @@ -246,14 +256,14 @@ class ReactorGLES {

private:
struct LiveHandle {
std::optional<GLuint> name;
std::optional<GLHandle> name;
std::optional<std::string> pending_debug_label;
bool pending_collection = false;
fml::ScopedCleanupClosure callback = {};

LiveHandle() = default;

explicit LiveHandle(std::optional<GLuint> p_name) : name(p_name) {}
explicit LiveHandle(std::optional<GLHandle> p_name) : name(p_name) {}

constexpr bool IsLive() const { return name.has_value(); }
};
Expand Down
68 changes: 68 additions & 0 deletions impeller/renderer/backend/gles/test/texture_gles_unittests.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
// 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 "flutter/impeller/playground/playground_test.h"
#include "flutter/impeller/renderer/backend/gles/context_gles.h"
#include "flutter/impeller/renderer/backend/gles/texture_gles.h"
#include "flutter/testing/testing.h"
#include "gtest/gtest.h"
#include "impeller/core/formats.h"
#include "impeller/core/texture_descriptor.h"
#include "impeller/renderer/backend/gles/handle_gles.h"
#include "impeller/renderer/backend/gles/proc_table_gles.h"

namespace impeller::testing {

using TextureGLESTest = PlaygroundTest;
INSTANTIATE_OPENGLES_PLAYGROUND_SUITE(TextureGLESTest);

TEST_P(TextureGLESTest, CanSetSyncFence) {
ContextGLES& context_gles = ContextGLES::Cast(*GetContext());
if (!context_gles.GetReactor()
->GetProcTable()
.GetDescription()
->GetGlVersion()
.IsAtLeast(Version{3, 0, 0})) {
GTEST_SKIP() << "GL Version too low to test sync fence.";
}

TextureDescriptor desc;
desc.storage_mode = StorageMode::kDevicePrivate;
desc.size = {100, 100};
desc.format = PixelFormat::kR8G8B8A8UNormInt;

auto texture = GetContext()->GetResourceAllocator()->CreateTexture(desc);
ASSERT_TRUE(!!texture);

EXPECT_TRUE(GetContext()->AddTrackingFence(texture));
EXPECT_TRUE(context_gles.GetReactor()->React());

std::optional<HandleGLES> sync_fence =
TextureGLES::Cast(*texture).GetSyncFence();
ASSERT_TRUE(sync_fence.has_value());
if (!sync_fence.has_value()) {
return;
}
EXPECT_EQ(sync_fence.value().type, HandleType::kFence);

std::optional<GLsync> sync =
context_gles.GetReactor()->GetGLFence(sync_fence.value());
ASSERT_TRUE(sync.has_value());
if (!sync.has_value()) {
return;
}

// Now queue up operation that binds texture to verify that sync fence is
// waited and removed.

EXPECT_TRUE(
context_gles.GetReactor()->AddOperation([&](const ReactorGLES& reactor) {
return TextureGLES::Cast(*texture).Bind();
}));

sync_fence = TextureGLES::Cast(*texture).GetSyncFence();
ASSERT_FALSE(sync_fence.has_value());
}

} // namespace impeller::testing
Loading