Vulkan: Fixing mem. corrupting in VulkanGpuParams

Source/BansheeVulkanRenderAPI/Include/BsVulkanGpuPipelineParamInfo.h

@@ -4,6 +4,7 @@
 
 #include "BsVulkanPrerequisites.h"
 #include "BsGpuPipelineParamInfo.h"
+#include "BsGroupAlloc.h"
 
 namespace bs
 {
@@ -24,6 +25,9 @@ namespace bs
 		/** Returns a pointer to an array of bindings for the layout at the specified index. */
 		VkDescriptorSetLayoutBinding* getBindings(UINT32 layoutIdx) const { return mLayoutInfos[layoutIdx].bindings; }
 
+		/** Returns the sequential index of the binding at the specificn set/slot. Returns -1 if slot is not used. */
+		UINT32 getBindingIdx(UINT32 set, UINT32 slot) const { return mSetExtraInfos[set].slotIndices[slot]; }
+
 		/** 
 		 * Returns a layout for the specified device, at the specified index. Returns null if no layout for the specified 
 		 * device index. 
@@ -41,12 +45,19 @@ namespace bs
 			UINT32 numBindings;
 		};
 
+		/** Information about a single set in the param info object. Complements SetInfo. */
+		struct SetExtraInfo
+		{
+			UINT32* slotIndices;
+		};
+
 		GpuDeviceFlags mDeviceMask;
 
+		SetExtraInfo* mSetExtraInfos;
 		VulkanDescriptorLayout** mLayouts[BS_MAX_DEVICES];
 		LayoutInfo* mLayoutInfos;
 
-		UINT8* mData;
+		GroupAlloc mAlloc;
 	};
 
 	/** @} */

Source/BansheeVulkanRenderAPI/Source/BsVulkanGpuParams.cpp

@@ -69,7 +69,8 @@ namespace bs
 		UINT32 perSetBytes = sizeof(PerSetData) * numSets;
 		UINT32 writeSetInfosBytes = sizeof(VkWriteDescriptorSet) * numBindings;
 		UINT32 writeInfosBytes = sizeof(WriteInfo) * numBindings;
-		mData = (UINT8*)bs_alloc(setsDirtyBytes + (perSetBytes + writeSetInfosBytes + writeInfosBytes) * numDevices);
+		UINT32 totalNumBytes = setsDirtyBytes + (perSetBytes + writeSetInfosBytes + writeInfosBytes) * numDevices;
+		mData = (UINT8*)bs_alloc(totalNumBytes);
 		UINT8* dataIter = mData;
 
 		Lock lock(mMutex); // Set write operations need to be thread safe
