szeged / webrender

A GPU-based renderer for the web
https://doc.servo.org/webrender/
Mozilla Public License 2.0
45 stars 7 forks source link

Fix validation layer error VkBufferImageCopy-bufferOffset #299

Closed imiklos closed 5 years ago

imiklos commented 5 years ago

Error: The bufferOffset must be a multiple of 4. On NVIDIA cards the optimalBufferCopyOffsetAlignment value is 1, therefore the alignment mask wasn't good in some cases. https://www.vulkan.gpuinfo.org/displayreport.php?id=6217#limits

kvark commented 5 years ago

What did the validation say? The code looks wrong to me for not using any of the actual limits for correcting the alignment.

zakorgy commented 5 years ago

I think the first line The bufferOffset must be a multiple of 4 is the validation layer error, at least that's the essential part of it. It comes from the VkBufferImageCopy call (copy_buffer_tom_image). We are already using the optimalBufferCopyOffsetAlignment for the alignment in this code https://github.com/szeged/webrender/pull/299/files#diff-5dbe2f57bb9273f9a363bcb71ec6834eR197 as self.copy_alignment_mask. But the value of this limit is unfortunately 1 on NVIDIA GeForce GTX 750 Ti, you can check it on the gpuinfo link @imiklos provided. So if we mask with that and the texel size is not a multiple of 4 we receive this error.

kvark commented 5 years ago

When validation layers tell that, they also mention (typically) where "4" comes from. Hence my request to see the full message. The optimalBufferCopyOffsetAlignment is not even a requirement, strictly speaking. Perhaps, it's the fact that buffer offset has to be aligned to the texel boundary? In this case, we should compute the texel size instead of hard-coding the 4.

zakorgy commented 5 years ago

I will ask @imiklos to publish the full error log here then, since the error occurs on his machine. I think we are using the texel size for this call here https://github.com/szeged/webrender/blob/master/webrender/src/device/gfx/image.rs#L218 as self.format.bytes_per_pixel() . And this value is lower than 4 in some cases https://github.com/szeged/webrender/blob/master/webrender_api/src/image.rs#L137

zakorgy commented 5 years ago

@kvark I totally forgot that @imiklos sent me the full error previously, so here it is:

VUID-VkBufferImageCopy-bufferOffset-00194(ERROR / SPEC): msgNum: 0 - vkCmdCopyBufferToImage(): pRegion[0] bufferOffset 0x213bd must be a multiple of 4.
The Vulkan spec states: bufferOffset must be a multiple of 4 
(https://www.khronos.org/registry/vulkan/specs/1.1-extensions/html/vkspec.html#VUID-VkBufferImageCopy-bufferOffset-00194)
    Objects: 1
        [0] 0x17e4, type: 10, name: NULL

So it's in the Vulkan spec: bufferOffset must be a multiple of 4 for VkBufferImageCopy. In this case we should move this check to apply only to the buffer related to the image copy here: https://github.com/szeged/webrender/blob/master/webrender/src/device/gfx/image.rs#L218, and it would be wise to check the specification in the link for this kind of limits, maybe we miss something else as well.