[Impeller] Use the typed allocation sizes in impeller::Allocation. (#54327)
diff --git a/impeller/base/allocation.cc b/impeller/base/allocation.cc index c7f65d4..88241f7 100644 --- a/impeller/base/allocation.cc +++ b/impeller/base/allocation.cc
@@ -21,15 +21,15 @@ return buffer_; } -size_t Allocation::GetLength() const { +Bytes Allocation::GetLength() const { return length_; } -size_t Allocation::GetReservedLength() const { +Bytes Allocation::GetReservedLength() const { return reserved_; } -bool Allocation::Truncate(size_t length, bool npot) { +bool Allocation::Truncate(Bytes length, bool npot) { const auto reserved = npot ? ReserveNPOT(length) : Reserve(length); if (!reserved) { return false; @@ -54,18 +54,18 @@ return x + 1; } -bool Allocation::ReserveNPOT(size_t reserved) { +bool Allocation::ReserveNPOT(Bytes reserved) { // Reserve at least one page of data. - reserved = std::max<size_t>(4096u, reserved); - return Reserve(NextPowerOfTwoSize(reserved)); + reserved = std::max(Bytes{4096u}, reserved); + return Reserve(Bytes{NextPowerOfTwoSize(reserved.GetByteSize())}); } -bool Allocation::Reserve(size_t reserved) { +bool Allocation::Reserve(Bytes reserved) { if (reserved <= reserved_) { return true; } - auto new_allocation = ::realloc(buffer_, reserved); + auto new_allocation = ::realloc(buffer_, reserved.GetByteSize()); if (!new_allocation) { // If new length is zero, a minimum non-zero sized allocation is returned. // So this check will not trip and this routine will indicate success as @@ -81,7 +81,7 @@ } std::shared_ptr<fml::Mapping> CreateMappingWithCopy(const uint8_t* contents, - size_t length) { + Bytes length) { if (contents == nullptr) { return nullptr; } @@ -91,7 +91,7 @@ return nullptr; } - std::memmove(allocation->GetBuffer(), contents, length); + std::memmove(allocation->GetBuffer(), contents, length.GetByteSize()); return CreateMappingFromAllocation(allocation); } @@ -103,7 +103,7 @@ } return std::make_shared<fml::NonOwnedMapping>( reinterpret_cast<const uint8_t*>(allocation->GetBuffer()), // - allocation->GetLength(), // + allocation->GetLength().GetByteSize(), // [allocation](auto, auto) {} // ); }
diff --git a/impeller/base/allocation.h b/impeller/base/allocation.h index 784c6ef..ae2337c 100644 --- a/impeller/base/allocation.h +++ b/impeller/base/allocation.h
@@ -13,45 +13,138 @@ namespace impeller { +//------------------------------------------------------------------------------ +/// @brief Describes an allocation on the heap. +/// +/// Managing allocations through this utility makes it harder to +/// miss allocation failures. +/// class Allocation { public: + //---------------------------------------------------------------------------- + /// @brief Constructs a new zero-sized allocation. + /// Allocation(); + //---------------------------------------------------------------------------- + /// @brief Destroys the allocation. + /// ~Allocation(); + //---------------------------------------------------------------------------- + /// @brief Gets the pointer to the start of the allocation. + /// + /// This pointer is only valid till the next call to `Truncate`. + /// + /// @return The pointer to the start of the allocation. + /// uint8_t* GetBuffer() const; - size_t GetLength() const; + //---------------------------------------------------------------------------- + /// @brief Gets the length of the allocation. + /// + /// @return The length. + /// + Bytes GetLength() const; - size_t GetReservedLength() const; + //---------------------------------------------------------------------------- + /// @brief Gets the reserved length of the allocation. Calls to truncate + /// may be ignored till the length exceeds the reserved length. + /// + /// @return The reserved length. + /// + Bytes GetReservedLength() const; - [[nodiscard]] bool Truncate(size_t length, bool npot = true); + //---------------------------------------------------------------------------- + /// @brief Resize the underlying allocation to at least given number of + /// bytes. + /// + /// In case of failure, false is returned and the underlying + /// allocation remains unchanged. + /// + /// @warning Pointers to buffers obtained via previous calls to `GetBuffer` + /// may become invalid at this point. + /// + /// @param[in] length The length. + /// @param[in] npot Whether to round up the length to the next power of + /// two. + /// + /// @return If the underlying allocation was resized to the new size. + /// + [[nodiscard]] bool Truncate(Bytes length, bool npot = true); + //---------------------------------------------------------------------------- + /// @brief Gets the next power of two size. + /// + /// @param[in] x The size. + /// + /// @return The next power of two of x. + /// static uint32_t NextPowerOfTwoSize(uint32_t x); private: uint8_t* buffer_ = nullptr; - size_t length_ = 0; - size_t reserved_ = 0; + Bytes length_; + Bytes reserved_; - [[nodiscard]] bool Reserve(size_t reserved); + [[nodiscard]] bool Reserve(Bytes reserved); - [[nodiscard]] bool ReserveNPOT(size_t reserved); + [[nodiscard]] bool ReserveNPOT(Bytes reserved); Allocation(const Allocation&) = delete; Allocation& operator=(const Allocation&) = delete; }; +//------------------------------------------------------------------------------ +/// @brief Creates a mapping with copy of the bytes. +/// +/// @param[in] contents The contents +/// @param[in] length The length +/// +/// @return The new mapping or nullptr if the copy could not be performed. +/// std::shared_ptr<fml::Mapping> CreateMappingWithCopy(const uint8_t* contents, - size_t length); + Bytes length); +//------------------------------------------------------------------------------ +/// @brief Creates a mapping from allocation. +/// +/// No data copy occurs. Only a reference to the underlying +/// allocation is bumped up. +/// +/// Changes to the underlying allocation will not be reflected in +/// the mapping and must not change. +/// +/// @param[in] allocation The allocation. +/// +/// @return A new mapping or nullptr if the argument allocation was invalid. +/// std::shared_ptr<fml::Mapping> CreateMappingFromAllocation( const std::shared_ptr<Allocation>& allocation); +//------------------------------------------------------------------------------ +/// @brief Creates a mapping with string data. +/// +/// Only a reference to the underlying string is bumped up and the +/// string is not copied. +/// +/// @param[in] string The string +/// +/// @return A new mapping or nullptr in case of allocation failures. +/// std::shared_ptr<fml::Mapping> CreateMappingWithString( std::shared_ptr<const std::string> string); +//------------------------------------------------------------------------------ +/// @brief Creates a mapping with string data. +/// +/// The string is copied. +/// +/// @param[in] string The string +/// +/// @return A new mapping or nullptr in case of allocation failures. +/// std::shared_ptr<fml::Mapping> CreateMappingWithString(std::string string); } // namespace impeller
diff --git a/impeller/base/allocation_size.h b/impeller/base/allocation_size.h index fd0717b..534a045 100644 --- a/impeller/base/allocation_size.h +++ b/impeller/base/allocation_size.h
@@ -5,8 +5,10 @@ #ifndef FLUTTER_IMPELLER_BASE_ALLOCATION_SIZE_H_ #define FLUTTER_IMPELLER_BASE_ALLOCATION_SIZE_H_ +#include <cmath> #include <cstddef> #include <cstdint> +#include <type_traits> namespace impeller { @@ -37,7 +39,9 @@ /// /// @param[in] size The size in `Period` number of bytes. /// - explicit constexpr AllocationSize(double size) : bytes_(size * Period) {} + template <class T, class = std::enable_if_t<std::is_arithmetic_v<T>>> + explicit constexpr AllocationSize(T size) + : bytes_(std::ceil(size) * Period) {} //---------------------------------------------------------------------------- /// @brief Create an allocation size from another instance with a @@ -158,37 +162,37 @@ // NOLINTNEXTLINE constexpr Bytes operator"" _bytes(unsigned long long int size) { - return Bytes{static_cast<double>(size)}; + return Bytes{size}; } // NOLINTNEXTLINE constexpr KiloBytes operator"" _kb(unsigned long long int size) { - return KiloBytes{static_cast<double>(size)}; + return KiloBytes{size}; } // NOLINTNEXTLINE constexpr MegaBytes operator"" _mb(unsigned long long int size) { - return MegaBytes{static_cast<double>(size)}; + return MegaBytes{size}; } // NOLINTNEXTLINE constexpr GigaBytes operator"" _gb(unsigned long long int size) { - return GigaBytes{static_cast<double>(size)}; + return GigaBytes{size}; } // NOLINTNEXTLINE constexpr KibiBytes operator"" _kib(unsigned long long int size) { - return KibiBytes{static_cast<double>(size)}; + return KibiBytes{size}; } // NOLINTNEXTLINE constexpr MebiBytes operator"" _mib(unsigned long long int size) { - return MebiBytes{static_cast<double>(size)}; + return MebiBytes{size}; } // NOLINTNEXTLINE constexpr GibiBytes operator"" _gib(unsigned long long int size) { - return GibiBytes{static_cast<double>(size)}; + return GibiBytes{size}; } } // namespace allocation_size_literals
diff --git a/impeller/base/allocation_size_unittests.cc b/impeller/base/allocation_size_unittests.cc index 370b645..dddae1e 100644 --- a/impeller/base/allocation_size_unittests.cc +++ b/impeller/base/allocation_size_unittests.cc
@@ -107,4 +107,19 @@ } } +TEST(AllocationSizeTest, CanConstructWithArith) { + { + Bytes a(1u); + ASSERT_EQ(a.GetByteSize(), 1u); + } + { + Bytes a(1.5); + ASSERT_EQ(a.GetByteSize(), 2u); + } + { + Bytes a(1.5f); + ASSERT_EQ(a.GetByteSize(), 2u); + } +} + } // namespace impeller::testing
diff --git a/impeller/playground/image/decompressed_image.cc b/impeller/playground/image/decompressed_image.cc index 70642ab..6e7f8fd 100644 --- a/impeller/playground/image/decompressed_image.cc +++ b/impeller/playground/image/decompressed_image.cc
@@ -75,7 +75,7 @@ } auto rgba_allocation = std::make_shared<Allocation>(); - if (!rgba_allocation->Truncate(size_.Area() * 4u, false)) { + if (!rgba_allocation->Truncate(Bytes{size_.Area() * 4u}, false)) { return {}; } @@ -114,9 +114,9 @@ return DecompressedImage{ size_, Format::kRGBA, std::make_shared<fml::NonOwnedMapping>( - rgba_allocation->GetBuffer(), // - rgba_allocation->GetLength(), // - [rgba_allocation](auto, auto) {}) // + rgba_allocation->GetBuffer(), // + rgba_allocation->GetLength().GetByteSize(), // + [rgba_allocation](auto, auto) {}) // }; }
diff --git a/impeller/renderer/backend/gles/allocator_gles.cc b/impeller/renderer/backend/gles/allocator_gles.cc index dc6a15c..cd062d8 100644 --- a/impeller/renderer/backend/gles/allocator_gles.cc +++ b/impeller/renderer/backend/gles/allocator_gles.cc
@@ -28,7 +28,7 @@ std::shared_ptr<DeviceBuffer> AllocatorGLES::OnCreateBuffer( const DeviceBufferDescriptor& desc) { auto backing_store = std::make_shared<Allocation>(); - if (!backing_store->Truncate(desc.size)) { + if (!backing_store->Truncate(Bytes{desc.size})) { return nullptr; } return std::make_shared<DeviceBufferGLES>(desc, //
diff --git a/impeller/renderer/backend/gles/device_buffer_gles.cc b/impeller/renderer/backend/gles/device_buffer_gles.cc index 3cbc9ea..a6a91e4 100644 --- a/impeller/renderer/backend/gles/device_buffer_gles.cc +++ b/impeller/renderer/backend/gles/device_buffer_gles.cc
@@ -46,7 +46,8 @@ return false; } - if (offset + source_range.length > backing_store_->GetLength()) { + if (offset + source_range.length > + backing_store_->GetLength().GetByteSize()) { return false; } @@ -87,9 +88,10 @@ gl.BindBuffer(target_type, buffer.value()); if (upload_generation_ != generation_) { - TRACE_EVENT1("impeller", "BufferData", "Bytes", - std::to_string(backing_store_->GetLength()).c_str()); - gl.BufferData(target_type, backing_store_->GetLength(), + TRACE_EVENT1( + "impeller", "BufferData", "Bytes", + std::to_string(backing_store_->GetLength().GetByteSize()).c_str()); + gl.BufferData(target_type, backing_store_->GetLength().GetByteSize(), backing_store_->GetBuffer(), GL_STATIC_DRAW); upload_generation_ = generation_; } @@ -119,7 +121,7 @@ update_buffer_data) { if (update_buffer_data) { update_buffer_data(backing_store_->GetBuffer(), - backing_store_->GetLength()); + backing_store_->GetLength().GetByteSize()); ++generation_; } }
diff --git a/impeller/renderer/backend/gles/proc_table_gles.cc b/impeller/renderer/backend/gles/proc_table_gles.cc index 8aeda6d..0ae8933 100644 --- a/impeller/renderer/backend/gles/proc_table_gles.cc +++ b/impeller/renderer/backend/gles/proc_table_gles.cc
@@ -407,7 +407,7 @@ length = std::min<GLint>(length, 1024); Allocation allocation; - if (!allocation.Truncate(length, false)) { + if (!allocation.Truncate(Bytes{length}, false)) { return ""; } GetProgramInfoLog(program, // program
diff --git a/impeller/renderer/backend/gles/texture_gles.cc b/impeller/renderer/backend/gles/texture_gles.cc index 7df092c..9ca4561 100644 --- a/impeller/renderer/backend/gles/texture_gles.cc +++ b/impeller/renderer/backend/gles/texture_gles.cc
@@ -201,7 +201,7 @@ bool TextureGLES::OnSetContents(const uint8_t* contents, size_t length, size_t slice) { - return OnSetContents(CreateMappingWithCopy(contents, length), slice); + return OnSetContents(CreateMappingWithCopy(contents, Bytes{length}), slice); } // |Texture|
diff --git a/impeller/renderer/backend/metal/allocator_mtl.mm b/impeller/renderer/backend/metal/allocator_mtl.mm index 834a529..75aea92 100644 --- a/impeller/renderer/backend/metal/allocator_mtl.mm +++ b/impeller/renderer/backend/metal/allocator_mtl.mm
@@ -100,8 +100,7 @@ } Bytes DebugAllocatorStats::GetAllocationSize() { - // RAM is measured in MiB, thus a divisor of 2^20 instead of 1,000,000. - return Bytes{static_cast<double>(size_)}; + return Bytes{size_.load()}; } AllocatorMTL::AllocatorMTL(id<MTLDevice> device, std::string label)
diff --git a/impeller/runtime_stage/runtime_stage_unittests.cc b/impeller/runtime_stage/runtime_stage_unittests.cc index 484fe45..8d6e466 100644 --- a/impeller/runtime_stage/runtime_stage_unittests.cc +++ b/impeller/runtime_stage/runtime_stage_unittests.cc
@@ -43,10 +43,11 @@ flutter::testing::OpenFixtureAsMapping("ink_sparkle.frag.iplr"); ASSERT_TRUE(fixture); auto junk_allocation = std::make_shared<Allocation>(); - ASSERT_TRUE(junk_allocation->Truncate(fixture->GetSize(), false)); + ASSERT_TRUE(junk_allocation->Truncate(Bytes{fixture->GetSize()}, false)); // Not meant to be secure. Just reject obviously bad blobs using magic // numbers. - ::memset(junk_allocation->GetBuffer(), 127, junk_allocation->GetLength()); + ::memset(junk_allocation->GetBuffer(), 127, + junk_allocation->GetLength().GetByteSize()); auto stages = RuntimeStage::DecodeRuntimeStages( CreateMappingFromAllocation(junk_allocation)); ASSERT_FALSE(stages[PlaygroundBackendToRuntimeStageBackend(GetBackend())]);