Skip to content

Commit a06c270

Browse files
committed
ICPUBuffer v2.0
- Replaced all wrappers usec-ases with new constructors and polymorphic memory resources - Allocation failures can now be properly checked - Don't re-allocate for shader cache writes Signed-off-by: Ali Cheraghi <[email protected]>
1 parent 08f54ba commit a06c270

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

54 files changed

+251
-280
lines changed

include/nbl/asset/CVectorCPUBuffer.h

Lines changed: 0 additions & 34 deletions
This file was deleted.

include/nbl/asset/ICPUBuffer.h

Lines changed: 71 additions & 116 deletions
Original file line numberDiff line numberDiff line change
@@ -6,73 +6,91 @@
66

77
#include <type_traits>
88

9-
#include "nbl/core/alloc/null_allocator.h"
10-
119
#include "nbl/asset/IBuffer.h"
1210
#include "nbl/asset/IAsset.h"
1311
#include "nbl/asset/IPreHashed.h"
1412

1513
namespace nbl::asset
1614
{
1715

16+
struct SICPUBufferCreationParams
17+
{
18+
size_t size;
19+
void* data = nullptr;
20+
size_t alignment = _NBL_SIMD_ALIGNMENT;
21+
std::pmr::memory_resource* memoryResource = nullptr;
22+
};
23+
1824
//! One of CPU class-object representing an Asset
25+
//! TODO: remove, alloc can fail, should be a static create method instead!
1926
/**
2027
One of Assets used for storage of large arrays, so that storage can be decoupled
2128
from other objects such as meshbuffers, images, animations and shader source/bytecode.
2229
2330
@see IAsset
2431
*/
25-
class ICPUBuffer : public asset::IBuffer, public IPreHashed
32+
class ICPUBuffer final : public asset::IBuffer, public IPreHashed
2633
{
27-
protected:
28-
//! Non-allocating constructor for CCustormAllocatorCPUBuffer derivative
29-
ICPUBuffer(size_t sizeInBytes, void* dat) : asset::IBuffer({ dat ? sizeInBytes : 0,EUF_TRANSFER_DST_BIT }), data(dat) {}
30-
3134
public:
32-
//! Constructor. TODO: remove, alloc can fail, should be a static create method instead!
33-
/** @param sizeInBytes Size in bytes. If `dat` argument is present, it denotes size of data pointed by `dat`, otherwise - size of data to be allocated.
34-
*/
35-
ICPUBuffer(size_t sizeInBytes) : asset::IBuffer({0,EUF_TRANSFER_DST_BIT})
36-
{
37-
data = _NBL_ALIGNED_MALLOC(sizeInBytes,_NBL_SIMD_ALIGNMENT);
38-
if (!data) // FIXME: cannot fail like that, need factory `create` methods
39-
return;
35+
ICPUBuffer(size_t size, void* data, std::pmr::memory_resource* memoryResource, size_t alignment, bool adopt_memory) :
36+
asset::IBuffer({ size, EUF_TRANSFER_DST_BIT }), m_data(data), m_mem_resource(memoryResource), m_alignment(alignment), m_adopt_memory(adopt_memory) {}
4037

41-
m_creationParams.size = sizeInBytes;
38+
//! allocates uninitialized memory, copies `data` into allocation if `!data` not nullptr
39+
core::smart_refctd_ptr<ICPUBuffer> static create(const SICPUBufferCreationParams& params) {
40+
std::pmr::memory_resource* memoryResource = params.memoryResource;
41+
if (!params.memoryResource)
42+
memoryResource = std::pmr::get_default_resource();
43+
44+
auto data = memoryResource->allocate(params.size, params.alignment);
45+
if (!data)
46+
return nullptr;
47+
48+
if (params.data)
49+
memcpy(data, params.data, params.size);
50+
51+
return core::make_smart_refctd_ptr<ICPUBuffer>(params.size, data, memoryResource, params.alignment, false);
52+
}
53+
54+
//! does not allocate memory, adopts the `data` pointer, no copies done
55+
core::smart_refctd_ptr<ICPUBuffer> static create(const SICPUBufferCreationParams& params, core::adopt_memory_t) {
56+
std::pmr::memory_resource* memoryResource;
57+
if (!params.memoryResource)
58+
memoryResource = std::pmr::get_default_resource();
59+
return core::make_smart_refctd_ptr<ICPUBuffer>(params.size, params.data, memoryResource, params.alignment, true);
4260
}
4361

4462
core::smart_refctd_ptr<IAsset> clone(uint32_t = ~0u) const override final
4563
{
46-
auto cp = core::make_smart_refctd_ptr<ICPUBuffer>(m_creationParams.size);
47-
memcpy(cp->getPointer(), data, m_creationParams.size);
64+
auto cp = create({ m_creationParams.size, m_data });
65+
memcpy(cp->getPointer(), m_data, m_creationParams.size);
4866
cp->setContentHash(getContentHash());
4967
return cp;
5068
}
5169

5270
constexpr static inline auto AssetType = ET_BUFFER;
5371
inline IAsset::E_TYPE getAssetType() const override final { return AssetType; }
5472

55-
inline size_t getDependantCount() const override {return 0;}
73+
inline size_t getDependantCount() const override { return 0; }
5674

5775
//
5876
inline core::blake3_hash_t computeContentHash() const override
5977
{
60-
core::blake3_hasher hasher;
61-
if (data)
62-
hasher.update(data,m_creationParams.size);
63-
return static_cast<core::blake3_hash_t>(hasher);
78+
core::blake3_hasher hasher;
79+
if (m_data)
80+
hasher.update(m_data, m_creationParams.size);
81+
return static_cast<core::blake3_hash_t>(hasher);
6482
}
6583

66-
inline bool missingContent() const override {return !data;}
84+
inline bool missingContent() const override { return !m_data; }
6785

6886
//! Returns pointer to data.
69-
const void* getPointer() const {return data;}
70-
void* getPointer()
71-
{
87+
const void* getPointer() const { return m_data; }
88+
void* getPointer()
89+
{
7290
assert(isMutable());
73-
return data;
91+
return m_data;
7492
}
75-
93+
7694
inline core::bitflag<E_USAGE_FLAGS> getUsageFlags() const
7795
{
7896
return m_creationParams.usage;
@@ -90,93 +108,30 @@ class ICPUBuffer : public asset::IBuffer, public IPreHashed
90108
return true;
91109
}
92110

93-
protected:
94-
inline IAsset* getDependant_impl(const size_t ix) override
95-
{
96-
return nullptr;
97-
}
98-
99-
inline void discardContent_impl() override
100-
{
101-
return freeData();
102-
}
103-
104-
// REMEMBER TO CALL FROM DTOR!
105-
// TODO: idea, make the `ICPUBuffer` an ADT, and use the default allocator CCPUBuffer instead for consistency
106-
// TODO: idea make a macro for overriding all `delete` operators of a class to enforce a finalizer that runs in reverse order to destructors (to allow polymorphic cleanups)
107-
virtual inline void freeData()
108-
{
109-
if (data)
110-
_NBL_ALIGNED_FREE(data);
111-
data = nullptr;
112-
m_creationParams.size = 0ull;
113-
}
114-
115-
void* data;
116-
};
117-
118-
119-
template<
120-
typename Allocator = _NBL_DEFAULT_ALLOCATOR_METATYPE<uint8_t>,
121-
bool = std::is_same<Allocator, core::null_allocator<typename Allocator::value_type> >::value
122-
>
123-
class CCustomAllocatorCPUBuffer;
124-
125-
using CDummyCPUBuffer = CCustomAllocatorCPUBuffer<core::null_allocator<uint8_t>, true>;
126-
127-
//! Specialization of ICPUBuffer capable of taking custom allocators
128-
/*
129-
Take a look that with this usage you have to specify custom alloctor
130-
passing an object type for allocation and a pointer to allocated
131-
data for it's storage by ICPUBuffer.
132-
133-
So the need for the class existence is for common following tricks - among others creating an
134-
\bICPUBuffer\b over an already existing \bvoid*\b array without any \imemcpy\i or \itaking over the memory ownership\i.
135-
You can use it with a \bnull_allocator\b that adopts memory (it is a bit counter intuitive because \badopt = take\b ownership,
136-
but a \inull allocator\i doesn't do anything, even free the memory, so you're all good).
137-
*/
138-
139-
template<typename Allocator>
140-
class CCustomAllocatorCPUBuffer<Allocator,true> : public ICPUBuffer
141-
{
142-
static_assert(sizeof(typename Allocator::value_type) == 1u, "Allocator::value_type must be of size 1");
143-
protected:
144-
Allocator m_allocator;
145-
146-
virtual ~CCustomAllocatorCPUBuffer() final
147-
{
148-
freeData();
149-
}
150-
inline void freeData() override
151-
{
152-
if (ICPUBuffer::data)
153-
m_allocator.deallocate(reinterpret_cast<typename Allocator::pointer>(ICPUBuffer::data), ICPUBuffer::m_creationParams.size);
154-
ICPUBuffer::data = nullptr; // so that ICPUBuffer won't try deallocating
155-
}
156-
157-
public:
158-
CCustomAllocatorCPUBuffer(size_t sizeInBytes, void* dat, core::adopt_memory_t, Allocator&& alctr = Allocator()) : ICPUBuffer(sizeInBytes,dat), m_allocator(std::move(alctr))
159-
{
160-
}
161-
};
162-
163-
template<typename Allocator>
164-
class CCustomAllocatorCPUBuffer<Allocator, false> : public CCustomAllocatorCPUBuffer<Allocator, true>
165-
{
166-
using Base = CCustomAllocatorCPUBuffer<Allocator, true>;
167-
168-
protected:
169-
virtual ~CCustomAllocatorCPUBuffer() = default;
170-
inline void freeData() override {}
171-
172-
public:
173-
using Base::Base;
174-
175-
// TODO: remove, alloc can fail, should be a static create method instead!
176-
CCustomAllocatorCPUBuffer(size_t sizeInBytes, const void* dat, Allocator&& alctr = Allocator()) : Base(sizeInBytes, alctr.allocate(sizeInBytes), core::adopt_memory, std::move(alctr))
177-
{
178-
memcpy(Base::data,dat,sizeInBytes);
179-
}
111+
protected:
112+
inline IAsset* getDependant_impl(const size_t ix) override
113+
{
114+
return nullptr;
115+
}
116+
117+
inline void discardContent_impl() override
118+
{
119+
return freeData();
120+
}
121+
122+
// REMEMBER TO CALL FROM DTOR!
123+
virtual inline void freeData()
124+
{
125+
if (!m_adopt_memory && m_data)
126+
m_mem_resource->deallocate(m_data, m_creationParams.size, m_alignment);
127+
m_data = nullptr;
128+
m_creationParams.size = 0ull;
129+
}
130+
131+
void* m_data;
132+
std::pmr::memory_resource* m_mem_resource;
133+
size_t m_alignment;
134+
bool m_adopt_memory;
180135
};
181136

182137
} // end namespace nbl::asset

include/nbl/asset/ICPUShader.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ class ICPUShader : public IAsset, public IShader
3333
: IShader(stage, std::move(filepathHint)), m_code(std::move(code)), m_contentType(contentType) {}
3434

3535
ICPUShader(const char* code, const E_SHADER_STAGE stage, const E_CONTENT_TYPE contentType, std::string&& filepathHint)
36-
: ICPUShader(core::make_smart_refctd_ptr<ICPUBuffer>(strlen(code) + 1u), stage, contentType, std::move(filepathHint))
36+
: ICPUShader(ICPUBuffer::create({ strlen(code) + 1u }), stage, contentType, std::move(filepathHint))
3737
{
3838
assert(contentType != E_CONTENT_TYPE::ECT_SPIRV); // because using strlen needs `code` to be null-terminated
3939
memcpy(m_code->getPointer(), code, m_code->getSize());

include/nbl/asset/filters/CFlattenRegionsImageFilter.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ class CFlattenRegionsImageFilter : public CImageFilter<CFlattenRegionsImageFilte
8989
assert(memsize.getNumerator()%memsize.getDenominator()==0u);
9090
bufferSize += memsize.getIntegerApprox();
9191
}
92-
auto buffer = core::make_smart_refctd_ptr<ICPUBuffer>(bufferSize);
92+
auto buffer = ICPUBuffer::create({ bufferSize });
9393
state->outImage->setBufferAndRegions(std::move(buffer),std::move(regions));
9494
};
9595

include/nbl/asset/filters/dithering/CPrecomputedDither.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ namespace nbl
5858
const size_t newDecodeBufferSize = extent.x * extent.y * extent.z * creationParams.arrayLayers * decodeTexelByteSize;
5959

6060
const core::vector3du32_SIMD decodeBufferByteStrides = TexelBlockInfo(decodeFormat).convert3DTexelStridesTo1DByteStrides(core::vector3du32_SIMD(extent.x, extent.y, extent.z));
61-
auto decodeFlattenBuffer = core::make_smart_refctd_ptr<ICPUBuffer>(newDecodeBufferSize);
61+
auto decodeFlattenBuffer = ICPUBuffer::create({ newDecodeBufferSize });
6262
decodeFlattenBuffer->setContentHash(IPreHashed::INVALID_HASH);
6363

6464
auto* inData = reinterpret_cast<uint8_t*>(flattenDitheringImage->getBuffer()->getPointer());

include/nbl/asset/interchange/IImageAssetHandlerBase.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ class IImageAssetHandlerBase : public virtual core::IReferenceCounted
110110
bufferSize += memsize.getIntegerApprox();
111111
}
112112

