[Impeller] Wrap provided FBO instead of defaulting to FBO0 and cleanup/document the texture API.

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",

impeller/renderer/backend/gles/surface_gles.cc

@@ -34,8 +34,8 @@ std::unique_ptr<Surface> SurfaceGLES::WrapFBO(
   color0_tex.storage_mode = StorageMode::kDevicePrivate;
 
   ColorAttachment color0;
-  color0.texture = std::make_shared<TextureGLES>(
-      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<Surface> SurfaceGLES::WrapFBO(
   depth_stencil_texture_desc.usage = TextureUsage::kRenderTarget;
   depth_stencil_texture_desc.sample_count = SampleCount::kCount1;
 
-  auto depth_stencil_tex = std::make_shared<TextureGLES>(
-      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;

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

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> TextureGLES::WrapFBO(ReactorGLES::Ref reactor,
                                                   TextureDescriptor desc,
                                                   GLuint fbo) {
-  return std::shared_ptr<TextureGLES>(
-      new TextureGLES(std::move(reactor), desc, true, fbo, std::nullopt));
+  auto texture = std::shared_ptr<TextureGLES>(
+      new TextureGLES(std::move(reactor), desc, fbo, std::nullopt));
+  if (!texture->IsValid()) {
+    return nullptr;
+  }
+  return texture;
+}
+
+std::shared_ptr<TextureGLES> 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<TextureGLES>(
+      new TextureGLES(std::move(reactor), desc, std::nullopt, external_handle));
+  if (!texture->IsValid()) {
+    return nullptr;
+  }
+  return texture;
+}
+
+std::shared_ptr<TextureGLES> 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<ReactorGLES> reactor,
                          TextureDescriptor desc,
-                         bool is_wrapped,
                          std::optional<GLuint> fbo,
                          std::optional<HandleGLES> external_handle)
     : Texture(desc),
@@ -176,7 +194,7 @@ TextureGLES::TextureGLES(std::shared_ptr<ReactorGLES> 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;
 }

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<TextureGLES> 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<TextureGLES> 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<TextureGLES> 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<GLuint> 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<ReactorGLES> reactor,
               TextureDescriptor desc,
-              bool is_wrapped,
               std::optional<GLuint> fbo,
               std::optional<HandleGLES> external_handle);
 

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<TextureGLES>(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.";

shell/platform/android/image_external_texture_gl_impeller.cc

@@ -38,8 +38,10 @@ sk_sp<flutter::DlImage> ImageExternalTextureGLImpeller::CreateDlImage(
                static_cast<int>(bounds.height())};
   desc.mip_count = 1;
   auto texture = std::make_shared<impeller::TextureGLES>(
-      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()) {

shell/platform/android/surface_texture_external_texture_gl_impeller.cc

@@ -33,8 +33,10 @@ void SurfaceTextureExternalTextureGLImpeller::ProcessFrame(
                  static_cast<int>(bounds.height())};
     desc.mip_count = 1;
     texture_ = std::make_shared<impeller::TextureGLES>(
-        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();

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<impeller::TextureGLES>(
-      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;

shell/platform/embedder/embedder_external_texture_gl.cc

@@ -147,8 +147,7 @@ sk_sp<DlImage> EmbedderExternalTextureGL::ResolveTextureImpeller(
   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);
+      impeller::TextureGLES::WrapTexture(context.GetReactor(), desc, handle);
 
   if (!image) {
     // In case Skia rejects the image, call the release proc so that