Fix ImGUI's layout transition issues and validation errors.

also add command pool bitwise ops for creationg flags

Author: devsh

examples_tests

@@ -41,7 +41,8 @@ class UI final : public core::IReferenceCounted
 		core::smart_refctd_ptr<video::IGPUDescriptorSetLayout> CreateDescriptorSetLayout();
 
 		void CreatePipeline(video::IGPURenderpass* renderpass, video::IGPUPipelineCache* pipelineCache);
-		void CreateFontTexture(video::IGPUCommandBuffer* cmdBuffer, video::IQueue* queue);
+		// TODO: just take an intended next submit instead of queue and cmdbuf, so we're consistent across utilities
+		video::ISemaphore::future_t<video::IQueue::RESULT> CreateFontTexture(video::IGPUCommandBuffer* cmdBuffer, video::IQueue* queue);
 		void UpdateDescriptorSets();
 		void createSystem();
 		void CreateFontSampler();

include/nbl/ext/ImGui/ImGui.h

@@ -822,6 +822,8 @@ class IGPUCommandPool::CCopyAccelerationStructureToOrFromMemoryCmd final : publi
         core::smart_refctd_ptr<const IGPUBuffer> m_buffer;
 };
 
+NBL_ENUM_ADD_BITWISE_OPERATORS(IGPUCommandPool::CREATE_FLAGS)
+
 }
 
 

include/nbl/video/IGPUCommandPool.h

@@ -32,7 +32,7 @@ namespace nbl::ext::imgui
 				.createFlags = IGPUDescriptorSetLayout::SBinding::E_CREATE_FLAGS::ECF_NONE,
 				.stageFlags = IShader::ESS_FRAGMENT,
 				.count = 1,
-				.samplers = nullptr // TODO: m_fontSampler?
+				.immutableSamplers = nullptr // TODO: m_fontSampler?
 			}
 		};
 
@@ -156,7 +156,7 @@ namespace nbl::ext::imgui
 		}
 	}
 
