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

Enhance/fix some little things in Vulkan RD #55954

Merged
merged 5 commits into from Jun 28, 2022

Conversation

RandomShaper
Copy link
Member

@RandomShaper RandomShaper commented Dec 15, 2021

I've grouped most changes in a single commit since they were minimal. Others have been kept in separate commits as I think they are important enough to be tracked separately.

Commits

  • Apply some small fixes/enhancements to the Vulkan RD

    • Initialize queue indices to values meaning 'unset'
    • Remove unused parameters & members
    • Make texture update access flags consistent with texture copy
    • Fix misuse of _buffer_memory_barrier()'s p_sync_with_draw parameter
    • Fix style and pass type of some parameters
    • Synchronize setup-draw in flush with a semaphore
    • Add no current list validation to draw_list_begin_splits()
    • Update texture usage flags on destination of copy
    • Fix misuse of Vulkan flag
    • Optimize texture update in Vulkan RD
    • Remove superfluous pre Vector resize checks 🆕
  • Fix passing dst stage mask only to single window semaphore in Vulkan RD (Removed since was done in another PR.)

  • Optimize texture update in Vulkan RD

  • Stop debug time full barriers preventing layout transitions in Vulkan RD

  • Issue finer grained barrier on texture creation in Vulkan RD

  • Clear confusion between Vulkan and RD storage buffer usage values 🆕

  • Redefine RenderingDevice::BARRIER_MASK_NO_BARRIER to be 0 🆕

  • Avoid manual memory management of certain arrays in Vulkan RD 🆕🆕

@RandomShaper RandomShaper force-pushed the vulkan_rd_goodies branch 8 times, most recently from 102c37b to ee580ba Compare December 23, 2021 13:00
@RandomShaper RandomShaper force-pushed the vulkan_rd_goodies branch 2 times, most recently from 46dc49d to 4e23797 Compare December 28, 2021 20:37
@RandomShaper RandomShaper force-pushed the vulkan_rd_goodies branch 2 times, most recently from 2f69ae9 to 0d9c76e Compare January 19, 2022 09:15
@RandomShaper RandomShaper force-pushed the vulkan_rd_goodies branch 2 times, most recently from 972fd78 to b52f9a8 Compare January 24, 2022 20:39
@@ -1963,7 +1963,7 @@ RID RenderingDeviceVulkan::texture_create(const TextureFormat &p_format, const T
image_memory_barrier.sType = VK_STRUCTURE_TYPE_IMAGE_MEMORY_BARRIER;
image_memory_barrier.pNext = nullptr;
image_memory_barrier.srcAccessMask = 0;
image_memory_barrier.dstAccessMask = VK_ACCESS_SHADER_READ_BIT;
image_memory_barrier.dstAccessMask = VK_ACCESS_SHADER_READ_BIT | VK_ACCESS_SHADER_WRITE_BIT;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can fine grain a bit more here. SHADER_WRITE_BIT would make sense only of the storage usage bit is set.
If the texture has color buffer usage, then we can mask COLOR_ATTACHMENT_OUTPUT_BIT. Likewise, if it has depth stencil usage you can use EARLY_FRAGMENT_TESTS_BIT, and if it can be updated, then TRANSFER_BIT.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've removed the change and added instead a new commit with new logic for this barrier.

drivers/vulkan/rendering_device_vulkan.cpp Outdated Show resolved Hide resolved
reduz
reduz previously approved these changes Feb 6, 2022
@reduz reduz dismissed their stale review February 6, 2022 08:54

My mistake

@reduz
Copy link
Member

reduz commented Feb 6, 2022

I think overall looks really good. I think the read/write hazzard fix may impose problems with barriers and my feeling is that this is a bug in the validation layer that we should discuss with Khronos.

@akien-mga
Copy link
Member

Might need a rebase for good measure as last update was a month ago.

- Initialize queue indices to values meaning 'unset'
- Remove unused parameters & members
- Make texture update access flags consistent with texture copy
- Fix style and pass type of some parameters
- Synchronize setup-draw in flush with a semaphore
- Add no current list validation to draw_list_begin_splits()
- Update texture usage flags on destination of copy
- Fix misuse of Vulkan flag
@RandomShaper
Copy link
Member Author

Rebased and smoke re-tested!

@akien-mga akien-mga merged commit 5ab59ee into godotengine:master Jun 28, 2022
@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.

None yet

3 participants