113-
auto texelBuffer = core::make_smart_refctd_ptr<ICPUBuffer>(bufferSize);
113+
auto texelBuffer = ICPUBuffer::create({ bufferSize });
114114
newImage->setBufferAndRegions(std::move(texelBuffer), newRegions);
115115
newImage->setContentHash(IPreHashed::INVALID_HASH);
116116
}

include/nbl/asset/utils/CCPUMeshPackerV1.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -296,11 +296,11 @@ void CCPUMeshPackerV1<MDIStructType>::instantiateDataStorage()
296296
const size_t vtxBuffSupportedByteSize = base_t::alctrTraits::get_total_size(base_t::m_vtxBuffAlctr);
297297
const size_t perInsBuffSupportedByteSize = base_t::alctrTraits::get_total_size(base_t::m_vtxBuffAlctr);
298298

299-
m_output.MDIDataBuffer = core::make_smart_refctd_ptr<ICPUBuffer>(MDIDataBuffSupportedByteSize);
300-
m_output.indexBuffer.buffer = core::make_smart_refctd_ptr<ICPUBuffer>(idxBuffSupportedByteSize);
299+
m_output.MDIDataBuffer = ICPUBuffer::create({ MDIDataBuffSupportedByteSize });
300+
m_output.indexBuffer.buffer = ICPUBuffer::create({ idxBuffSupportedByteSize });
301301