@@ -170,11 +171,19 @@ namespace bs
 	{
 		GpuParamsCore::setParamBlockBuffer(set, slot, paramBlockBuffer);
 
+		VulkanGpuPipelineParamInfo& vkParamInfo = static_cast<VulkanGpuPipelineParamInfo&>(*mParamInfo);
+		UINT32 bindingIdx = vkParamInfo.getBindingIdx(set, slot);
+		if(bindingIdx == -1)
+		{
+			LOGERR("Provided set/slot combination is not used by the GPU program: " + toString(set) + "," + 
+				toString(slot) + ".");
+			return;
+		}
+
 		Lock(mMutex);
 
 		VulkanGpuParamBlockBufferCore* vulkanParamBlockBuffer =
 			static_cast<VulkanGpuParamBlockBufferCore*>(paramBlockBuffer.get());
-
 		for (UINT32 i = 0; i < BS_MAX_DEVICES; i++)
 		{
 			if (mPerDeviceData[i].perSetData == nullptr)
@@ -187,9 +196,9 @@ namespace bs
 				bufferRes = nullptr;
 
 			if (bufferRes != nullptr)
-				mPerDeviceData[i].perSetData[set].writeInfos[slot].buffer.buffer = bufferRes->getHandle();
+				mPerDeviceData[i].perSetData[set].writeInfos[bindingIdx].buffer.buffer = bufferRes->getHandle();
 			else
-				mPerDeviceData[i].perSetData[set].writeInfos[slot].buffer.buffer = VK_NULL_HANDLE;
+				mPerDeviceData[i].perSetData[set].writeInfos[bindingIdx].buffer.buffer = VK_NULL_HANDLE;
 		}
 
 		mSetsDirty[set] = true;
@@ -199,6 +208,15 @@ namespace bs
 	{
 		GpuParamsCore::setTexture(set, slot, texture);
 
+		VulkanGpuPipelineParamInfo& vkParamInfo = static_cast<VulkanGpuPipelineParamInfo&>(*mParamInfo);
+		UINT32 bindingIdx = vkParamInfo.getBindingIdx(set, slot);
+		if (bindingIdx == -1)
+		{
+			LOGERR("Provided set/slot combination is not used by the GPU program: " + toString(set) + "," +
+				   toString(slot) + ".");
+			return;
+		}
+
 		Lock(mMutex);
 
 		VulkanTextureCore* vulkanTexture = static_cast<VulkanTextureCore*>(texture.get());
@@ -207,7 +225,7 @@ namespace bs
 			if (mPerDeviceData[i].perSetData == nullptr)
 				continue;
 
-			mPerDeviceData[i].perSetData[set].writeInfos[slot].image.imageView = vulkanTexture->getView(i);
+			mPerDeviceData[i].perSetData[set].writeInfos[bindingIdx].image.imageView = vulkanTexture->getView(i);
 		}
 
 		mSetsDirty[set] = true;
@@ -218,6 +236,15 @@ namespace bs
 	{
 		GpuParamsCore::setLoadStoreTexture(set, slot, texture, surface);
 
+		VulkanGpuPipelineParamInfo& vkParamInfo = static_cast<VulkanGpuPipelineParamInfo&>(*mParamInfo);
+		UINT32 bindingIdx = vkParamInfo.getBindingIdx(set, slot);
+		if (bindingIdx == -1)
+		{
+			LOGERR("Provided set/slot combination is not used by the GPU program: " + toString(set) + "," +
+				   toString(slot) + ".");
+			return;
+		}
+
 		Lock(mMutex);
 
 		VulkanTextureCore* vulkanTexture = static_cast<VulkanTextureCore*>(texture.get());
@@ -226,7 +253,7 @@ namespace bs
 			if (mPerDeviceData[i].perSetData == nullptr)
 				continue;
 
-			mPerDeviceData[i].perSetData[set].writeInfos[slot].image.imageView = vulkanTexture->getView(i, surface);
+			mPerDeviceData[i].perSetData[set].writeInfos[bindingIdx].image.imageView = vulkanTexture->getView(i, surface);
 		}
 
 		mSetsDirty[set] = true;
@@ -236,6 +263,15 @@ namespace bs
 	{
 		GpuParamsCore::setBuffer(set, slot, buffer);
 
+		VulkanGpuPipelineParamInfo& vkParamInfo = static_cast<VulkanGpuPipelineParamInfo&>(*mParamInfo);
+		UINT32 bindingIdx = vkParamInfo.getBindingIdx(set, slot);
+		if (bindingIdx == -1)
+		{
+			LOGERR("Provided set/slot combination is not used by the GPU program: " + toString(set) + "," +
+				   toString(slot) + ".");
+			return;
+		}
+
 		Lock(mMutex);
 
 		VulkanGpuBufferCore* vulkanBuffer = static_cast<VulkanGpuBufferCore*>(buffer.get());
@@ -246,9 +282,9 @@ namespace bs
 
 			VulkanBuffer* bufferRes = vulkanBuffer->getResource(i);
 			if (bufferRes != nullptr)
-				mPerDeviceData[i].perSetData[set].writeInfos[slot].bufferView = bufferRes->getView();
+				mPerDeviceData[i].perSetData[set].writeInfos[bindingIdx].bufferView = bufferRes->getView();
 			else
-				mPerDeviceData[i].perSetData[set].writeInfos[slot].bufferView = VK_NULL_HANDLE;
+				mPerDeviceData[i].perSetData[set].writeInfos[bindingIdx].bufferView = VK_NULL_HANDLE;
 		}
 
 		mSetsDirty[set] = true;
@@ -258,6 +294,15 @@ namespace bs
 	{
 		GpuParamsCore::setSamplerState(set, slot, sampler);
 
+		VulkanGpuPipelineParamInfo& vkParamInfo = static_cast<VulkanGpuPipelineParamInfo&>(*mParamInfo);
+		UINT32 bindingIdx = vkParamInfo.getBindingIdx(set, slot);
+		if (bindingIdx == -1)
+		{
+			LOGERR("Provided set/slot combination is not used by the GPU program: " + toString(set) + "," +
+				   toString(slot) + ".");
+			return;
+		}
+
 		Lock(mMutex);
 
 		VulkanSamplerStateCore* vulkanSampler = static_cast<VulkanSamplerStateCore*>(sampler.get());
@@ -268,9 +313,9 @@ namespace bs
 
 			VulkanSampler* samplerRes = vulkanSampler->getResource(i);
 			if (samplerRes != nullptr)
-				mPerDeviceData[i].perSetData[set].writeInfos[slot].image.sampler = samplerRes->getHandle();
+				mPerDeviceData[i].perSetData[set].writeInfos[bindingIdx].image.sampler = samplerRes->getHandle();
 			else
-				mPerDeviceData[i].perSetData[set].writeInfos[slot].image.sampler = VK_NULL_HANDLE;
+				mPerDeviceData[i].perSetData[set].writeInfos[bindingIdx].image.sampler = VK_NULL_HANDLE;
 		}
 
 		mSetsDirty[set] = true;
@@ -280,6 +325,15 @@ namespace bs
 	{
 		GpuParamsCore::setLoadStoreSurface(set, slot, surface);
 
+		VulkanGpuPipelineParamInfo& vkParamInfo = static_cast<VulkanGpuPipelineParamInfo&>(*mParamInfo);
+		UINT32 bindingIdx = vkParamInfo.getBindingIdx(set, slot);
+		if (bindingIdx == -1)
+		{
+			LOGERR("Provided set/slot combination is not used by the GPU program: " + toString(set) + "," +
+				   toString(slot) + ".");
+			return;
+		}
+
 		SPtr<TextureCore> texture = getLoadStoreTexture(set, slot);
 		if (texture == nullptr)
 			return;
@@ -292,7 +346,7 @@ namespace bs
 			if (mPerDeviceData[i].perSetData == nullptr)
 				continue;
 
-			mPerDeviceData[i].perSetData[set].writeInfos[slot].image.imageView = vulkanTexture->getView(i, surface);
+			mPerDeviceData[i].perSetData[set].writeInfos[bindingIdx].image.imageView = vulkanTexture->getView(i, surface);
 		}
 
 		mSetsDirty[set] = true;

Source/BansheeVulkanRenderAPI/Source/BsVulkanGpuPipelineParamInfo.cpp

@@ -9,12 +9,13 @@
 namespace bs
 {
 	VulkanGpuPipelineParamInfo::VulkanGpuPipelineParamInfo(const GPU_PIPELINE_PARAMS_DESC& desc, GpuDeviceFlags deviceMask)
-		:GpuPipelineParamInfoCore(desc, deviceMask), mDeviceMask(deviceMask), mLayouts(), mLayoutInfos(), mData(nullptr)
+		: GpuPipelineParamInfoCore(desc, deviceMask), mDeviceMask(deviceMask), mSetExtraInfos(nullptr), mLayouts()
+		, mLayoutInfos()
 	{ }
 
 	VulkanGpuPipelineParamInfo::~VulkanGpuPipelineParamInfo()
 	{
-		bs_free(mData);
+
 	}
 
 	void VulkanGpuPipelineParamInfo::initialize()
@@ -31,18 +32,19 @@ namespace bs
 				numDevices++;
 		}
 
-		UINT32 bindingsSize = sizeof(VkDescriptorSetLayoutBinding) * mNumElements;
-		UINT32 layoutInfoSize = sizeof(LayoutInfo) * mNumSets;
-		UINT32 layoutsSize = sizeof(VulkanDescriptorLayout*) * mNumSets;
-
-		mData = (UINT8*)bs_alloc(bindingsSize + layoutInfoSize + layoutsSize * numDevices);
-		UINT8* dataPtr = mData;
+		UINT32 totalNumSlots = 0;
+		for (UINT32 i = 0; i < mNumSets; i++)
+			totalNumSlots += mSetInfos[i].numSlots;
 
-		mLayoutInfos = (LayoutInfo*)dataPtr;
-		dataPtr += layoutInfoSize;
+		mAlloc.reserve<VkDescriptorSetLayoutBinding>(mNumElements)
+			.reserve<LayoutInfo>(mNumSets)
+			.reserve<VulkanDescriptorLayout*>(mNumSets * numDevices)
+			.reserve<SetExtraInfo>(mNumSets)
+			.reserve<UINT32>(totalNumSlots)
+			.init();
 
-		VkDescriptorSetLayoutBinding* bindings = (VkDescriptorSetLayoutBinding*)dataPtr;
-		dataPtr += bindingsSize;
+		mLayoutInfos = mAlloc.alloc<LayoutInfo>(mNumSets);
+		VkDescriptorSetLayoutBinding* bindings = mAlloc.alloc<VkDescriptorSetLayoutBinding>(mNumElements);
 
 		for (UINT32 i = 0; i < BS_MAX_DEVICES; i++)
 		{
@@ -52,26 +54,34 @@ namespace bs
 				continue;
 			}
 
-			mLayouts[i] = (VulkanDescriptorLayout**)dataPtr;
-			dataPtr += layoutsSize;
+			mLayouts[i] = mAlloc.alloc<VulkanDescriptorLayout*>(mNumSets);
 		}
 
