Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ci/licenses_golden/excluded_files
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@
../../../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
Expand Down Expand Up @@ -187,7 +188,6 @@
../../../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
Expand Down
1 change: 0 additions & 1 deletion impeller/base/allocation.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
#define FLUTTER_IMPELLER_BASE_ALLOCATION_H_

#include <cstdint>
#include <limits>
#include <memory>

#include "flutter/fml/mapping.h"
Expand Down
2 changes: 2 additions & 0 deletions impeller/core/device_buffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ class DeviceBuffer : public Buffer,

virtual uint8_t* OnGetContents() const = 0;

virtual void Flush(std::optional<Range> range) const {}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Place definition in device_buffer.cc?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done


protected:
const DeviceBufferDescriptor desc_;

Expand Down
129 changes: 69 additions & 60 deletions impeller/core/host_buffer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,22 +4,31 @@

#include "impeller/core/host_buffer.h"

#include <algorithm>
#include <cstring>

#include "flutter/fml/logging.h"
#include <tuple>

#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 {

std::shared_ptr<HostBuffer> HostBuffer::Create() {
return std::shared_ptr<HostBuffer>(new HostBuffer());
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));
}

HostBuffer::HostBuffer() = default;
HostBuffer::HostBuffer(const std::shared_ptr<Allocator>& allocator) {
state_->allocator = allocator;
DeviceBufferDescriptor desc;
desc.size = kAllocatorBlockSize;
desc.storage_mode = StorageMode::kHostVisible;
state_->device_buffers.push_back(allocator->CreateBuffer(desc));
}

HostBuffer::~HostBuffer() = default;

Expand All @@ -30,104 +39,101 @@ void HostBuffer::SetLabel(std::string label) {
BufferView HostBuffer::Emplace(const void* buffer,
size_t length,
size_t align) {
auto [device_buffer, range] = state_->Emplace(buffer, length, align);
auto [data, range, device_buffer] = state_->Emplace(buffer, length, align);
if (!device_buffer) {
return {};
}
return BufferView{state_, device_buffer, range};
return BufferView{std::move(device_buffer), data, range};
}

BufferView HostBuffer::Emplace(const void* buffer, size_t length) {
auto [device_buffer, range] = state_->Emplace(buffer, length);
auto [data, range, device_buffer] = state_->Emplace(buffer, length);
if (!device_buffer) {
return {};
}
return BufferView{state_, device_buffer, range};
return BufferView{std::move(device_buffer), data, range};
}

BufferView HostBuffer::Emplace(size_t length,
size_t align,
const EmplaceProc& cb) {
auto [buffer, range] = state_->Emplace(length, align, cb);
if (!buffer) {
auto [data, range, device_buffer] = state_->Emplace(length, align, cb);
if (!device_buffer) {
return {};
}
return BufferView{state_, buffer, range};
return BufferView{std::move(device_buffer), data, range};
}

std::shared_ptr<const DeviceBuffer> HostBuffer::GetDeviceBuffer(
Allocator& allocator) const {
return state_->GetDeviceBuffer(allocator);
return nullptr;
}

void HostBuffer::Reset() {
state_->Reset();
}

size_t HostBuffer::GetSize() const {
return state_->GetReservedLength();
}

size_t HostBuffer::GetLength() const {
return state_->GetLength();
void HostBuffer::HostBufferState::MaybeCreateNewBuffer(size_t required_size) {
if (current_buffer + 1 >= device_buffers.size()) {
if (required_size > kAllocatorBlockSize) {
FML_LOG(ERROR) << "Created oversized buffer: " << required_size;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be addressed later when we start doing DeviceBuffer reuse, but it would probably be a good to track oversized DeviceBuffers separately from the block list. Otherwise one oversized draw could end up causing a few oversized blocked to get allocated while scrolling around, for example.

In the doc for flutter/flutter#138161 I recommended just throwing away oversized buffers and not tracking when starting out... But perhaps later we could form a heap for tracking oversize buffers sorted by size.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

}
DeviceBufferDescriptor desc;
desc.size = std::max(kAllocatorBlockSize, required_size);
desc.storage_mode = StorageMode::kHostVisible;
device_buffers.push_back(allocator->CreateBuffer(desc));
}
current_buffer++;
offset = 0;
}

std::pair<uint8_t*, Range> HostBuffer::HostBufferState::Emplace(
size_t length,
size_t align,
const EmplaceProc& cb) {
std::tuple<uint8_t*, Range, std::shared_ptr<DeviceBuffer>>
HostBuffer::HostBufferState::Emplace(size_t length,
size_t align,
const EmplaceProc& cb) {
if (!cb) {
return {};
}
auto old_length = GetLength();
if (!Truncate(old_length + length)) {
return {};
if (old_length + length > kAllocatorBlockSize) {
MaybeCreateNewBuffer(length);
}
generation++;
cb(GetBuffer() + old_length);

return std::make_pair(GetBuffer(), Range{old_length, length});
}
cb(GetCurrentBuffer()->OnGetContents() + old_length);
GetCurrentBuffer()->Flush(Range{old_length, length});

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;
offset += length;
return std::make_tuple(GetCurrentBuffer()->OnGetContents(),
Range{old_length, length}, GetCurrentBuffer());
}

std::pair<uint8_t*, Range> HostBuffer::HostBufferState::Emplace(
const void* buffer,
size_t length) {
std::tuple<uint8_t*, Range, std::shared_ptr<DeviceBuffer>>
HostBuffer::HostBufferState::Emplace(const void* buffer, size_t length) {
auto old_length = GetLength();
if (!Truncate(old_length + length)) {
return {};
if (old_length + length > kAllocatorBlockSize) {
MaybeCreateNewBuffer(length);
}
generation++;

if (buffer) {
::memmove(GetBuffer() + old_length, buffer, length);
::memmove(GetCurrentBuffer()->OnGetContents() + old_length, buffer, length);
GetCurrentBuffer()->Flush(Range{old_length, length});
}
return std::make_pair(GetBuffer(), Range{old_length, length});
offset += length;
return std::make_tuple(GetCurrentBuffer()->OnGetContents(),
Range{old_length, length}, GetCurrentBuffer());
}

std::pair<uint8_t*, Range> HostBuffer::HostBufferState::Emplace(
const void* buffer,
size_t length,
size_t align) {
std::tuple<uint8_t*, Range, std::shared_ptr<DeviceBuffer>>
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] = Emplace(nullptr, align - (GetLength() % align));
auto [buffer, range, device_buffer] =
Emplace(nullptr, align - (GetLength() % align));
if (!buffer) {
return {};
}
Expand All @@ -137,10 +143,13 @@ std::pair<uint8_t*, Range> HostBuffer::HostBufferState::Emplace(
}

void HostBuffer::HostBufferState::Reset() {
generation += 1;
device_buffer = nullptr;
bool did_truncate = Truncate(0);
FML_CHECK(did_truncate);
offset = 0u;
current_buffer = 0u;
device_buffers.clear();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any plans for reusing the DeviceBuffers across frames? One easy solution would be to rotate through MaxFramesInFlight vectors IMO.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that seems simple enough. Added this logic plus some trimming of buffers if they go unused.

DeviceBufferDescriptor desc;
desc.size = kAllocatorBlockSize;
desc.storage_mode = StorageMode::kHostVisible;
device_buffers.push_back(allocator->CreateBuffer(desc));
}

} // namespace impeller
48 changes: 22 additions & 26 deletions impeller/core/host_buffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@
#define FLUTTER_IMPELLER_CORE_HOST_BUFFER_H_

#include <algorithm>
#include <functional>
#include <memory>
#include <string>
#include <type_traits>

#include "impeller/base/allocation.h"
#include "impeller/core/buffer.h"
#include "impeller/core/buffer_view.h"
#include "impeller/core/platform.h"
Expand All @@ -19,7 +19,8 @@ namespace impeller {

class HostBuffer final : public Buffer {
public:
static std::shared_ptr<HostBuffer> Create();
static std::shared_ptr<HostBuffer> Create(
const std::shared_ptr<Allocator>& allocator);

// |Buffer|
virtual ~HostBuffer();
Expand Down Expand Up @@ -114,36 +115,31 @@ class HostBuffer final : public Buffer {
/// reused.
void Reset();

//----------------------------------------------------------------------------
/// @brief Returns the capacity of the HostBuffer in memory in bytes.
size_t GetSize() const;
private:
struct HostBufferState {
[[nodiscard]] std::tuple<uint8_t*, Range, std::shared_ptr<DeviceBuffer>>
Emplace(const void* buffer, size_t length);

//----------------------------------------------------------------------------
/// @brief Returns the size of the currently allocated HostBuffer memory in
/// bytes.
size_t GetLength() const;
std::tuple<uint8_t*, Range, std::shared_ptr<DeviceBuffer>>
Emplace(size_t length, size_t align, const EmplaceProc& cb);

private:
struct HostBufferState : public Buffer, public Allocation {
std::shared_ptr<const DeviceBuffer> GetDeviceBuffer(
Allocator& allocator) const override;
std::tuple<uint8_t*, Range, std::shared_ptr<DeviceBuffer>>
Emplace(const void* buffer, size_t length, size_t align);

[[nodiscard]] std::pair<uint8_t*, Range> Emplace(const void* buffer,
size_t length);
void Reset();

std::pair<uint8_t*, Range> Emplace(size_t length,
size_t align,
const EmplaceProc& cb);
size_t GetLength() const { return offset; }

std::pair<uint8_t*, Range> Emplace(const void* buffer,
size_t length,
size_t align);
void MaybeCreateNewBuffer(size_t required_size);

void Reset();
std::shared_ptr<DeviceBuffer> GetCurrentBuffer() {
return device_buffers[current_buffer];
}

mutable std::shared_ptr<DeviceBuffer> device_buffer;
mutable size_t device_buffer_generation = 0u;
size_t generation = 1u;
std::shared_ptr<Allocator> allocator;
std::vector<std::shared_ptr<DeviceBuffer>> device_buffers;
size_t current_buffer = 0u;
size_t offset = 0u;
std::string label;
};

Expand All @@ -155,7 +151,7 @@ class HostBuffer final : public Buffer {

[[nodiscard]] BufferView Emplace(const void* buffer, size_t length);

HostBuffer();
explicit HostBuffer(const std::shared_ptr<Allocator>& allocator);

HostBuffer(const HostBuffer&) = delete;

Expand Down
1 change: 1 addition & 0 deletions impeller/entity/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,7 @@ impeller_component("entity_unittests") {
"contents/filters/directional_gaussian_blur_filter_contents_unittests.cc",
"contents/filters/gaussian_blur_filter_contents_unittests.cc",
"contents/filters/inputs/filter_input_unittests.cc",
"contents/host_buffer_unittests.cc",
"contents/tiled_texture_contents_unittests.cc",
"contents/vertices_contents_unittests.cc",
"entity_pass_target_unittests.cc",
Expand Down
6 changes: 3 additions & 3 deletions impeller/entity/contents/atlas_contents.cc
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ bool AtlasContents::Render(const ContentContext& renderer,
VertexBufferBuilder<VS::PerVertexData> vtx_builder;
vtx_builder.Reserve(texture_coords_.size() * 6);
const auto texture_size = texture_->GetSize();
auto& host_buffer = pass.GetTransientsBuffer();
auto& host_buffer = renderer.GetTransientsBuffer();

for (size_t i = 0; i < texture_coords_.size(); i++) {
auto sample_rect = texture_coords_[i];
Expand Down Expand Up @@ -399,7 +399,7 @@ bool AtlasTextureContents::Render(const ContentContext& renderer,
Command cmd;
DEBUG_COMMAND_INFO(cmd, "AtlasTexture");

auto& host_buffer = pass.GetTransientsBuffer();
auto& host_buffer = renderer.GetTransientsBuffer();

VS::FrameInfo frame_info;
frame_info.mvp = pass.GetOrthographicTransform() * entity.GetTransform();
Expand Down Expand Up @@ -486,7 +486,7 @@ bool AtlasColorContents::Render(const ContentContext& renderer,
Command cmd;
DEBUG_COMMAND_INFO(cmd, "AtlasColors");

auto& host_buffer = pass.GetTransientsBuffer();
auto& host_buffer = renderer.GetTransientsBuffer();

VS::FrameInfo frame_info;
frame_info.mvp = pass.GetOrthographicTransform() * entity.GetTransform();
Expand Down
2 changes: 1 addition & 1 deletion impeller/entity/contents/checkerboard_contents.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ CheckerboardContents::~CheckerboardContents() = default;
bool CheckerboardContents::Render(const ContentContext& renderer,
const Entity& entity,
RenderPass& pass) const {
auto& host_buffer = pass.GetTransientsBuffer();
auto& host_buffer = renderer.GetTransientsBuffer();

using VS = CheckerboardPipeline::VertexShader;
using FS = CheckerboardPipeline::FragmentShader;
Expand Down
Loading