Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Impeller] remove Buffer type and associated abstractions. #49702

Merged
merged 2 commits into from
Jan 12, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
4 changes: 0 additions & 4 deletions ci/licenses_golden/licenses_flutter
Original file line number Diff line number Diff line change
Expand Up @@ -5050,8 +5050,6 @@ ORIGIN: ../../../flutter/impeller/compiler/utilities.cc + ../../../flutter/LICEN
ORIGIN: ../../../flutter/impeller/compiler/utilities.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/core/allocator.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/core/allocator.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/core/buffer.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/core/buffer.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/core/buffer_view.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/core/buffer_view.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/core/capture.cc + ../../../flutter/LICENSE
Expand Down Expand Up @@ -7877,8 +7875,6 @@ FILE: ../../../flutter/impeller/compiler/utilities.cc
FILE: ../../../flutter/impeller/compiler/utilities.h
FILE: ../../../flutter/impeller/core/allocator.cc
FILE: ../../../flutter/impeller/core/allocator.h
FILE: ../../../flutter/impeller/core/buffer.cc
FILE: ../../../flutter/impeller/core/buffer.h
FILE: ../../../flutter/impeller/core/buffer_view.cc
FILE: ../../../flutter/impeller/core/buffer_view.h
FILE: ../../../flutter/impeller/core/capture.cc
Expand Down
2 changes: 0 additions & 2 deletions impeller/base/backend_cast.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@
#ifndef FLUTTER_IMPELLER_BASE_BACKEND_CAST_H_
#define FLUTTER_IMPELLER_BASE_BACKEND_CAST_H_

#include "flutter/fml/macros.h"

