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
19 changes: 11 additions & 8 deletions impeller/renderer/backend/gles/render_pass_gles.cc
Original file line number Diff line number Diff line change
Expand Up @@ -170,19 +170,22 @@ struct RenderPassData {
}
});

const auto is_default_fbo =
TextureGLES::Cast(*pass_data.color_attachment).IsWrapped();
TextureGLES& color_gles = TextureGLES::Cast(*pass_data.color_attachment);
const bool is_default_fbo = color_gles.IsWrapped();

if (!is_default_fbo) {
if (is_default_fbo) {
if (color_gles.GetFBO().has_value()) {
// NOLINTNEXTLINE(bugprone-unchecked-optional-access)
gl.BindFramebuffer(GL_FRAMEBUFFER, *color_gles.GetFBO());
}
} else {
// Create and bind an offscreen FBO.
gl.GenFramebuffers(1u, &fbo);
gl.BindFramebuffer(GL_FRAMEBUFFER, fbo);

if (auto color = TextureGLES::Cast(pass_data.color_attachment.get())) {
if (!color->SetAsFramebufferAttachment(
GL_FRAMEBUFFER, TextureGLES::AttachmentType::kColor0)) {
return false;
}
if (!color_gles.SetAsFramebufferAttachment(
GL_FRAMEBUFFER, TextureGLES::AttachmentType::kColor0)) {
return false;
}

if (auto depth = TextureGLES::Cast(pass_data.depth_attachment.get())) {
Expand Down
17 changes: 13 additions & 4 deletions impeller/renderer/backend/gles/texture_gles.cc
Original file line number Diff line number Diff line change
Expand Up @@ -68,21 +68,30 @@ HandleType ToHandleType(TextureGLES::Type type) {
}

TextureGLES::TextureGLES(ReactorGLES::Ref reactor, TextureDescriptor desc)
: TextureGLES(std::move(reactor), desc, false) {}
: TextureGLES(std::move(reactor), desc, false, std::nullopt) {}

TextureGLES::TextureGLES(ReactorGLES::Ref reactor,
TextureDescriptor desc,
enum IsWrapped wrapped)
: TextureGLES(std::move(reactor), desc, true) {}
: TextureGLES(std::move(reactor), desc, true, std::nullopt) {}

std::shared_ptr<TextureGLES> TextureGLES::WrapFBO(ReactorGLES::Ref reactor,
TextureDescriptor desc,
GLuint fbo) {
return std::shared_ptr<TextureGLES>(
new TextureGLES(std::move(reactor), desc, true, fbo));
}

TextureGLES::TextureGLES(std::shared_ptr<ReactorGLES> reactor,
TextureDescriptor desc,
bool is_wrapped)
bool is_wrapped,
std::optional<GLuint> fbo)
: Texture(desc),
reactor_(std::move(reactor)),
type_(GetTextureTypeFromDescriptor(GetTextureDescriptor())),
handle_(reactor_->CreateHandle(ToHandleType(type_))),
is_wrapped_(is_wrapped) {
is_wrapped_(is_wrapped),
wrapped_fbo_(fbo) {
// Ensure the texture descriptor itself is valid.
if (!GetTextureDescriptor().IsValid()) {
VALIDATION_LOG << "Invalid texture descriptor.";
Expand Down
10 changes: 9 additions & 1 deletion impeller/renderer/backend/gles/texture_gles.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ class TextureGLES final : public Texture,
TextureDescriptor desc,
IsWrapped wrapped);

static std::shared_ptr<TextureGLES> WrapFBO(ReactorGLES::Ref reactor,
TextureDescriptor desc,
GLuint fbo);

// |Texture|
~TextureGLES() override;

Expand All @@ -54,6 +58,8 @@ class TextureGLES final : public Texture,

bool IsWrapped() const { return is_wrapped_; }

std::optional<GLuint> GetFBO() const { return wrapped_fbo_; }

private:
friend class AllocatorMTL;

Expand All @@ -62,11 +68,13 @@ class TextureGLES final : public Texture,
HandleGLES handle_;
mutable bool contents_initialized_ = false;
const bool is_wrapped_;
Copy link
Member

Choose a reason for hiding this comment

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

You should be able to get rid of the is_wrapped_ bool and replace it with wrapped_fbo_.has_value()

Copy link
Member Author

Choose a reason for hiding this comment

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

SurfaceGLES is using textures with the kWrapped flag that are not associated with a specific FBO. Apparently this means that the texture should use whatever FBO is bound when encoding happens.
(see https://github.com/flutter/engine/blob/main/impeller/renderer/backend/gles/surface_gles.cc#L38)

const std::optional<GLuint> wrapped_fbo_;
bool is_valid_ = false;

TextureGLES(std::shared_ptr<ReactorGLES> reactor,
TextureDescriptor desc,
bool is_wrapped);
bool is_wrapped,
std::optional<GLuint> fbo);

// |Texture|
void SetLabel(std::string_view label) override;
Expand Down
5 changes: 2 additions & 3 deletions shell/platform/embedder/embedder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -979,9 +979,8 @@ MakeRenderTargetFromBackingStoreImpeller(
color0_tex.storage_mode = impeller::StorageMode::kDevicePrivate;

impeller::ColorAttachment color0;
color0.texture = std::make_shared<impeller::TextureGLES>(
gl_context.GetReactor(), color0_tex,
impeller::TextureGLES::IsWrapped::kWrapped);
color0.texture = impeller::TextureGLES::WrapFBO(
gl_context.GetReactor(), color0_tex, framebuffer->name);
color0.clear_color = impeller::Color::DarkSlateGray();
color0.load_action = impeller::LoadAction::kClear;
color0.store_action = impeller::StoreAction::kStore;
Expand Down
21 changes: 20 additions & 1 deletion shell/platform/embedder/tests/embedder_gl_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4747,7 +4747,8 @@ TEST_F(EmbedderTest,
}

TEST_F(EmbedderTest, CanRenderWithImpellerOpenGL) {
auto& context = GetEmbedderContext(EmbedderTestContextType::kOpenGLContext);
EmbedderTestContextGL& context = static_cast<EmbedderTestContextGL&>(
GetEmbedderContext(EmbedderTestContextType::kOpenGLContext));
EmbedderConfigBuilder builder(context);

bool present_called = false;
Expand All @@ -4768,6 +4769,24 @@ TEST_F(EmbedderTest, CanRenderWithImpellerOpenGL) {
auto engine = builder.LaunchEngine();
ASSERT_TRUE(engine.is_valid());

// Bind to an arbitrary FBO in order to verify that Impeller binds to the
// provided FBO during rendering.
typedef void (*glGenFramebuffersProc)(GLsizei n, GLuint* ids);
typedef void (*glBindFramebufferProc)(GLenum target, GLuint framebuffer);
auto glGenFramebuffers = reinterpret_cast<glGenFramebuffersProc>(
context.GLGetProcAddress("glGenFramebuffers"));
auto glBindFramebuffer = reinterpret_cast<glBindFramebufferProc>(
context.GLGetProcAddress("glBindFramebuffer"));
const flutter::Shell& shell = ToEmbedderEngine(engine.get())->GetShell();
fml::AutoResetWaitableEvent raster_event;
shell.GetTaskRunners().GetRasterTaskRunner()->PostTask([&] {
GLuint fbo;
glGenFramebuffers(1, &fbo);
glBindFramebuffer(GL_FRAMEBUFFER, fbo);
raster_event.Signal();
});
raster_event.Wait();

// Send a window metrics events so frames may be scheduled.
FlutterWindowMetricsEvent event = {};
event.struct_size = sizeof(event);
Expand Down
4 changes: 2 additions & 2 deletions shell/platform/embedder/tests/embedder_test_context_gl.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ class EmbedderTestContextGL : public EmbedderTestContext {
void GLPopulateExistingDamage(const intptr_t id,
FlutterDamage* existing_damage);

void* GLGetProcAddress(const char* name);

protected:
virtual void SetupCompositor() override;

Expand All @@ -88,8 +90,6 @@ class EmbedderTestContextGL : public EmbedderTestContext {

bool GLMakeResourceCurrent();

void* GLGetProcAddress(const char* name);

FML_DISALLOW_COPY_AND_ASSIGN(EmbedderTestContextGL);
};

Expand Down