TODOs for @PrzemoG

Also where is CGPUMeshPackerV1 that can be constructed from CCPUMeshPackerV1?

Author: devshgraphicsprogramming

examples_tests/21.DynamicTextureIndexing/main.cpp

@@ -7,7 +7,6 @@
 
 //! I advise to check out this file, its a basic input handler
 #include "../common/QToQuitEventReceiver.h"
-#include "nbl/asset/utils/CCPUMeshPacker.h"
 
 using namespace nbl;
 using namespace core;

include/nbl/asset/utils/CCPUMeshPackerV1.h

@@ -11,8 +11,6 @@
 
 //AFTER SAFE SHRINK FIX TODO LIST:
 //1. new size for buffers (obviously)
-//3. Chandle case where (maxIdx - minIdx) > 0xFFFF
-//4. fix handling of per instance attributes 
 //5. make it work for multiple `alloc` calls
 //6. provide `free` method
 //7. assertions on buffer overflow
@@ -73,6 +71,7 @@ class CCPUMeshPackerV1 final : public IMeshPacker<ICPUMeshBuffer, MDIStructType>
 	using IdxBufferParams = typename base_t::IdxBufferParams;
 
 public:
+	// TODO: dont track the supported bytesize after constructing the mesh packer, its redundant information... its in your per_instance allocator size
 	struct AllocationParams : IMeshPackerBase::AllocationParamsCommon
 	{
 		// Maximum byte size of per instance vertex data allocation
@@ -126,7 +125,7 @@ class CCPUMeshPackerV1 final : public IMeshPacker<ICPUMeshBuffer, MDIStructType>
 	ReservedAllocationMeshBuffers alloc(const MeshBufferIterator mbBegin, const MeshBufferIterator mbEnd);
 
 	//TODO: test
-	void free(const ReservedAllocationMeshBuffers& ramb)
+	void free(const ReservedAllocationMeshBuffers& ramb)  // TODO: this isn't special so do it in the base class
 	{
 		if (ramb.indexAllocationOffset != INVALID_ADDRESS)
 			m_idxBuffAlctr.free_addr(ramb.indexAllocationOffset, ramb.indexAllocationReservedCnt);
@@ -157,12 +156,13 @@ class CCPUMeshPackerV1 final : public IMeshPacker<ICPUMeshBuffer, MDIStructType>
 		MeshPackerConfigParams<Iterator>* packerParamsOut);
 
 	//! shrinks byte size of all output buffers, so they are large enough to fit currently allocated contents. Call this function before `instantiateDataStorage`
-	void shrinkOutputBuffersSize()
+	void shrinkOutputBuffersSize() // TODO: shrink isn't special, can only happen before data storage is instantiated, so do it in the base class
 	{
 		m_allocParams.MDIDataBuffSupportedCnt = m_MDIDataAlctr.safe_shrink_size(0u, 1u);
 		m_allocParams.perInstanceVertexBuffSupportedByteSize = m_perInsVtxBuffAlctr.safe_shrink_size(0u, 1u);
 		m_allocParams.indexBuffSupportedCnt = m_idxBuffAlctr.safe_shrink_size(0u, 1u);
 		m_allocParams.vertexBuffSupportedByteSize = m_vtxBuffAlctr.safe_shrink_size(0u, 1u);
+		// TODO: SHRINK ACTUAL ALLOCATORS AND THEIR RESERVED SPACES! (CREATE NEW ALLOCATORS WITH NEW RESERVED SPACES, THEN DELETE OLD RESERVED, THEN SWAP ALLOCATOR AND RESERVED MEMBERS) 
 	}
 
 private:
@@ -173,7 +173,7 @@ class CCPUMeshPackerV1 final : public IMeshPacker<ICPUMeshBuffer, MDIStructType>
 
 	uint32_t m_vtxSize;
 	uint32_t m_perInsVtxSize;
-	AllocationParams m_allocParams;
+	AllocationParams m_allocParams; // TODO: only track the min sizes, the other stuff is redundant with allocators
 
 	bool isInstancingEnabled;
 	void* m_perInsVtxBuffAlctrResSpc;
@@ -233,6 +233,7 @@ CCPUMeshPackerV1<MDIStructType>::CCPUMeshPackerV1(const SVertexInputParams& preD
 	);
 }
 
+// TODO: why cant this implementation of `alloc` be common to both CPU and CPU?
 template <typename MDIStructType>
 //`Iterator` may be only an Iterator or pointer to pointer
 template <typename MeshBufferIterator>
