[Impeller] delete points compute shader. (#52346)

We only use this for drawPoints, a rarely used API. On local tests, this is just as fast with the CPU backend implementations. While this was intended to be the first in a series of compute based rendering experiments, it hasn't really been worth the carrying cost. So lets shrink the complexity and and remove another shader to boot. Fixes flutter/flutter#147184
flutter · Apr 24, 2024 · 88fc864 · 88fc864
1 parent c330ca8
commit 88fc864
Show file tree

Hide file tree

Showing 11 changed files with 12 additions and 317 deletions.
diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter
@@ -40396,7 +40396,6 @@ ORIGIN: ../../../flutter/impeller/entity/shaders/gaussian_blur/kernel.glsl + ../
 ORIGIN: ../../../flutter/impeller/entity/shaders/gaussian_blur/kernel.vert + ../../../flutter/LICENSE
 ORIGIN: ../../../flutter/impeller/entity/shaders/gaussian_blur/kernel_decal.frag + ../../../flutter/LICENSE
 ORIGIN: ../../../flutter/impeller/entity/shaders/gaussian_blur/kernel_nodecal.frag + ../../../flutter/LICENSE
-ORIGIN: ../../../flutter/impeller/entity/shaders/geometry/points.comp + ../../../flutter/LICENSE
 ORIGIN: ../../../flutter/impeller/entity/shaders/glyph_atlas.frag + ../../../flutter/LICENSE
 ORIGIN: ../../../flutter/impeller/entity/shaders/glyph_atlas.vert + ../../../flutter/LICENSE
 ORIGIN: ../../../flutter/impeller/entity/shaders/glyph_atlas_color.frag + ../../../flutter/LICENSE
@@ -43278,7 +43277,6 @@ FILE: ../../../flutter/impeller/entity/shaders/gaussian_blur/kernel.glsl
 FILE: ../../../flutter/impeller/entity/shaders/gaussian_blur/kernel.vert
 FILE: ../../../flutter/impeller/entity/shaders/gaussian_blur/kernel_decal.frag
 FILE: ../../../flutter/impeller/entity/shaders/gaussian_blur/kernel_nodecal.frag
-FILE: ../../../flutter/impeller/entity/shaders/geometry/points.comp
 FILE: ../../../flutter/impeller/entity/shaders/glyph_atlas.frag
 FILE: ../../../flutter/impeller/entity/shaders/glyph_atlas.vert
 FILE: ../../../flutter/impeller/entity/shaders/glyph_atlas_color.frag

diff --git a/impeller/aiks/aiks_unittests.cc b/impeller/aiks/aiks_unittests.cc
@@ -2349,10 +2349,6 @@ TEST_P(AiksTest, DrawPaintTransformsBounds) {
 }
 
 TEST_P(AiksTest, CanDrawPoints) {
-  if (GetBackend() == PlaygroundBackend::kMetal) {
-    // https://github.com/flutter/flutter/issues/147184
-    GTEST_SKIP() << "Draw Points is currently broken on the metal m1 backend.";
-  }
   std::vector<Point> points = {
       {0, 0},      //
       {100, 100},  //
@@ -2447,10 +2443,6 @@ TEST_P(AiksTest, DrawAtlasAdvancedAndTransform) {
 }
 
 TEST_P(AiksTest, CanDrawPointsWithTextureMap) {
-  if (GetBackend() == PlaygroundBackend::kMetal) {
-    // https://github.com/flutter/flutter/issues/147184
-    GTEST_SKIP() << "Draw Points is currently broken on the metal m1 backend.";
-  }
   auto texture = CreateTextureForFixture("table_mountain_nx.png",
                                          /*enable_mipmapping=*/true);
 

diff --git a/impeller/entity/BUILD.gn b/impeller/entity/BUILD.gn
@@ -81,7 +81,6 @@ impeller_shaders("modern_entity_shaders") {
     "shaders/gradients/linear_gradient_ssbo_fill.frag",
     "shaders/gradients/radial_gradient_ssbo_fill.frag",
     "shaders/gradients/sweep_gradient_ssbo_fill.frag",
-    "shaders/geometry/points.comp",
   ]
 }
 

diff --git a/impeller/entity/contents/content_context.cc b/impeller/entity/contents/content_context.cc
@@ -450,12 +450,6 @@ ContentContext::ContentContext(
     tiled_texture_external_pipelines_.CreateDefault(*context_, options);
   }
 #endif  // IMPELLER_ENABLE_OPENGLES
-  if (context_->GetCapabilities()->SupportsCompute()) {
-    auto pipeline_desc =
-        PointsComputeShaderPipeline::MakeDefaultPipelineDescriptor(*context_);
-    point_field_compute_pipelines_ =
-        context_->GetPipelineLibrary()->GetPipeline(pipeline_desc).Get();
-  }
 
   is_valid_ = true;
   InitializeCommonlyUsedShadersIfNeeded();

diff --git a/impeller/entity/contents/content_context.h b/impeller/entity/contents/content_context.h
@@ -44,7 +44,6 @@
 #include "impeller/entity/linear_to_srgb_filter.frag.h"
 #include "impeller/entity/morphology_filter.frag.h"
 #include "impeller/entity/morphology_filter.vert.h"
-#include "impeller/entity/points.comp.h"
 #include "impeller/entity/porter_duff_blend.frag.h"
 #include "impeller/entity/porter_duff_blend.vert.h"
 #include "impeller/entity/radial_gradient_fill.frag.h"
@@ -256,9 +255,6 @@ using FramebufferBlendSoftLightPipeline =
 using VerticesUberShader =
     RenderPipelineHandle<VerticesUberVertexShader, VerticesUberFragmentShader>;
 
-/// Geometry Pipelines
-using PointsComputeShaderPipeline = ComputePipelineBuilder<PointsComputeShader>;
-
 #ifdef IMPELLER_ENABLE_OPENGLES
 using TiledTextureExternalPipeline =
     RenderPipelineHandle<TextureUvFillVertexShader,
@@ -719,12 +715,6 @@ class ContentContext {
     return GetPipeline(vertices_uber_shader_, opts);
   }
 
-  std::shared_ptr<Pipeline<ComputePipelineDescriptor>> GetPointComputePipeline()
-      const {
-    FML_DCHECK(GetDeviceCapabilities().SupportsCompute());
-    return point_field_compute_pipelines_;
-  }
-
   std::shared_ptr<Context> GetContext() const;
 
   const Capabilities& GetDeviceCapabilities() const;
@@ -987,8 +977,6 @@ class ContentContext {
   mutable Variants<FramebufferBlendSoftLightPipeline>
       framebuffer_blend_softlight_pipelines_;
   mutable Variants<VerticesUberShader> vertices_uber_shader_;
-  mutable std::shared_ptr<Pipeline<ComputePipelineDescriptor>>
-      point_field_compute_pipelines_;
 
   template <class TypedPipeline>
   std::shared_ptr<Pipeline<PipelineDescriptor>> GetPipeline(

diff --git a/impeller/entity/entity_unittests.cc b/impeller/entity/entity_unittests.cc
@@ -2519,24 +2519,6 @@ TEST_P(EntityTest, TiledTextureContentsIsOpaque) {
   ASSERT_FALSE(contents.IsOpaque());
 }
 
-TEST_P(EntityTest, PointFieldGeometryDivisions) {
-  // Square always gives 4 divisions.
-  ASSERT_EQ(PointFieldGeometry::ComputeCircleDivisions(24.0, false), 4u);
-  ASSERT_EQ(PointFieldGeometry::ComputeCircleDivisions(2.0, false), 4u);
-  ASSERT_EQ(PointFieldGeometry::ComputeCircleDivisions(200.0, false), 4u);
-
-  ASSERT_EQ(PointFieldGeometry::ComputeCircleDivisions(0.5, true), 4u);
-  ASSERT_EQ(PointFieldGeometry::ComputeCircleDivisions(1.5, true), 8u);
-  ASSERT_EQ(PointFieldGeometry::ComputeCircleDivisions(5.5, true), 24u);
-  ASSERT_EQ(PointFieldGeometry::ComputeCircleDivisions(12.5, true), 34u);
-  ASSERT_EQ(PointFieldGeometry::ComputeCircleDivisions(22.3, true), 22u);
-  ASSERT_EQ(PointFieldGeometry::ComputeCircleDivisions(40.5, true), 40u);
-  ASSERT_EQ(PointFieldGeometry::ComputeCircleDivisions(100.0, true), 100u);
-  // Caps at 140.
-  ASSERT_EQ(PointFieldGeometry::ComputeCircleDivisions(1000.0, true), 140u);
-  ASSERT_EQ(PointFieldGeometry::ComputeCircleDivisions(20000.0, true), 140u);
-}
-
 TEST_P(EntityTest, PointFieldGeometryCoverage) {
   std::vector<Point> points = {{10, 20}, {100, 200}};
   auto geometry = Geometry::MakePointField(points, 5.0, false);

diff --git a/impeller/entity/geometry/point_field_geometry.cc b/impeller/entity/geometry/point_field_geometry.cc
@@ -4,7 +4,8 @@
 
 #include "impeller/entity/geometry/point_field_geometry.h"
 
-#include "impeller/geometry/color.h"
+#include "impeller/core/vertex_buffer.h"
+#include "impeller/entity/geometry/geometry.h"
 #include "impeller/renderer/command_buffer.h"
 
 namespace impeller {
@@ -18,38 +19,19 @@ GeometryResult PointFieldGeometry::GetPositionBuffer(
     const ContentContext& renderer,
     const Entity& entity,
     RenderPass& pass) const {
-  if (renderer.GetDeviceCapabilities().SupportsCompute()) {
-    return GetPositionBufferGPU(renderer, entity, pass);
-  }
-  auto vtx_builder = GetPositionBufferCPU(renderer, entity, pass);
-  if (!vtx_builder.has_value()) {
-    return {};
-  }
-
-  auto& host_buffer = renderer.GetTransientsBuffer();
-  return {
-      .type = PrimitiveType::kTriangleStrip,
-      .vertex_buffer = vtx_builder->CreateVertexBuffer(host_buffer),
-      .transform = entity.GetShaderTransform(pass),
-  };
-}
-
-std::optional<VertexBufferBuilder<SolidFillVertexShader::PerVertexData>>
-PointFieldGeometry::GetPositionBufferCPU(const ContentContext& renderer,
-                                         const Entity& entity,
-                                         RenderPass& pass) const {
   if (radius_ < 0.0) {
-    return std::nullopt;
+    return {};
   }
-  auto transform = entity.GetTransform();
-  auto determinant = transform.GetDeterminant();
+  Matrix transform = entity.GetTransform();
+  Scalar determinant = transform.GetDeterminant();
   if (determinant == 0) {
-    return std::nullopt;
+    return {};
   }
 
   Scalar min_size = 1.0f / sqrt(std::abs(determinant));
   Scalar radius = std::max(radius_, min_size);
 
+  HostBuffer& host_buffer = renderer.GetTransientsBuffer();
   VertexBufferBuilder<SolidFillVertexShader::PerVertexData> vtx_builder;
 
   if (round_) {
@@ -94,114 +76,13 @@ PointFieldGeometry::GetPositionBufferCPU(const ContentContext& renderer,
     }
   }
 
-  return vtx_builder;
-}
-
-GeometryResult PointFieldGeometry::GetPositionBufferGPU(
-    const ContentContext& renderer,
-    const Entity& entity,
-    RenderPass& pass) const {
-  FML_DCHECK(renderer.GetDeviceCapabilities().SupportsCompute());
-  if (radius_ < 0.0) {
-    return {};
-  }
-  Scalar determinant = entity.GetTransform().GetDeterminant();
-  if (determinant == 0) {
-    return {};
-  }
-
-  Scalar min_size = 1.0f / sqrt(std::abs(determinant));
-  Scalar radius = std::max(radius_, min_size);
-
-  size_t vertices_per_geom = ComputeCircleDivisions(
-      entity.GetTransform().GetMaxBasisLength() * radius, round_);
-
-  size_t points_per_circle = 3 + (vertices_per_geom - 3) * 3;
-  size_t total = points_per_circle * points_.size();
-
-  std::shared_ptr<CommandBuffer> cmd_buffer =
-      renderer.GetContext()->CreateCommandBuffer();
-  std::shared_ptr<ComputePass> compute_pass = cmd_buffer->CreateComputePass();
-  HostBuffer& host_buffer = renderer.GetTransientsBuffer();
-
-  BufferView points_data =
-      host_buffer.Emplace(points_.data(), points_.size() * sizeof(Point),
-                          DefaultUniformAlignment());
-
-  BufferView geometry_buffer =
-      host_buffer.Emplace(nullptr, total * sizeof(Point),
-                          std::max(DefaultUniformAlignment(), alignof(Point)));
-
-  BufferView output;
-  {
-    using PS = PointsComputeShader;
-
-    compute_pass->SetPipeline(renderer.GetPointComputePipeline());
-    compute_pass->SetCommandLabel("Points Geometry");
-
-    PS::FrameInfo frame_info;
-    frame_info.count = points_.size();
-    frame_info.radius = round_ ? radius : radius * kSqrt2;
-    frame_info.radian_start = round_ ? 0.0f : kPiOver4;
-    frame_info.radian_step = k2Pi / vertices_per_geom;
-    frame_info.points_per_circle = points_per_circle;
-    frame_info.divisions_per_circle = vertices_per_geom;
-
-    PS::BindFrameInfo(*compute_pass, host_buffer.EmplaceUniform(frame_info));
-    PS::BindGeometryData(*compute_pass, geometry_buffer);
-    PS::BindPointData(*compute_pass, points_data);
-
-    if (!compute_pass->Compute(ISize(total, 1)).ok()) {
-      return {};
-    }
-    output = geometry_buffer;
-  }
-
-  if (!compute_pass->EncodeCommands()) {
-    return {};
-  }
-  if (!renderer.GetContext()
-           ->GetCommandQueue()
-           ->Submit({std::move(cmd_buffer)})
-           .ok()) {
-    return {};
-  }
-
-  return {
-      .type = PrimitiveType::kTriangle,
-      .vertex_buffer = {.vertex_buffer = std::move(output),
-                        .vertex_count = total,
-                        .index_type = IndexType::kNone},
+  return GeometryResult{
+      .type = PrimitiveType::kTriangleStrip,
+      .vertex_buffer = vtx_builder.CreateVertexBuffer(host_buffer),
       .transform = entity.GetShaderTransform(pass),
   };
 }
 
-/// @brief Compute the number of vertices to divide each circle into.
-///
-/// @return the number of vertices.
-size_t PointFieldGeometry::ComputeCircleDivisions(Scalar scaled_radius,
-                                                  bool round) {
-  if (!round) {
-    return 4;
-  }
-
-  // Note: these values are approximated based on the values returned from
-  // the decomposition of 4 cubics performed by Path::CreatePolyline.
-  if (scaled_radius < 1.0) {
-    return 4;
-  }
-  if (scaled_radius < 2.0) {
-    return 8;
-  }
-  if (scaled_radius < 12.0) {
-    return 24;
-  }
-  if (scaled_radius < 22.0) {
-    return 34;
-  }
-  return std::min(scaled_radius, 140.0f);
-}
-
 // |Geometry|
 std::optional<Rect> PointFieldGeometry::GetCoverage(
     const Matrix& transform) const {

diff --git a/impeller/entity/geometry/point_field_geometry.h b/impeller/entity/geometry/point_field_geometry.h
@@ -15,8 +15,6 @@ class PointFieldGeometry final : public Geometry {
 
   ~PointFieldGeometry() = default;
 
-  static size_t ComputeCircleDivisions(Scalar scaled_radius, bool round);
-
  private:
   // |Geometry|
   GeometryResult GetPositionBuffer(const ContentContext& renderer,
@@ -26,15 +24,6 @@ class PointFieldGeometry final : public Geometry {
   // |Geometry|
   std::optional<Rect> GetCoverage(const Matrix& transform) const override;
 
-  GeometryResult GetPositionBufferGPU(const ContentContext& renderer,
-                                      const Entity& entity,
-                                      RenderPass& pass) const;
-
-  std::optional<VertexBufferBuilder<SolidFillVertexShader::PerVertexData>>
-  GetPositionBufferCPU(const ContentContext& renderer,
-                       const Entity& entity,
-                       RenderPass& pass) const;
-
   std::vector<Point> points_;
   Scalar radius_;
   bool round_;

diff --git a/impeller/entity/shaders/geometry/points.comp b/impeller/entity/shaders/geometry/points.comp