302-
core::smart_refctd_ptr<ICPUBuffer> unifiedVtxBuff = core::make_smart_refctd_ptr<ICPUBuffer>(vtxBuffSupportedByteSize);
303-
core::smart_refctd_ptr<ICPUBuffer> unifiedInsBuff = core::make_smart_refctd_ptr<ICPUBuffer>(perInsBuffSupportedByteSize);
302+
core::smart_refctd_ptr<ICPUBuffer> unifiedVtxBuff = ICPUBuffer::create({ vtxBuffSupportedByteSize });
303+
core::smart_refctd_ptr<ICPUBuffer> unifiedInsBuff = ICPUBuffer::create({ perInsBuffSupportedByteSize });
304304

305305
//divide unified vtx buffers
306306
//proportions: sizeOfAttr1 : sizeOfAttr2 : ... : sizeOfAttrN

include/nbl/asset/utils/CCPUMeshPackerV2.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,9 +61,9 @@ void CCPUMeshPackerV2<MDIStructType>::instantiateDataStorage()
6161
const uint32_t idxBuffByteSize = base_t::m_idxBuffAlctr.get_total_size() * sizeof(uint16_t);
6262
const uint32_t vtxBuffByteSize = base_t::m_vtxBuffAlctr.get_total_size();
6363

