[Impeller] Don't discard GLES buffer for each copy; respect destinati…

…on offset (flutter#33545)
engine-flutter-autoroll · May 23, 2022 · 7442ecf · 7442ecf
1 parent 1c55450
commit 7442ecf
Show file tree

Hide file tree

Showing 4 changed files with 45 additions and 22 deletions.
diff --git 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 <memory>
+
+#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<DeviceBuffer> AllocatorGLES::CreateBuffer(StorageMode mode,
                                                           size_t length) {
-  return std::make_shared<DeviceBufferGLES>(reactor_, length, mode);
+  auto backing_store = std::make_shared<Allocation>();
+  if (!backing_store->Truncate(length)) {
+    return nullptr;
+  }
+  return std::make_shared<DeviceBufferGLES>(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
@@ -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
@@ -4,6 +4,9 @@
 
 #include "impeller/renderer/backend/gles/device_buffer_gles.h"
 
+#include <cstring>
+#include <memory>
+
 #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<Allocation> 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<fml::Mapping> 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
@@ -4,7 +4,10 @@
 
 #pragma once
 
+#include <memory>
+
 #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<DeviceBufferGLES, DeviceBuffer> {
  public:
-  DeviceBufferGLES(ReactorGLES::Ref reactor, size_t size, StorageMode mode);
+  DeviceBufferGLES(ReactorGLES::Ref reactor,
+                   std::shared_ptr<Allocation> buffer,
+                   size_t size,
+                   StorageMode mode);
 
   // |DeviceBuffer|
   ~DeviceBufferGLES() override;
 
-  std::shared_ptr<fml::Mapping> 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<fml::Mapping> data_;
-  mutable bool uploaded_ = false;
+  mutable std::shared_ptr<Allocation> backing_store_;
+  mutable uint32_t generation_ = 0;
+  mutable uint32_t upload_generation_ = 0;
 
   // |DeviceBuffer|
   bool CopyHostBuffer(const uint8_t* source,