[Impeller] Device default attachment pixel formats (flutter/engine#39655)

* [Impeller] Device default attachment pixel formats fixes: https://github.com/flutter/flutter/issues/120498 * remove commented out defaults * fix CR comments
2023-02-17 14:04:50 -05:00
parent 25a795ffe7
commit 98aaa25559
15 changed files with 107 additions and 40 deletions
--- a/engine/src/flutter/impeller/entity/contents/content_context.cc
+++ b/engine/src/flutter/impeller/entity/contents/content_context.cc
@@ -28,7 +28,13 @@ void ContentContextOptions::ApplyToPipelineDescriptor(
  desc.SetSampleCount(sample_count);

  ColorAttachmentDescriptor color0 = *desc.GetColorAttachmentDescriptor(0u);
-  color0.format = color_attachment_pixel_format;
+  if (!color_attachment_pixel_format.has_value()) {
+    VALIDATION_LOG << "Color attachment pixel format must be set.";
+    color0.format = PixelFormat::kB8G8R8A8UNormInt;
+  } else {
+    color0.format = *color_attachment_pixel_format;
+  }
+  color0.format = *color_attachment_pixel_format;
  color0.alpha_blend_op = BlendOperation::kAdd;
  color0.color_blend_op = BlendOperation::kAdd;

@@ -145,7 +151,9 @@ static std::unique_ptr<PipelineT> CreateDefaultPipeline(
    return nullptr;
  }
  // Apply default ContentContextOptions to the descriptor.
-  ContentContextOptions{}.ApplyToPipelineDescriptor(*desc);
+  const auto default_color_fmt = context.GetColorAttachmentPixelFormat();
+  ContentContextOptions{.color_attachment_pixel_format = default_color_fmt}
+      .ApplyToPipelineDescriptor(*desc);
  return std::make_unique<PipelineT>(context, desc);
 }

--- a/engine/src/flutter/impeller/entity/contents/content_context.h
+++ b/engine/src/flutter/impeller/entity/contents/content_context.h
@@ -5,6 +5,7 @@
 #pragma once

 #include <memory>
+#include <optional>
 #include <unordered_map>

 #include "flutter/fml/hash_combine.h"
@@ -265,7 +266,7 @@ struct ContentContextOptions {
  CompareFunction stencil_compare = CompareFunction::kEqual;
  StencilOperation stencil_operation = StencilOperation::kKeep;
  PrimitiveType primitive_type = PrimitiveType::kTriangle;
-  PixelFormat color_attachment_pixel_format = PixelFormat::kDefaultColor;
+  std::optional<PixelFormat> color_attachment_pixel_format;
  bool has_stencil_attachment = true;

  struct Hash {
--- a/engine/src/flutter/impeller/entity/contents/runtime_effect_contents.cc
+++ b/engine/src/flutter/impeller/entity/contents/runtime_effect_contents.cc
@@ -102,6 +102,11 @@ bool RuntimeEffectContents::Render(const ContentContext& renderer,
  /// Get or create runtime stage pipeline.
  ///

+  const auto& device_capabilities = context->GetDeviceCapabilities();
+  const auto color_attachment_format = context->GetColorAttachmentPixelFormat();
+  const auto stencil_attachment_format =
+      device_capabilities.GetDefaultStencilFormat();
+
  using VS = RuntimeEffectVertexShader;
  PipelineDescriptor desc;
  desc.SetLabel("Runtime Stage");
@@ -115,9 +120,9 @@ bool RuntimeEffectContents::Render(const ContentContext& renderer,
  }
  desc.SetVertexDescriptor(std::move(vertex_descriptor));
  desc.SetColorAttachmentDescriptor(
-      0u, {.format = PixelFormat::kDefaultColor, .blending_enabled = true});
+      0u, {.format = color_attachment_format, .blending_enabled = true});
  desc.SetStencilAttachmentDescriptors({});
-  desc.SetStencilPixelFormat(PixelFormat::kDefaultStencil);
+  desc.SetStencilPixelFormat(stencil_attachment_format);

  auto options = OptionsFromPassAndEntity(pass, entity);
  if (geometry_result.prevent_overdraw) {
--- a/engine/src/flutter/impeller/playground/backend/metal/playground_impl_mtl.mm
+++ b/engine/src/flutter/impeller/playground/backend/metal/playground_impl_mtl.mm
@@ -75,7 +75,8 @@ PlaygroundImplMTL::PlaygroundImplMTL()
  data_->metal_layer = [CAMetalLayer layer];
  data_->metal_layer.device = ContextMTL::Cast(*context).GetMTLDevice();
  // This pixel format is one of the documented supported formats.
-  data_->metal_layer.pixelFormat = ToMTLPixelFormat(PixelFormat::kDefaultColor);
+  const auto color_fmt = context->GetColorAttachmentPixelFormat();
+  data_->metal_layer.pixelFormat = ToMTLPixelFormat(color_fmt);
  data_->metal_layer.framebufferOnly = NO;
  cocoa_window.contentView.layer = data_->metal_layer;
  cocoa_window.contentView.wantsLayer = YES;
--- a/engine/src/flutter/impeller/renderer/backend/gles/context_gles.cc
+++ b/engine/src/flutter/impeller/renderer/backend/gles/context_gles.cc
@@ -71,11 +71,14 @@ ContextGLES::ContextGLES(std::unique_ptr<ProcTableGLES> gl,

  // Create the device capabilities.
  {
-    device_capabilities_ = DeviceCapabilitiesBuilder()
-                               .SetHasThreadingRestrictions(true)
-                               .SetSupportsOffscreenMSAA(false)
-                               .SetSupportsSSBO(false)
-                               .Build();
+    device_capabilities_ =
+        DeviceCapabilitiesBuilder()
+            .SetHasThreadingRestrictions(true)
+            .SetSupportsOffscreenMSAA(false)
+            .SetSupportsSSBO(false)
+            .SetDefaultColorFormat(PixelFormat::kB8G8R8A8UNormInt)
+            .SetDefaultStencilFormat(PixelFormat::kS8UInt)
+            .Build();
  }

  is_valid_ = true;
--- a/engine/src/flutter/impeller/renderer/backend/metal/context_mtl.mm
+++ b/engine/src/flutter/impeller/renderer/backend/metal/context_mtl.mm
@@ -90,11 +90,14 @@ ContextMTL::ContextMTL(id<MTLDevice> device,
 #endif

  {
-    device_capabilities_ = DeviceCapabilitiesBuilder()
-                               .SetHasThreadingRestrictions(false)
-                               .SetSupportsOffscreenMSAA(true)
-                               .SetSupportsSSBO(true)
-                               .Build();
+    device_capabilities_ =
+        DeviceCapabilitiesBuilder()
+            .SetHasThreadingRestrictions(false)
+            .SetSupportsOffscreenMSAA(true)
+            .SetSupportsSSBO(true)
+            .SetDefaultColorFormat(PixelFormat::kB8G8R8A8UNormInt)
+            .SetDefaultStencilFormat(PixelFormat::kS8UInt)
+            .Build();
  }

  is_valid_ = true;
--- a/engine/src/flutter/impeller/renderer/backend/metal/surface_mtl.mm
+++ b/engine/src/flutter/impeller/renderer/backend/metal/surface_mtl.mm
@@ -81,7 +81,8 @@ std::unique_ptr<SurfaceMTL> SurfaceMTL::WrapCurrentMetalLayerDrawable(
  stencil0_tex.storage_mode = StorageMode::kDeviceTransient;
  stencil0_tex.type = TextureType::kTexture2DMultisample;
  stencil0_tex.sample_count = SampleCount::kCount4;
-  stencil0_tex.format = PixelFormat::kDefaultStencil;
+  stencil0_tex.format =
+      context->GetDeviceCapabilities().GetDefaultStencilFormat();
  stencil0_tex.size = color0_tex_desc.size;
  stencil0_tex.usage =
      static_cast<TextureUsageMask>(TextureUsage::kRenderTarget);
--- a/engine/src/flutter/impeller/renderer/backend/vulkan/context_vk.cc
+++ b/engine/src/flutter/impeller/renderer/backend/vulkan/context_vk.cc
@@ -499,11 +499,14 @@ ContextVK::ContextVK(
  transfer_queue_ =
      device_->getQueue(transfer_queue->family, transfer_queue->index);

-  device_capabilities_ = DeviceCapabilitiesBuilder()
-                             .SetHasThreadingRestrictions(false)
-                             .SetSupportsOffscreenMSAA(true)
-                             .SetSupportsSSBO(false)
-                             .Build();
+  device_capabilities_ =
+      DeviceCapabilitiesBuilder()
+          .SetHasThreadingRestrictions(false)
+          .SetSupportsOffscreenMSAA(true)
+          .SetSupportsSSBO(false)
+          .SetDefaultColorFormat(PixelFormat::kB8G8R8A8UNormInt)
+          .SetDefaultStencilFormat(PixelFormat::kS8UInt)
+          .Build();

  is_valid_ = true;
 }
--- a/engine/src/flutter/impeller/renderer/context.cc
+++ b/engine/src/flutter/impeller/renderer/context.cc
@@ -15,7 +15,7 @@ std::shared_ptr<GPUTracer> Context::GetGPUTracer() const {
 }

 PixelFormat Context::GetColorAttachmentPixelFormat() const {
-  return PixelFormat::kDefaultColor;
+  return GetDeviceCapabilities().GetDefaultColorFormat();
 }

 }  // namespace impeller
--- a/engine/src/flutter/impeller/renderer/device_capabilities.cc
+++ b/engine/src/flutter/impeller/renderer/device_capabilities.cc
@@ -8,10 +8,14 @@ namespace impeller {

 IDeviceCapabilities::IDeviceCapabilities(bool threading_restrictions,
                                         bool offscreen_msaa,
-                                         bool supports_ssbo)
+                                         bool supports_ssbo,
+                                         PixelFormat default_color_format,
+                                         PixelFormat default_stencil_format)
    : threading_restrictions_(threading_restrictions),
      offscreen_msaa_(offscreen_msaa),
-      supports_ssbo_(supports_ssbo) {}
+      supports_ssbo_(supports_ssbo),
+      default_color_format_(default_color_format),
+      default_stencil_format_(default_stencil_format) {}

 IDeviceCapabilities::~IDeviceCapabilities() = default;

@@ -27,6 +31,14 @@ bool IDeviceCapabilities::SupportsSSBO() const {
  return supports_ssbo_;
 }

+PixelFormat IDeviceCapabilities::GetDefaultColorFormat() const {
+  return default_color_format_;
+}
+
+PixelFormat IDeviceCapabilities::GetDefaultStencilFormat() const {
+  return default_stencil_format_;
+}
+
 DeviceCapabilitiesBuilder::DeviceCapabilitiesBuilder() = default;

 DeviceCapabilitiesBuilder::~DeviceCapabilitiesBuilder() = default;
@@ -49,9 +61,27 @@ DeviceCapabilitiesBuilder& DeviceCapabilitiesBuilder::SetSupportsSSBO(
  return *this;
 }

+DeviceCapabilitiesBuilder& DeviceCapabilitiesBuilder::SetDefaultColorFormat(
+    PixelFormat value) {
+  default_color_format_ = value;
+  return *this;
+}
+
+DeviceCapabilitiesBuilder& DeviceCapabilitiesBuilder::SetDefaultStencilFormat(
+    PixelFormat value) {
+  default_stencil_format_ = value;
+  return *this;
+}
+
 std::unique_ptr<IDeviceCapabilities> DeviceCapabilitiesBuilder::Build() {
+  FML_CHECK(default_color_format_.has_value())
+      << "Default color format not set";
+  FML_CHECK(default_stencil_format_.has_value())
+      << "Default stencil format not set";
+
  IDeviceCapabilities* capabilities = new IDeviceCapabilities(
-      threading_restrictions_, offscreen_msaa_, supports_ssbo_);
+      threading_restrictions_, offscreen_msaa_, supports_ssbo_,
+      *default_color_format_, *default_stencil_format_);
  return std::unique_ptr<IDeviceCapabilities>(capabilities);
 }

--- a/engine/src/flutter/impeller/renderer/device_capabilities.h
+++ b/engine/src/flutter/impeller/renderer/device_capabilities.h
@@ -7,6 +7,7 @@
 #include <memory>

 #include "flutter/fml/macros.h"
+#include "impeller/renderer/formats.h"

 namespace impeller {

@@ -20,16 +21,24 @@ class IDeviceCapabilities {

  bool SupportsSSBO() const;

+  PixelFormat GetDefaultColorFormat() const;
+
+  PixelFormat GetDefaultStencilFormat() const;
+
 private:
  IDeviceCapabilities(bool threading_restrictions,
                      bool offscreen_msaa,
-                      bool supports_ssbo);
+                      bool supports_ssbo,
+                      PixelFormat default_color_format,
+                      PixelFormat default_stencil_format);

  friend class DeviceCapabilitiesBuilder;

  bool threading_restrictions_ = false;
  bool offscreen_msaa_ = false;
  bool supports_ssbo_ = false;
+  PixelFormat default_color_format_;
+  PixelFormat default_stencil_format_;

  FML_DISALLOW_COPY_AND_ASSIGN(IDeviceCapabilities);
 };
@@ -46,12 +55,18 @@ class DeviceCapabilitiesBuilder {

  DeviceCapabilitiesBuilder& SetSupportsSSBO(bool value);

+  DeviceCapabilitiesBuilder& SetDefaultColorFormat(PixelFormat value);
+
+  DeviceCapabilitiesBuilder& SetDefaultStencilFormat(PixelFormat value);
+
  std::unique_ptr<IDeviceCapabilities> Build();

 private:
  bool threading_restrictions_ = false;
  bool offscreen_msaa_ = false;
  bool supports_ssbo_ = false;
+  std::optional<PixelFormat> default_color_format_ = std::nullopt;
+  std::optional<PixelFormat> default_stencil_format_ = std::nullopt;

  FML_DISALLOW_COPY_AND_ASSIGN(DeviceCapabilitiesBuilder);
 };
--- a/engine/src/flutter/impeller/renderer/formats.h
+++ b/engine/src/flutter/impeller/renderer/formats.h
@@ -95,14 +95,6 @@ enum class PixelFormat {
  // Depth and stencil formats.
  kS8UInt,
  kD32FloatS8UInt,
-
-  // Defaults. If you don't know which ones to use, these are usually a safe
-  // bet.
-  //
-  // On Metal, this is a support format for layer drawable and can be used to
-  // specify the format of the resolve texture if needed.
-  kDefaultColor = kB8G8R8A8UNormInt,
-  kDefaultStencil = kS8UInt,
 };

 enum class BlendFactor {
--- a/engine/src/flutter/impeller/renderer/pipeline_builder.h
+++ b/engine/src/flutter/impeller/renderer/pipeline_builder.h
@@ -127,7 +127,8 @@ struct PipelineBuilder {
      StencilAttachmentDescriptor stencil0;
      stencil0.stencil_compare = CompareFunction::kEqual;
      desc.SetStencilAttachmentDescriptors(stencil0);
-      desc.SetStencilPixelFormat(PixelFormat::kDefaultStencil);
+      desc.SetStencilPixelFormat(
+          context.GetDeviceCapabilities().GetDefaultStencilFormat());
    }

    return true;
--- a/engine/src/flutter/impeller/renderer/render_target.cc
+++ b/engine/src/flutter/impeller/renderer/render_target.cc
@@ -225,7 +225,8 @@ RenderTarget RenderTarget::CreateOffscreen(
  if (stencil_attachment_config.has_value()) {
    TextureDescriptor stencil_tex0;
    stencil_tex0.storage_mode = stencil_attachment_config->storage_mode;
-    stencil_tex0.format = PixelFormat::kDefaultStencil;
+    stencil_tex0.format =
+        context.GetDeviceCapabilities().GetDefaultStencilFormat();
    stencil_tex0.size = size;
    stencil_tex0.usage =
        static_cast<TextureUsageMask>(TextureUsage::kRenderTarget);
@@ -318,7 +319,8 @@ RenderTarget RenderTarget::CreateOffscreenMSAA(
    stencil_tex0.storage_mode = stencil_attachment_config->storage_mode;
    stencil_tex0.type = TextureType::kTexture2DMultisample;
    stencil_tex0.sample_count = SampleCount::kCount4;
-    stencil_tex0.format = PixelFormat::kDefaultStencil;
+    stencil_tex0.format =
+        context.GetDeviceCapabilities().GetDefaultStencilFormat();
    stencil_tex0.size = size;
    stencil_tex0.usage =
        static_cast<TextureUsageMask>(TextureUsage::kRenderTarget);
--- a/engine/src/flutter/impeller/runtime_stage/runtime_stage_unittests.cc
+++ b/engine/src/flutter/impeller/runtime_stage/runtime_stage_unittests.cc
@@ -242,12 +242,14 @@ TEST_P(RuntimeStageTest, CanCreatePipelineFromRuntimeStage) {
  ASSERT_TRUE(vertex_descriptor->SetStageInputs(VS::kAllShaderStageInputs));
  desc.SetVertexDescriptor(std::move(vertex_descriptor));
  ColorAttachmentDescriptor color0;
-  color0.format = PixelFormat::kDefaultColor;
+  color0.format = GetContext()->GetColorAttachmentPixelFormat();
  StencilAttachmentDescriptor stencil0;
  stencil0.stencil_compare = CompareFunction::kEqual;
  desc.SetColorAttachmentDescriptor(0u, color0);
  desc.SetStencilAttachmentDescriptors(stencil0);
-  desc.SetStencilPixelFormat(PixelFormat::kDefaultStencil);
+  const auto stencil_fmt =
+      GetContext()->GetDeviceCapabilities().GetDefaultStencilFormat();
+  desc.SetStencilPixelFormat(stencil_fmt);
  auto pipeline = GetContext()->GetPipelineLibrary()->GetPipeline(desc).Get();
  ASSERT_NE(pipeline, nullptr);
 }