64-
base_t::m_packerDataStore.MDIDataBuffer = core::make_smart_refctd_ptr<ICPUBuffer>(MDIDataBuffByteSize);
65-
base_t::m_packerDataStore.indexBuffer = core::make_smart_refctd_ptr<ICPUBuffer>(idxBuffByteSize);
66-
base_t::m_packerDataStore.vertexBuffer = core::make_smart_refctd_ptr<ICPUBuffer>(vtxBuffByteSize);
64+
base_t::m_packerDataStore.MDIDataBuffer = ICPUBuffer::create({ MDIDataBuffByteSize });
65+
base_t::m_packerDataStore.indexBuffer = ICPUBuffer::create({ idxBuffByteSize });
66+
base_t::m_packerDataStore.vertexBuffer = ICPUBuffer::create({ vtxBuffByteSize });
6767
}
6868

6969
/*

include/nbl/asset/utils/CDirQuantCacheBase.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -297,7 +297,7 @@ class CDirQuantCacheBase : public impl::CDirQuantCacheBase
297297
if (!file)
298298
return false;
299299

300-
auto buffer = core::make_smart_refctd_ptr<asset::ICPUBuffer>(file->getSize());
300+
auto buffer = asset::ICPUBuffer::create({ file->getSize() });
301301

302302
system::IFile::success_t succ;
303303
file->read(succ, buffer->getPointer(), 0, file->getSize());
@@ -346,7 +346,7 @@ class CDirQuantCacheBase : public impl::CDirQuantCacheBase
346346
asset::SBufferRange<asset::ICPUBuffer> bufferRange;
347347
bufferRange.offset = 0;
348348
bufferRange.size = getSerializedCacheSizeInBytes<CacheFormat>();
349-
bufferRange.buffer = core::make_smart_refctd_ptr<asset::ICPUBuffer>(bufferRange.size);
349+
bufferRange.buffer = asset::ICPUBuffer::create({ bufferRange.size });
350350

351351
saveCacheToBuffer<CacheFormat>(bufferRange);
352352

include/nbl/asset/utils/IMeshManipulator.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -557,7 +557,7 @@ class NBL_API2 IMeshManipulator : public virtual core::IReferenceCounted
557557
core::smart_refctd_ptr<ICPUBuffer> iotaUint32Buffer;
558558
if (iotaLength)
559559
{
560-
iotaUint32Buffer = core::make_smart_refctd_ptr<ICPUBuffer>(sizeof(uint32_t)*iotaLength);
560+
iotaUint32Buffer = ICPUBuffer::create({ sizeof(uint32_t)*iotaLength });
561561
auto ptr = reinterpret_cast<uint32_t*>(iotaUint32Buffer->getPointer());
562562
std::iota(ptr,ptr+iotaLength,0u);
563563
}
@@ -599,13 +599,13 @@ class NBL_API2 IMeshManipulator : public virtual core::IReferenceCounted
599599
if (indexType==EIT_16BIT)
600600
{
601601
auto inPtr = reinterpret_cast<const uint16_t*>(correctlyOffsetIndexBufferPtr);
602-
newIndexBuffer = core::make_smart_refctd_ptr<ICPUBuffer>(sizeof(uint32_t)*indexCount);
602+
newIndexBuffer = ICPUBuffer::create({ sizeof(uint32_t)*indexCount });
603603
std::copy(inPtr,inPtr+indexCount,reinterpret_cast<uint32_t*>(newIndexBuffer->getPointer()));
604604
}
605605
else
606606
{
607607
auto inPtr = reinterpret_cast<const uint32_t*>(correctlyOffsetIndexBufferPtr);
608-
newIndexBuffer = core::make_smart_refctd_ptr<ICPUBuffer>(sizeof(uint16_t)*indexCount);
608+
newIndexBuffer = ICPUBuffer::create({ sizeof(uint16_t)*indexCount });
609609
std::copy(inPtr,inPtr+indexCount,reinterpret_cast<uint16_t*>(newIndexBuffer->getPointer()));
610610
}
611611
}

0 commit comments

Comments
 (0)