From 7442ecfc2d4d99fad220a0f537a085975ea37ac0 Mon Sep 17 00:00:00 2001 From: Brandon DeRosier Date: Mon, 23 May 2022 13:43:41 -0700 Subject: [PATCH] [Impeller] Don't discard GLES buffer for each copy; respect destination offset (#33545) --- .../renderer/backend/gles/allocator_gles.cc | 10 ++++- .../backend/gles/buffer_bindings_gles.cc | 4 +- .../backend/gles/device_buffer_gles.cc | 38 +++++++++++-------- .../backend/gles/device_buffer_gles.h | 15 ++++++-- 4 files changed, 45 insertions(+), 22 deletions(-) diff --git a/impeller/renderer/backend/gles/allocator_gles.cc b/impeller/renderer/backend/gles/allocator_gles.cc index 52b3ede4a447c..5f08256ab0031 100644 --- a/impeller/renderer/backend/gles/allocator_gles.cc +++ b/impeller/renderer/backend/gles/allocator_gles.cc @@ -4,6 +4,9 @@ #include "impeller/renderer/backend/gles/allocator_gles.h" +#include + +#include "impeller/base/allocation.h" #include "impeller/base/config.h" #include "impeller/renderer/backend/gles/device_buffer_gles.h" #include "impeller/renderer/backend/gles/texture_gles.h" @@ -24,7 +27,12 @@ bool AllocatorGLES::IsValid() const { // |Allocator| std::shared_ptr AllocatorGLES::CreateBuffer(StorageMode mode, size_t length) { - return std::make_shared(reactor_, length, mode); + auto backing_store = std::make_shared(); + if (!backing_store->Truncate(length)) { + return nullptr; + } + return std::make_shared(reactor_, std::move(backing_store), + length, mode); } // |Allocator| diff --git a/impeller/renderer/backend/gles/buffer_bindings_gles.cc b/impeller/renderer/backend/gles/buffer_bindings_gles.cc index 8ff893b82038e..33cd7e627a62b 100644 --- a/impeller/renderer/backend/gles/buffer_bindings_gles.cc +++ b/impeller/renderer/backend/gles/buffer_bindings_gles.cc @@ -192,8 +192,8 @@ bool BufferBindingsGLES::BindUniformBuffer(const ProcTableGLES& gl, return false; } const auto& device_buffer_gles = DeviceBufferGLES::Cast(*device_buffer); - const uint8_t* buffer_ptr = device_buffer_gles.GetBufferData()->GetMapping() + - buffer.resource.range.offset; + const uint8_t* buffer_ptr = + device_buffer_gles.GetBufferData() + buffer.resource.range.offset; if (metadata->members.empty()) { VALIDATION_LOG << "Uniform buffer had no members. This is currently " diff --git a/impeller/renderer/backend/gles/device_buffer_gles.cc b/impeller/renderer/backend/gles/device_buffer_gles.cc index 6ee452972cce6..702e34a97b85c 100644 --- a/impeller/renderer/backend/gles/device_buffer_gles.cc +++ b/impeller/renderer/backend/gles/device_buffer_gles.cc @@ -4,6 +4,9 @@ #include "impeller/renderer/backend/gles/device_buffer_gles.h" +#include +#include + #include "flutter/fml/trace_event.h" #include "impeller/base/allocation.h" #include "impeller/base/config.h" @@ -12,12 +15,14 @@ namespace impeller { DeviceBufferGLES::DeviceBufferGLES(ReactorGLES::Ref reactor, + std::shared_ptr backing_store, size_t size, StorageMode mode) : DeviceBuffer(size, mode), reactor_(std::move(reactor)), handle_(reactor_ ? reactor_->CreateHandle(HandleType::kBuffer) - : HandleGLES::DeadHandle()) {} + : HandleGLES::DeadHandle()), + backing_store_(std::move(backing_store)) {} // |DeviceBuffer| DeviceBufferGLES::~DeviceBufferGLES() { @@ -35,21 +40,23 @@ bool DeviceBufferGLES::CopyHostBuffer(const uint8_t* source, return false; } - if (offset + source_range.length > size_) { - // Out of bounds of this buffer. + if (!reactor_) { return false; } - if (!reactor_) { + if (offset + source_range.length > size_) { + // Out of bounds of this buffer. return false; } - auto mapping = - CreateMappingWithCopy(source + source_range.offset, source_range.length); - if (!mapping) { + if (offset + source_range.length > backing_store_->GetLength()) { return false; } - data_ = std::move(mapping); + + std::memmove(backing_store_->GetBuffer() + offset, + source + source_range.offset, source_range.length); + ++generation_; + return true; } @@ -78,11 +85,12 @@ bool DeviceBufferGLES::BindAndUploadDataIfNecessary(BindingType type) const { gl.BindBuffer(target_type, buffer.value()); - if (!uploaded_) { + if (upload_generation_ != generation_) { TRACE_EVENT0("impeller", "BufferData"); - gl.BufferData(target_type, data_->GetSize(), data_->GetMapping(), - GL_STATIC_DRAW); - uploaded_ = true; + gl.BufferData(target_type, backing_store_->GetLength(), + backing_store_->GetBuffer(), GL_STATIC_DRAW); + upload_generation_ = generation_; + reactor_->SetDebugLabel(handle_, label_); } @@ -92,7 +100,7 @@ bool DeviceBufferGLES::BindAndUploadDataIfNecessary(BindingType type) const { // |DeviceBuffer| bool DeviceBufferGLES::SetLabel(const std::string& label) { label_ = label; - if (uploaded_) { + if (upload_generation_ > 0) { reactor_->SetDebugLabel(handle_, label_); } return true; @@ -105,7 +113,7 @@ bool DeviceBufferGLES::SetLabel(const std::string& label, Range range) { return SetLabel(label); } -std::shared_ptr DeviceBufferGLES::GetBufferData() const { - return data_; +const uint8_t* DeviceBufferGLES::GetBufferData() const { + return backing_store_->GetBuffer(); } } // namespace impeller diff --git a/impeller/renderer/backend/gles/device_buffer_gles.h b/impeller/renderer/backend/gles/device_buffer_gles.h index bd2a48f98b340..8305167845c01 100644 --- a/impeller/renderer/backend/gles/device_buffer_gles.h +++ b/impeller/renderer/backend/gles/device_buffer_gles.h @@ -4,7 +4,10 @@ #pragma once +#include + #include "flutter/fml/macros.h" +#include "impeller/base/allocation.h" #include "impeller/base/backend_cast.h" #include "impeller/renderer/backend/gles/reactor_gles.h" #include "impeller/renderer/device_buffer.h" @@ -15,12 +18,15 @@ class DeviceBufferGLES final : public DeviceBuffer, public BackendCast { public: - DeviceBufferGLES(ReactorGLES::Ref reactor, size_t size, StorageMode mode); + DeviceBufferGLES(ReactorGLES::Ref reactor, + std::shared_ptr buffer, + size_t size, + StorageMode mode); // |DeviceBuffer| ~DeviceBufferGLES() override; - std::shared_ptr GetBufferData() const; + const uint8_t* GetBufferData() const; enum class BindingType { kArrayBuffer, @@ -33,8 +39,9 @@ class DeviceBufferGLES final ReactorGLES::Ref reactor_; HandleGLES handle_; std::string label_; - mutable std::shared_ptr data_; - mutable bool uploaded_ = false; + mutable std::shared_ptr backing_store_; + mutable uint32_t generation_ = 0; + mutable uint32_t upload_generation_ = 0; // |DeviceBuffer| bool CopyHostBuffer(const uint8_t* source,