@@ -245,7 +246,7 @@ typename CCPUMeshPackerV1<MDIStructType>::ReservedAllocationMeshBuffers CCPUMesh
 
 	//TODO:
 	//allocation should be happening even if processed mesh buffer doesn't have attribute that was declared in pre defined `SVertexInputParams`, if mesh buffer has any attributes that are not declared in pre defined `SVertexInputParams` then these should be always ignored
-	//include it in the ducumentation? 
+	//include it in the ducumentation? YES
 
 	//validation
 	for (auto it = mbBegin; it != mbEnd; it++)

include/nbl/asset/utils/CCPUMeshPackerV2.h

@@ -40,9 +40,10 @@ class CCPUMeshPackerV2 final : public IMeshPackerV2<ICPUBuffer, ICPUMeshBuffer,
     //! shrinks byte size of all output buffers, so they are large enough to fit currently allocated contents. Call this function before `instantiateDataStorage`
     void shrinkOutputBuffersSize()
     {
-        m_allocParams.MDIDataBuffSupportedCnt = m_MDIDataAlctr.safe_shrink_size(0u, 1u);
-        m_allocParams.indexBuffSupportedCnt = m_idxBuffAlctr.safe_shrink_size(0u, 1u);
-        m_allocParams.vertexBuffSupportedByteSize = m_vtxBuffAlctr.safe_shrink_size(0u, 1u);
+        m_allocParams.MDIDataBuffSupportedCnt = m_MDIDataAlctr.safe_shrink_size(0u,1u);
+        m_allocParams.indexBuffSupportedCnt = m_idxBuffAlctr.safe_shrink_size(0u,1u);
+        m_allocParams.vertexBuffSupportedByteSize = m_vtxBuffAlctr.safe_shrink_size(0u,1u);
+        // TODO: SHRINK ACTUAL ALLOCATORS AND THEIR RESERVED SPACES! (CREATE NEW ALLOCATORS WITH NEW RESERVED SPACES, THEN DELETE OLD RESERVED, THEN SWAP ALLOCATOR AND RESERVED MEMBERS)
     }
 
     /**

include/nbl/asset/utils/IMeshPacker.h

@@ -48,6 +48,17 @@ class IMeshPackerBase : public virtual core::IReferenceCounted
             _NBL_ALIGNED_FREE(m_vtxBuffAlctrResSpc);
         }
 
+        struct MinimumAllocationParamsCommon
+        {
+            // Minimum count of 16 bit indicies allocated per allocation
+            size_t indexBufferMinAllocCnt = 256ull;
+
+            // Minimum bytes of vertex data allocated per allocation
+            size_t vertexBufferMinAllocByteSize = 32ull;
+
+            // Minimum count of MDI structs allocated per allocation
+            size_t MDIDataBuffMinAllocCnt = 32ull;
+        };
         struct AllocationParamsCommon
         {
             // Maximum number of 16 bit indicies that may be allocated
@@ -62,6 +73,9 @@ class IMeshPackerBase : public virtual core::IReferenceCounted
             // Maximum number of MDI structs that may be allocated
             size_t MDIDataBuffSupportedCnt = 16777216ull;                  /*   16MB assuming MDIStructType is DrawElementsIndirectCommand_t*/
 
+            // TODO: get rid of these, replace with
+            // MinimumAllocationParamsCommon minAllocParams;
+            // 
             // Minimum count of 16 bit indicies allocated per allocation
             size_t indexBufferMinAllocCnt = 256ull;
 
@@ -72,6 +86,7 @@ class IMeshPackerBase : public virtual core::IReferenceCounted
             size_t MDIDataBuffMinAllocCnt = 32ull;
         };
 
+        // TODO: make a function which initializes allocators as a copy (incl resized copy)
         void initializeCommonAllocators(const AllocationParamsCommon& allocParams)
         {
             if (allocParams.indexBuffSupportedCnt)

include/nbl/asset/utils/IMeshPackerV2.h

@@ -235,6 +235,7 @@ class IMeshPackerV2 : public IMeshPacker<MeshBufferType,MDIStructType>, public I
     static_assert(std::is_base_of<IBuffer, BufferType>::value);
 
 	using base_t = IMeshPacker<MeshBufferType, MDIStructType>;
+    using MinimumAllocationParams = IMeshPackerBase::MinimumAllocationParamsCommon;
     using AllocationParams = IMeshPackerBase::AllocationParamsCommon;
 
 public:
@@ -632,11 +633,8 @@ class IMeshPackerV2 : public IMeshPacker<MeshBufferType,MDIStructType>, public I
     inline PackerDataStore getPackerDataStore() { return m_packerDataStore; };
 
 protected:
-    //core::vector<VirtualAttribute> virtualAttribTable; // Is this variable even used!?
-    //uint16_t enabledAttribFlagsCombined = 0u; // Is this variable even used!?
-
     PackerDataStore m_packerDataStore;
-    AllocationParams m_allocParams;
+    AllocationParams m_allocParams; // TODO: only hold onto MinimumAllocationParams, derive rest from the allocator sizes! (if you even need to)
 
 };
 

