Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

[Impeller] Don't discard GLES buffer for each copy; respect destination offset #33545

Merged
merged 2 commits into from
May 23, 2022
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
10 changes: 9 additions & 1 deletion impeller/renderer/backend/gles/allocator_gles.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@

#include "impeller/renderer/backend/gles/allocator_gles.h"

#include <memory>
Copy link
Member

Choose a reason for hiding this comment

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

Nit: Newline between the header types.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.


#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"
Expand All @@ -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|
Expand Down
4 changes: 2 additions & 2 deletions impeller/renderer/backend/gles/buffer_bindings_gles.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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 "
Expand Down
38 changes: 23 additions & 15 deletions impeller/renderer/backend/gles/device_buffer_gles.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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() {
Expand All @@ -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;
}

Expand Down Expand Up @@ -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_);
}

Expand All @@ -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;
Expand All @@ -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
15 changes: 11 additions & 4 deletions impeller/renderer/backend/gles/device_buffer_gles.h
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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,
Expand All @@ -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,
Expand Down