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 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 @@ -22,6 +22,7 @@ impeller_component("gles_unittests") {
"test/mock_gles_unittests.cc",
"test/pipeline_library_gles_unittests.cc",
"test/proc_table_gles_unittests.cc",
"test/reactor_unittests.cc",
"test/specialization_constants_unittests.cc",
]
deps = [
Expand Down
14 changes: 14 additions & 0 deletions impeller/renderer/backend/gles/reactor_gles.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include <algorithm>

#include "flutter/fml/trace_event.h"
#include "fml/closure.h"
#include "fml/logging.h"
#include "impeller/base/validation.h"

Expand Down Expand Up @@ -85,6 +86,19 @@ bool ReactorGLES::AddOperation(Operation operation) {
return true;
}

bool ReactorGLES::RegisterCleanupCallback(const HandleGLES& handle,
const fml::closure& callback) {
if (handle.IsDead()) {
return false;
}
WriterLock handles_lock(handles_mutex_);
if (auto found = handles_.find(handle); found != handles_.end()) {
found->second.callback = fml::ScopedCleanupClosure(callback);
return true;
}
return false;
}

static std::optional<GLuint> CreateGLHandle(const ProcTableGLES& gl,
HandleType type) {
GLuint handle = GL_NONE;
Expand Down
19 changes: 19 additions & 0 deletions impeller/renderer/backend/gles/reactor_gles.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include <memory>
#include <vector>

#include "fml/closure.h"
#include "impeller/base/thread.h"
#include "impeller/renderer/backend/gles/handle_gles.h"
#include "impeller/renderer/backend/gles/proc_table_gles.h"
Expand Down Expand Up @@ -216,6 +217,23 @@ class ReactorGLES {
///
[[nodiscard]] bool AddOperation(Operation operation);

//----------------------------------------------------------------------------
/// @brief Register a cleanup callback that will be invokved with the
/// provided user data when the handle is destroyed.
///
/// This operation is not guaranteed to run immediately. It will
/// complete in a finite amount of time on any thread as long as
/// there is a reactor worker and the reactor itself is not being
/// torn down.
///
/// @param[in] handle The handle to attach the cleanup to.
/// @param[in] callback The cleanup callback to execute.
///
/// @return If the operation was successfully queued for completion.
///
bool RegisterCleanupCallback(const HandleGLES& handle,
const fml::closure& callback);

//----------------------------------------------------------------------------
/// @brief Perform a reaction on the current thread if able.
///
Expand All @@ -231,6 +249,7 @@ class ReactorGLES {
std::optional<GLuint> name;
std::optional<std::string> pending_debug_label;
bool pending_collection = false;
fml::ScopedCleanupClosure callback = {};

LiveHandle() = default;

Expand Down
47 changes: 47 additions & 0 deletions impeller/renderer/backend/gles/test/reactor_unittests.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
// 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 <memory>
#include "flutter/testing/testing.h" // IWYU pragma: keep
#include "gtest/gtest.h"
#include "impeller/renderer/backend/gles/handle_gles.h"
#include "impeller/renderer/backend/gles/proc_table_gles.h"
#include "impeller/renderer/backend/gles/reactor_gles.h"
#include "impeller/renderer/backend/gles/test/mock_gles.h"

namespace impeller {
namespace testing {

class TestWorker : public ReactorGLES::Worker {
public:
bool CanReactorReactOnCurrentThreadNow(
const ReactorGLES& reactor) const override {
return true;
}
};

TEST(ReactorGLES, CanAttachCleanupCallbacksToHandles) {
auto mock_gles = MockGLES::Init();
ProcTableGLES::Resolver resolver = kMockResolverGLES;
auto proc_table = std::make_unique<ProcTableGLES>(resolver);
auto worker = std::make_shared<TestWorker>();
auto reactor = std::make_shared<ReactorGLES>(std::move(proc_table));
reactor->AddWorker(worker);

int value = 0;
auto handle = reactor->CreateHandle(HandleType::kTexture, 1123);
auto added =
reactor->RegisterCleanupCallback(handle, [&value]() { value++; });

EXPECT_TRUE(added);
EXPECT_TRUE(reactor->React());

reactor->CollectHandle(handle);
EXPECT_TRUE(reactor->AddOperation([](const ReactorGLES& reactor) {}));
EXPECT_TRUE(reactor->React());
EXPECT_EQ(value, 1);
}

} // namespace testing
} // namespace impeller
63 changes: 62 additions & 1 deletion shell/platform/embedder/embedder_external_texture_gl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,14 @@
#include "flutter/shell/platform/embedder/embedder_external_texture_gl.h"

#include "flutter/fml/logging.h"
#include "include/core/SkCanvas.h"
#include "impeller/core/texture_descriptor.h"
#include "impeller/display_list/aiks_context.h"
#include "impeller/display_list/dl_image_impeller.h"
#include "impeller/geometry/size.h"
#include "impeller/renderer/backend/gles/context_gles.h"
#include "impeller/renderer/backend/gles/handle_gles.h"
#include "impeller/renderer/backend/gles/texture_gles.h"

#include "include/core/SkPaint.h"
#include "third_party/skia/include/core/SkAlphaType.h"
#include "third_party/skia/include/core/SkColorSpace.h"
Expand Down Expand Up @@ -62,6 +69,17 @@ sk_sp<DlImage> EmbedderExternalTextureGL::ResolveTexture(
GrDirectContext* context,
impeller::AiksContext* aiks_context,
const SkISize& size) {
if (!!aiks_context) {
return ResolveTextureImpeller(texture_id, aiks_context, size);
} else {
return ResolveTextureSkia(texture_id, context, size);
}
}

sk_sp<DlImage> EmbedderExternalTextureGL::ResolveTextureSkia(
int64_t texture_id,
GrDirectContext* context,
const SkISize& size) {
context->flushAndSubmit();
context->resetContext(kAll_GrBackendState);
std::unique_ptr<FlutterOpenGLTexture> texture =
Expand Down Expand Up @@ -110,6 +128,49 @@ sk_sp<DlImage> EmbedderExternalTextureGL::ResolveTexture(
return DlImage::Make(std::move(image));
}

sk_sp<DlImage> EmbedderExternalTextureGL::ResolveTextureImpeller(
int64_t texture_id,
impeller::AiksContext* aiks_context,
const SkISize& size) {
std::unique_ptr<FlutterOpenGLTexture> texture =
external_texture_callback_(texture_id, size.width(), size.height());

if (!texture) {
return nullptr;
}

impeller::TextureDescriptor desc;
desc.size = impeller::ISize(texture->width, texture->height);

impeller::ContextGLES& context =
impeller::ContextGLES::Cast(*aiks_context->GetContext());
impeller::HandleGLES handle = context.GetReactor()->CreateHandle(
impeller::HandleType::kTexture, texture->target);
std::shared_ptr<impeller::TextureGLES> image =
std::make_shared<impeller::TextureGLES>(context.GetReactor(), desc,
handle);

if (!image) {
// In case Skia rejects the image, call the release proc so that
// embedders can perform collection of intermediates.
if (texture->destruction_callback) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@chinmaygarde do we have an existing way to track handles / a place for me to attach to a destruction callback? Otherwise I was just going to add an optional member to texture_gles.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is a case of the embedder giving us a texture handle, us using it, and then asking the embedder to collect it for us when we are done.

The most accurate way to do this is using the reactor. It knows when the texture handle is no longer in use. Attaching it to TextureGLES isn't going to be safe because of the skew between the destruction of the texture (which can happen on any thread) and the collection of the handle in the reactor (which must happen on one of the GL threads).

I did something similar (but not the same) here where the ownership of the handle is transferred in. See the wrap here of an external_handle. You could augment reactor handles to pass in a callback instead of a known destructor by handle (which essentially calls glDeleteTextures). That way, the handle will be collected at the right time on the right thread.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it, thank you!

texture->destruction_callback(texture->user_data);
}
FML_LOG(ERROR) << "Could not create external texture";
return nullptr;
}
if (texture->destruction_callback &&
!context.GetReactor()->RegisterCleanupCallback(
handle,
[callback = texture->destruction_callback,
user_data = texture->user_data]() { callback(user_data); })) {
FML_LOG(ERROR) << "Could not register destruction callback";
return nullptr;
}

return impeller::DlImageImpeller::Make(image);
}

// |flutter::Texture|
void EmbedderExternalTextureGL::OnGrContextCreated() {}

Expand Down
8 changes: 8 additions & 0 deletions shell/platform/embedder/embedder_external_texture_gl.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,14 @@ class EmbedderExternalTextureGL : public flutter::Texture {
impeller::AiksContext* aiks_context,
const SkISize& size);

sk_sp<DlImage> ResolveTextureSkia(int64_t texture_id,
GrDirectContext* context,
const SkISize& size);

sk_sp<DlImage> ResolveTextureImpeller(int64_t texture_id,
impeller::AiksContext* aiks_context,
const SkISize& size);

// |flutter::Texture|
void Paint(PaintContext& context,
const SkRect& bounds,
Expand Down