[Impeller] Reland: Switch from transient stencil-only to depth+stencil buffer. (#49838)

bdero · web-flow · commit 4a398d399b9f · 2024-01-22T09:37:59.000-08:00
Part of flutter/flutter#138460. In preparation for [draw order optimization](flutter/flutter#114402) and [StC](flutter/flutter#123671). Use a transient depth+stencil texture instead of a stencil-only texture. Doing this in isolation to detect/weed out any bugs with handling the attachment.
diff --git a/impeller/aiks/aiks_unittests.cc b/impeller/aiks/aiks_unittests.cc
@@ -3557,6 +3557,8 @@ 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/contents/content_context.cc b/impeller/entity/contents/content_context.cc
@@ -136,7 +136,8 @@ void ContentContextOptions::ApplyToPipelineDescriptor(
   }
   desc.SetColorAttachmentDescriptor(0u, color0);
 
-  if (!has_stencil_attachment) {
+  if (!has_depth_stencil_attachments) {
+    desc.ClearDepthAttachment();
     desc.ClearStencilAttachments();
   }
 
diff --git a/impeller/entity/contents/content_context.h b/impeller/entity/contents/content_context.h
@@ -285,7 +285,7 @@ struct ContentContextOptions {
   StencilOperation stencil_operation = StencilOperation::kKeep;
   PrimitiveType primitive_type = PrimitiveType::kTriangle;
   PixelFormat color_attachment_pixel_format = PixelFormat::kUnknown;
-  bool has_stencil_attachment = true;
+  bool has_depth_stencil_attachments = true;
   bool wireframe = false;
   bool is_for_rrect_blur_clear = false;
 
@@ -301,7 +301,7 @@ struct ContentContextOptions {
 
       return (o.is_for_rrect_blur_clear ? 1llu : 0llu) << 0 |
              (o.wireframe ? 1llu : 0llu) << 1 |
-             (o.has_stencil_attachment ? 1llu : 0llu) << 2 |
+             (o.has_depth_stencil_attachments ? 1llu : 0llu) << 2 |
              // enums
              static_cast<uint64_t>(o.color_attachment_pixel_format) << 16 |
              static_cast<uint64_t>(o.primitive_type) << 24 |
@@ -322,7 +322,8 @@ struct ContentContextOptions {
              lhs.primitive_type == rhs.primitive_type &&
              lhs.color_attachment_pixel_format ==
                  rhs.color_attachment_pixel_format &&
-             lhs.has_stencil_attachment == rhs.has_stencil_attachment &&
+             lhs.has_depth_stencil_attachments ==
+                 rhs.has_depth_stencil_attachments &&
              lhs.wireframe == rhs.wireframe &&
              lhs.is_for_rrect_blur_clear == rhs.is_for_rrect_blur_clear;
     }
diff --git a/impeller/entity/contents/contents.cc b/impeller/entity/contents/contents.cc
@@ -21,7 +21,7 @@ ContentContextOptions OptionsFromPass(const RenderPass& pass) {
   ContentContextOptions opts;
   opts.sample_count = pass.GetSampleCount();
   opts.color_attachment_pixel_format = pass.GetRenderTargetPixelFormat();
-  opts.has_stencil_attachment = pass.HasStencilAttachment();
+  opts.has_depth_stencil_attachments = pass.HasStencilAttachment();
   return opts;
 }
 
diff --git a/impeller/entity/entity_pass.cc b/impeller/entity/entity_pass.cc
@@ -431,7 +431,8 @@ 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();
-  if (stencil_attachment.has_value()) {
+  auto depth_attachment = root_render_target.GetDepthAttachment();
+  if (stencil_attachment.has_value() && depth_attachment.has_value()) {
     auto stencil_texture = stencil_attachment->texture;
     if (!stencil_texture) {
       VALIDATION_LOG << "The root RenderTarget must have a stencil texture.";
@@ -450,7 +451,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.SetupStencilAttachment(
+    root_render_target.SetupDepthStencilAttachments(
         *renderer.GetContext(), *renderer.GetRenderTargetCache(),
         color0.texture->GetSize(),
         renderer.GetContext()->GetCapabilities()->SupportsOffscreenMSAA(),
diff --git a/impeller/entity/entity_unittests.cc b/impeller/entity/entity_unittests.cc
@@ -2619,9 +2619,9 @@ TEST_P(EntityTest, SpecializationConstantsAreAppliedToVariants) {
       ContentContext(GetContext(), TypographerContextSkia::Make());
 
   auto default_color_burn = content_context.GetBlendColorBurnPipeline(
-      {.has_stencil_attachment = false});
+      {.has_depth_stencil_attachments = false});
   auto alt_color_burn = content_context.GetBlendColorBurnPipeline(
-      {.has_stencil_attachment = true});
+      {.has_depth_stencil_attachments = true});
 
   ASSERT_NE(default_color_burn, alt_color_burn);
   ASSERT_EQ(default_color_burn->GetDescriptor().GetSpecializationConstants(),
@@ -2690,7 +2690,7 @@ TEST_P(EntityTest, ContentContextOptionsHasReasonableHashFunctions) {
   opts.blend_mode = BlendMode::kColorBurn;
   auto hash_b = ContentContextOptions::Hash{}(opts);
 
-  opts.has_stencil_attachment = false;
+  opts.has_depth_stencil_attachments = false;
   auto hash_c = ContentContextOptions::Hash{}(opts);
 
   opts.primitive_type = PrimitiveType::kPoint;
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::AttachmentPoint::kColor0)) {
+          fbo_type, TextureGLES::AttachmentType::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::AttachmentPoint::kColor0)) {
+              GL_FRAMEBUFFER, TextureGLES::AttachmentType::kColor0)) {
         return false;
       }
     }
 
     if (auto depth = TextureGLES::Cast(pass_data.depth_attachment.get())) {
       if (!depth->SetAsFramebufferAttachment(
-              GL_FRAMEBUFFER, TextureGLES::AttachmentPoint::kDepth)) {
+              GL_FRAMEBUFFER, TextureGLES::AttachmentType::kDepth)) {
         return false;
       }
     }
     if (auto stencil = TextureGLES::Cast(pass_data.stencil_attachment.get())) {
       if (!stencil->SetAsFramebufferAttachment(
-              GL_FRAMEBUFFER, TextureGLES::AttachmentPoint::kStencil)) {
+              GL_FRAMEBUFFER, TextureGLES::AttachmentType::kStencil)) {
         return false;
       }
     }
diff --git a/impeller/renderer/backend/gles/surface_gles.cc b/impeller/renderer/backend/gles/surface_gles.cc
@@ -40,24 +40,34 @@ std::unique_ptr<Surface> SurfaceGLES::WrapFBO(
   color0.load_action = LoadAction::kClear;
   color0.store_action = StoreAction::kStore;
 
-  TextureDescriptor stencil0_tex;
-  stencil0_tex.type = TextureType::kTexture2D;
-  stencil0_tex.format = color_format;
-  stencil0_tex.size = fbo_size;
-  stencil0_tex.usage =
+  TextureDescriptor depth_stencil_texture_desc;
+  depth_stencil_texture_desc.type = TextureType::kTexture2D;
+  depth_stencil_texture_desc.format = color_format;
+  depth_stencil_texture_desc.size = fbo_size;
+  depth_stencil_texture_desc.usage =
       static_cast<TextureUsageMask>(TextureUsage::kRenderTarget);
-  stencil0_tex.sample_count = SampleCount::kCount1;
+  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);
+
+  DepthAttachment depth0;
+  depth0.clear_depth = 0;
+  depth0.texture = depth_stencil_tex;
+  depth0.load_action = LoadAction::kClear;
+  depth0.store_action = StoreAction::kDontCare;
 
   StencilAttachment stencil0;
   stencil0.clear_stencil = 0;
-  stencil0.texture = std::make_shared<TextureGLES>(
-      gl_context.GetReactor(), stencil0_tex, TextureGLES::IsWrapped::kWrapped);
+  stencil0.texture = depth_stencil_tex;
   stencil0.load_action = LoadAction::kClear;
   stencil0.store_action = StoreAction::kDontCare;
 
   RenderTarget render_target_desc;
 
   render_target_desc.SetColorAttachment(color0, 0u);
+  render_target_desc.SetDepthAttachment(depth0);
   render_target_desc.SetStencilAttachment(stencil0);
 
 #ifdef IMPELLER_DEBUG
diff --git a/impeller/renderer/backend/gles/texture_gles.cc b/impeller/renderer/backend/gles/texture_gles.cc
@@ -7,6 +7,7 @@
 #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"
@@ -17,13 +18,37 @@
 
 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 && desc.format == PixelFormat::kS8UInt) {
+  if (usage == render_target && IsDepthStencilFormat(desc.format)) {
     return is_msaa ? TextureGLES::Type::kRenderBufferMultisampled
                    : TextureGLES::Type::kRenderBuffer;
   }
@@ -457,19 +482,20 @@ TextureGLES::Type TextureGLES::GetType() const {
   return type_;
 }
 
-static GLenum ToAttachmentPoint(TextureGLES::AttachmentPoint point) {
+static GLenum ToAttachmentType(TextureGLES::AttachmentType point) {
   switch (point) {
-    case TextureGLES::AttachmentPoint::kColor0:
+    case TextureGLES::AttachmentType::kColor0:
       return GL_COLOR_ATTACHMENT0;
-    case TextureGLES::AttachmentPoint::kDepth:
+    case TextureGLES::AttachmentType::kDepth:
       return GL_DEPTH_ATTACHMENT;
-    case TextureGLES::AttachmentPoint::kStencil:
+    case TextureGLES::AttachmentType::kStencil:
       return GL_STENCIL_ATTACHMENT;
   }
 }
 
-bool TextureGLES::SetAsFramebufferAttachment(GLenum target,
-                                             AttachmentPoint point) const {
+bool TextureGLES::SetAsFramebufferAttachment(
+    GLenum target,
+    AttachmentType attachment_type) const {
   if (!IsValid()) {
     return false;
   }
@@ -482,29 +508,30 @@ bool TextureGLES::SetAsFramebufferAttachment(GLenum target,
 
   switch (type_) {
     case Type::kTexture:
-      gl.FramebufferTexture2D(target,                    // target
-                              ToAttachmentPoint(point),  // attachment
-                              GL_TEXTURE_2D,             // textarget
-                              handle.value(),            // texture
-                              0                          // level
+      gl.FramebufferTexture2D(target,                             // target
+                              ToAttachmentType(attachment_type),  // attachment
+                              GL_TEXTURE_2D,                      // textarget
+                              handle.value(),                     // texture
+                              0                                   // level
       );
       break;
     case Type::kTextureMultisampled:
       gl.FramebufferTexture2DMultisampleEXT(
-          target,                    // target
-          ToAttachmentPoint(point),  // attachment
-          GL_TEXTURE_2D,             // textarget
-          handle.value(),            // texture
-          0,                         // level
-          4                          // samples
+          target,                             // target
+          ToAttachmentType(attachment_type),  // attachment
+          GL_TEXTURE_2D,                      // textarget
+          handle.value(),                     // texture
+          0,                                  // level
+          4                                   // samples
       );
       break;
     case Type::kRenderBuffer:
     case Type::kRenderBufferMultisampled:
-      gl.FramebufferRenderbuffer(target,                    // target
-                                 ToAttachmentPoint(point),  // attachment
-                                 GL_RENDERBUFFER,  // render-buffer target
-                                 handle.value()    // render-buffer
+      gl.FramebufferRenderbuffer(
+          target,                             // target
+          ToAttachmentType(attachment_type),  // 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,13 +41,14 @@ class TextureGLES final : public Texture,
 
   [[nodiscard]] bool GenerateMipmap();
 
-  enum class AttachmentPoint {
+  enum class AttachmentType {
     kColor0,
     kDepth,
     kStencil,
   };
-  [[nodiscard]] bool SetAsFramebufferAttachment(GLenum target,
-                                                AttachmentPoint point) const;
+  [[nodiscard]] bool SetAsFramebufferAttachment(
+      GLenum target,
+      AttachmentType attachment_type) const;
 
   Type GetType() const;
 
diff --git a/impeller/renderer/backend/vulkan/render_pass_vk.cc b/impeller/renderer/backend/vulkan/render_pass_vk.cc
@@ -196,7 +196,8 @@ SharedHandleVK<vk::RenderPass> RenderPassVK::CreateVKRenderPass(
     }
   }
 
-  if (auto depth = render_target_.GetDepthAttachment(); depth.has_value()) {
+  auto depth = render_target_.GetDepthAttachment();
+  if (depth.has_value()) {
     depth_stencil_ref = vk::AttachmentReference{
         static_cast<uint32_t>(attachments.size()),
         vk::ImageLayout::eDepthStencilAttachmentOptimal};
@@ -213,8 +214,14 @@ SharedHandleVK<vk::RenderPass> RenderPassVK::CreateVKRenderPass(
         vk::ImageLayout::eDepthStencilAttachmentOptimal};
     attachments.emplace_back(CreateAttachmentDescription(
         stencil.value(), &Attachment::texture, supports_framebuffer_fetch));
-    SetTextureLayout(stencil.value(), attachments.back(), command_buffer,
-                     &Attachment::texture);
+
+    // If the depth and stencil are stored in the same texture, then we've
+    // already inserted a memory barrier to transition this texture as part of
+    // the depth branch above.
+    if (depth.has_value() && depth->texture != stencil->texture) {
+      SetTextureLayout(stencil.value(), attachments.back(), command_buffer,
+                       &Attachment::texture);
+    }
   }
 
   vk::SubpassDescription subpass_desc;
diff --git a/impeller/renderer/compute_subgroup_unittests.cc b/impeller/renderer/compute_subgroup_unittests.cc
@@ -140,7 +140,7 @@ TEST_P(ComputeSubgroupTest, PathPlayground) {
     options.sample_count = pass.GetRenderTarget().GetSampleCount();
     options.color_attachment_pixel_format =
         pass.GetRenderTarget().GetRenderTargetPixelFormat();
-    options.has_stencil_attachment =
+    options.has_depth_stencil_attachments =
         pass.GetRenderTarget().GetStencilAttachment().has_value();
     options.blend_mode = BlendMode::kSourceIn;
     options.primitive_type = PrimitiveType::kTriangleStrip;
@@ -337,7 +337,7 @@ TEST_P(ComputeSubgroupTest, LargePath) {
     options.sample_count = pass.GetRenderTarget().GetSampleCount();
     options.color_attachment_pixel_format =
         pass.GetRenderTarget().GetRenderTargetPixelFormat();
-    options.has_stencil_attachment =
+    options.has_depth_stencil_attachments =
         pass.GetRenderTarget().GetStencilAttachment().has_value();
     options.blend_mode = BlendMode::kSourceIn;
     options.primitive_type = PrimitiveType::kTriangleStrip;
@@ -415,7 +415,7 @@ TEST_P(ComputeSubgroupTest, QuadAndCubicInOnePath) {
     options.sample_count = pass.GetRenderTarget().GetSampleCount();
     options.color_attachment_pixel_format =
         pass.GetRenderTarget().GetRenderTargetPixelFormat();
-    options.has_stencil_attachment =
+    options.has_depth_stencil_attachments =
         pass.GetRenderTarget().GetStencilAttachment().has_value();
     options.blend_mode = BlendMode::kSourceIn;
     options.primitive_type = PrimitiveType::kTriangleStrip;
diff --git a/impeller/renderer/pipeline_builder.h b/impeller/renderer/pipeline_builder.h
@@ -107,13 +107,22 @@ struct PipelineBuilder {
       desc.SetColorAttachmentDescriptor(0u, color0);
     }
 
+    // Setup default depth buffer descriptions.
+    {
+      DepthAttachmentDescriptor depth0;
+      depth0.depth_compare = CompareFunction::kAlways;
+      desc.SetDepthStencilAttachmentDescriptor(depth0);
+      desc.SetDepthPixelFormat(
+          context.GetCapabilities()->GetDefaultDepthStencilFormat());
+    }
+
     // Setup default stencil buffer descriptions.
     {
       StencilAttachmentDescriptor stencil0;
       stencil0.stencil_compare = CompareFunction::kEqual;
       desc.SetStencilAttachmentDescriptors(stencil0);
       desc.SetStencilPixelFormat(
-          context.GetCapabilities()->GetDefaultStencilFormat());
+          context.GetCapabilities()->GetDefaultDepthStencilFormat());
     }
 
     return true;
diff --git a/impeller/renderer/render_target.cc b/impeller/renderer/render_target.cc
diff --git a/impeller/renderer/render_target.h b/impeller/renderer/render_target.h
diff --git a/impeller/renderer/renderer_unittests.cc b/impeller/renderer/renderer_unittests.cc

Original file line number	Diff line number	Diff line change
`@@ -136,7 +136,8 @@ void ContentContextOptions::ApplyToPipelineDescriptor(`
`136`	`136`	`}`
`137`	`137`	`desc.SetColorAttachmentDescriptor(0u, color0);`
`138`	`138`
`139`		`- if (!has_stencil_attachment) {`
	`139`	`+ if (!has_depth_stencil_attachments) {`
	`140`	`+ desc.ClearDepthAttachment();`
`140`	`141`	`desc.ClearStencilAttachments();`
`141`	`142`	`}`
`142`	`143`
Original file line number	Diff line number	Diff line change
`@@ -21,7 +21,7 @@ ContentContextOptions OptionsFromPass(const RenderPass& pass) {`
`21`	`21`	`ContentContextOptions opts;`
`22`	`22`	`opts.sample_count = pass.GetSampleCount();`
`23`	`23`	`opts.color_attachment_pixel_format = pass.GetRenderTargetPixelFormat();`
`24`		`- opts.has_stencil_attachment = pass.HasStencilAttachment();`
	`24`	`+ opts.has_depth_stencil_attachments = pass.HasStencilAttachment();`
`25`	`25`	`return opts;`
`26`	`26`	`}`
`27`	`27`
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::AttachmentPoint::kColor0)) {`
	`189`	`+ GL_FRAMEBUFFER, TextureGLES::AttachmentType::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::AttachmentPoint::kDepth)) {`
	`196`	`+ GL_FRAMEBUFFER, TextureGLES::AttachmentType::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::AttachmentPoint::kStencil)) {`
	`202`	`+ GL_FRAMEBUFFER, TextureGLES::AttachmentType::kStencil)) {`
`203`	`203`	`return false;`
`204`	`204`	`}`
`205`	`205`	`}`