asset: fix several bugs

alichraghi · alichraghi · commit cf9a86662331 · 2024-11-15T00:53:50.000+03:30
Signed-off-by: Ali Cheraghi &lt;alichraghi@proton.me&gt;
diff --git a/include/nbl/asset/ICPUBuffer.h b/include/nbl/asset/ICPUBuffer.h
@@ -40,7 +40,8 @@ class ICPUBuffer final : public asset::IBuffer, public IPreHashed
         };
 
         //! allocates uninitialized memory, copies `data` into allocation if `!data` not nullptr
-        core::smart_refctd_ptr<ICPUBuffer> static create(SCreationParams&& params) {
+        core::smart_refctd_ptr<ICPUBuffer> static create(SCreationParams&& params)
+        {
             if (!params.memoryResource)
                 params.memoryResource = core::getDefaultMemoryResource();
 
@@ -55,7 +56,8 @@ class ICPUBuffer final : public asset::IBuffer, public IPreHashed
         }
 
         //! does not allocate memory, adopts the `data` pointer, no copies done
-        core::smart_refctd_ptr<ICPUBuffer> static create(SCreationParams&& params, core::adopt_memory_t) {
+        core::smart_refctd_ptr<ICPUBuffer> static create(SCreationParams&& params, core::adopt_memory_t)
+        {
             if (!params.data)
                 return nullptr;
             if (!params.memoryResource)
@@ -122,6 +124,7 @@ class ICPUBuffer final : public asset::IBuffer, public IPreHashed
         if (m_data)
             m_mem_resource->deallocate(m_data, m_creationParams.size, m_alignment);
         m_data = nullptr;
+        m_mem_resource = nullptr;
         m_creationParams.size = 0ull;
     }
 
@@ -131,8 +134,7 @@ class ICPUBuffer final : public asset::IBuffer, public IPreHashed
         m_mem_resource(params.memoryResource), m_alignment(params.alignment) {}
 
     ~ICPUBuffer() override {
-        if (m_data)
-            m_mem_resource->deallocate(m_data, m_creationParams.size, m_alignment);
+        discardContent_impl();
     }
 
     void* m_data;
diff --git a/include/nbl/asset/ICPUMeshBuffer.h b/include/nbl/asset/ICPUMeshBuffer.h
@@ -544,7 +544,7 @@ class ICPUMeshBuffer final : public IMeshBuffer<ICPUBuffer,ICPUDescriptorSet,ICP
             const bool scaled = isScaledFormat(format);
             if (!dst || !(scaled || isIntegerFormat(format)))
                 return false;
-            uint8_t* vxPtr = (uint8_t*)dst;
+            uint8_t* vxPtr = reinterpret_cast<uint8_t*>(dst);
 
             if (isSignedFormat(format))
             {
diff --git a/include/nbl/core/alloc/VectorViewNullMemoryResource.h b/include/nbl/core/alloc/VectorViewNullMemoryResource.h
@@ -17,29 +17,34 @@ class VectorViewNullMemoryResource : public std::pmr::memory_resource
 {
     public:
         // only create the resource from an already sized vector
-        VectorViewNullMemoryResource(core::vector<uint8_t>&& buffer) : buffer(buffer)
+        VectorViewNullMemoryResource(core::vector<uint8_t>&& buffer) : buffer(std::move(buffer)), already_called(false)
         {
             assert(buffer.size());
-        };
+        }
 
         void* data() {
             return buffer.data();
         }
 
     protected:
         void* do_allocate(size_t bytes, size_t alignment) override {
-            assert(bytes <= buffer.size());
+            if (already_called || bytes > buffer.size() || !core::is_aligned_to(bytes, alignment))
+                return nullptr;
+            already_called = true;
             return buffer.data();
         }
 
-        void do_deallocate(void* p, size_t bytes, size_t alignment) override {}
+        void do_deallocate(void* p, size_t bytes, size_t alignment) override {
+            assert(p == buffer.data());
+        }
 
         bool do_is_equal(const std::pmr::memory_resource& other) const noexcept override {
             return this == &other;
         }
 
     private:
         core::vector<uint8_t> buffer;
+        bool already_called;
 };
 
 }
diff --git a/include/nbl/core/alloc/refctd_memory_resource.h b/include/nbl/core/alloc/refctd_memory_resource.h
@@ -11,12 +11,10 @@
 
 #include <memory_resource>
 
-using namespace nbl;
-
 namespace nbl::core
 {
 
-class refctd_memory_resource : public core::IReferenceCounted
+class refctd_memory_resource : public IReferenceCounted
 {
     public:
         refctd_memory_resource(std::pmr::memory_resource* pmr) : m_pmr(pmr) {};
diff --git a/src/nbl/asset/ICPUImage.cpp b/src/nbl/asset/ICPUImage.cpp
@@ -122,7 +122,7 @@ class CFlattenRegionsStreamHashImageFilter : public CMatchedSizeInOutImageFilter
 
 		auto getScratchAsBuffer = [&memory = state->scratch.memory](size_t size, size_t offset = 0ull)
 		{
-				return ICPUBuffer::create({ .size = size, .data = (uint8_t*)memory + offset, .memoryResource = core::getNullMemoryResource() }, core::adopt_memory); // adopt memory & don't free it on exit
+				return ICPUBuffer::create({ .size = size, .data = reinterpret_cast<uint8_t*>(memory) + offset, .memoryResource = core::getNullMemoryResource() }, core::adopt_memory); // adopt memory & don't free it on exit
 		};
 
 		/*
@@ -303,7 +303,7 @@ core::blake3_hash_t ICPUImage::computeContentHash() const
 
 	// TODO, Arek: Matt if you want this templated then assign me to update instead of hardcoding or change it yourself otherwise delete this comment
 	const bool passed = filter.execute(std::execution::par, &state);
-	_NBL_DELETE_ARRAY((uint8_t*)state.scratch.memory, state.scratch.size);
+	_NBL_DELETE_ARRAY(reinterpret_cast<uint8_t*>(state.scratch.memory), state.scratch.size);
 	assert(passed); // actually this should never fail, leaving in case
 
 	return state.outHash;
diff --git a/src/nbl/asset/interchange/CImageWriterJPG.cpp b/src/nbl/asset/interchange/CImageWriterJPG.cpp
@@ -160,7 +160,7 @@ static bool writeJPEGFile(system::IFile* file, system::ISystem* sys, const asset
 		JSAMPROW row_pointer[1];      /* pointer to JSAMPLE row[s] */
 		row_pointer[0] = dest;
 		
-		uint8_t* src = (uint8_t*)convertedImage->getBuffer()->getPointer();
+		uint8_t* src = reinterpret_cast<uint8_t*>(convertedImage->getBuffer()->getPointer());
 		
 		/* Switch up, write from bottom -> top because the texture is flipped from OpenGL side */
 		uint32_t eof = cinfo.image_height * cinfo.image_width * cinfo.input_components;
diff --git a/src/nbl/asset/interchange/CImageWriterPNG.cpp b/src/nbl/asset/interchange/CImageWriterPNG.cpp
@@ -177,7 +177,7 @@ bool CImageWriterPNG::writeAsset(system::IFile* _file, const SAssetWriteParams&
 			}
 	}
 	
-	uint8_t* data = (uint8_t*)convertedImage->getBuffer()->getPointer();
+	uint8_t* data = reinterpret_cast<uint8_t*>(convertedImage->getBuffer()->getPointer());
 
 	constexpr uint32_t maxPNGFileHeight = 16u * 1024u; // arbitrary limit
 	if (trueExtent.Y>maxPNGFileHeight)
diff --git a/src/nbl/asset/interchange/CImageWriterTGA.cpp b/src/nbl/asset/interchange/CImageWriterTGA.cpp
@@ -120,7 +120,7 @@ bool CImageWriterTGA::writeAsset(system::IFile* _file, const SAssetWriteParams&
 			return false;
 	}
 
-	uint8_t* scan_lines = (uint8_t*)convertedImage->getBuffer()->getPointer();
+	uint8_t* scan_lines = reinterpret_cast<uint8_t*>(convertedImage->getBuffer()->getPointer());
 	if (!scan_lines)
 		return false;
 
diff --git a/src/nbl/asset/interchange/CSTLMeshFileLoader.cpp b/src/nbl/asset/interchange/CSTLMeshFileLoader.cpp
@@ -283,7 +283,7 @@ SAssetBundle CSTLMeshFileLoader::loadAsset(system::IFile* _file, const IAssetLoa
 	{
 		if (i % 3 == 0)
 			normal = quantNormalCache->quantize<EF_A2B10G10R10_SNORM_PACK32>(normals[i / 3]);
-		uint8_t* ptr = ((uint8_t*)(vertexBuf->getPointer())) + i * vtxSize;
+		uint8_t* ptr = (reinterpret_cast<uint8_t*>(vertexBuf->getPointer())) + i * vtxSize;
 		memcpy(ptr, positions[i].pointer, 3 * 4);
 
 		*reinterpret_cast<quant_normal_t*>(ptr + 12) = normal;
diff --git a/src/nbl/asset/utils/CMeshManipulator.cpp b/src/nbl/asset/utils/CMeshManipulator.cpp
@@ -444,7 +444,7 @@ static bool cmpVertices(ICPUMeshBuffer* _inbuf, const void* _va, const void* _vb
 
     constexpr uint32_t MAX_ATTRIBS = ICPUMeshBuffer::MAX_VERTEX_ATTRIB_COUNT;
 
-    const uint8_t* va = (uint8_t*)_va, *vb = (uint8_t*)_vb;
+    const uint8_t* va = reinterpret_cast<const uint8_t*>(_va), *vb = reinterpret_cast<const uint8_t*>(_vb);
     for (size_t i = 0u; i < MAX_ATTRIBS; ++i)
     {
         if (!_inbuf->isAttributeEnabled(i))
@@ -517,7 +517,7 @@ core::smart_refctd_ptr<ICPUMeshBuffer> IMeshManipulator::createMeshBufferWelded(
 
     uint32_t maxRedirect = 0;
 
-    uint8_t* epicData = (uint8_t*)_NBL_ALIGNED_MALLOC(vertexSize*vertexCount,_NBL_SIMD_ALIGNMENT);
+    uint8_t* epicData = reinterpret_cast<uint8_t*>(_NBL_ALIGNED_MALLOC(vertexSize*vertexCount,_NBL_SIMD_ALIGNMENT));
     for (auto i=0u; i<vertexCount; i++)
     {
         uint8_t* currentVertexPtr = epicData+i*vertexSize;
@@ -750,8 +750,8 @@ core::smart_refctd_ptr<ICPUMeshBuffer> IMeshManipulator::createOptimizedMeshBuff
             const size_t bufsz = outbuffer->getAttribBoundBuffer(posId).buffer->getSize();
 
 			const size_t vertexSize = pipeline->getCachedCreationParams().vertexInput.bindings[0].stride;
-			uint8_t* const v = (uint8_t*)(outbuffer->getAttribBoundBuffer(posId).buffer->getPointer()); // after prefetch optim. we have guarantee of single vertex buffer so we can do like this
-			uint8_t* const vCopy = (uint8_t*)_NBL_ALIGNED_MALLOC(bufsz, _NBL_SIMD_ALIGNMENT);
+			uint8_t* const v = reinterpret_cast<uint8_t*>(outbuffer->getAttribBoundBuffer(posId).buffer->getPointer()); // after prefetch optim. we have guarantee of single vertex buffer so we can do like this
+			uint8_t* const vCopy = reinterpret_cast<uint8_t*>(_NBL_ALIGNED_MALLOC(bufsz, _NBL_SIMD_ALIGNMENT));
 			memcpy(vCopy, v, bufsz);
 
 			size_t baseVtx = outbuffer->getBaseVertex();
@@ -894,7 +894,7 @@ void CMeshManipulator::_filterInvalidTriangles(ICPUMeshBuffer* _input)
     struct Triangle
     {
         IdxT i[3];
-    } *const begin = (Triangle*)copy, *const end = (Triangle*)((uint8_t*)copy + size);
+    } *const begin = (Triangle*)copy, *const end = (Triangle*)(reinterpret_cast<uint8_t*>(copy) + size);
 
     Triangle* const newEnd = std::remove_if(begin, end,
         [&_input](const Triangle& _t) {
diff --git a/src/nbl/asset/utils/IShaderCompiler.cpp b/src/nbl/asset/utils/IShaderCompiler.cpp
@@ -155,7 +155,7 @@ auto IShaderCompiler::CIncludeFinder::getIncludeStandard(const system::path& req
 
 
     core::blake3_hasher hasher;
-    hasher.update((uint8_t*)(retVal.contents.data()), retVal.contents.size() * (sizeof(char) / sizeof(uint8_t)));
+    hasher.update(reinterpret_cast<uint8_t*>(retVal.contents.data()), retVal.contents.size() * (sizeof(char) / sizeof(uint8_t)));
     retVal.hash = static_cast<core::blake3_hash_t>(hasher);
     return retVal;
 }
@@ -171,7 +171,7 @@ auto IShaderCompiler::CIncludeFinder::getIncludeRelative(const system::path& req
     else retVal = std::move(trySearchPaths(includeName));
 
     core::blake3_hasher hasher;
-    hasher.update((uint8_t*)(retVal.contents.data()), retVal.contents.size() * (sizeof(char) / sizeof(uint8_t)));
+    hasher.update(reinterpret_cast<uint8_t*>(retVal.contents.data()), retVal.contents.size() * (sizeof(char) / sizeof(uint8_t)));
     retVal.hash = static_cast<core::blake3_hash_t>(hasher);
     return retVal;
 }
@@ -327,7 +327,8 @@ core::smart_refctd_ptr<ICPUBuffer> IShaderCompiler::CCache::serialize() const
     uint64_t dumpedContainerJsonLength = dumpedContainerJson.size();
 
     // Create a buffer able to hold all shaders + the containerJson
-    core::vector<uint8_t> retVal(shaderBufferSize + SHADER_BUFFER_SIZE_BYTES + dumpedContainerJsonLength);
+    size_t retValSize = shaderBufferSize + SHADER_BUFFER_SIZE_BYTES + dumpedContainerJsonLength;
+    core::vector<uint8_t> retVal(retValSize);
 
     // first SHADER_BUFFER_SIZE_BYTES (8) in the buffer are the size of the shader buffer
     memcpy(retVal.data(), &shaderBufferSize, SHADER_BUFFER_SIZE_BYTES);
@@ -343,7 +344,7 @@ core::smart_refctd_ptr<ICPUBuffer> IShaderCompiler::CCache::serialize() const
     memcpy(retVal.data() + SHADER_BUFFER_SIZE_BYTES + shaderBufferSize, dumpedContainerJson.data(), dumpedContainerJsonLength);
 
     auto memoryResource = new core::VectorViewNullMemoryResource(std::move(retVal));
-    return ICPUBuffer::create({ .size = retVal.size(), .data = retVal.data(), .memoryResource = core::make_smart_refctd_ptr<core::refctd_memory_resource>(memoryResource) });
+    return ICPUBuffer::create({ .size = retValSize, .data = memoryResource->data(), .memoryResource = core::make_smart_refctd_ptr<core::refctd_memory_resource>(memoryResource) });
 }
 
 core::smart_refctd_ptr<IShaderCompiler::CCache> IShaderCompiler::CCache::deserialize(const std::span<const uint8_t> serializedCache)
diff --git a/src/nbl/core/alloc/refctd_memory_resource.cpp b/src/nbl/core/alloc/refctd_memory_resource.cpp
@@ -7,11 +7,11 @@
 using namespace nbl;
 using namespace core;
 
-static smart_refctd_ptr<refctd_memory_resource> null_memory_resource = nullptr;
 static smart_refctd_ptr<refctd_memory_resource> default_memory_resource = nullptr;
 
 smart_refctd_ptr<refctd_memory_resource> core::getNullMemoryResource()
 {
+    static smart_refctd_ptr<refctd_memory_resource> null_memory_resource = nullptr;
     if (!null_memory_resource)
         null_memory_resource = make_smart_refctd_ptr<refctd_memory_resource>(std::pmr::null_memory_resource());
     return null_memory_resource;
diff --git a/src/nbl/system/CArchiveLoaderTar.cpp b/src/nbl/system/CArchiveLoaderTar.cpp
@@ -76,15 +76,15 @@ bool CArchiveLoaderTar::isALoadableFileFormat(IFile* file) const
 	memset(fHead.Checksum, ' ', 8);
 
 	// old header
-	for (uint8_t* p = (uint8_t*)(&fHead); p < (uint8_t*)(&fHead.Magic[0]); ++p)
+	for (uint8_t* p = reinterpret_cast<uint8_t*>(&fHead); p < reinterpret_cast<uint8_t*>(&fHead.Magic[0]); ++p)
 	{
 		checksum1 += *p;
 		checksum2 += char(*p);
 	}
 
 	if (!strncmp(fHead.Magic, "ustar", 5))
 	{
-		for (uint8_t* p = (uint8_t*)(&fHead.Magic[0]); p < (uint8_t*)(&fHead) + sizeof(fHead); ++p)
+		for (uint8_t* p = reinterpret_cast<uint8_t*>(&fHead.Magic[0]); p < reinterpret_cast<uint8_t*>(&fHead) + sizeof(fHead); ++p)
 		{
 			checksum1 += *p;
 			checksum2 += char(*p);

Original file line number	Diff line number	Diff line change
`@@ -544,7 +544,7 @@ class ICPUMeshBuffer final : public IMeshBuffer<ICPUBuffer,ICPUDescriptorSet,ICP`
`544`	`544`	`const bool scaled = isScaledFormat(format);`
`545`	`545`	`if (!dst \|\| !(scaled \|\| isIntegerFormat(format)))`
`546`	`546`	`return false;`
`547`		`- uint8_t* vxPtr = (uint8_t*)dst;`
	`547`	`+ uint8_t* vxPtr = reinterpret_cast<uint8_t*>(dst);`
`548`	`548`
`549`	`549`	`if (isSignedFormat(format))`
`550`	`550`	`{`
Original file line number	Diff line number	Diff line change
`@@ -17,29 +17,34 @@ class VectorViewNullMemoryResource : public std::pmr::memory_resource`
`17`	`17`	`{`
`18`	`18`	`public:`
`19`	`19`	`// only create the resource from an already sized vector`
`20`		`- VectorViewNullMemoryResource(core::vector<uint8_t>&& buffer) : buffer(buffer)`
	`20`	`+ VectorViewNullMemoryResource(core::vector<uint8_t>&& buffer) : buffer(std::move(buffer)), already_called(false)`
`21`	`21`	`{`
`22`	`22`	`assert(buffer.size());`
`23`		`- };`
	`23`	`+ }`
`24`	`24`
`25`	`25`	`void* data() {`
`26`	`26`	`return buffer.data();`
`27`	`27`	`}`
`28`	`28`
`29`	`29`	`protected:`
`30`	`30`	`void* do_allocate(size_t bytes, size_t alignment) override {`
`31`		`- assert(bytes <= buffer.size());`
	`31`	`+ if (already_called \|\| bytes > buffer.size() \|\| !core::is_aligned_to(bytes, alignment))`
	`32`	`+ return nullptr;`
	`33`	`+ already_called = true;`
`32`	`34`	`return buffer.data();`
`33`	`35`	`}`
`34`	`36`
`35`		`- void do_deallocate(void* p, size_t bytes, size_t alignment) override {}`
	`37`	`+ void do_deallocate(void* p, size_t bytes, size_t alignment) override {`
	`38`	`+ assert(p == buffer.data());`
	`39`	`+ }`
`36`	`40`
`37`	`41`	`bool do_is_equal(const std::pmr::memory_resource& other) const noexcept override {`
`38`	`42`	`return this == &other;`
`39`	`43`	`}`
`40`	`44`
`41`	`45`	`private:`
`42`	`46`	`core::vector<uint8_t> buffer;`
	`47`	`+ bool already_called;`
`43`	`48`	`};`
`44`	`49`
`45`	`50`	`}`
Original file line number	Diff line number	Diff line change
`@@ -11,12 +11,10 @@`
`11`	`11`
`12`	`12`	`#include <memory_resource>`
`13`	`13`
`14`		`-using namespace nbl;`
`15`		`-`
`16`	`14`	`namespace nbl::core`
`17`	`15`	`{`
`18`	`16`
`19`		`-class refctd_memory_resource : public core::IReferenceCounted`
	`17`	`+class refctd_memory_resource : public IReferenceCounted`
`20`	`18`	`{`
`21`	`19`	`public:`
`22`	`20`	`refctd_memory_resource(std::pmr::memory_resource* pmr) : m_pmr(pmr) {};`
Original file line number	Diff line number	Diff line change
`@@ -177,7 +177,7 @@ bool CImageWriterPNG::writeAsset(system::IFile* _file, const SAssetWriteParams&`
`177`	`177`	`}`
`178`	`178`	`}`
`179`	`179`
`180`		`- uint8_t* data = (uint8_t*)convertedImage->getBuffer()->getPointer();`
	`180`	`+ uint8_t* data = reinterpret_cast<uint8_t*>(convertedImage->getBuffer()->getPointer());`
`181`	`181`
`182`	`182`	`constexpr uint32_t maxPNGFileHeight = 16u * 1024u; // arbitrary limit`
`183`	`183`	`if (trueExtent.Y>maxPNGFileHeight)`
Original file line number	Diff line number	Diff line change
`@@ -120,7 +120,7 @@ bool CImageWriterTGA::writeAsset(system::IFile* _file, const SAssetWriteParams&`
`120`	`120`	`return false;`
`121`	`121`	`}`
`122`	`122`
`123`		`- uint8_t* scan_lines = (uint8_t*)convertedImage->getBuffer()->getPointer();`
	`123`	`+ uint8_t* scan_lines = reinterpret_cast<uint8_t*>(convertedImage->getBuffer()->getPointer());`
`124`	`124`	`if (!scan_lines)`
`125`	`125`	`return false;`
`126`	`126`
Original file line number	Diff line number	Diff line change
`@@ -283,7 +283,7 @@ SAssetBundle CSTLMeshFileLoader::loadAsset(system::IFile* _file, const IAssetLoa`
`283`	`283`	`{`
`284`	`284`	`if (i % 3 == 0)`
`285`	`285`	`normal = quantNormalCache->quantize<EF_A2B10G10R10_SNORM_PACK32>(normals[i / 3]);`
`286`		`- uint8_t* ptr = ((uint8_t)(vertexBuf->getPointer())) + i vtxSize;`
	`286`	`+ uint8_t* ptr = (reinterpret_cast<uint8_t>(vertexBuf->getPointer())) + i vtxSize;`
`287`	`287`	`memcpy(ptr, positions[i].pointer, 3 * 4);`
`288`	`288`
`289`	`289`	`reinterpret_cast<quant_normal_t>(ptr + 12) = normal;`