VDR: Transfer commands #2561
VDR: Transfer commands #2561
Conversation
|
CI gfxreconstruct build queued with queue ID 601674. |
|
CI gfxreconstruct build # 8353 running. |
|
CI gfxreconstruct build # 8353 passed. |
panos-lunarg
force-pushed
the
VDR_transfer_commands
branch
from
4bcd76c to
2756de4
Compare
|
CI gfxreconstruct build queued with queue ID 601798. |
|
CI gfxreconstruct build # 8355 running. |
|
CI gfxreconstruct build # 8355 passed. |
panos-lunarg
force-pushed
the
VDR_transfer_commands
branch
from
2756de4 to
b43b869
Compare
|
CI gfxreconstruct build queued with queue ID 601922. |
|
CI gfxreconstruct build # 8356 running. |
|
CI gfxreconstruct build # 8356 passed. |
panos-lunarg
force-pushed
the
VDR_transfer_commands
branch
from
b43b869 to
a10d23f
Compare
|
CI gfxreconstruct build queued with queue ID 602778. |
|
CI gfxreconstruct build # 8357 running. |
panos-lunarg
force-pushed
the
VDR_transfer_commands
branch
from
a10d23f to
0eefbe8
Compare
|
CI gfxreconstruct build queued with queue ID 602877. |
|
CI gfxreconstruct build # 8358 running. |
|
CI gfxreconstruct build # 8358 passed. |
panos-lunarg
force-pushed
the
VDR_transfer_commands
branch
from
0eefbe8 to
4e38fba
Compare
|
CI gfxreconstruct build queued with queue ID 603105. |
|
CI gfxreconstruct build # 8361 running. |
panos-lunarg
force-pushed
the
VDR_transfer_commands
branch
from
4e38fba to
62415c7
Compare
|
CI gfxreconstruct build queued with queue ID 603133. |
|
CI gfxreconstruct build # 8362 running. |
|
CI gfxreconstruct build # 8362 passed. |
panos-lunarg
force-pushed
the
VDR_transfer_commands
branch
from
62415c7 to
1d8a485
Compare
|
CI gfxreconstruct build queued with queue ID 604037. |
|
CI gfxreconstruct build # 8375 running. |
|
CI gfxreconstruct build # 8375 passed. |
panos-lunarg
force-pushed
the
VDR_transfer_commands
branch
from
1d8a485 to
5b2a3b6
Compare
| return true; | ||
| } | ||
|
|
||
| void DefaultVulkanDumpResourcesDelegate::GenerateOutputJsonTransferInfo( |
There was a problem hiding this comment.
This is a really long function - would it make sense to break it out into smaller utility subfunctions for readability? Maybe just the chunks on the inside of the case blocks?
|
|
||
| if (bc) | ||
| { | ||
| before_params = CopyBuffer(s, d); |
There was a problem hiding this comment.
It looks like before_params is identical to params; can you help me understand what the purpose is of having separate params and before_params?
| VulkanImageInfo image_info; | ||
| }; | ||
|
|
||
| // kInitBufferCommand |
There was a problem hiding this comment.
I think these would be better as a polymorphic construct, like TransferParamsBase, and then subclasses - virtual destructors would remove the get_if chain and the variants wouldn't need to be maintained when the list of classes changes. There are a lot of dependencies on things, like sometimes there's a before_params and sometimes not, and it would be nice if that was presented directly in the subclass and not left to being an empty reference or variant.
bradgrantham-lunarg
left a comment
bradgrantham-lunarg
left a comment
There was a problem hiding this comment.
In this change you're storing the contexts in the map as smart pointers but still returning a vector of raw pointers. Store them in the map as shared_ptr and return a vector of shared_pointer instead of a vector of raw pointers. (I may have said "unique_ptr" before and if so I apologize.)
The maps that hold the dumping contexts store the contexts themselves inside the map. This makes them vulnerable to errors where a pointer/reference to a contexts becomes invalid when there's a modification to the map. This commits switches to storing them as smart pointers instead, making them more robust against map changes.
IsRecording()'s job is to guard the resource dumping logic to be called when not actually dumping anything. In its current logic it searches among available dumping contexts in an inefficient and complicated way which is not 100% correct. This commit simplifies the logic by keeping track of the currently active dumping contexts.
ReleaseDrawCallContexts, ReleaseDispatchTraceRaysContexts, and ReleaseTransferContexts iterator over existing contexts with iterator based for loops. These are replaces with range-based loops
TransferDumpingContext is storing each different transfer command using a variant. This commit refactors this by removing the variant and use inheritance instead. A base class TransferParamsBase is defined and each transfer command is a child of the base class. This should simplify adding new commands
Each transfer command dumps only one "resource" so DumpedResourcesInfo::dumped_transfer_commands is made from a vector into a smart pointer.
Split DefaultVulkanDumpResourcesDelegate::GenerateOutputJsonTransferInfo into smaller ones that handle each case separately
DumpedBuffer::buffer_info now is initialized in DumpedBuffer's constructors initialization lists
panos-lunarg
force-pushed
the
VDR_transfer_commands
branch
from
223fbef to
5b461c3
Compare
|
CI gfxreconstruct build queued with queue ID 614190. |
|
CI gfxreconstruct build # 8486 running. |
|
CI gfxreconstruct build # 8486 failed. |
panos-lunarg
force-pushed
the
VDR_transfer_commands
branch
from
5b461c3 to
fb2ad32
Compare
|
CI gfxreconstruct build queued with queue ID 614215. |
|
CI gfxreconstruct build # 8487 running. |
|
CI gfxreconstruct build # 8487 failed. |
CloneBuffer is removed as it has a deceiving name and does the same job as CreateVkBuffer. For same reasons CloneImage is renamed into CreateVkImage
GetMemoryTypeIndex() from vulkan_replay_dump_resources_common.h/cpp is remove as there is an identical function in vulkan_device_util.h/cpp
Move helper functions ScaleToMipLevel and ScaleExtent from vulkan_replay_dump_resources_common.h into graphics/vulkan_util.h
The pattern of vkCmdCopyBuffer + PipelineBarrier was repeating in several places.
Some of the Find*Context were not used. Also some of them are renamed for uniformity
Move dumped resources structures that are related to the delegate class into a separate file. Also move AccelerationStructureDumpResourcesContext related code into a separate file
panos-lunarg
force-pushed
the
VDR_transfer_commands
branch
from
fb2ad32 to
741683c
Compare
|
CI gfxreconstruct build queued with queue ID 614307. |
|
CI gfxreconstruct build # 8488 running. |
|
CI gfxreconstruct build # 8488 passed. |
3 participants
VDR: Transfer commands
It is now possible to dump resources targeted by transfer API calls. The
following transfer commands are added:
vkCmdCopyBuffer
vkCmdCopyBuffer2
vkCmdCopyBuffer2KHR
vkCmdCopyBufferToImage
vkCmdCopyBufferToImage2
vkCmdCopyBufferToImage2KHR
vkCmdCopyImage
vkCmdCopyImage2
vkCmdCopyImage2KHR
vkCmdCopyImageToBuffer
vkCmdCopyImageToBuffer2
vkCmdCopyImageToBuffer2KHR
vkCmdBuildAccelerationStructuresKHR
vkCmdCopyAccelerationStructureKHR
Additionaly the following GFXR transfer meta commands are also
supported:
kInitBufferCommand
kInitImageCommand
kVulkanBuildAccelerationStructuresCommand
kVulkanCopyAccelerationStructuresCommand
More details can be found in the vulkan_dump_resources.md documentation