Reverts "[Impeller] have Hostbuffer write directly to block allocated device buffers."

ci/licenses_golden/excluded_files

@@ -146,7 +146,6 @@
 ../../../flutter/impeller/entity/contents/filters/directional_gaussian_blur_filter_contents_unittests.cc
 ../../../flutter/impeller/entity/contents/filters/gaussian_blur_filter_contents_unittests.cc
 ../../../flutter/impeller/entity/contents/filters/inputs/filter_input_unittests.cc
-../../../flutter/impeller/entity/contents/host_buffer_unittests.cc
 ../../../flutter/impeller/entity/contents/test
 ../../../flutter/impeller/entity/contents/tiled_texture_contents_unittests.cc
 ../../../flutter/impeller/entity/contents/vertices_contents_unittests.cc
@@ -188,6 +187,7 @@
 ../../../flutter/impeller/renderer/compute_subgroup_unittests.cc
 ../../../flutter/impeller/renderer/compute_unittests.cc
 ../../../flutter/impeller/renderer/device_buffer_unittests.cc
+../../../flutter/impeller/renderer/host_buffer_unittests.cc
 ../../../flutter/impeller/renderer/pipeline_descriptor_unittests.cc
 ../../../flutter/impeller/renderer/pool_unittests.cc
 ../../../flutter/impeller/renderer/renderer_dart_unittests.cc

impeller/aiks/aiks_context.cc

@@ -40,19 +40,14 @@ ContentContext& AiksContext::GetContentContext() const {
   return *content_context_;
 }
 
-bool AiksContext::Render(const Picture& picture,
-                         RenderTarget& render_target,
-                         bool reset_host_buffer) {
+bool AiksContext::Render(const Picture& picture, RenderTarget& render_target) {
   if (!IsValid()) {
     return false;
   }
 
   if (picture.pass) {
     return picture.pass->Render(*content_context_, render_target);
   }
-  if (reset_host_buffer) {
-    content_context_->GetTransientsBuffer().Reset();
-  }
 
   return true;
 }

impeller/aiks/aiks_context.h

@@ -39,9 +39,7 @@ class AiksContext {
 
   ContentContext& GetContentContext() const;
 
-  bool Render(const Picture& picture,
-              RenderTarget& render_target,
-              bool reset_host_buffer);
+  bool Render(const Picture& picture, RenderTarget& render_target);
 
  private:
   std::shared_ptr<Context> context_;

impeller/aiks/aiks_playground.cc

@@ -53,7 +53,7 @@ bool AiksPlayground::OpenPlaygroundHere(AiksPlaygroundCallback callback) {
         if (!picture.has_value()) {
           return false;
         }
-        return renderer.Render(*picture, render_target, true);
+        return renderer.Render(*picture, render_target);
       });
 }
 

impeller/aiks/picture.cc

@@ -84,7 +84,7 @@ std::shared_ptr<Texture> Picture::RenderToTexture(
     return nullptr;
   }
 
-  if (!context.Render(*this, target, false)) {
+  if (!context.Render(*this, target)) {
     VALIDATION_LOG << "Could not render Picture to Texture.";
     return nullptr;
   }

impeller/base/allocation.h

@@ -6,6 +6,7 @@
 #define FLUTTER_IMPELLER_BASE_ALLOCATION_H_
 
 #include <cstdint>
+#include <limits>
 #include <memory>
 
 #include "flutter/fml/mapping.h"

impeller/core/buffer.h

@@ -16,7 +16,8 @@ class Buffer {
  public:
   virtual ~Buffer();
 
-  virtual std::shared_ptr<const DeviceBuffer> GetDeviceBuffer() const = 0;
+  virtual std::shared_ptr<const DeviceBuffer> GetDeviceBuffer(
+      Allocator& allocator) const = 0;
 };
 
 }  // namespace impeller

impeller/core/device_buffer.cc

