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

Change Vulkan RHI CommandList::SetStreamBuffers to not do 2 memory allocation per draw by using fixed_vector #6393

Conversation

rgba16f
Copy link
Contributor

@rgba16f rgba16f commented Dec 13, 2021

…location per draw by using fixed_vector

Signed-off-by: rgba16f <82187279+rgba16f@users.noreply.github.com>
@rgba16f rgba16f merged commit a60fba5 into o3de:stabilization/2111RTE Dec 14, 2021
@rgba16f rgba16f deleted the Atom/rgba16f/VulkanRHICpuOptimization branch December 14, 2021 20:15
AZStd::vector<VkBuffer> nativeBuffers(numBuffers, VK_NULL_HANDLE);
AZStd::vector<VkDeviceSize> offsets(numBuffers, 0);
AZStd::fixed_vector<VkBuffer, RHI::Limits::Pipeline::StreamCountMax> nativeBuffers(numBuffers, VK_NULL_HANDLE);
AZStd::fixed_vector<VkDeviceSize, RHI::Limits::Pipeline::StreamCountMax> offsets(numBuffers, 0);
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure if you'd want to add it to this PR, but there's another small gain to be had at this point:

for (uint32_t i = 0; i < numBuffers; ++i)
{
    const RHI::StreamBufferView& bufferView = streams[i + interval.m_min];
    if (!bufferView.GetBuffer())
    {
        continue; // in this case entries in nativeBuffers and offsets are unchanged from their state pre-initialized above
    } 
    const auto* bufferMemoryView = static_cast<const Buffer*>(bufferView.GetBuffer())->GetBufferMemoryView();
    nativeBuffers[i] = bufferMemoryView->GetNativeBuffer();
    offsets[i] = bufferMemoryView->GetOffset() + bufferView.GetByteOffset();
}

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

Successfully merging this pull request may close these issues.

4 participants