-		memset(bindings, 0, bindingsSize);
+		mSetExtraInfos = mAlloc.alloc<SetExtraInfo>(mNumSets);
+
+		if(bindings != nullptr)
+			bs_zero_out(bindings, mNumElements);
 
 		UINT32 globalBindingIdx = 0;
 		for (UINT32 i = 0; i < mNumSets; i++)
 		{
+			mSetExtraInfos[i].slotIndices = mAlloc.alloc<UINT32>(mSetInfos[i].numSlots);
+
 			mLayoutInfos[i].numBindings = 0;
 			mLayoutInfos[i].bindings = nullptr;
 
 			for (UINT32 j = 0; j < mSetInfos[i].numSlots; j++)
 			{
 				if (mSetInfos[i].slotIndices[j] == -1)
+				{
+					mSetExtraInfos[i].slotIndices[j] = -1;
 					continue;
+				}
 
 				VkDescriptorSetLayoutBinding& binding = bindings[globalBindingIdx];
 				binding.binding = j;
 
+				mSetExtraInfos[i].slotIndices[j] = globalBindingIdx;
 				mLayoutInfos[i].numBindings++;
 				globalBindingIdx++;
 			}
@@ -103,19 +113,14 @@ namespace bs
 			{
 				for (auto& entry : params)
 				{
+					UINT32 bindingIdx = getBindingIdx(entry.second.set, entry.second.slot);
+					assert(bindingIdx != -1);
+
 					LayoutInfo& layoutInfo = mLayoutInfos[entry.second.set];
-					for(UINT32 j = 0; j < layoutInfo.numBindings; j++)
-					{
-						if(layoutInfo.bindings[j].binding == entry.second.slot)
-						{
-							VkDescriptorSetLayoutBinding& binding = layoutInfo.bindings[j];
-							binding.descriptorCount = 1;
-							binding.stageFlags |= stageFlagsLookup[i];
-							binding.descriptorType = descType;
-
-							break;
-						}
-					}
+					VkDescriptorSetLayoutBinding& binding = layoutInfo.bindings[bindingIdx];
+					binding.descriptorCount = 1;
+					binding.stageFlags |= stageFlagsLookup[i];
+					binding.descriptorType = descType;
 				}
 			};
 
@@ -131,19 +136,15 @@ namespace bs
 				bool isLoadStore = entry.second.type != GPOT_BYTE_BUFFER &&
 					entry.second.type != GPOT_STRUCTURED_BUFFER;
 
+				UINT32 bindingIdx = getBindingIdx(entry.second.set, entry.second.slot);
+				assert(bindingIdx != -1);
+
 				LayoutInfo& layoutInfo = mLayoutInfos[entry.second.set];
-				for (UINT32 j = 0; j < layoutInfo.numBindings; j++)
-				{
-					if (layoutInfo.bindings[j].binding == entry.second.slot)
-					{
-						VkDescriptorSetLayoutBinding& binding = layoutInfo.bindings[j];
-						binding.descriptorCount = 1;
-						binding.stageFlags |= stageFlagsLookup[i];
-						binding.descriptorType = isLoadStore ? VK_DESCRIPTOR_TYPE_STORAGE_TEXEL_BUFFER : VK_DESCRIPTOR_TYPE_UNIFORM_TEXEL_BUFFER;
-
-						break;
-					}
-				}
+				VkDescriptorSetLayoutBinding& binding = layoutInfo.bindings[bindingIdx];
+				binding.descriptorCount = 1;
+				binding.stageFlags |= stageFlagsLookup[i];
+				binding.descriptorType = 
+					isLoadStore ? VK_DESCRIPTOR_TYPE_STORAGE_TEXEL_BUFFER : VK_DESCRIPTOR_TYPE_UNIFORM_TEXEL_BUFFER;
 			}
 		}
 

Source/BansheeVulkanRenderAPI/Source/BsVulkanGpuPipelineState.cpp

@@ -484,7 +484,7 @@ namespace bs
 
 		VkPipeline pipeline;
 		VkResult result = vkCreateGraphicsPipelines(vkDevice, VK_NULL_HANDLE, 1, &mPipelineInfo, gVulkanAllocator, &pipeline);
-		assert(result != VK_SUCCESS);
+		assert(result == VK_SUCCESS);
 
 		// Restore previous stencil op states
 		mDepthStencilInfo.front.passOp = oldFrontPassOp;