-	void UI::CreateFontTexture(video::IGPUCommandBuffer* cmdBuffer, video::IQueue* transfer)
+	ISemaphore::future_t<IQueue::RESULT> UI::CreateFontTexture(video::IGPUCommandBuffer* cmdBuffer, video::IQueue* transfer)
 	{
 		// Load Fonts
 		// - If no fonts are loaded, dear imgui will use the default font. You can also load multiple fonts and use ImGui::PushFont()/PopFont() to select them.
@@ -173,17 +173,14 @@ namespace nbl::ext::imgui
 		//ImFont* font = io.Fonts->AddFontFromFileTTF("c:\\Windows\\Fonts\\ArialUni.ttf", 18.0f, NULL, io.Fonts->GetGlyphRangesJapanese());
 		ImGuiIO& io = ImGui::GetIO();
 
+		// TODO: don't `pixels` need to be freed somehow!? (Use a uniqueptr with custom deleter lambda)
 		uint8_t* pixels = nullptr;
 		int32_t width, height;
 		io.Fonts->GetTexDataAsRGBA32(&pixels, &width, &height);
-		assert(pixels != nullptr);
-		assert(width > 0);
-		assert(height > 0);
-		const size_t componentsCount = 4, image_size = width * height * componentsCount * sizeof(uint8_t);
-		
-		_NBL_STATIC_INLINE_CONSTEXPR auto NBL_FORMAT_FONT = EF_R8G8B8A8_UNORM;
-		const auto buffer = core::make_smart_refctd_ptr< asset::CCustomAllocatorCPUBuffer<core::null_allocator<uint8_t>, true> >(image_size, pixels, core::adopt_memory);
+		if (!pixels || width<=0 || height<=0)
+			return IQueue::RESULT::OTHER_ERROR;
 
+		constexpr auto NBL_FORMAT_FONT = EF_R8G8B8A8_UNORM;
 		IGPUImage::SCreationParams params;
 		params.flags = static_cast<IImage::E_CREATE_FLAGS>(0u);
 		params.type = IImage::ET_2D;
@@ -192,7 +189,7 @@ namespace nbl::ext::imgui
 		params.mipLevels = 1;
 		params.arrayLayers = 1u;
 		params.samples = IImage::ESCF_1_BIT;
-		params.usage |= IGPUImage::EUF_TRANSFER_DST_BIT | IGPUImage::EUF_SAMPLED_BIT | IGPUImage::E_USAGE_FLAGS::EUF_TRANSFER_SRC_BIT;
+		params.usage |= IGPUImage::EUF_TRANSFER_DST_BIT | IGPUImage::EUF_SAMPLED_BIT | IGPUImage::E_USAGE_FLAGS::EUF_TRANSFER_SRC_BIT; // do you really need the SRC bit?
 
 		struct
 		{
@@ -224,57 +221,80 @@ namespace nbl::ext::imgui
 		if (!image)
 		{
 			logger->log("Could not create font image!", system::ILogger::ELL_ERROR);
-			assert(false);
+			return IQueue::RESULT::OTHER_ERROR;
 		}
+		image->setObjectDebugName("Nabla IMGUI extension Font Image");
 
 		if (!m_device->allocate(image->getMemoryReqs(), image.get()).isValid())
 		{
 			logger->log("Could not allocate memory for font image!", system::ILogger::ELL_ERROR);
-			assert(false);
+			return IQueue::RESULT::OTHER_ERROR;
 		}
-		
-		image->setObjectDebugName("Nabla IMGUI extension Font Image");
+
+		SIntendedSubmitInfo sInfo;
 		{
 			IQueue::SSubmitInfo::SCommandBufferInfo cmdInfo = { cmdBuffer };
-			SIntendedSubmitInfo sInfo;
 
 			auto scratchSemaphore = m_device->createSemaphore(0);
 			if (!scratchSemaphore)
 			{
 				logger->log("Could not create scratch semaphore", system::ILogger::ELL_ERROR);
-				assert(false);
+				return IQueue::RESULT::OTHER_ERROR;
 			}
 			scratchSemaphore->setObjectDebugName("Nabla IMGUI extension Scratch Semaphore");
 
 			sInfo.queue = transfer;
 			sInfo.waitSemaphores = {};
 			sInfo.commandBuffers = { &cmdInfo, 1 };
-			sInfo.scratchSemaphore = // TODO: do I really need it?
+			sInfo.scratchSemaphore = // TODO: do I really need it? YES, ALWAYS!
 			{
 				.semaphore = scratchSemaphore.get(),
 				.value = 0,
 				.stageMask = PIPELINE_STAGE_FLAGS::ALL_TRANSFER_BITS
 			};
-
-			const SMemoryBarrier toTransferBarrier = {
+			
+			// we have no explicit source stage and access to sync against, brand new clean image.
+			const asset::SMemoryBarrier toTransferDep = {
 				.dstStageMask = PIPELINE_STAGE_FLAGS::COPY_BIT,
-				.dstAccessMask = ACCESS_FLAGS::TRANSFER_WRITE_BIT
+				.dstAccessMask = ACCESS_FLAGS::TRANSFER_WRITE_BIT,
 			};
-
-			cmdBuffer->begin(IGPUCommandBuffer::USAGE::ONE_TIME_SUBMIT_BIT);
-			const IGPUCommandBuffer::SImageMemoryBarrier<IGPUCommandBuffer::SOwnershipTransferBarrier> barriers[] = 
+			const auto transferLayout = IGPUImage::LAYOUT::TRANSFER_DST_OPTIMAL;
+			// transition to TRANSFER_DST
+			IGPUCommandBuffer::SImageMemoryBarrier<IGPUCommandBuffer::SOwnershipTransferBarrier> barriers[] = 
 			{ 
 				{
-					.barrier = { .dep = toTransferBarrier },
+					.barrier = {.dep = toTransferDep},
 					.image = image.get(),
 					.subresourceRange = regions.subresource,
-					.newLayout = IGPUImage::LAYOUT::TRANSFER_DST_OPTIMAL
+					.oldLayout = IGPUImage::LAYOUT::UNDEFINED, // wiping transition
+					.newLayout = transferLayout
 				} 
 			};
 
-			cmdBuffer->pipelineBarrier(E_DEPENDENCY_FLAGS::EDF_NONE, { .imgBarriers = barriers });
+			cmdBuffer->begin(IGPUCommandBuffer::USAGE::ONE_TIME_SUBMIT_BIT);
+			cmdBuffer->pipelineBarrier(E_DEPENDENCY_FLAGS::EDF_NONE,{.imgBarriers=barriers});
+			// We cannot use the `AutoSubmit` variant of the util because we need to add a pipeline barrier with a transition onto the command buffer after the upload.
+			// old layout is UNDEFINED because we don't want a content preserving transition, we can just put ourselves in transfer right away
+			if (!utilities->updateImageViaStagingBuffer(sInfo,pixels,image->getCreationParameters().format,image.get(),transferLayout,regions.range))
+			{
+				logger->log("Could not upload font image contents", system::ILogger::ELL_ERROR);
+				return IQueue::RESULT::OTHER_ERROR;
+			}
 
-			utilities->updateImageViaStagingBufferAutoSubmit(sInfo, buffer->getPointer(), NBL_FORMAT_FONT, image.get(), IGPUImage::LAYOUT::TRANSFER_DST_OPTIMAL, regions.range);
+			// we only need to sync with semaphore signal
+			barriers[0].barrier.dep = toTransferDep.nextBarrier(sInfo.scratchSemaphore.stageMask,ACCESS_FLAGS::NONE);
+			// transition to READ_ONLY_OPTIMAL ready for rendering with sampling
+			barriers[0].oldLayout = barriers[0].newLayout;
+			barriers[0].newLayout = IGPUImage::LAYOUT::READ_ONLY_OPTIMAL;
+			cmdBuffer->pipelineBarrier(E_DEPENDENCY_FLAGS::EDF_NONE,{.imgBarriers=barriers});
+			cmdBuffer->end();
+
+			const auto submit = sInfo.popSubmit({});
+			if (transfer->submit(submit)!=IQueue::RESULT::SUCCESS)
+			{
+				logger->log("Could not submit workload for font texture upload.", system::ILogger::ELL_ERROR);
+				return IQueue::RESULT::OTHER_ERROR;
+			}
 		}
 
 		{
@@ -286,6 +306,10 @@ namespace nbl::ext::imgui
 
 			m_fontTexture = m_device->createImageView(std::move(params));
 		}
+		
+        ISemaphore::future_t<IQueue::RESULT> retval(IQueue::RESULT::SUCCESS);
+        retval.set({sInfo.scratchSemaphore.semaphore,sInfo.scratchSemaphore.value});
+        return retval;
 	}
 
 	void prepareKeyMapForDesktop()
@@ -328,8 +352,8 @@ namespace nbl::ext::imgui
 		IGPUDescriptorSet::SDescriptorInfo info;
 		{
 			info.desc = m_fontTexture;
-			info.info.image.sampler = m_fontSampler;
-			info.info.image.imageLayout = nbl::asset::IImage::LAYOUT::READ_ONLY_OPTIMAL;
+			info.info.combinedImageSampler.sampler = m_fontSampler;
+			info.info.combinedImageSampler.imageLayout = nbl::asset::IImage::LAYOUT::READ_ONLY_OPTIMAL;
 		}
 
 		IGPUDescriptorSet::SWriteDescriptorSet writeDescriptorSet{};
@@ -462,7 +486,9 @@ namespace nbl::ext::imgui
 
 		smart_refctd_ptr<nbl::video::IGPUCommandBuffer> transistentCMD;
 		{
-			smart_refctd_ptr<nbl::video::IGPUCommandPool> pool = device->createCommandPool(families.id.transfer, IGPUCommandPool::CREATE_FLAGS::TRANSIENT_BIT);
+			using pool_flags_t = IGPUCommandPool::CREATE_FLAGS;
+			// need to be individually resettable such that we can form a valid SIntendedSubmit out of the commandbuffer allocated from the pool
+			smart_refctd_ptr<nbl::video::IGPUCommandPool> pool = device->createCommandPool(families.id.transfer, pool_flags_t::RESET_COMMAND_BUFFER_BIT|pool_flags_t::TRANSIENT_BIT);
 			if (!pool)
 			{
 				logger->log("Could not create command pool!", system::ILogger::ELL_ERROR);