[Impeller] Encode directly to command buffer for Vulkan. #49780

jonahwilliams · 2024-01-13T00:09:31Z

Rather than using impeller::Command, the impeller::RenderPass records most state directly into the Vulkan command buffer. This should remove allocation/free overhead of the intermediary structures and make further improvements to the backend even easier. This required a number of other changes to the renderer:

The render pass holds a strong ptr to the context. This helps avoid locking continually while encoding, which is quite slow.
barriers need to be encoded on the producing side, and not the consuming side. This is because we'll actually run the consuming code before the producing code. i.e. we transition to shader read at the end of a render pass instead of when binding.
I've updated the binding code to also provide the descriptor type so that we don't need to look it up from the desc. set.
I added a test render pass class that records commands.

…_vulkan

impeller/renderer/backend/vulkan/gpu_tracer_vk.cc

impeller/renderer/backend/vulkan/context_vk.cc

impeller/renderer/backend/vulkan/render_pass_vk.cc

impeller/entity/inline_pass_context.cc

jonahwilliams · 2024-01-13T03:08:24Z

impeller/compiler/code_gen_template.h

@@ -161,7 +161,7 @@ struct {{camel_case(shader_name)}}{{camel_case(shader_stage)}}Shader {
 {% endfor %}) {
    return {{ proto.args.0.argument_name }}.BindResource({% for arg in proto.args %}
  {% if loop.is_first %}
-{{to_shader_stage(shader_stage)}}, kResource{{ proto.name }}, kMetadata{{ proto.name }}, {% else %}
+{{to_shader_stage(shader_stage)}}, {{ proto.descriptor_type }}, kResource{{ proto.name }}, kMetadata{{ proto.name }}, {% else %}


Add the descriptor type so that bind calls don't need to look up the layout information to determine if a BufferView is a uniform buffer or storage buffer.

…ine into direct_encoding_vulkan

jonahwilliams · 2024-01-16T18:20:18Z

impeller/entity/contents/test/recording_render_pass.cc

+
+namespace impeller {
+
+RecordingRenderPass::RecordingRenderPass(


This is a test only class that lets us inspect individual commands.

jonahwilliams · 2024-01-16T18:23:44Z

impeller/renderer/backend/vulkan/render_pass_vk.cc

@@ -199,61 +248,87 @@ SharedHandleVK<vk::RenderPass> RenderPassVK::CreateVKRenderPass(

 RenderPassVK::RenderPassVK(const std::shared_ptr<const Context>& context,


Note that the pass bindings cache is mostly removed, as I was not able to measure a performance improvement from it.

bdero

LGTM

bdero · 2024-01-16T20:08:29Z

lib/gpu/render_pass.cc

-    render_pass_->BindResource(impeller::ShaderStage::kVertex, texture.slot,
-                               *texture.texture.GetMetadata(),
+    render_pass_->BindResource(impeller::ShaderStage::kVertex,
+                               impeller::DescriptorType::kUniformBuffer,


Suggested change

impeller::DescriptorType::kUniformBuffer,

impeller::DescriptorType::kSampledImage,

bdero · 2024-01-16T20:08:47Z

lib/gpu/render_pass.cc

-    render_pass_->BindResource(impeller::ShaderStage::kFragment, buffer.slot,
-                               *buffer.view.GetMetadata(),
+    render_pass_->BindResource(impeller::ShaderStage::kFragment,
+                               impeller::DescriptorType::kSampledImage,


Suggested change

impeller::DescriptorType::kSampledImage,

impeller::DescriptorType::kUniformBuffer,

bdero · 2024-01-16T20:19:46Z

impeller/entity/contents/vertices_contents_unittests.cc


-  const auto& cmd = render_pass->GetCommands()[0];
+  const auto& cmd = recording_pass->GetCommands()[0];


nit: assert command list is not empty before indexing

bdero · 2024-01-16T20:20:41Z

impeller/entity/contents/test/recording_render_pass.cc

+  return delegate_->BindResource(stage, type, slot, metadata, texture, sampler);
+}
+
+}  // namespace impeller


Suggested change

} // namespace impeller

} // namespace impeller

bdero · 2024-01-16T20:45:01Z

impeller/renderer/backend/vulkan/render_pass_vk.h

@@ -25,20 +25,92 @@ class RenderPassVK final : public RenderPass {
 private:
  friend class CommandBufferVK;

-  std::weak_ptr<CommandBufferVK> command_buffer_;
+  std::shared_ptr<CommandBufferVK> command_buffer_;


Is this just to remove weak check churn?

Yes, locking weak ptrs is actually very espensive. Since the RenderPass is immediately doing the recording and using the ContextVK to allocate descriptor sets, it would potentially lock multiple times per command otherwise.

…lutter/engine#49780)

…141651) flutter/engine@d4b6b7e...021a5ff 2024-01-16 [email protected] [web] Leave blob URLs untouched in TT policy. (flutter/engine#49782) 2024-01-16 [email protected] [Impeller] Fix a race between SwapchainImplVK::Present and WaitForFence (flutter/engine#49777) 2024-01-16 [email protected] Reland "[Windows] Move to FlutterCompositor for rendering" (flutter/engine#49726) 2024-01-16 [email protected] [Impeller] Add an API for sampling strictly within the bounds of the source rect in DrawImageRect (flutter/engine#49696) 2024-01-16 [email protected] [Impeller] Encode directly to command buffer for Vulkan. (flutter/engine#49780) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

jonahwilliams · 2024-01-17T01:15:01Z

Reverting as this broke the query pool reset and is breaking the framework tree.

)" This reverts commit 602c354.

…49818) Reverts #49780 Initiated by: jonahwilliams This change reverts the following previous change: Original Description: Part of flutter/flutter#140804 Rather than using impeller::Command, the impeller::RenderPass records most state directly into the Vulkan command buffer. This should remove allocation/free overhead of the intermediary structures and make further improvements to the backend even easier. This required a number of other changes to the renderer: 1. The render pass holds a strong ptr to the context. This helps avoid locking continually while encoding, which is quite slow. 2. barriers need to be encoded on the _producing_ side, and not the consuming side. This is because we'll actually run the consuming code before the producing code. i.e. we transition to shader read at the end of a render pass instead of when binding. 3. I've updated the binding code to also provide the descriptor type so that we don't need to look it up from the desc. set. 4. I added a test render pass class that records commands.

…isions)" (#141659) Reverts #141651 Initiated by: jonahwilliams This change reverts the following previous change: Original Description: flutter/engine@d4b6b7e...021a5ff 2024-01-16 [email protected] [web] Leave blob URLs untouched in TT policy. (flutter/engine#49782) 2024-01-16 [email protected] [Impeller] Fix a race between SwapchainImplVK::Present and WaitForFence (flutter/engine#49777) 2024-01-16 [email protected] Reland "[Windows] Move to FlutterCompositor for rendering" (flutter/engine#49726) 2024-01-16 [email protected] [Impeller] Add an API for sampling strictly within the bounds of the source rect in DrawImageRect (flutter/engine#49696) 2024-01-16 [email protected] [Impeller] Encode directly to command buffer for Vulkan. (flutter/engine#49780) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

…lutter/engine#49780)

…141664) flutter/engine@d4b6b7e...1382ff7 2024-01-16 [email protected] Remove iOS 12 availability checks (flutter/engine#49771) 2024-01-16 [email protected] [Impeller] generate mipmaps for imagefilters (flutter/engine#49794) 2024-01-16 [email protected] [web] Leave blob URLs untouched in TT policy. (flutter/engine#49782) 2024-01-16 [email protected] [Impeller] Fix a race between SwapchainImplVK::Present and WaitForFence (flutter/engine#49777) 2024-01-16 [email protected] Reland "[Windows] Move to FlutterCompositor for rendering" (flutter/engine#49726) 2024-01-16 [email protected] [Impeller] Add an API for sampling strictly within the bounds of the source rect in DrawImageRect (flutter/engine#49696) 2024-01-16 [email protected] [Impeller] Encode directly to command buffer for Vulkan. (flutter/engine#49780) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

jonahwilliams added 2 commits January 12, 2024 13:28

merge desc set changes.

9a84921

[Impeller] encode commands to Vulkan command buffer.

6285909

jonahwilliams changed the title ~~Direct encoding vulkan~~ [Impeller] Encode directly to command buffer. Jan 13, 2024

github-actions bot added the e: impeller label Jan 13, 2024

Merge branch 'main' of github.com:flutter/engine into direct_encoding…

6341c9e

…_vulkan