Revert "[Impeller] Switch from transient stencil-only to depth+stencil buffer. (#47987)"

auto-submit[bot] · auto-submit[bot] · commit 8717e858d493 · 2024-01-17T16:13:01.000Z
This reverts commit 8897901.
diff --git a/impeller/aiks/aiks_unittests.cc b/impeller/aiks/aiks_unittests.cc
@@ -3557,8 +3557,6 @@ TEST_P(AiksTest, GaussianBlurWithoutDecalSupport) {
       .WillRepeatedly(::testing::Return(false));
   FLT_FORWARD(mock_capabilities, old_capabilities, GetDefaultColorFormat);
   FLT_FORWARD(mock_capabilities, old_capabilities, GetDefaultStencilFormat);
-  FLT_FORWARD(mock_capabilities, old_capabilities,
-              GetDefaultDepthStencilFormat);
   FLT_FORWARD(mock_capabilities, old_capabilities, SupportsOffscreenMSAA);
   FLT_FORWARD(mock_capabilities, old_capabilities,
               SupportsImplicitResolvingMSAA);
diff --git a/impeller/entity/entity_pass.cc b/impeller/entity/entity_pass.cc
@@ -423,8 +423,7 @@ bool EntityPass::Render(ContentContext& renderer,
   // If a root stencil was provided by the caller, then verify that it has a
   // configuration which can be used to render this pass.
   auto stencil_attachment = root_render_target.GetStencilAttachment();
-  auto depth_attachment = root_render_target.GetDepthAttachment();
-  if (stencil_attachment.has_value() && depth_attachment.has_value()) {
+  if (stencil_attachment.has_value()) {
     auto stencil_texture = stencil_attachment->texture;
     if (!stencil_texture) {
       VALIDATION_LOG << "The root RenderTarget must have a stencil texture.";
@@ -443,7 +442,7 @@ bool EntityPass::Render(ContentContext& renderer,
   // Setup a new root stencil with an optimal configuration if one wasn't
   // provided by the caller.
   else {
-    root_render_target.SetupDepthStencilAttachments(
+    root_render_target.SetupStencilAttachment(
         *renderer.GetContext(), *renderer.GetRenderTargetCache(),
         color0.texture->GetSize(),
         renderer.GetContext()->GetCapabilities()->SupportsOffscreenMSAA(),
diff --git a/impeller/renderer/backend/gles/blit_command_gles.cc b/impeller/renderer/backend/gles/blit_command_gles.cc
@@ -41,7 +41,7 @@ static std::optional<GLuint> ConfigureFBO(
   gl.BindFramebuffer(fbo_type, fbo);
 
   if (!TextureGLES::Cast(*texture).SetAsFramebufferAttachment(
-          fbo_type, TextureGLES::AttachmentType::kColor0)) {
+          fbo_type, TextureGLES::AttachmentPoint::kColor0)) {
     VALIDATION_LOG << "Could not attach texture to framebuffer.";
     DeleteFBO(gl, fbo, fbo_type);
     return std::nullopt;
diff --git a/impeller/renderer/backend/gles/render_pass_gles.cc b/impeller/renderer/backend/gles/render_pass_gles.cc
@@ -186,20 +186,20 @@ struct RenderPassData {
 
     if (auto color = TextureGLES::Cast(pass_data.color_attachment.get())) {
       if (!color->SetAsFramebufferAttachment(
-              GL_FRAMEBUFFER, TextureGLES::AttachmentType::kColor0)) {
+              GL_FRAMEBUFFER, TextureGLES::AttachmentPoint::kColor0)) {
         return false;
       }
     }
 
     if (auto depth = TextureGLES::Cast(pass_data.depth_attachment.get())) {
       if (!depth->SetAsFramebufferAttachment(
-              GL_FRAMEBUFFER, TextureGLES::AttachmentType::kDepth)) {
+              GL_FRAMEBUFFER, TextureGLES::AttachmentPoint::kDepth)) {
         return false;
       }
     }
     if (auto stencil = TextureGLES::Cast(pass_data.stencil_attachment.get())) {
       if (!stencil->SetAsFramebufferAttachment(
-              GL_FRAMEBUFFER, TextureGLES::AttachmentType::kStencil)) {
+              GL_FRAMEBUFFER, TextureGLES::AttachmentPoint::kStencil)) {
         return false;
       }
     }
diff --git a/impeller/renderer/backend/gles/texture_gles.cc b/impeller/renderer/backend/gles/texture_gles.cc
@@ -7,7 +7,6 @@
 #include <optional>
 #include <utility>
 
-#include "flutter/fml/logging.h"
 #include "flutter/fml/mapping.h"
 #include "flutter/fml/trace_event.h"
 #include "impeller/base/allocation.h"
@@ -18,37 +17,13 @@
 
 namespace impeller {
 
-static bool IsDepthStencilFormat(PixelFormat format) {
-  switch (format) {
-    case PixelFormat::kS8UInt:
-    case PixelFormat::kD24UnormS8Uint:
-    case PixelFormat::kD32FloatS8UInt:
-      return true;
-    case PixelFormat::kUnknown:
-    case PixelFormat::kA8UNormInt:
-    case PixelFormat::kR8UNormInt:
-    case PixelFormat::kR8G8UNormInt:
-    case PixelFormat::kR8G8B8A8UNormInt:
-    case PixelFormat::kR8G8B8A8UNormIntSRGB:
-    case PixelFormat::kB8G8R8A8UNormInt:
-    case PixelFormat::kB8G8R8A8UNormIntSRGB:
-    case PixelFormat::kR32G32B32A32Float:
-    case PixelFormat::kR16G16B16A16Float:
-    case PixelFormat::kB10G10R10XR:
-    case PixelFormat::kB10G10R10XRSRGB:
-    case PixelFormat::kB10G10R10A10XR:
-      return false;
-  }
-  FML_UNREACHABLE();
-}
-
 static TextureGLES::Type GetTextureTypeFromDescriptor(
     const TextureDescriptor& desc) {
   const auto usage = static_cast<TextureUsageMask>(desc.usage);
   const auto render_target =
       static_cast<TextureUsageMask>(TextureUsage::kRenderTarget);
   const auto is_msaa = desc.sample_count == SampleCount::kCount4;
-  if (usage == render_target && IsDepthStencilFormat(desc.format)) {
+  if (usage == render_target && desc.format == PixelFormat::kS8UInt) {
     return is_msaa ? TextureGLES::Type::kRenderBufferMultisampled
                    : TextureGLES::Type::kRenderBuffer;
   }
@@ -482,20 +457,19 @@ TextureGLES::Type TextureGLES::GetType() const {
   return type_;
 }
 
-static GLenum ToAttachmentType(TextureGLES::AttachmentType point) {
+static GLenum ToAttachmentPoint(TextureGLES::AttachmentPoint point) {
   switch (point) {
-    case TextureGLES::AttachmentType::kColor0:
+    case TextureGLES::AttachmentPoint::kColor0:
       return GL_COLOR_ATTACHMENT0;
-    case TextureGLES::AttachmentType::kDepth:
+    case TextureGLES::AttachmentPoint::kDepth:
       return GL_DEPTH_ATTACHMENT;
-    case TextureGLES::AttachmentType::kStencil:
+    case TextureGLES::AttachmentPoint::kStencil:
       return GL_STENCIL_ATTACHMENT;
   }
 }
 
-bool TextureGLES::SetAsFramebufferAttachment(
-    GLenum target,
-    AttachmentType attachment_type) const {
+bool TextureGLES::SetAsFramebufferAttachment(GLenum target,
+                                             AttachmentPoint point) const {
   if (!IsValid()) {
     return false;
   }
@@ -508,30 +482,29 @@ bool TextureGLES::SetAsFramebufferAttachment(
 
   switch (type_) {
     case Type::kTexture:
-      gl.FramebufferTexture2D(target,                             // target
-                              ToAttachmentType(attachment_type),  // attachment
-                              GL_TEXTURE_2D,                      // textarget
-                              handle.value(),                     // texture
-                              0                                   // level
+      gl.FramebufferTexture2D(target,                    // target
+                              ToAttachmentPoint(point),  // attachment
+                              GL_TEXTURE_2D,             // textarget
+                              handle.value(),            // texture
+                              0                          // level
       );
       break;
     case Type::kTextureMultisampled:
       gl.FramebufferTexture2DMultisampleEXT(
-          target,                             // target
-          ToAttachmentType(attachment_type),  // attachment
-          GL_TEXTURE_2D,                      // textarget
-          handle.value(),                     // texture
-          0,                                  // level
-          4                                   // samples
+          target,                    // target
+          ToAttachmentPoint(point),  // attachment
+          GL_TEXTURE_2D,             // textarget
+          handle.value(),            // texture
+          0,                         // level
+          4                          // samples
       );
       break;
     case Type::kRenderBuffer:
     case Type::kRenderBufferMultisampled:
-      gl.FramebufferRenderbuffer(
-          target,                             // target
-          ToAttachmentType(attachment_type),  // attachment
-          GL_RENDERBUFFER,                    // render-buffer target
-          handle.value()                      // render-buffer
+      gl.FramebufferRenderbuffer(target,                    // target
+                                 ToAttachmentPoint(point),  // attachment
+                                 GL_RENDERBUFFER,  // render-buffer target
+                                 handle.value()    // render-buffer
       );
       break;
   }
diff --git a/impeller/renderer/backend/gles/texture_gles.h b/impeller/renderer/backend/gles/texture_gles.h
@@ -41,14 +41,13 @@ class TextureGLES final : public Texture,
 
   [[nodiscard]] bool GenerateMipmap();
 
-  enum class AttachmentType {
+  enum class AttachmentPoint {
     kColor0,
     kDepth,
     kStencil,
   };
-  [[nodiscard]] bool SetAsFramebufferAttachment(
-      GLenum target,
-      AttachmentType attachment_type) const;
+  [[nodiscard]] bool SetAsFramebufferAttachment(GLenum target,
+                                                AttachmentPoint point) const;
 
   Type GetType() const;
 
diff --git a/impeller/renderer/render_target.cc b/impeller/renderer/render_target.cc
@@ -255,8 +255,8 @@ RenderTarget RenderTarget::CreateOffscreen(
   target.SetColorAttachment(color0, 0u);
 
   if (stencil_attachment_config.has_value()) {
-    target.SetupDepthStencilAttachments(context, allocator, size, false, label,
-                                        stencil_attachment_config.value());
+    target.SetupStencilAttachment(context, allocator, size, false, label,
+                                  stencil_attachment_config.value());
   } else {
     target.SetStencilAttachment(std::nullopt);
   }
@@ -347,56 +347,43 @@ RenderTarget RenderTarget::CreateOffscreenMSAA(
   // Create MSAA stencil texture.
 
   if (stencil_attachment_config.has_value()) {
-    target.SetupDepthStencilAttachments(context, allocator, size, true, label,
-                                        stencil_attachment_config.value());
+    target.SetupStencilAttachment(context, allocator, size, true, label,
+                                  stencil_attachment_config.value());
   } else {
     target.SetStencilAttachment(std::nullopt);
   }
 
   return target;
 }
 
-void RenderTarget::SetupDepthStencilAttachments(
+void RenderTarget::SetupStencilAttachment(
     const Context& context,
     RenderTargetAllocator& allocator,
     ISize size,
     bool msaa,
     const std::string& label,
     AttachmentConfig stencil_attachment_config) {
-  TextureDescriptor depth_stencil_texture_desc;
-  depth_stencil_texture_desc.storage_mode =
-      stencil_attachment_config.storage_mode;
+  TextureDescriptor stencil_tex0;
+  stencil_tex0.storage_mode = stencil_attachment_config.storage_mode;
   if (msaa) {
-    depth_stencil_texture_desc.type = TextureType::kTexture2DMultisample;
-    depth_stencil_texture_desc.sample_count = SampleCount::kCount4;
+    stencil_tex0.type = TextureType::kTexture2DMultisample;
+    stencil_tex0.sample_count = SampleCount::kCount4;
   }
-  depth_stencil_texture_desc.format =
-      context.GetCapabilities()->GetDefaultDepthStencilFormat();
-  depth_stencil_texture_desc.size = size;
-  depth_stencil_texture_desc.usage =
+  stencil_tex0.format = context.GetCapabilities()->GetDefaultStencilFormat();
+  stencil_tex0.size = size;
+  stencil_tex0.usage =
       static_cast<TextureUsageMask>(TextureUsage::kRenderTarget);
 
-  auto depth_stencil_texture =
-      allocator.CreateTexture(depth_stencil_texture_desc);
-  if (!depth_stencil_texture) {
-    return;  // Error messages are handled by `Allocator::CreateTexture`.
-  }
-
-  DepthAttachment depth0;
-  depth0.load_action = stencil_attachment_config.load_action;
-  depth0.store_action = stencil_attachment_config.store_action;
-  depth0.clear_depth = 0u;
-  depth0.texture = depth_stencil_texture;
-
   StencilAttachment stencil0;
   stencil0.load_action = stencil_attachment_config.load_action;
   stencil0.store_action = stencil_attachment_config.store_action;
   stencil0.clear_stencil = 0u;
-  stencil0.texture = depth_stencil_texture;
+  stencil0.texture = allocator.CreateTexture(stencil_tex0);
 
-  stencil0.texture->SetLabel(
-      SPrintF("%s Depth+Stencil Texture", label.c_str()));
-  SetDepthAttachment(std::move(depth0));
+  if (!stencil0.texture) {
+    return;  // Error messages are handled by `Allocator::CreateTexture`.
+  }
+  stencil0.texture->SetLabel(SPrintF("%s Stencil Texture", label.c_str()));
   SetStencilAttachment(std::move(stencil0));
 }
 
@@ -416,9 +403,6 @@ size_t RenderTarget::GetTotalAttachmentCount() const {
   if (stencil_.has_value()) {
     count++;
   }
-  if (depth_.has_value()) {
-    count++;
-  }
   return count;
 }
 
diff --git a/impeller/renderer/render_target.h b/impeller/renderer/render_target.h
@@ -109,13 +109,13 @@ class RenderTarget final {
 
   bool IsValid() const;
 
-  void SetupDepthStencilAttachments(const Context& context,
-                                    RenderTargetAllocator& allocator,
-                                    ISize size,
-                                    bool msaa,
-                                    const std::string& label = "Offscreen",
-                                    AttachmentConfig stencil_attachment_config =
-                                        kDefaultStencilAttachmentConfig);
+  void SetupStencilAttachment(const Context& context,
+                              RenderTargetAllocator& allocator,
+                              ISize size,
+                              bool msaa,
+                              const std::string& label = "Offscreen",
+                              AttachmentConfig stencil_attachment_config =
+                                  kDefaultStencilAttachmentConfig);
 
   SampleCount GetSampleCount() const;
 
diff --git a/impeller/renderer/renderer_unittests.cc b/impeller/renderer/renderer_unittests.cc
@@ -1161,9 +1161,9 @@ TEST_P(RendererTest, StencilMask) {
       stencil_config.storage_mode = StorageMode::kHostVisible;
       auto render_target_allocator =
           RenderTargetAllocator(context->GetResourceAllocator());
-      render_target.SetupDepthStencilAttachments(
-          *context, render_target_allocator,
-          render_target.GetRenderTargetSize(), true, "stencil", stencil_config);
+      render_target.SetupStencilAttachment(*context, render_target_allocator,
+                                           render_target.GetRenderTargetSize(),
+                                           true, "stencil", stencil_config);
       // Fill the stencil buffer with an checkerboard pattern.
       const auto target_width = render_target.GetRenderTargetSize().width;
       const auto target_height = render_target.GetRenderTargetSize().height;
@@ -1289,20 +1289,6 @@ TEST_P(RendererTest, CanLookupRenderTargetProperties) {
             render_target.GetRenderTargetSize());
 }
 
-TEST_P(RendererTest,
-       RenderTargetCreateOffscreenMSAASetsDefaultDepthStencilFormat) {
-  auto context = GetContext();
-  auto render_target_cache = std::make_shared<RenderTargetAllocator>(
-      GetContext()->GetResourceAllocator());
-
-  RenderTarget render_target = RenderTarget::CreateOffscreenMSAA(
-      *context, *render_target_cache, {100, 100}, /*mip_count=*/1);
-  EXPECT_EQ(render_target.GetDepthAttachment()
-                ->texture->GetTextureDescriptor()
-                .format,
-            GetContext()->GetCapabilities()->GetDefaultDepthStencilFormat());
-}
-
 }  // namespace testing
 }  // namespace impeller
 

Original file line number	Diff line number	Diff line change
`@@ -186,20 +186,20 @@ struct RenderPassData {`
`186`	`186`
`187`	`187`	`if (auto color = TextureGLES::Cast(pass_data.color_attachment.get())) {`
`188`	`188`	`if (!color->SetAsFramebufferAttachment(`
`189`		`- GL_FRAMEBUFFER, TextureGLES::AttachmentType::kColor0)) {`
	`189`	`+ GL_FRAMEBUFFER, TextureGLES::AttachmentPoint::kColor0)) {`
`190`	`190`	`return false;`
`191`	`191`	`}`
`192`	`192`	`}`
`193`	`193`
`194`	`194`	`if (auto depth = TextureGLES::Cast(pass_data.depth_attachment.get())) {`
`195`	`195`	`if (!depth->SetAsFramebufferAttachment(`
`196`		`- GL_FRAMEBUFFER, TextureGLES::AttachmentType::kDepth)) {`
	`196`	`+ GL_FRAMEBUFFER, TextureGLES::AttachmentPoint::kDepth)) {`
`197`	`197`	`return false;`
`198`	`198`	`}`
`199`	`199`	`}`
`200`	`200`	`if (auto stencil = TextureGLES::Cast(pass_data.stencil_attachment.get())) {`
`201`	`201`	`if (!stencil->SetAsFramebufferAttachment(`
`202`		`- GL_FRAMEBUFFER, TextureGLES::AttachmentType::kStencil)) {`
	`202`	`+ GL_FRAMEBUFFER, TextureGLES::AttachmentPoint::kStencil)) {`
`203`	`203`	`return false;`
`204`	`204`	`}`
`205`	`205`	`}`