namespace impeller {

template <class Sub, class Base>
Expand Down
2 changes: 0 additions & 2 deletions impeller/core/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@ impeller_component("core") {
sources = [
"allocator.cc",
"allocator.h",
"buffer.cc",
"buffer.h",
"buffer_view.cc",
"buffer_view.h",
"capture.cc",
Expand Down
11 changes: 0 additions & 11 deletions impeller/core/buffer.cc

This file was deleted.

24 changes: 0 additions & 24 deletions impeller/core/buffer.h

This file was deleted.

7 changes: 4 additions & 3 deletions impeller/core/buffer_view.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,15 @@
#ifndef FLUTTER_IMPELLER_CORE_BUFFER_VIEW_H_
#define FLUTTER_IMPELLER_CORE_BUFFER_VIEW_H_

#include "impeller/core/buffer.h"
#include <memory>
#include "impeller/core/range.h"

namespace impeller {

class DeviceBuffer;

struct BufferView {
std::shared_ptr<const Buffer> buffer;
uint8_t* contents;
std::shared_ptr<const DeviceBuffer> buffer;
Range range;

constexpr explicit operator bool() const { return static_cast<bool>(buffer); }
Expand Down
13 changes: 4 additions & 9 deletions impeller/core/device_buffer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,13 @@ DeviceBuffer::DeviceBuffer(DeviceBufferDescriptor desc) : desc_(desc) {}

DeviceBuffer::~DeviceBuffer() = default;

// |Buffer|
std::shared_ptr<const DeviceBuffer> DeviceBuffer::GetDeviceBuffer() const {
return shared_from_this();
}

void DeviceBuffer::Flush(std::optional<Range> range) const {}

BufferView DeviceBuffer::AsBufferView() const {
// static
BufferView DeviceBuffer::AsBufferView(std::shared_ptr<DeviceBuffer> buffer) {
BufferView view;
view.buffer = shared_from_this();
view.contents = OnGetContents();
view.range = {0u, desc_.size};
view.buffer = std::move(buffer);
view.range = {0u, view.buffer->desc_.size};
return view;
}

Expand Down
10 changes: 3 additions & 7 deletions impeller/core/device_buffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,14 @@
#include <string>

#include "impeller/core/allocator.h"
#include "impeller/core/buffer.h"
#include "impeller/core/buffer_view.h"
#include "impeller/core/device_buffer_descriptor.h"
#include "impeller/core/range.h"
#include "impeller/core/texture.h"

namespace impeller {

class DeviceBuffer : public Buffer,
public std::enable_shared_from_this<DeviceBuffer> {
class DeviceBuffer {
public:
virtual ~DeviceBuffer();

Expand All @@ -30,16 +28,14 @@ class DeviceBuffer : public Buffer,

virtual bool SetLabel(const std::string& label, Range range) = 0;

BufferView AsBufferView() const;
/// @brief Create a buffer view of this entire buffer.
static BufferView AsBufferView(std::shared_ptr<DeviceBuffer> buffer);

virtual std::shared_ptr<Texture> AsTexture(
Allocator& allocator,
const TextureDescriptor& descriptor,
uint16_t row_bytes) const;

// |Buffer|
std::shared_ptr<const DeviceBuffer> GetDeviceBuffer() const;

const DeviceBufferDescriptor& GetDeviceBufferDescriptor() const;

virtual uint8_t* OnGetContents() const = 0;
Expand Down
63 changes: 30 additions & 33 deletions impeller/core/host_buffer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -41,29 +41,29 @@ void HostBuffer::SetLabel(std::string label) {
BufferView HostBuffer::Emplace(const void* buffer,
size_t length,
size_t align) {
auto [data, range, device_buffer] = EmplaceInternal(buffer, length, align);
auto [range, device_buffer] = EmplaceInternal(buffer, length, align);
if (!device_buffer) {
return {};
}
return BufferView{std::move(device_buffer), data, range};
return BufferView{std::move(device_buffer), range};
}

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

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

HostBuffer::TestStateQuery HostBuffer::GetStateForTest() {
Expand All @@ -74,10 +74,9 @@ HostBuffer::TestStateQuery HostBuffer::GetStateForTest() {
};
}

void HostBuffer::MaybeCreateNewBuffer(size_t required_size) {
void HostBuffer::MaybeCreateNewBuffer() {
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;
Expand All @@ -86,10 +85,10 @@ void HostBuffer::MaybeCreateNewBuffer(size_t required_size) {
offset_ = 0;
}

std::tuple<uint8_t*, Range, std::shared_ptr<DeviceBuffer>>
HostBuffer::EmplaceInternal(size_t length,
size_t align,
const EmplaceProc& cb) {
std::tuple<Range, std::shared_ptr<DeviceBuffer>> HostBuffer::EmplaceInternal(
size_t length,
size_t align,
const EmplaceProc& cb) {
if (!cb) {
return {};
}
Expand All @@ -108,28 +107,27 @@ HostBuffer::EmplaceInternal(size_t length,
cb(device_buffer->OnGetContents());
device_buffer->Flush(Range{0, length});
}
return std::make_tuple(device_buffer->OnGetContents(), Range{0, length},
device_buffer);
return std::make_tuple(Range{0, length}, device_buffer);
}

auto old_length = GetLength();
if (old_length + length > kAllocatorBlockSize) {
MaybeCreateNewBuffer(length);
MaybeCreateNewBuffer();
}
old_length = GetLength();

auto current_buffer = GetCurrentBuffer();
cb(current_buffer->OnGetContents() + old_length);
auto contents = current_buffer->OnGetContents();
cb(contents + old_length);
current_buffer->Flush(Range{old_length, length});

offset_ += length;
auto contents = current_buffer->OnGetContents();
return std::make_tuple(contents, Range{old_length, length},
std::move(current_buffer));
return std::make_tuple(Range{old_length, length}, std::move(current_buffer));
}

std::tuple<uint8_t*, Range, std::shared_ptr<DeviceBuffer>>
HostBuffer::EmplaceInternal(const void* buffer, size_t length) {
std::tuple<Range, std::shared_ptr<DeviceBuffer>> HostBuffer::EmplaceInternal(
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) {
Expand All @@ -146,38 +144,37 @@ HostBuffer::EmplaceInternal(const void* buffer, size_t length) {
return {};
}
}
return std::make_tuple(device_buffer->OnGetContents(), Range{0, length},
device_buffer);
return std::make_tuple(Range{0, length}, device_buffer);
}

auto old_length = GetLength();
if (old_length + length > kAllocatorBlockSize) {
MaybeCreateNewBuffer(length);
MaybeCreateNewBuffer();
}
old_length = GetLength();

auto current_buffer = GetCurrentBuffer();
auto contents = current_buffer->OnGetContents();
if (buffer) {
::memmove(current_buffer->OnGetContents() + old_length, buffer, length);
::memmove(contents + old_length, buffer, length);
current_buffer->Flush(Range{old_length, length});
}
offset_ += length;
auto contents = current_buffer->OnGetContents();
return std::make_tuple(contents, Range{old_length, length},
std::move(current_buffer));
return std::make_tuple(Range{old_length, length}, std::move(current_buffer));
}

std::tuple<uint8_t*, Range, std::shared_ptr<DeviceBuffer>>
std::tuple<Range, std::shared_ptr<DeviceBuffer>>
HostBuffer::EmplaceInternal(const void* buffer, size_t length, size_t align) {
if (align == 0 || (GetLength() % align) == 0) {
return EmplaceInternal(buffer, length);
}

{
auto [buffer, range, device_buffer] =
EmplaceInternal(nullptr, align - (GetLength() % align));
if (!buffer) {
return {};
auto padding = align - (GetLength() % align);
if (offset_ + padding < kAllocatorBlockSize) {
offset_ += padding;
} else {
MaybeCreateNewBuffer();
}
}

Expand Down
10 changes: 5 additions & 5 deletions impeller/core/host_buffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
#include <string>
#include <type_traits>

#include "impeller/core/buffer.h"
#include "impeller/core/allocator.h"
#include "impeller/core/buffer_view.h"
#include "impeller/core/platform.h"

Expand Down Expand Up @@ -134,18 +134,18 @@ class HostBuffer {
TestStateQuery GetStateForTest();

private:
[[nodiscard]] std::tuple<uint8_t*, Range, std::shared_ptr<DeviceBuffer>>
[[nodiscard]] std::tuple<Range, std::shared_ptr<DeviceBuffer>>
EmplaceInternal(const void* buffer, size_t length);

std::tuple<uint8_t*, Range, std::shared_ptr<DeviceBuffer>>
std::tuple<Range, std::shared_ptr<DeviceBuffer>>
EmplaceInternal(size_t length, size_t align, const EmplaceProc& cb);

std::tuple<uint8_t*, Range, std::shared_ptr<DeviceBuffer>>
std::tuple<Range, std::shared_ptr<DeviceBuffer>>
EmplaceInternal(const void* buffer, size_t length, size_t align);

size_t GetLength() const { return offset_; }

void MaybeCreateNewBuffer(size_t required_size);
void MaybeCreateNewBuffer();

std::shared_ptr<DeviceBuffer>& GetCurrentBuffer() {
return device_buffers_[frame_index_][current_buffer_];
Expand Down
8 changes: 4 additions & 4 deletions impeller/entity/contents/test/contents_test_helpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ typename T::VertInfo* GetVertInfo(const Command& command) {
return nullptr;
}

auto data =
(resource->view.resource.contents + resource->view.resource.range.offset);
auto data = (resource->view.resource.buffer->OnGetContents() +
resource->view.resource.range.offset);
return reinterpret_cast<typename T::VertInfo*>(data);
}

Expand All @@ -38,8 +38,8 @@ typename T::FragInfo* GetFragInfo(const Command& command) {
return nullptr;
}

auto data =
(resource->view.resource.contents + resource->view.resource.range.offset);
auto data = (resource->view.resource.buffer->OnGetContents() +
resource->view.resource.range.offset);
return reinterpret_cast<typename T::FragInfo*>(data);
}

Expand Down
13 changes: 5 additions & 8 deletions impeller/entity/geometry/point_field_geometry.cc
Original file line number Diff line number Diff line change
Expand Up @@ -162,10 +162,8 @@ GeometryResult PointFieldGeometry::GetPositionBufferGPU(
buffer_desc.size = total * sizeof(Point);
buffer_desc.storage_mode = StorageMode::kDevicePrivate;

auto geometry_buffer = renderer.GetContext()
->GetResourceAllocator()
->CreateBuffer(buffer_desc)
->AsBufferView();
auto geometry_buffer = DeviceBuffer::AsBufferView(
renderer.GetContext()->GetResourceAllocator()->CreateBuffer(buffer_desc));

BufferView output;
{
Expand Down Expand Up @@ -197,10 +195,9 @@ GeometryResult PointFieldGeometry::GetPositionBufferGPU(
buffer_desc.size = total * sizeof(Vector4);
buffer_desc.storage_mode = StorageMode::kDevicePrivate;

auto geometry_uv_buffer = renderer.GetContext()
->GetResourceAllocator()
->CreateBuffer(buffer_desc)
->AsBufferView();
auto geometry_uv_buffer = DeviceBuffer::AsBufferView(
renderer.GetContext()->GetResourceAllocator()->CreateBuffer(
buffer_desc));

using UV = UvComputeShader;

Expand Down
2 changes: 1 addition & 1 deletion impeller/playground/playground.cc
Original file line number Diff line number Diff line change
Expand Up @@ -422,7 +422,7 @@ static std::shared_ptr<Texture> CreateTextureForDecompressedImage(
return nullptr;
}
blit_pass->SetLabel("Mipmap Blit Pass");
blit_pass->AddCopy(buffer->AsBufferView(), dest_texture);
blit_pass->AddCopy(DeviceBuffer::AsBufferView(buffer), dest_texture);
if (enable_mipmapping) {
blit_pass->GenerateMipmap(dest_texture);
}
Expand Down
Loading