From ebd057a46fd09b3d155293340147c84b95e3664f Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Fri, 6 Dec 2024 14:03:11 -0800 Subject: [PATCH] [Impeller] store vertex buffers in render pass for gles. (flutter/engine#56991) Move vertex buffer storage off of the command object. Because we can bind up to 16 (but usually 1) vertex buffers per cmd, storing it on the command requires allocating storage for the full 16 buffers. MOving this to a secondary vector decreases the size of the command object from ~800 bytes to ~200 bytes. --- .../contents/test/recording_render_pass.cc | 1 - .../renderer/backend/gles/render_pass_gles.cc | 8 +++++-- .../src/flutter/impeller/renderer/command.cc | 17 +------------- .../src/flutter/impeller/renderer/command.h | 22 ++++--------------- .../flutter/impeller/renderer/render_pass.cc | 10 +++++++-- .../flutter/impeller/renderer/render_pass.h | 2 ++ 6 files changed, 21 insertions(+), 39 deletions(-) diff --git a/engine/src/flutter/impeller/entity/contents/test/recording_render_pass.cc b/engine/src/flutter/impeller/entity/contents/test/recording_render_pass.cc index 9d7877e1a1..39a8f69b50 100644 --- a/engine/src/flutter/impeller/entity/contents/test/recording_render_pass.cc +++ b/engine/src/flutter/impeller/entity/contents/test/recording_render_pass.cc @@ -74,7 +74,6 @@ void RecordingRenderPass::SetInstanceCount(size_t count) { // |RenderPass| bool RecordingRenderPass::SetVertexBuffer(VertexBuffer buffer) { - pending_.BindVertices(buffer); if (delegate_) { return delegate_->SetVertexBuffer(buffer); } diff --git a/engine/src/flutter/impeller/renderer/backend/gles/render_pass_gles.cc b/engine/src/flutter/impeller/renderer/backend/gles/render_pass_gles.cc index 24b0ccecd9..c8900d6e0c 100644 --- a/engine/src/flutter/impeller/renderer/backend/gles/render_pass_gles.cc +++ b/engine/src/flutter/impeller/renderer/backend/gles/render_pass_gles.cc @@ -10,6 +10,7 @@ #include "fml/closure.h" #include "fml/logging.h" #include "impeller/base/validation.h" +#include "impeller/core/buffer_view.h" #include "impeller/core/formats.h" #include "impeller/renderer/backend/gles/buffer_bindings_gles.h" #include "impeller/renderer/backend/gles/context_gles.h" @@ -191,6 +192,7 @@ void RenderPassGLES::ResetGLState(const ProcTableGLES& gl) { const RenderPassData& pass_data, const ReactorGLES& reactor, const std::vector& commands, + const std::vector& vertex_buffers, const std::vector& bound_textures, const std::vector& bound_buffers, const std::shared_ptr& tracer) { @@ -441,8 +443,9 @@ void RenderPassGLES::ResetGLState(const ProcTableGLES& gl) { /// `RenderPass::ValidateIndexBuffer` here, as validation already runs /// when the vertex/index buffers are set on the command. /// - for (size_t i = 0; i < command.vertex_buffer_count; i++) { - if (!BindVertexBuffer(gl, vertex_desc_gles, command.vertex_buffers[i], + for (size_t i = 0; i < command.vertex_buffers.length; i++) { + if (!BindVertexBuffer(gl, vertex_desc_gles, + vertex_buffers[i + command.vertex_buffers.offset], i)) { return false; } @@ -615,6 +618,7 @@ bool RenderPassGLES::OnEncodeCommands(const Context& context) const { /*pass_data=*/*pass_data, // /*reactor=*/reactor, // /*commands=*/render_pass->commands_, // + /*vertex_buffers=*/render_pass->vertex_buffers_, // /*bound_textures=*/render_pass->bound_textures_, // /*bound_buffers=*/render_pass->bound_buffers_, // /*tracer=*/tracer // diff --git a/engine/src/flutter/impeller/renderer/command.cc b/engine/src/flutter/impeller/renderer/command.cc index bd17e02a50..a981cac626 100644 --- a/engine/src/flutter/impeller/renderer/command.cc +++ b/engine/src/flutter/impeller/renderer/command.cc @@ -4,23 +4,8 @@ #include "impeller/renderer/command.h" -#include "impeller/base/validation.h" -#include "impeller/core/formats.h" - namespace impeller { -bool Command::BindVertices(const VertexBuffer& buffer) { - if (buffer.index_type == IndexType::kUnknown) { - VALIDATION_LOG << "Cannot bind vertex buffer with an unknown index type."; - return false; - } - - vertex_buffers = {buffer.vertex_buffer}; - vertex_buffer_count = 1u; - element_count = buffer.vertex_count; - index_buffer = buffer.index_buffer; - index_type = buffer.index_type; - return true; -} +// } // namespace impeller diff --git a/engine/src/flutter/impeller/renderer/command.h b/engine/src/flutter/impeller/renderer/command.h index 13ee566317..24b6471666 100644 --- a/engine/src/flutter/impeller/renderer/command.h +++ b/engine/src/flutter/impeller/renderer/command.h @@ -15,7 +15,6 @@ #include "impeller/core/sampler.h" #include "impeller/core/shader_types.h" #include "impeller/core/texture.h" -#include "impeller/core/vertex_buffer.h" #include "impeller/geometry/rect.h" #include "impeller/renderer/pipeline.h" @@ -130,13 +129,10 @@ struct Command { size_t instance_count = 1u; //---------------------------------------------------------------------------- - /// The vertex buffers used by the vertex shader stage. - std::array vertex_buffers; - - //---------------------------------------------------------------------------- - /// The number of vertex buffers in the vertex_buffers array. Must not exceed - /// kMaxVertexBuffers. - size_t vertex_buffer_count = 0u; + /// An offset and range of vertex buffers bound for this draw call. + /// + /// The vertex buffers are stored on the render pass object. + Range vertex_buffers; //---------------------------------------------------------------------------- /// The index buffer binding used by the vertex shader stage. @@ -152,16 +148,6 @@ struct Command { /// packed in the index buffer. IndexType index_type = IndexType::kUnknown; - //---------------------------------------------------------------------------- - /// @brief Specify the vertex and index buffer to use for this command. - /// - /// @param[in] buffer The vertex and index buffer definition. If possible, - /// this value should be moved and not copied. - /// - /// @return returns if the binding was updated. - /// - bool BindVertices(const VertexBuffer& buffer); - bool IsValid() const { return pipeline && pipeline->IsValid(); } }; diff --git a/engine/src/flutter/impeller/renderer/render_pass.cc b/engine/src/flutter/impeller/renderer/render_pass.cc index e498589021..a7f00ff286 100644 --- a/engine/src/flutter/impeller/renderer/render_pass.cc +++ b/engine/src/flutter/impeller/renderer/render_pass.cc @@ -142,9 +142,13 @@ bool RenderPass::SetVertexBuffer(BufferView vertex_buffers[], return false; } - pending_.vertex_buffer_count = vertex_buffer_count; + if (!vertex_buffers_start_.has_value()) { + vertex_buffers_start_ = vertex_buffers_.size(); + } + + pending_.vertex_buffers.length += vertex_buffer_count; for (size_t i = 0; i < vertex_buffer_count; i++) { - pending_.vertex_buffers[i] = std::move(vertex_buffers[i]); + vertex_buffers_.push_back(vertex_buffers[i]); } return true; } @@ -196,10 +200,12 @@ bool RenderPass::ValidateIndexBuffer(const BufferView& index_buffer, fml::Status RenderPass::Draw() { pending_.bound_buffers.offset = bound_buffers_start_.value_or(0u); pending_.bound_textures.offset = bound_textures_start_.value_or(0u); + pending_.vertex_buffers.offset = vertex_buffers_start_.value_or(0u); auto result = AddCommand(std::move(pending_)); pending_ = Command{}; bound_textures_start_ = std::nullopt; bound_buffers_start_ = std::nullopt; + vertex_buffers_start_ = std::nullopt; if (result) { return fml::Status(); } diff --git a/engine/src/flutter/impeller/renderer/render_pass.h b/engine/src/flutter/impeller/renderer/render_pass.h index 2ef60aecf1..20d85111d9 100644 --- a/engine/src/flutter/impeller/renderer/render_pass.h +++ b/engine/src/flutter/impeller/renderer/render_pass.h @@ -244,6 +244,7 @@ class RenderPass : public ResourceBinder { const ISize render_target_size_; const RenderTarget render_target_; std::vector commands_; + std::vector vertex_buffers_; std::vector bound_buffers_; std::vector bound_textures_; const Matrix orthographic_; @@ -289,6 +290,7 @@ class RenderPass : public ResourceBinder { Command pending_; std::optional bound_buffers_start_ = std::nullopt; std::optional bound_textures_start_ = std::nullopt; + std::optional vertex_buffers_start_ = std::nullopt; }; } // namespace impeller