diff --git a/impeller/renderer/backend/gles/BUILD.gn b/impeller/renderer/backend/gles/BUILD.gn index 3562eae59b82c..b3e54aefae8b7 100644 --- a/impeller/renderer/backend/gles/BUILD.gn +++ b/impeller/renderer/backend/gles/BUILD.gn @@ -24,6 +24,7 @@ impeller_component("gles_unittests") { "test/proc_table_gles_unittests.cc", "test/reactor_unittests.cc", "test/specialization_constants_unittests.cc", + "test/surface_gles_unittests.cc", ] deps = [ ":gles", diff --git a/impeller/renderer/backend/gles/surface_gles.cc b/impeller/renderer/backend/gles/surface_gles.cc index d225831d0896d..494351913fde4 100644 --- a/impeller/renderer/backend/gles/surface_gles.cc +++ b/impeller/renderer/backend/gles/surface_gles.cc @@ -34,8 +34,8 @@ std::unique_ptr SurfaceGLES::WrapFBO( color0_tex.storage_mode = StorageMode::kDevicePrivate; ColorAttachment color0; - color0.texture = std::make_shared( - gl_context.GetReactor(), color0_tex, TextureGLES::IsWrapped::kWrapped); + color0.texture = + TextureGLES::WrapFBO(gl_context.GetReactor(), color0_tex, fbo); color0.clear_color = Color::DarkSlateGray(); color0.load_action = LoadAction::kClear; color0.store_action = StoreAction::kStore; @@ -47,9 +47,10 @@ std::unique_ptr SurfaceGLES::WrapFBO( depth_stencil_texture_desc.usage = TextureUsage::kRenderTarget; depth_stencil_texture_desc.sample_count = SampleCount::kCount1; - auto depth_stencil_tex = std::make_shared( - gl_context.GetReactor(), depth_stencil_texture_desc, - TextureGLES::IsWrapped::kWrapped); + auto depth_stencil_tex = + TextureGLES::CreatePlaceholder(gl_context.GetReactor(), // + depth_stencil_texture_desc // + ); DepthAttachment depth0; depth0.clear_depth = 0; diff --git a/impeller/renderer/backend/gles/test/surface_gles_unittests.cc b/impeller/renderer/backend/gles/test/surface_gles_unittests.cc new file mode 100644 index 0000000000000..dbef03075a088 --- /dev/null +++ b/impeller/renderer/backend/gles/test/surface_gles_unittests.cc @@ -0,0 +1,32 @@ +// 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/surface_gles.h" +#include "flutter/impeller/renderer/backend/gles/texture_gles.h" +#include "flutter/testing/testing.h" + +namespace impeller::testing { + +using SurfaceGLESTest = PlaygroundTest; +INSTANTIATE_OPENGLES_PLAYGROUND_SUITE(SurfaceGLESTest); + +TEST_P(SurfaceGLESTest, CanWrapNonZeroFBO) { + const GLuint fbo = 1988; + auto surface = + SurfaceGLES::WrapFBO(GetContext(), []() { return true; }, fbo, + PixelFormat::kR8G8B8A8UNormInt, {100, 100}); + ASSERT_TRUE(!!surface); + ASSERT_TRUE(surface->IsValid()); + ASSERT_TRUE(surface->GetRenderTarget().HasColorAttachment(0)); + const auto& texture = TextureGLES::Cast( + *(surface->GetRenderTarget().GetColorAttachments().at(0).texture)); + auto wrapped = texture.GetFBO(); + ASSERT_TRUE(wrapped.has_value()); + // NOLINTNEXTLINE(bugprone-unchecked-optional-access) + ASSERT_EQ(wrapped.value(), fbo); +} + +} // namespace impeller::testing diff --git a/impeller/renderer/backend/gles/texture_gles.cc b/impeller/renderer/backend/gles/texture_gles.cc index f2167de515c08..1902fd22ae7c7 100644 --- a/impeller/renderer/backend/gles/texture_gles.cc +++ b/impeller/renderer/backend/gles/texture_gles.cc @@ -140,34 +140,52 @@ HandleType ToHandleType(TextureGLES::Type type) { FML_UNREACHABLE(); } -TextureGLES::TextureGLES(ReactorGLES::Ref reactor, TextureDescriptor desc) - : TextureGLES(std::move(reactor), desc, false, std::nullopt, std::nullopt) { -} - -TextureGLES::TextureGLES(ReactorGLES::Ref reactor, - TextureDescriptor desc, - enum IsWrapped wrapped) - : TextureGLES(std::move(reactor), desc, true, std::nullopt, std::nullopt) {} - -TextureGLES::TextureGLES(ReactorGLES::Ref reactor, - TextureDescriptor desc, - HandleGLES external_handle) - : TextureGLES(std::move(reactor), - desc, - true, - std::nullopt, - external_handle) {} - std::shared_ptr TextureGLES::WrapFBO(ReactorGLES::Ref reactor, TextureDescriptor desc, GLuint fbo) { - return std::shared_ptr( - new TextureGLES(std::move(reactor), desc, true, fbo, std::nullopt)); + auto texture = std::shared_ptr( + new TextureGLES(std::move(reactor), desc, fbo, std::nullopt)); + if (!texture->IsValid()) { + return nullptr; + } + return texture; +} + +std::shared_ptr TextureGLES::WrapTexture( + ReactorGLES::Ref reactor, + TextureDescriptor desc, + HandleGLES external_handle) { + if (external_handle.IsDead()) { + VALIDATION_LOG << "Cannot wrap a dead handle."; + return nullptr; + } + if (external_handle.type != HandleType::kTexture) { + VALIDATION_LOG << "Cannot wrap a non-texture handle."; + return nullptr; + } + auto texture = std::shared_ptr( + new TextureGLES(std::move(reactor), desc, std::nullopt, external_handle)); + if (!texture->IsValid()) { + return nullptr; + } + return texture; +} + +std::shared_ptr TextureGLES::CreatePlaceholder( + ReactorGLES::Ref reactor, + TextureDescriptor desc) { + return TextureGLES::WrapFBO(std::move(reactor), desc, 0u); } +TextureGLES::TextureGLES(ReactorGLES::Ref reactor, TextureDescriptor desc) + : TextureGLES(std::move(reactor), // + desc, // + std::nullopt, // + std::nullopt // + ) {} + TextureGLES::TextureGLES(std::shared_ptr reactor, TextureDescriptor desc, - bool is_wrapped, std::optional fbo, std::optional external_handle) : Texture(desc), @@ -176,7 +194,7 @@ TextureGLES::TextureGLES(std::shared_ptr reactor, handle_(external_handle.has_value() ? external_handle.value() : reactor_->CreateHandle(ToHandleType(type_))), - is_wrapped_(is_wrapped), + is_wrapped_(fbo.has_value() || external_handle.has_value()), wrapped_fbo_(fbo) { // Ensure the texture descriptor itself is valid. if (!GetTextureDescriptor().IsValid()) { @@ -479,6 +497,12 @@ bool TextureGLES::Bind() const { return true; } +void TextureGLES::MarkContentsInitialized() { + for (size_t i = 0; i < slices_initialized_.size(); i++) { + slices_initialized_[i] = true; + } +} + void TextureGLES::MarkSliceInitialized(size_t slice) const { slices_initialized_[slice] = true; } diff --git a/impeller/renderer/backend/gles/texture_gles.h b/impeller/renderer/backend/gles/texture_gles.h index 05ad1d952c11b..2f5a9d176104e 100644 --- a/impeller/renderer/backend/gles/texture_gles.h +++ b/impeller/renderer/backend/gles/texture_gles.h @@ -24,24 +24,56 @@ class TextureGLES final : public Texture, kRenderBufferMultisampled, }; - enum class IsWrapped { - kWrapped, - }; - - TextureGLES(ReactorGLES::Ref reactor, TextureDescriptor desc); - - TextureGLES(ReactorGLES::Ref reactor, - TextureDescriptor desc, - IsWrapped wrapped); - - TextureGLES(ReactorGLES::Ref reactor, - TextureDescriptor desc, - HandleGLES external_handle); - + //---------------------------------------------------------------------------- + /// @brief Create a texture by wrapping an external framebuffer object + /// whose lifecycle is owned by the caller. + /// + /// This is useful for creating a render target for the default + /// window managed framebuffer. + /// + /// @param[in] reactor The reactor + /// @param[in] desc The description + /// @param[in] fbo The fbo + /// + /// @return If a texture representation of the framebuffer could be + /// created. + /// static std::shared_ptr WrapFBO(ReactorGLES::Ref reactor, TextureDescriptor desc, GLuint fbo); + //---------------------------------------------------------------------------- + /// @brief Create a texture by wrapping an external OpenGL texture + /// handle. Ownership of the texture handle is assumed by the + /// reactor. + /// + /// @param[in] reactor The reactor + /// @param[in] desc The description + /// @param[in] external_handle The external handle + /// + /// @return If a texture representation of the framebuffer could be + /// created. + /// + static std::shared_ptr WrapTexture(ReactorGLES::Ref reactor, + TextureDescriptor desc, + HandleGLES external_handle); + + //---------------------------------------------------------------------------- + /// @brief Create a "texture" that is never expected to be bound/unbound + /// explicitly or initialized in any way. It only exists to setup + /// a render pass description. + /// + /// @param[in] reactor The reactor + /// @param[in] desc The description + /// + /// @return If a texture placeholder could be created. + /// + static std::shared_ptr CreatePlaceholder( + ReactorGLES::Ref reactor, + TextureDescriptor desc); + + TextureGLES(ReactorGLES::Ref reactor, TextureDescriptor desc); + // |Texture| ~TextureGLES() override; @@ -69,9 +101,22 @@ class TextureGLES final : public Texture, std::optional GetFBO() const; - // For non cubemap textures, 0 indicates uninitialized and 1 indicates - // initialized. For cubemap textures, each face is initialized separately with - // each bit tracking the initialization of the corresponding slice. + //---------------------------------------------------------------------------- + /// @brief Indicates that all texture storage has already been allocated + /// and contents initialized. + /// + /// This is similar to calling `MarkSliceInitialized` with all + /// slices. + /// + /// @see MarkSliceInitialized. + /// + void MarkContentsInitialized(); + + //---------------------------------------------------------------------------- + /// @brief Indicates that a specific texture slice has been initialized. + /// + /// @param[in] slice The slice to mark as being initialized. + /// void MarkSliceInitialized(size_t slice) const; bool IsSliceInitialized(size_t slice) const; @@ -87,7 +132,6 @@ class TextureGLES final : public Texture, TextureGLES(std::shared_ptr reactor, TextureDescriptor desc, - bool is_wrapped, std::optional fbo, std::optional external_handle); diff --git a/impeller/toolkit/interop/impeller.cc b/impeller/toolkit/interop/impeller.cc index 03999398268ad..7681efe515f9c 100644 --- a/impeller/toolkit/interop/impeller.cc +++ b/impeller/toolkit/interop/impeller.cc @@ -528,13 +528,6 @@ ImpellerTexture ImpellerTextureCreateWithOpenGLTextureHandleNew( const auto& impeller_context_gl = ContextGLES::Cast(*impeller_context); const auto& reactor = impeller_context_gl.GetReactor(); - auto wrapped_external_gl_handle = - reactor->CreateHandle(HandleType::kTexture, external_gl_handle); - if (wrapped_external_gl_handle.IsDead()) { - VALIDATION_LOG << "Could not wrap external handle."; - return nullptr; - } - TextureDescriptor desc; desc.storage_mode = StorageMode::kDevicePrivate; desc.type = TextureType::kTexture2D; @@ -543,9 +536,11 @@ ImpellerTexture ImpellerTextureCreateWithOpenGLTextureHandleNew( desc.mip_count = std::min(descriptor->mip_count, 1u); desc.usage = TextureUsage::kShaderRead; desc.compression_type = CompressionType::kLossless; - auto texture = std::make_shared(reactor, // - desc, // - wrapped_external_gl_handle // + + auto texture = TextureGLES::WrapTexture( + reactor, // + desc, // + reactor->CreateHandle(HandleType::kTexture, external_gl_handle) // ); if (!texture || !texture->IsValid()) { VALIDATION_LOG << "Could not wrap external texture."; diff --git a/shell/platform/android/image_external_texture_gl_impeller.cc b/shell/platform/android/image_external_texture_gl_impeller.cc index f14fea8882beb..7b2e5ce706016 100644 --- a/shell/platform/android/image_external_texture_gl_impeller.cc +++ b/shell/platform/android/image_external_texture_gl_impeller.cc @@ -38,8 +38,10 @@ sk_sp ImageExternalTextureGLImpeller::CreateDlImage( static_cast(bounds.height())}; desc.mip_count = 1; auto texture = std::make_shared( - impeller_context_->GetReactor(), desc, - impeller::TextureGLES::IsWrapped::kWrapped); + impeller_context_->GetReactor(), desc); + // The contents will be initialized later in the call to + // `glEGLImageTargetTexture2DOES` instead of by Impeller. + texture->MarkContentsInitialized(); texture->SetCoordinateSystem( impeller::TextureCoordinateSystem::kUploadFromHost); if (!texture->Bind()) { diff --git a/shell/platform/android/surface_texture_external_texture_gl_impeller.cc b/shell/platform/android/surface_texture_external_texture_gl_impeller.cc index 227610a524889..69f4b5e2c6906 100644 --- a/shell/platform/android/surface_texture_external_texture_gl_impeller.cc +++ b/shell/platform/android/surface_texture_external_texture_gl_impeller.cc @@ -33,8 +33,10 @@ void SurfaceTextureExternalTextureGLImpeller::ProcessFrame( static_cast(bounds.height())}; desc.mip_count = 1; texture_ = std::make_shared( - impeller_context_->GetReactor(), desc, - impeller::TextureGLES::IsWrapped::kWrapped); + impeller_context_->GetReactor(), desc); + // The contents will be initialized later in the call to `Attach` instead of + // by Impeller. + texture_->MarkContentsInitialized(); texture_->SetCoordinateSystem( impeller::TextureCoordinateSystem::kUploadFromHost); auto maybe_handle = texture_->GetGLHandle(); diff --git a/shell/platform/embedder/embedder.cc b/shell/platform/embedder/embedder.cc index 05fed0e016d29..8bec5367a87b4 100644 --- a/shell/platform/embedder/embedder.cc +++ b/shell/platform/embedder/embedder.cc @@ -1196,9 +1196,8 @@ MakeRenderTargetFromBackingStoreImpeller( depth_stencil_texture_desc.sample_count = impeller::SampleCount::kCount1; } - auto depth_stencil_tex = std::make_shared( - gl_context.GetReactor(), depth_stencil_texture_desc, - impeller::TextureGLES::IsWrapped::kWrapped); + auto depth_stencil_tex = impeller::TextureGLES::CreatePlaceholder( + gl_context.GetReactor(), depth_stencil_texture_desc); impeller::DepthAttachment depth0; depth0.clear_depth = 0; diff --git a/shell/platform/embedder/embedder_external_texture_gl.cc b/shell/platform/embedder/embedder_external_texture_gl.cc index 892b064af080f..852e7390a25ae 100644 --- a/shell/platform/embedder/embedder_external_texture_gl.cc +++ b/shell/platform/embedder/embedder_external_texture_gl.cc @@ -147,8 +147,7 @@ sk_sp EmbedderExternalTextureGL::ResolveTextureImpeller( impeller::HandleGLES handle = context.GetReactor()->CreateHandle( impeller::HandleType::kTexture, texture->target); std::shared_ptr image = - std::make_shared(context.GetReactor(), desc, - handle); + impeller::TextureGLES::WrapTexture(context.GetReactor(), desc, handle); if (!image) { // In case Skia rejects the image, call the release proc so that