Move primitive type to pipeline descriptor (#37315)

iskakaushik · web-flow · commit 2ac9c73f664c · 2022-11-04T18:01:16.000Z
This is the correct abstraction for Vulkan as we need this during primitive assembly. We also know this ahead of time so its useful to just wire it in. Fixes flutter/flutter#106379
diff --git a/impeller/entity/contents/clip_contents.cc b/impeller/entity/contents/clip_contents.cc
@@ -86,7 +86,6 @@ bool ClipContents::Render(const ContentContext& renderer,
     {
       cmd.label = "Difference Clip (Increment)";
 
-      cmd.primitive_type = PrimitiveType::kTriangleStrip;
       auto points = Rect(Size(pass.GetRenderTargetSize())).GetPoints();
       auto vertices =
           VertexBufferBuilder<VS::PerVertexData>{}
@@ -104,7 +103,6 @@ bool ClipContents::Render(const ContentContext& renderer,
     {
       cmd.label = "Difference Clip (Punch)";
 
-      cmd.primitive_type = PrimitiveType::kTriangle;
       cmd.stencil_reference = entity.GetStencilDepth() + 1;
       options.stencil_compare = CompareFunction::kEqual;
       options.stencil_operation = StencilOperation::kDecrementClamp;
@@ -115,12 +113,13 @@ bool ClipContents::Render(const ContentContext& renderer,
     options.stencil_operation = StencilOperation::kIncrementClamp;
   }
 
+  auto geometry_result = geometry_->GetPositionBuffer(renderer, entity, pass);
+  options.primitive_type = geometry_result.type;
   cmd.pipeline = renderer.GetClipPipeline(options);
 
   auto allocator = renderer.GetContext()->GetResourceAllocator();
-  auto geometry_result = geometry_->GetPositionBuffer(renderer, entity, pass);
   cmd.BindVertices(geometry_result.vertex_buffer);
-  cmd.primitive_type = geometry_result.type;
+
   info.mvp = Matrix::MakeOrthographic(pass.GetRenderTargetSize()) *
              entity.GetTransformation();
   VS::BindVertInfo(cmd, pass.GetTransientsBuffer().EmplaceUniform(info));
diff --git a/impeller/entity/contents/content_context.cc b/impeller/entity/contents/content_context.cc
@@ -127,6 +127,8 @@ void ContentContextOptions::ApplyToPipelineDescriptor(
     stencil.depth_stencil_pass = stencil_operation;
     desc.SetStencilAttachmentDescriptors(stencil);
   }
+
+  desc.SetPrimitiveType(primitive_type);
 }
 
 template <typename PipelineT>
diff --git a/impeller/entity/contents/content_context.h b/impeller/entity/contents/content_context.h
@@ -170,11 +170,12 @@ struct ContentContextOptions {
   BlendMode blend_mode = BlendMode::kSourceOver;
   CompareFunction stencil_compare = CompareFunction::kEqual;
   StencilOperation stencil_operation = StencilOperation::kKeep;
+  PrimitiveType primitive_type = PrimitiveType::kTriangle;
 
   struct Hash {
     constexpr std::size_t operator()(const ContentContextOptions& o) const {
       return fml::HashCombine(o.sample_count, o.blend_mode, o.stencil_compare,
-                              o.stencil_operation);
+                              o.stencil_operation, o.primitive_type);
     }
   };
 
@@ -184,7 +185,8 @@ struct ContentContextOptions {
       return lhs.sample_count == rhs.sample_count &&
              lhs.blend_mode == rhs.blend_mode &&
              lhs.stencil_compare == rhs.stencil_compare &&
-             lhs.stencil_operation == rhs.stencil_operation;
+             lhs.stencil_operation == rhs.stencil_operation &&
+             lhs.primitive_type == rhs.primitive_type;
     }
   };
 
diff --git a/impeller/entity/contents/contents.cc b/impeller/entity/contents/contents.cc
@@ -8,6 +8,7 @@
 #include "fml/logging.h"
 #include "impeller/entity/contents/content_context.h"
 #include "impeller/renderer/command_buffer.h"
+#include "impeller/renderer/formats.h"
 #include "impeller/renderer/render_pass.h"
 
 namespace impeller {
diff --git a/impeller/entity/contents/linear_gradient_contents.cc b/impeller/entity/contents/linear_gradient_contents.cc
@@ -82,10 +82,10 @@ bool LinearGradientContents::Render(const ContentContext& renderer,
     options.stencil_compare = CompareFunction::kEqual;
     options.stencil_operation = StencilOperation::kIncrementClamp;
   }
+  options.primitive_type = geometry_result.type;
   cmd.pipeline = renderer.GetLinearGradientFillPipeline(options);
 
   cmd.BindVertices(geometry_result.vertex_buffer);
-  cmd.primitive_type = geometry_result.type;
   FS::BindGradientInfo(
       cmd, pass.GetTransientsBuffer().EmplaceUniform(gradient_info));
   SamplerDescriptor sampler_desc;
diff --git a/impeller/entity/contents/radial_gradient_contents.cc b/impeller/entity/contents/radial_gradient_contents.cc
@@ -82,10 +82,10 @@ bool RadialGradientContents::Render(const ContentContext& renderer,
     options.stencil_compare = CompareFunction::kEqual;
     options.stencil_operation = StencilOperation::kIncrementClamp;
   }
+  options.primitive_type = geometry_result.type;
   cmd.pipeline = renderer.GetRadialGradientFillPipeline(options);
 
   cmd.BindVertices(geometry_result.vertex_buffer);
-  cmd.primitive_type = geometry_result.type;
   FS::BindGradientInfo(
       cmd, pass.GetTransientsBuffer().EmplaceUniform(gradient_info));
   SamplerDescriptor sampler_desc;
diff --git a/impeller/entity/contents/rrect_shadow_contents.cc b/impeller/entity/contents/rrect_shadow_contents.cc
@@ -79,11 +79,11 @@ bool RRectShadowContents::Render(const ContentContext& renderer,
 
   Command cmd;
   cmd.label = "RRect Shadow";
-  cmd.pipeline =
-      renderer.GetRRectBlurPipeline(OptionsFromPassAndEntity(pass, entity));
+  auto opts = OptionsFromPassAndEntity(pass, entity);
+  opts.primitive_type = PrimitiveType::kTriangle;
+  cmd.pipeline = renderer.GetRRectBlurPipeline(opts);
   cmd.stencil_reference = entity.GetStencilDepth();
 
-  cmd.primitive_type = PrimitiveType::kTriangle;
   cmd.BindVertices(vtx_builder.CreateVertexBuffer(pass.GetTransientsBuffer()));
 
   VS::VertInfo vert_info;
diff --git a/impeller/entity/contents/runtime_effect_contents.cc b/impeller/entity/contents/runtime_effect_contents.cc
@@ -122,6 +122,7 @@ bool RuntimeEffectContents::Render(const ContentContext& renderer,
     options.stencil_compare = CompareFunction::kEqual;
     options.stencil_operation = StencilOperation::kIncrementClamp;
   }
+  options.primitive_type = geometry_result.type;
   options.ApplyToPipelineDescriptor(desc);
 
   auto pipeline = context->GetPipelineLibrary()->GetPipeline(desc).get();
@@ -135,7 +136,6 @@ bool RuntimeEffectContents::Render(const ContentContext& renderer,
   cmd.pipeline = pipeline;
   cmd.stencil_reference = entity.GetStencilDepth();
   cmd.BindVertices(geometry_result.vertex_buffer);
-  cmd.primitive_type = geometry_result.type;
 
   //--------------------------------------------------------------------------
   /// Vertex stage uniforms.
diff --git a/impeller/entity/contents/solid_color_contents.cc b/impeller/entity/contents/solid_color_contents.cc
@@ -66,9 +66,9 @@ bool SolidColorContents::Render(const ContentContext& renderer,
     options.stencil_operation = StencilOperation::kIncrementClamp;
   }
 
+  options.primitive_type = geometry_result.type;
   cmd.pipeline = renderer.GetSolidFillPipeline(options);
   cmd.BindVertices(geometry_result.vertex_buffer);
-  cmd.primitive_type = geometry_result.type;
 
   VS::VertInfo vert_info;
   vert_info.mvp = Matrix::MakeOrthographic(pass.GetRenderTargetSize()) *
diff --git a/impeller/entity/contents/sweep_gradient_contents.cc b/impeller/entity/contents/sweep_gradient_contents.cc
@@ -88,10 +88,10 @@ bool SweepGradientContents::Render(const ContentContext& renderer,
     options.stencil_compare = CompareFunction::kEqual;
     options.stencil_operation = StencilOperation::kIncrementClamp;
   }
+  options.primitive_type = geometry_result.type;
   cmd.pipeline = renderer.GetSweepGradientFillPipeline(options);
 
   cmd.BindVertices(geometry_result.vertex_buffer);
-  cmd.primitive_type = geometry_result.type;
   FS::BindGradientInfo(
       cmd, pass.GetTransientsBuffer().EmplaceUniform(gradient_info));
   VS::BindFrameInfo(cmd, pass.GetTransientsBuffer().EmplaceUniform(frame_info));
diff --git a/impeller/entity/contents/text_contents.cc b/impeller/entity/contents/text_contents.cc
@@ -189,9 +189,9 @@ bool TextContents::RenderSdf(const ContentContext& renderer,
   // Information shared by all glyph draw calls.
   Command cmd;
   cmd.label = "TextFrameSDF";
-  cmd.primitive_type = PrimitiveType::kTriangle;
-  cmd.pipeline =
-      renderer.GetGlyphAtlasSdfPipeline(OptionsFromPassAndEntity(pass, entity));
+  auto opts = OptionsFromPassAndEntity(pass, entity);
+  opts.primitive_type = PrimitiveType::kTriangle;
+  cmd.pipeline = renderer.GetGlyphAtlasSdfPipeline(opts);
   cmd.stencil_reference = entity.GetStencilDepth();
 
   return CommonRender<GlyphAtlasSdfPipeline>(renderer, entity, pass, color_,
@@ -224,9 +224,9 @@ bool TextContents::Render(const ContentContext& renderer,
   // Information shared by all glyph draw calls.
   Command cmd;
   cmd.label = "TextFrame";
-  cmd.primitive_type = PrimitiveType::kTriangle;
-  cmd.pipeline =
-      renderer.GetGlyphAtlasPipeline(OptionsFromPassAndEntity(pass, entity));
+  auto opts = OptionsFromPassAndEntity(pass, entity);
+  opts.primitive_type = PrimitiveType::kTriangle;
+  cmd.pipeline = renderer.GetGlyphAtlasPipeline(opts);
   cmd.stencil_reference = entity.GetStencilDepth();
 
   return CommonRender<GlyphAtlasPipeline>(renderer, entity, pass, color_,
diff --git a/impeller/entity/contents/tiled_texture_contents.cc b/impeller/entity/contents/tiled_texture_contents.cc
@@ -77,10 +77,10 @@ bool TiledTextureContents::Render(const ContentContext& renderer,
     options.stencil_compare = CompareFunction::kEqual;
     options.stencil_operation = StencilOperation::kIncrementClamp;
   }
+  options.primitive_type = geometry_result.type;
   cmd.pipeline = renderer.GetTiledTexturePipeline(options);
 
   cmd.BindVertices(geometry_result.vertex_buffer);
-  cmd.primitive_type = geometry_result.type;
   VS::BindVertInfo(cmd, host_buffer.EmplaceUniform(vert_info));
   FS::BindFragInfo(cmd, host_buffer.EmplaceUniform(frag_info));
   FS::BindTextureSampler(cmd, texture_,
diff --git a/impeller/entity/contents/vertices_contents.cc b/impeller/entity/contents/vertices_contents.cc
@@ -47,15 +47,16 @@ bool VerticesContents::Render(const ContentContext& renderer,
   cmd.label = "Vertices";
   cmd.stencil_reference = entity.GetStencilDepth();
 
+  auto opts = OptionsFromPassAndEntity(pass, entity);
+
   switch (vertex_type) {
     case GeometryVertexType::kColor: {
       using VS = GeometryColorPipeline::VertexShader;
 
       auto geometry_result = geometry_->GetPositionColorBuffer(
           renderer, entity, pass, color_, blend_mode_);
-      cmd.pipeline = renderer.GetGeometryColorPipeline(
-          OptionsFromPassAndEntity(pass, entity));
-      cmd.primitive_type = geometry_result.type;
+      opts.primitive_type = geometry_result.type;
+      cmd.pipeline = renderer.GetGeometryColorPipeline(opts);
       cmd.BindVertices(geometry_result.vertex_buffer);
 
       VS::VertInfo vert_info;
@@ -70,9 +71,8 @@ bool VerticesContents::Render(const ContentContext& renderer,
 
       auto geometry_result =
           geometry_->GetPositionBuffer(renderer, entity, pass);
-      cmd.pipeline = renderer.GetGeometryPositionPipeline(
-          OptionsFromPassAndEntity(pass, entity));
-      cmd.primitive_type = geometry_result.type;
+      opts.primitive_type = geometry_result.type;
+      cmd.pipeline = renderer.GetGeometryPositionPipeline(opts);
       cmd.BindVertices(geometry_result.vertex_buffer);
 
       VS::VertInfo vert_info;
diff --git a/impeller/entity/entity_unittests.cc b/impeller/entity/entity_unittests.cc
@@ -757,6 +757,7 @@ TEST_P(EntityTest, BlendingModeOptions) {
       cmd.label = "Blended Rectangle";
       auto options = OptionsFromPass(pass);
       options.blend_mode = blend_mode;
+      options.primitive_type = PrimitiveType::kTriangle;
       cmd.pipeline = context.GetSolidFillPipeline(options);
       cmd.BindVertices(
           vtx_builder.CreateVertexBuffer(pass.GetTransientsBuffer()));
@@ -772,8 +773,6 @@ TEST_P(EntityTest, BlendingModeOptions) {
       FS::BindFragInfo(cmd,
                        pass.GetTransientsBuffer().EmplaceUniform(frag_info));
 
-      cmd.primitive_type = PrimitiveType::kTriangle;
-
       return pass.AddCommand(std::move(cmd));
     };
 
diff --git a/impeller/playground/imgui/imgui_impl_impeller.cc b/impeller/playground/imgui/imgui_impl_impeller.cc
@@ -264,7 +264,6 @@ void ImGui_ImplImpeller_RenderDrawData(ImDrawData* draw_data,
         vertex_buffer.index_type = impeller::IndexType::k16bit;
         cmd.BindVertices(vertex_buffer);
         cmd.base_vertex = pcmd->VtxOffset;
-        cmd.primitive_type = impeller::PrimitiveType::kTriangle;
 
         render_pass.AddCommand(std::move(cmd));
       }
diff --git a/impeller/renderer/backend/gles/render_pass_gles.cc b/impeller/renderer/backend/gles/render_pass_gles.cc
@@ -411,7 +411,8 @@ struct RenderPassData {
     //--------------------------------------------------------------------------
     /// Finally! Invoke the draw call.
     ///
-    gl.DrawElements(ToMode(command.primitive_type),   // mode
+    PrimitiveType primitive_type = pipeline.GetDescriptor().GetPrimitiveType();
+    gl.DrawElements(ToMode(primitive_type),           // mode
                     command.index_count,              // count
                     ToIndexType(command.index_type),  // type
                     reinterpret_cast<const GLvoid*>(static_cast<GLsizei>(
diff --git a/impeller/renderer/backend/metal/render_pass_mtl.mm b/impeller/renderer/backend/metal/render_pass_mtl.mm
@@ -494,6 +494,8 @@ static bool Bind(PassBindingsCache& pass,
       return false;
     }
 
+    const PrimitiveType primitive_type = pipeline_desc.GetPrimitiveType();
+
     FML_DCHECK(command.index_count *
                    (command.index_type == IndexType::k16bit ? 2 : 4) ==
                command.index_buffer.range.length);
@@ -503,7 +505,7 @@ static bool Bind(PassBindingsCache& pass,
       VALIDATION_LOG << "iOS Simulator does not support instanced rendering.";
       return false;
 #endif
-      [encoder drawIndexedPrimitives:ToMTLPrimitiveType(command.primitive_type)
+      [encoder drawIndexedPrimitives:ToMTLPrimitiveType(primitive_type)
                           indexCount:command.index_count
                            indexType:ToMTLIndexType(command.index_type)
                          indexBuffer:mtl_index_buffer
@@ -512,7 +514,7 @@ static bool Bind(PassBindingsCache& pass,
                           baseVertex:command.base_vertex
                         baseInstance:0u];
     } else {
-      [encoder drawIndexedPrimitives:ToMTLPrimitiveType(command.primitive_type)
+      [encoder drawIndexedPrimitives:ToMTLPrimitiveType(primitive_type)
                           indexCount:command.index_count
                            indexType:ToMTLIndexType(command.index_type)
                          indexBuffer:mtl_index_buffer
diff --git a/impeller/renderer/backend/vulkan/formats_vk.h b/impeller/renderer/backend/vulkan/formats_vk.h
@@ -311,4 +311,19 @@ constexpr vk::IndexType ToVKIndexType(IndexType index_type) {
   }
 }
 
+constexpr vk::PrimitiveTopology ToVKPrimitiveTopology(PrimitiveType primitive) {
+  switch (primitive) {
+    case PrimitiveType::kTriangle:
+      return vk::PrimitiveTopology::eTriangleList;
+    case PrimitiveType::kTriangleStrip:
+      return vk::PrimitiveTopology::eTriangleStrip;
+    case PrimitiveType::kLine:
+      return vk::PrimitiveTopology::eLineList;
+    case PrimitiveType::kLineStrip:
+      return vk::PrimitiveTopology::eLineStrip;
+    case PrimitiveType::kPoint:
+      return vk::PrimitiveTopology::ePointList;
+  }
+}
+
 }  // namespace impeller
diff --git a/impeller/renderer/backend/vulkan/pipeline_library_vk.cc b/impeller/renderer/backend/vulkan/pipeline_library_vk.cc
@@ -281,11 +281,9 @@ std::unique_ptr<PipelineCreateInfoVK> PipelineLibraryVK::CreatePipeline(
 
   //----------------------------------------------------------------------------
   /// Primitive Input Assembly State
-  /// TODO(106379): Move primitive topology to the the pipeline instead of it
-  ///               being on the draw call. This is hard-coded right now.
-  ///
   vk::PipelineInputAssemblyStateCreateInfo input_assembly;
-  input_assembly.setTopology(vk::PrimitiveTopology::eTriangleList);
+  const auto topology = ToVKPrimitiveTopology(desc.GetPrimitiveType());
+  input_assembly.setTopology(topology);
   pipeline_info.setPInputAssemblyState(&input_assembly);
 
   //----------------------------------------------------------------------------
diff --git a/impeller/renderer/command.h b/impeller/renderer/command.h
@@ -103,13 +103,6 @@ struct Command {
   ///
   std::string label;
   //----------------------------------------------------------------------------
-  /// The type of primitives in the vertex buffer. Set the vertex and index
-  /// buffers using a call to `BindVertices`.
-  ///
-  /// @see         `BindVertices`
-  ///
-  PrimitiveType primitive_type = PrimitiveType::kTriangle;
-  //----------------------------------------------------------------------------
   /// The reference value to use in stenciling operations. Stencil configuration
   /// is part of pipeline setup and can be read from the pipelines descriptor.
   ///
diff --git a/impeller/renderer/pipeline_descriptor.cc b/impeller/renderer/pipeline_descriptor.cc
@@ -229,4 +229,12 @@ WindingOrder PipelineDescriptor::GetWindingOrder() const {
   return winding_order_;
 }
 
+void PipelineDescriptor::SetPrimitiveType(PrimitiveType type) {
+  primitive_type_ = type;
+}
+
+PrimitiveType PipelineDescriptor::GetPrimitiveType() const {
+  return primitive_type_;
+}
+
 }  // namespace impeller
diff --git a/impeller/renderer/pipeline_descriptor.h b/impeller/renderer/pipeline_descriptor.h
@@ -115,6 +115,10 @@ class PipelineDescriptor final : public Comparable<PipelineDescriptor> {
 
   WindingOrder GetWindingOrder() const;
 
+  void SetPrimitiveType(PrimitiveType type);
+
+  PrimitiveType GetPrimitiveType() const;
+
  private:
   std::string label_;
   SampleCount sample_count_ = SampleCount::kCount1;
@@ -131,6 +135,7 @@ class PipelineDescriptor final : public Comparable<PipelineDescriptor> {
       front_stencil_attachment_descriptor_;
   std::optional<StencilAttachmentDescriptor>
       back_stencil_attachment_descriptor_;
+  PrimitiveType primitive_type_ = PrimitiveType::kTriangle;
 };
 
 using PipelineMap = std::unordered_map<
diff --git a/impeller/renderer/renderer_unittests.cc b/impeller/renderer/renderer_unittests.cc
@@ -101,8 +101,6 @@ TEST_P(RendererTest, CanCreateBoxPrimitive) {
                       pass.GetTransientsBuffer().EmplaceUniform(frame_info));
     FS::BindContents1(cmd, boston, sampler);
     FS::BindContents2(cmd, bridge, sampler);
-
-    cmd.primitive_type = PrimitiveType::kTriangle;
     if (!pass.AddCommand(std::move(cmd))) {
       return false;
     }
@@ -192,8 +190,6 @@ TEST_P(RendererTest, CanRenderPerspectiveCube) {
         Matrix::MakeRotationZ(Radians(euler_angles.z));
     VS::BindUniformBuffer(cmd,
                           pass.GetTransientsBuffer().EmplaceUniform(uniforms));
-
-    cmd.primitive_type = PrimitiveType::kTriangle;
     if (!pass.AddCommand(std::move(cmd))) {
       return false;
     }
@@ -254,8 +250,6 @@ TEST_P(RendererTest, CanRenderMultiplePrimitives) {
     FS::BindContents1(cmd, boston, sampler);
     FS::BindContents2(cmd, bridge, sampler);
 
-    cmd.primitive_type = PrimitiveType::kTriangle;
-
     for (size_t i = 0; i < 1; i++) {
       for (size_t j = 0; j < 1; j++) {
         VS::UniformBuffer uniforms;
@@ -369,8 +363,6 @@ TEST_P(RendererTest, CanRenderToTexture) {
   FS::BindContents1(cmd, boston, sampler);
   FS::BindContents2(cmd, bridge, sampler);
 
-  cmd.primitive_type = PrimitiveType::kTriangle;
-
   VS::UniformBuffer uniforms;
   uniforms.mvp = Matrix::MakeOrthographic(ISize{1024, 768}) *
                  Matrix::MakeTranslation({50.0f, 50.0f, 0.0f});

Original file line number	Diff line number	Diff line change
`@@ -127,6 +127,8 @@ void ContentContextOptions::ApplyToPipelineDescriptor(`
`127`	`127`	`stencil.depth_stencil_pass = stencil_operation;`
`128`	`128`	`desc.SetStencilAttachmentDescriptors(stencil);`
`129`	`129`	`}`
	`130`	`+`
	`131`	`+ desc.SetPrimitiveType(primitive_type);`
`130`	`132`	`}`
`131`	`133`
`132`	`134`	`template <typename PipelineT>`
Original file line number	Diff line number	Diff line change
`@@ -66,9 +66,9 @@ bool SolidColorContents::Render(const ContentContext& renderer,`
`66`	`66`	`options.stencil_operation = StencilOperation::kIncrementClamp;`
`67`	`67`	`}`
`68`	`68`
	`69`	`+ options.primitive_type = geometry_result.type;`
`69`	`70`	`cmd.pipeline = renderer.GetSolidFillPipeline(options);`
`70`	`71`	`cmd.BindVertices(geometry_result.vertex_buffer);`
`71`		`- cmd.primitive_type = geometry_result.type;`
`72`	`72`
`73`	`73`	`VS::VertInfo vert_info;`
`74`	`74`	`vert_info.mvp = Matrix::MakeOrthographic(pass.GetRenderTargetSize()) *`