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.

Sign up for GitHub

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

Polish a few things in Vulkan RD #81912

Merged
merged 1 commit into from
Sep 20, 2023
Merged

Polish a few things in Vulkan RD #81912

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 4 additions & 7 deletions drivers/vulkan/rendering_device_vulkan.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3112,7 +3112,7 @@ Error RenderingDeviceVulkan::texture_copy(RID p_from_texture, RID p_to_texture,
image_memory_barrier.srcQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED;
image_memory_barrier.dstQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED;
image_memory_barrier.image = dst_tex->image;
image_memory_barrier.subresourceRange.aspectMask = VK_IMAGE_ASPECT_COLOR_BIT;
image_memory_barrier.subresourceRange.aspectMask = dst_tex->read_aspect_mask;
RandomShaper marked this conversation as resolved.
Show resolved Hide resolved
image_memory_barrier.subresourceRange.baseMipLevel = p_src_mipmap;
image_memory_barrier.subresourceRange.levelCount = 1;
image_memory_barrier.subresourceRange.baseArrayLayer = p_src_layer;
Expand Down Expand Up @@ -3294,7 +3294,7 @@ Error RenderingDeviceVulkan::texture_resolve_multisample(RID p_from_texture, RID
image_memory_barrier.srcQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED;
image_memory_barrier.dstQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED;
image_memory_barrier.image = dst_tex->image;
image_memory_barrier.subresourceRange.aspectMask = VK_IMAGE_ASPECT_COLOR_BIT;
image_memory_barrier.subresourceRange.aspectMask = dst_tex->read_aspect_mask;
image_memory_barrier.subresourceRange.baseMipLevel = dst_tex->base_mipmap;
image_memory_barrier.subresourceRange.levelCount = 1;
image_memory_barrier.subresourceRange.baseArrayLayer = dst_tex->base_layer;
Expand Down Expand Up @@ -5918,7 +5918,7 @@ Error RenderingDeviceVulkan::buffer_copy(RID p_src_buffer, RID p_dst_buffer, uin

// This method assumes the barriers have been pushed prior to being called, therefore no barriers are pushed
// for the source or destination buffers before performing the copy. These masks are effectively ignored.
VkPipelineShaderStageCreateFlags src_stage_mask = 0;
VkPipelineStageFlags src_stage_mask = 0;
Copy link
Member Author

Choose a reason for hiding this comment

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

This worked because in the end it's a plain int, but was the wrong typedef.

VkAccessFlags src_access_mask = 0;
Buffer *src_buffer = _get_buffer_from_owner(p_src_buffer, src_stage_mask, src_access_mask, BARRIER_MASK_NO_BARRIER);
if (!src_buffer) {
Expand Down Expand Up @@ -6038,9 +6038,6 @@ Error RenderingDeviceVulkan::buffer_clear(RID p_buffer, uint32_t p_offset, uint3
ERR_FAIL_COND_V_MSG(p_offset + p_size > buffer->size, ERR_INVALID_PARAMETER,
"Attempted to write buffer (" + itos((p_offset + p_size) - buffer->size) + " bytes) past the end.");

// Should not be needed.
// _buffer_memory_barrier(buffer->buffer, p_offset, p_size, dst_stage_mask, VK_PIPELINE_STAGE_TRANSFER_BIT, dst_access, VK_ACCESS_TRANSFER_WRITE_BIT, p_post_barrier);
Comment on lines -6041 to -6042
Copy link
Member Author

Choose a reason for hiding this comment

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

By now we'd already know. Anyway, we can always find out it's not true and re-add, tracking this removal if needed.


vkCmdFillBuffer(frames[frame].draw_command_buffer, buffer->buffer, p_offset, p_size, 0);

#ifdef FORCE_FULL_BARRIER
Expand All @@ -6050,7 +6047,7 @@ Error RenderingDeviceVulkan::buffer_clear(RID p_buffer, uint32_t p_offset, uint3
dst_stage_mask = VK_PIPELINE_STAGE_BOTTOM_OF_PIPE_BIT;
}

_buffer_memory_barrier(buffer->buffer, p_offset, p_size, VK_PIPELINE_STAGE_TRANSFER_BIT, dst_stage_mask, VK_ACCESS_TRANSFER_WRITE_BIT, dst_access, dst_stage_mask);
_buffer_memory_barrier(buffer->buffer, p_offset, p_size, VK_PIPELINE_STAGE_TRANSFER_BIT, dst_stage_mask, VK_ACCESS_TRANSFER_WRITE_BIT, dst_access, true);
Copy link
Member Author

Choose a reason for hiding this comment

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

This worked before the change because dst_stage_mask would always be non-zero, that is, true would be passed as the argument, but the code was awkward.

Copy link

Choose a reason for hiding this comment

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

buffer_update() above is very similar to this function but it wraps this barrier in a condition. Should this function also have the condition?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's very likely, but since this PR aims to be about very silly little things, the reasoning to be sure is out of its scope.


#endif
return OK;
Expand Down