@@ -11,12 +11,11 @@ DeviceBuffer::DeviceBuffer(DeviceBufferDescriptor desc) : desc_(desc) {}
 DeviceBuffer::~DeviceBuffer() = default;
 
 // |Buffer|
-std::shared_ptr<const DeviceBuffer> DeviceBuffer::GetDeviceBuffer() const {
+std::shared_ptr<const DeviceBuffer> DeviceBuffer::GetDeviceBuffer(
+    Allocator& allocator) const {
   return shared_from_this();
 }
 
-void DeviceBuffer::Flush(std::optional<Range> range) const {}
-
 BufferView DeviceBuffer::AsBufferView() const {
   BufferView view;
   view.buffer = shared_from_this();

impeller/core/device_buffer.h

@@ -38,21 +38,13 @@ class DeviceBuffer : public Buffer,
       uint16_t row_bytes) const;
 
   // |Buffer|
-  std::shared_ptr<const DeviceBuffer> GetDeviceBuffer() const;
+  std::shared_ptr<const DeviceBuffer> GetDeviceBuffer(
+      Allocator& allocator) const;
 
   const DeviceBufferDescriptor& GetDeviceBufferDescriptor() const;
 
   virtual uint8_t* OnGetContents() const = 0;
 
-  /// Make any pending writes visible to the GPU.
-  ///
-  /// This method must be called if the device pointer provided by
-  /// [OnGetContents] is written to without using [CopyHostBuffer]. On Devices
-  /// with coherent host memory, this method will not perform extra work.
-  ///
-  /// If the range is not provided, the entire buffer is flushed.
-  virtual void Flush(std::optional<Range> range = std::nullopt) const;
-
  protected:
   const DeviceBufferDescriptor desc_;
 

impeller/core/host_buffer.cc

@@ -4,33 +4,22 @@
 
 #include "impeller/core/host_buffer.h"
 
+#include <algorithm>
 #include <cstring>
-#include <tuple>
+
+#include "flutter/fml/logging.h"
 
 #include "impeller/core/allocator.h"
 #include "impeller/core/buffer_view.h"
 #include "impeller/core/device_buffer.h"
-#include "impeller/core/device_buffer_descriptor.h"
-#include "impeller/core/formats.h"
 
 namespace impeller {
 
-constexpr size_t kAllocatorBlockSize = 1024000;  // 1024 Kb.
-
-std::shared_ptr<HostBuffer> HostBuffer::Create(
-    const std::shared_ptr<Allocator>& allocator) {
-  return std::shared_ptr<HostBuffer>(new HostBuffer(allocator));
+std::shared_ptr<HostBuffer> HostBuffer::Create() {
+  return std::shared_ptr<HostBuffer>(new HostBuffer());
 }
 
-HostBuffer::HostBuffer(const std::shared_ptr<Allocator>& allocator) {
-  state_->allocator = allocator;
-  DeviceBufferDescriptor desc;
-  desc.size = kAllocatorBlockSize;
-  desc.storage_mode = StorageMode::kHostVisible;
-  for (auto i = 0u; i < kHostBufferArenaSize; i++) {
-    state_->device_buffers[i].push_back(allocator->CreateBuffer(desc));
-  }
-}
+HostBuffer::HostBuffer() = default;
 
 HostBuffer::~HostBuffer() = default;
 
@@ -41,143 +30,104 @@ void HostBuffer::SetLabel(std::string label) {
 BufferView HostBuffer::Emplace(const void* buffer,
                                size_t length,
                                size_t align) {
-  auto [data, range, device_buffer] = state_->Emplace(buffer, length, align);
+  auto [device_buffer, range] = state_->Emplace(buffer, length, align);
   if (!device_buffer) {
     return {};
   }
-  return BufferView{std::move(device_buffer), data, range};
+  return BufferView{state_, device_buffer, range};
 }
 
 BufferView HostBuffer::Emplace(const void* buffer, size_t length) {
-  auto [data, range, device_buffer] = state_->Emplace(buffer, length);
+  auto [device_buffer, range] = state_->Emplace(buffer, length);
   if (!device_buffer) {
     return {};
   }
-  return BufferView{std::move(device_buffer), data, range};
+  return BufferView{state_, device_buffer, range};
 }
 
 BufferView HostBuffer::Emplace(size_t length,
                                size_t align,
                                const EmplaceProc& cb) {
-  auto [data, range, device_buffer] = state_->Emplace(length, align, cb);
-  if (!device_buffer) {
+  auto [buffer, range] = state_->Emplace(length, align, cb);
+  if (!buffer) {
     return {};
   }
-  return BufferView{std::move(device_buffer), data, range};
+  return BufferView{state_, buffer, range};
 }
 
-HostBuffer::TestStateQuery HostBuffer::GetStateForTest() {
-  return HostBuffer::TestStateQuery{
-      .current_frame = state_->frame_index,
-      .current_buffer = state_->current_buffer,
-      .total_buffer_count = state_->device_buffers[state_->frame_index].size(),
-  };
+std::shared_ptr<const DeviceBuffer> HostBuffer::GetDeviceBuffer(
+    Allocator& allocator) const {
+  return state_->GetDeviceBuffer(allocator);
 }
 
 void HostBuffer::Reset() {
   state_->Reset();
 }
 
-void HostBuffer::HostBufferState::MaybeCreateNewBuffer(size_t required_size) {
-  current_buffer++;
-  if (current_buffer >= device_buffers[frame_index].size()) {
-    FML_DCHECK(required_size <= kAllocatorBlockSize);
-    DeviceBufferDescriptor desc;
-    desc.size = kAllocatorBlockSize;
-    desc.storage_mode = StorageMode::kHostVisible;
-    device_buffers[frame_index].push_back(allocator->CreateBuffer(desc));
-  }
-  offset = 0;
+size_t HostBuffer::GetSize() const {
+  return state_->GetReservedLength();
 }
 
-std::tuple<uint8_t*, Range, std::shared_ptr<DeviceBuffer>>
-HostBuffer::HostBufferState::Emplace(size_t length,
-                                     size_t align,
-                                     const EmplaceProc& cb) {
+size_t HostBuffer::GetLength() const {
+  return state_->GetLength();
+}
+
+std::pair<uint8_t*, Range> HostBuffer::HostBufferState::Emplace(
+    size_t length,
+    size_t align,
+    const EmplaceProc& cb) {
   if (!cb) {
     return {};
   }
-
-  // If the requested allocation is bigger than the block size, create a one-off
-  // device buffer and write to that.
-  if (length > kAllocatorBlockSize) {
-    DeviceBufferDescriptor desc;
-    desc.size = length;
-    desc.storage_mode = StorageMode::kHostVisible;
-    auto device_buffer = allocator->CreateBuffer(desc);
-    if (!device_buffer) {
-      return {};
-    }
-    if (cb) {
-      cb(device_buffer->OnGetContents());
-      device_buffer->Flush(Range{0, length});
-    }
-    return std::make_tuple(device_buffer->OnGetContents(), Range{0, length},
-                           device_buffer);
-  }
-
   auto old_length = GetLength();
-  if (old_length + length > kAllocatorBlockSize) {
-    MaybeCreateNewBuffer(length);
+  if (!Truncate(old_length + length)) {
+    return {};
   }
-  old_length = GetLength();
-
-  cb(GetCurrentBuffer()->OnGetContents() + old_length);
-  GetCurrentBuffer()->Flush(Range{old_length, length});
+  generation++;
+  cb(GetBuffer() + old_length);
 
-  offset += length;
-  return std::make_tuple(GetCurrentBuffer()->OnGetContents(),
-                         Range{old_length, length}, GetCurrentBuffer());
+  return std::make_pair(GetBuffer(), Range{old_length, length});
 }
 
-std::tuple<uint8_t*, Range, std::shared_ptr<DeviceBuffer>>
-HostBuffer::HostBufferState::Emplace(const void* buffer, size_t length) {
-  // If the requested allocation is bigger than the block size, create a one-off
-  // device buffer and write to that.
-  if (length > kAllocatorBlockSize) {
-    DeviceBufferDescriptor desc;
-    desc.size = length;
-    desc.storage_mode = StorageMode::kHostVisible;
-    auto device_buffer = allocator->CreateBuffer(desc);
-    if (!device_buffer) {
-      return {};
-    }
-    if (buffer) {
-      if (!device_buffer->CopyHostBuffer(static_cast<const uint8_t*>(buffer),
-                                         Range{0, length})) {
-        return {};
-      }
-    }
-    return std::make_tuple(device_buffer->OnGetContents(), Range{0, length},
-                           device_buffer);
+std::shared_ptr<const DeviceBuffer>
+HostBuffer::HostBufferState::GetDeviceBuffer(Allocator& allocator) const {
+  if (generation == device_buffer_generation) {
+    return device_buffer;
   }
+  auto new_buffer = allocator.CreateBufferWithCopy(GetBuffer(), GetLength());
+  if (!new_buffer) {
+    return nullptr;
+  }
+  new_buffer->SetLabel(label);
+  device_buffer_generation = generation;
+  device_buffer = std::move(new_buffer);
+  return device_buffer;
+}
 
+std::pair<uint8_t*, Range> HostBuffer::HostBufferState::Emplace(
+    const void* buffer,
+    size_t length) {
   auto old_length = GetLength();
-  if (old_length + length > kAllocatorBlockSize) {
-    MaybeCreateNewBuffer(length);
+  if (!Truncate(old_length + length)) {
+    return {};
   }
-  old_length = GetLength();
-
+  generation++;
   if (buffer) {
-    ::memmove(GetCurrentBuffer()->OnGetContents() + old_length, buffer, length);
-    GetCurrentBuffer()->Flush(Range{old_length, length});
+    ::memmove(GetBuffer() + old_length, buffer, length);
   }
-  offset += length;
-  return std::make_tuple(GetCurrentBuffer()->OnGetContents(),
-                         Range{old_length, length}, GetCurrentBuffer());
+  return std::make_pair(GetBuffer(), Range{old_length, length});
 }
 
-std::tuple<uint8_t*, Range, std::shared_ptr<DeviceBuffer>>
-HostBuffer::HostBufferState::Emplace(const void* buffer,
-                                     size_t length,
-                                     size_t align) {
+std::pair<uint8_t*, Range> HostBuffer::HostBufferState::Emplace(
+    const void* buffer,
+    size_t length,
+    size_t align) {
   if (align == 0 || (GetLength() % align) == 0) {
     return Emplace(buffer, length);
   }
 
   {
-    auto [buffer, range, device_buffer] =
-        Emplace(nullptr, align - (GetLength() % align));
+    auto [buffer, range] = Emplace(nullptr, align - (GetLength() % align));
     if (!buffer) {
       return {};
     }
@@ -187,15 +137,10 @@ HostBuffer::HostBufferState::Emplace(const void* buffer,
 }
 
 void HostBuffer::HostBufferState::Reset() {
-  // When resetting the host buffer state at the end of the frame, check if
-  // there are any unused buffers and remove them.
-  while (device_buffers[frame_index].size() > current_buffer + 1) {
-    device_buffers[frame_index].pop_back();
-  }
-
-  offset = 0u;
-  current_buffer = 0u;
-  frame_index = (frame_index + 1) % kHostBufferArenaSize;
+  generation += 1;
+  device_buffer = nullptr;
+  bool did_truncate = Truncate(0);
+  FML_CHECK(did_truncate);
 }
 
 }  // namespace impeller