Vulkan2DRenderer icon indicating copy to clipboard operation
Vulkan2DRenderer copied to clipboard

Possible bug in RenderTargetTexture.cpp

Open exuvo opened this issue 2 years ago • 1 comments

In RenderTargetTexture's mipmap blit code, the last mipmap level is transitioned from its write state to shader read. But the code incorrectly says the srcAccessMask is VK_ACCESS_TRANSFER_READ_BIT which should be VK_ACCESS_TRANSFER_WRITE_BIT as that is what is set at the start and the last mipmap level is never changed hence the need for the last bit of code.

Line 2969: image_memory_barriers[ 0 ].srcAccessMask = VK_ACCESS_TRANSFER_READ_BIT;

The relevant code block:

// Memory barrier to convert the last mip image layout to vk_sampled_image_final_layout.
// After this the complete image is ready to be used.
if( std::size( mipmap_levels ) > 1 ) {
	std::array<VkImageMemoryBarrier, 1> image_memory_barriers;

	// memory barrier to change source image layout.
	image_memory_barriers[ 0 ].sType							= VK_STRUCTURE_TYPE_IMAGE_MEMORY_BARRIER;
	image_memory_barriers[ 0 ].pNext						= nullptr;
	image_memory_barriers[ 0 ].srcAccessMask					= VK_ACCESS_TRANSFER_READ_BIT;
	image_memory_barriers[ 0 ].dstAccessMask					= vk_sampled_image_final_access_mask;
	image_memory_barriers[ 0 ].oldLayout						= VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL;
	image_memory_barriers[ 0 ].newLayout						= vk_sampled_image_final_layout;
	image_memory_barriers[ 0 ].srcQueueFamilyIndex			= VK_QUEUE_FAMILY_IGNORED;
	image_memory_barriers[ 0 ].dstQueueFamilyIndex			= VK_QUEUE_FAMILY_IGNORED;
	image_memory_barriers[ 0 ].image						= destination_image.image;
	image_memory_barriers[ 0 ].subresourceRange.aspectMask		= VK_IMAGE_ASPECT_COLOR_BIT;
	image_memory_barriers[ 0 ].subresourceRange.baseMipLevel	= uint32_t( std::size( mipmap_levels ) - 1 );
	image_memory_barriers[ 0 ].subresourceRange.levelCount		= 1;
	image_memory_barriers[ 0 ].subresourceRange.baseArrayLayer	= 0;
	image_memory_barriers[ 0 ].subresourceRange.layerCount		= 1;

	vkCmdPipelineBarrier(
		command_buffer,
		VK_PIPELINE_STAGE_TRANSFER_BIT,
		VK_PIPELINE_STAGE_FRAGMENT_SHADER_BIT,
		0,
		0, nullptr,
		0, nullptr,
		uint32_t( std::size( image_memory_barriers ) ), image_memory_barriers.data()
	);
}

Or am i mistaken here too?

exuvo avatar Apr 23 '22 23:04 exuvo

Hey. I think you're right. I quickly went over this and I think there's a few other things that should be looked at as well. I'll take a proper look when I have more time.

Noxagonal avatar Jun 20 '22 11:06 Noxagonal