include/nbl/video/CGPUMeshPackerV2.h

@@ -36,8 +36,8 @@ class CGPUMeshPackerV2 final : public IMeshPackerV2<video::IGPUBuffer,video::IGP
              m_driver(driver)
         {}
 
-        CGPUMeshPackerV2(video::IVideoDriver* driver, const asset::CCPUMeshPackerV2<MDIStructType>& cpuMP)
-            :IMeshPackerV2<video::IGPUBuffer, video::IGPUMeshBuffer, MDIStructType>(cpuMP.m_allocParams, cpuMP.m_minTriangleCountPerMDIData, cpuMP.m_maxTriangleCountPerMDIData),
+        CGPUMeshPackerV2(video::IVideoDriver* driver, const asset::CCPUMeshPackerV2<MDIStructType>* cpuMP)
+            :IMeshPackerV2<video::IGPUBuffer,video::IGPUMeshBuffer,MDIStructType>(cpuMP.m_allocParams, cpuMP.m_minTriangleCountPerMDIData, cpuMP.m_maxTriangleCountPerMDIData),
              m_driver(driver)
         {
             m_virtualAttribConfig = cpuMP.m_virtualAttribConfig;
@@ -46,31 +46,14 @@ class CGPUMeshPackerV2 final : public IMeshPackerV2<video::IGPUBuffer,video::IGP
             auto& cpuIdxBuff = cpuMP.m_packerDataStore.indexBuffer;
             auto& cpuVtxBuff = cpuMP.m_packerDataStore.vertexBuffer;
 
+            // TODO: why are the allocators not copied!?
+
+            // TODO: call instantiateDataStorage() here and then copy CPU data to the initialized storage
             m_packerDataStore.MDIDataBuffer = driver->createFilledDeviceLocalGPUBufferOnDedMem(cpuMDIBuff->getSize(), cpuMDIBuff->getPointer());
             m_packerDataStore.indexBuffer = driver->createFilledDeviceLocalGPUBufferOnDedMem(cpuMDIBuff->getSize(), cpuMDIBuff->getPointer());
             m_packerDataStore.vertexBuffer = driver->createFilledDeviceLocalGPUBufferOnDedMem(cpuMDIBuff->getSize(), cpuMDIBuff->getPointer());
         }
 
-        CGPUMeshPackerV2(video::IVideoDriver* driver, asset::CCPUMeshPackerV2<MDIStructType>&& cpuMP)
-            :IMeshPackerV2<video::IGPUBuffer, video::IGPUMeshBuffer, MDIStructType>(cpuMP.m_allocParams, cpuMP.m_minTriangleCountPerMDIData, cpuMP.m_maxTriangleCountPerMDIData),
-             m_driver(driver)
-        {
-            m_virtualAttribConfig = std::move(cpuMP.m_virtualAttribConfig);
-
-            auto cpuMDIBuff = std::move(cpuMP.m_packerDataStore.MDIDataBuffer);
-            auto cpuIdxBuff = std::move(cpuMP.m_packerDataStore.indexBuffer);
-            auto cpuVtxBuff = std::move(cpuMP.m_packerDataStore.vertexBuffer);
-
-            m_packerDataStore.MDIDataBuffer = driver->createFilledDeviceLocalGPUBufferOnDedMem(cpuMDIBuff->getSize(), cpuMDIBuff->getPointer());
-            m_packerDataStore.indexBuffer = driver->createFilledDeviceLocalGPUBufferOnDedMem(cpuIdxBuff->getSize(), cpuIdxBuff->getPointer());
-            m_packerDataStore.vertexBuffer = driver->createFilledDeviceLocalGPUBufferOnDedMem(cpuVtxBuff->getSize(), cpuVtxBuff->getPointer());
-
-            cpuMP.m_vtxBuffAlctr.reset();
-            cpuMP.m_idxBuffAlctr.reset();
-            cpuMP.m_MDIDataAlctr.reset();
-        }
-
-
         void instantiateDataStorage();
 
         template <typename MeshBufferIterator>