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

Jump to bottom

Fix various image invalidation issues #4399

Merged
merged 4 commits into from
Oct 25, 2024
+69 −53
Merged

Fix various image invalidation issues #4399

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
cd43389
[dxvk] Do not emit useless image barriers in relocateResources
doitsujin Oct 24, 2024
95563ac
[dxvk] Properly flush everything before recording defrag commands
doitsujin Oct 24, 2024
bb4c0fe
[dxvk] Fix potential issues in invalidateImageWithUsage
doitsujin Oct 24, 2024
145d741
[dxvk] Remove obsolete comments
doitsujin Oct 24, 2024
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
115 changes: 69 additions & 46 deletions src/dxvk/dxvk_context.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ namespace dxvk {

Rc<DxvkCommandList> DxvkContext::endRecording() {
this->endCurrentCommands();
this->relocateQueuedResources();

if (m_descriptorPool->shouldSubmit(false)) {
m_cmd->trackDescriptorPool(m_descriptorPool, m_descriptorManager);
Expand All @@ -102,8 +103,6 @@ namespace dxvk {


void DxvkContext::flushCommandList(DxvkSubmitStatus* status) {
relocateQueuedResources();

m_device->submitCommandList(
this->endRecording(), status);

Expand Down Expand Up @@ -1430,9 +1429,6 @@ namespace dxvk {
void DxvkContext::invalidateImage(
const Rc<DxvkImage>& image,
Rc<DxvkResourceAllocation>&& slice) {
// Ensure image is in the correct layout and not currently tracked
prepareImage(image, image->getAvailableSubresources());

invalidateImageWithUsage(image, std::move(slice), DxvkImageUsageInfo());
}

Expand All @@ -1441,28 +1437,43 @@ namespace dxvk {
const Rc<DxvkImage>& image,
Rc<DxvkResourceAllocation>&& slice,
const DxvkImageUsageInfo& usageInfo) {
Rc<DxvkResourceAllocation> prevAllocation = image->assignStorageWithUsage(std::move(slice), usageInfo);
m_cmd->track(std::move(prevAllocation));

if (usageInfo.stableGpuAddress)
m_common->memoryManager().lockResourceGpuAddress(image->storage());

VkImageUsageFlags usage = image->info().usage;

// Invalidate active image descriptors
if (usage & (VK_IMAGE_USAGE_SAMPLED_BIT | VK_IMAGE_USAGE_STORAGE_BIT))
m_descriptorState.dirtyViews(image->getShaderStages());

// Ensure that the image is in its default layout before invalidation
// and is not being tracked by the render pass layout logic. This does
// assume that the new backing storage is also in the default layout.
if (usage & (VK_IMAGE_USAGE_COLOR_ATTACHMENT_BIT | VK_IMAGE_USAGE_DEPTH_STENCIL_ATTACHMENT_BIT)) {
// Interrupt the current render pass if the image is bound for rendering
for (uint32_t i = 0; i < m_state.om.framebufferInfo.numAttachments(); i++) {
if (m_state.om.framebufferInfo.getAttachment(i).view->image() == image) {
this->spillRenderPass(false);
bool found = false;

m_flags.set(DxvkContextFlag::GpDirtyFramebuffer);
break;
}
}
for (uint32_t i = 0; i < m_state.om.framebufferInfo.numAttachments() && !found; i++)
found = m_state.om.framebufferInfo.getAttachment(i).view->image() == image;

if (found)
m_flags.set(DxvkContextFlag::GpDirtyFramebuffer);

spillRenderPass(true);

prepareImage(image, image->getAvailableSubresources());
}

// If the image has any pending layout transitions, flush them accordingly.
// There might be false positives here, but those do not affect correctness.
if (resourceHasAccess(*image, image->getAvailableSubresources(), DxvkAccess::Write)) {
spillRenderPass(true);

flushBarriers();
}

// Actually replace backing storage and make sure to keep the old one alive
Rc<DxvkResourceAllocation> prevAllocation = image->assignStorageWithUsage(std::move(slice), usageInfo);
m_cmd->track(std::move(prevAllocation));

if (usageInfo.stableGpuAddress)
m_common->memoryManager().lockResourceGpuAddress(image->storage());
}


Expand Down Expand Up @@ -6289,15 +6300,15 @@ namespace dxvk {

small_vector<VkImageMemoryBarrier2, 16> imageBarriers;

VkMemoryBarrier2 bufferBarrier = { VK_STRUCTURE_TYPE_MEMORY_BARRIER_2 };
bufferBarrier.dstStageMask |= VK_PIPELINE_STAGE_TRANSFER_BIT;
bufferBarrier.dstAccessMask |= VK_ACCESS_TRANSFER_WRITE_BIT | VK_ACCESS_TRANSFER_READ_BIT;
VkMemoryBarrier2 memoryBarrier = { VK_STRUCTURE_TYPE_MEMORY_BARRIER_2 };
memoryBarrier.dstStageMask |= VK_PIPELINE_STAGE_TRANSFER_BIT;
memoryBarrier.dstAccessMask |= VK_ACCESS_TRANSFER_WRITE_BIT | VK_ACCESS_TRANSFER_READ_BIT;

for (size_t i = 0; i < bufferCount; i++) {
const auto& info = bufferInfos[i];

bufferBarrier.srcStageMask |= info.buffer->info().stages;
bufferBarrier.srcAccessMask |= info.buffer->info().access;
memoryBarrier.srcStageMask |= info.buffer->info().stages;
memoryBarrier.srcAccessMask |= info.buffer->info().access;
}

for (size_t i = 0; i < imageCount; i++) {
Expand Down Expand Up @@ -6335,6 +6346,8 @@ namespace dxvk {
dstBarrier.image = info.storage->getImageInfo().image;
dstBarrier.subresourceRange = subresourceRange;

imageBarriers.push_back(dstBarrier);

VkImageMemoryBarrier2 srcBarrier = { VK_STRUCTURE_TYPE_IMAGE_MEMORY_BARRIER_2 };
srcBarrier.srcStageMask = info.image->info().stages;
srcBarrier.srcAccessMask = info.image->info().access;
Expand All @@ -6347,8 +6360,12 @@ namespace dxvk {
srcBarrier.image = oldStorage->getImageInfo().image;
srcBarrier.subresourceRange = subresourceRange;

imageBarriers.push_back(dstBarrier);
imageBarriers.push_back(srcBarrier);
if (srcBarrier.oldLayout != srcBarrier.newLayout) {
imageBarriers.push_back(srcBarrier);
} else {
memoryBarrier.srcStageMask |= srcBarrier.srcStageMask;
memoryBarrier.srcAccessMask |= srcBarrier.srcAccessMask;
}
}
}
}
Expand All @@ -6357,19 +6374,26 @@ namespace dxvk {
// Submit all pending barriers in one go
VkDependencyInfo depInfo = { VK_STRUCTURE_TYPE_DEPENDENCY_INFO };

if (imageCount) {
if (!imageBarriers.empty()) {
depInfo.imageMemoryBarrierCount = imageBarriers.size();
depInfo.pImageMemoryBarriers = imageBarriers.data();
}

if (bufferCount) {
if (memoryBarrier.srcStageMask) {
depInfo.memoryBarrierCount = 1u;
depInfo.pMemoryBarriers = &bufferBarrier;
depInfo.pMemoryBarriers = &memoryBarrier;
}

m_cmd->cmdPipelineBarrier(DxvkCmdBuffer::ExecBuffer, &depInfo);
m_cmd->addStatCtr(DxvkStatCounter::CmdBarrierCount, 1);

// Set up post-copy barriers
depInfo = { VK_STRUCTURE_TYPE_DEPENDENCY_INFO };

memoryBarrier = { VK_STRUCTURE_TYPE_MEMORY_BARRIER_2 };
memoryBarrier.srcStageMask |= VK_PIPELINE_STAGE_TRANSFER_BIT;
memoryBarrier.srcAccessMask |= VK_ACCESS_TRANSFER_WRITE_BIT;

imageBarriers.clear();

// Copy and invalidate all buffers
Expand All @@ -6395,6 +6419,9 @@ namespace dxvk {
m_cmd->track(info.buffer, DxvkAccess::Write);

invalidateBuffer(info.buffer, Rc<DxvkResourceAllocation>(info.storage));

memoryBarrier.dstStageMask |= info.buffer->info().stages;
memoryBarrier.dstAccessMask |= info.buffer->info().access;
}

// Copy and invalidate all images
Expand Down Expand Up @@ -6463,9 +6490,14 @@ namespace dxvk {
dstBarrier.srcQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED;
dstBarrier.dstQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED;
dstBarrier.image = info.storage->getImageInfo().image;
dstBarrier.subresourceRange = vk::makeSubresourceRange(region.dstSubresource),
dstBarrier.subresourceRange = vk::makeSubresourceRange(region.dstSubresource);

imageBarriers.push_back(dstBarrier);
if (dstBarrier.oldLayout != dstBarrier.newLayout) {
imageBarriers.push_back(dstBarrier);
} else {
memoryBarrier.dstStageMask |= dstBarrier.dstStageMask;
memoryBarrier.dstAccessMask |= dstBarrier.dstAccessMask;
}
}
}
}
Expand All @@ -6485,23 +6517,16 @@ namespace dxvk {
invalidateImageWithUsage(info.image, Rc<DxvkResourceAllocation>(info.storage), info.usageInfo);
}

// Submit post-copy barriers
bufferBarrier = { VK_STRUCTURE_TYPE_MEMORY_BARRIER_2 };
bufferBarrier.srcStageMask |= VK_PIPELINE_STAGE_TRANSFER_BIT;
bufferBarrier.srcAccessMask |= VK_ACCESS_TRANSFER_WRITE_BIT;

for (size_t i = 0; i < bufferCount; i++) {
const auto& info = bufferInfos[i];

bufferBarrier.dstStageMask |= info.buffer->info().stages;
bufferBarrier.dstAccessMask |= info.buffer->info().access;
}

if (imageCount) {
if (!imageBarriers.empty()) {
depInfo.imageMemoryBarrierCount = imageBarriers.size();
depInfo.pImageMemoryBarriers = imageBarriers.data();
}

if (memoryBarrier.dstStageMask) {
depInfo.memoryBarrierCount = 1u;
depInfo.pMemoryBarriers = &memoryBarrier;
}

m_cmd->cmdPipelineBarrier(DxvkCmdBuffer::ExecBuffer, &depInfo);
m_cmd->addStatCtr(DxvkStatCounter::CmdBarrierCount, 1);
}
Expand Down Expand Up @@ -6551,8 +6576,6 @@ namespace dxvk {
// If there are any resources to relocate, we have to stall the transfer
// queue so that subsequent resource uploads do not overlap with resource
// copies on the graphics timeline.
this->spillRenderPass(true);

relocateResources(
bufferInfos.size(), bufferInfos.data(),
imageInfos.size(), imageInfos.data());
Expand Down
7 changes: 0 additions & 7 deletions src/dxvk/dxvk_context.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -936,9 +936,6 @@ namespace dxvk {
* backing resource. This allows the host to access
* the buffer while the GPU is still accessing the
* original backing resource.
*
* \warning If the buffer is used by another context,
* invalidating it will result in undefined behaviour.
* \param [in] buffer The buffer to invalidate
* \param [in] slice New buffer slice
*/
Expand All @@ -962,8 +959,6 @@ namespace dxvk {
* \brief Invalidates image content
*
* Replaces the backing storage of an image.
* \warning If the image is used by another context,
* invalidating it will result in undefined behaviour.
* \param [in] buffer The buffer to invalidate
* \param [in] slice New buffer slice
*/
Expand All @@ -975,8 +970,6 @@ namespace dxvk {
* \brief Invalidates image content and add usage flag
*
* Replaces the backing storage of an image.
* \warning If the image is used by another context,
* invalidating it will result in undefined behaviour.
* \param [in] buffer The buffer to invalidate
* \param [in] slice New buffer slice
* \param [in] usageInfo Added usage info
Expand Down