Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RenderingDevice: Leave handling of compressed block granularity to the driver #94069

Merged
merged 1 commit into from
Jul 18, 2024

Conversation

RandomShaper
Copy link
Member

@RandomShaper RandomShaper commented Jul 8, 2024

Fixes the following Vulkan validation error in command_copy_texture_to_buffer():

drivers\vulkan\rendering_context_driver_vulkan.cpp:304 - VALIDATION - Message Id Number: 1465181769 | Message Id Name: VUID-vkCmdCopyImageToBuffer-imageSubresource-07971
	Validation Error: [ VUID-vkCmdCopyImageToBuffer-imageSubresource-07971 ] Object 0: handle = 0x1a304de76f0, type = VK_OBJECT_TYPE_COMMAND_BUFFER; Object 1: handle = 0xc39db0000004713, name = RID:670869596676284, type = VK_OBJECT_TYPE_IMAGE; | MessageID = 0x5754e649 | vkCmdCopyImageToBuffer(): pRegions[10] imageOffset.x (0) and (imageExtent.width + imageOffset.x) (4) must be >= zero or <= image subresource width (2). The Vulkan spec states: For each element of pRegions, imageOffset.x and (imageExtent.width + imageOffset.x) must both be greater than or equal to 0 and less than or equal to the width of the specified imageSubresource of srcImage (https://vulkan.lunarg.com/doc/view/1.3.275.0/windows/1.3-extensions/vkspec.html#VUID-vkCmdCopyImageToBuffer-imageSubresource-07971)
	Objects - 2
		Object[0] - VK_OBJECT_TYPE_COMMAND_BUFFER, Handle 1799672985328
		Object[1] - VK_OBJECT_TYPE_IMAGE, Handle 880975995174143763, Name "RID:670869596676284"

The issue is that Vulkan expects the dimensions be passed there without stepifying to the block size.

This PR changes the implicit contract between RenderingDevice and RenderingDeviceDriver so drivers now have to be aware that they will get logical sizes and they have to perform the rounding to physical block size according to what the underlying graphics API expects. That's OK for Vulkan, for obvious reasons, and also fine for Direct3D 12 because RenderingDeviceDriverD3D12 performs the rounding itself so it doesn' really care. I'd just like to let @stuartcarnie know so he can adapt the Metal RDD to this "spec" change.

@RandomShaper RandomShaper added this to the 4.3 milestone Jul 8, 2024
@RandomShaper RandomShaper requested a review from a team as a code owner July 8, 2024 10:06
@stuartcarnie
Copy link
Contributor

Is there an MRP that I can use to validate Metal?

@RandomShaper
Copy link
Member Author

Is there an MRP that I can use to validate Metal?

Nope, but any project that involves textured imported as compressed will exercise the affected path during editor's resource preview generation. That's how I stumbled upon this in the first place.

@akien-mga akien-mga merged commit 0f1e2c3 into godotengine:master Jul 18, 2024
18 checks passed
@akien-mga
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants