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

Add delegate for Vulkan dump resources #1941

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

locke-lunarg
Copy link
Contributor

@locke-lunarg locke-lunarg commented Jan 4, 2025

Move code about writing data from vulkan_replay_dump_resources_draw_calls and vulkan_replay_dump_resources_compute_ray_tracing to DefaultVulkanDumpResourcesDelegate on vulkan_replay_dump_resources_json vulkan_replay_dump_resources_delegate.

Closed: #1764

@ci-tester-lunarg
Copy link

CI gfxreconstruct build queued with queue ID 336698.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build # 5687 running.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build queued with queue ID 336714.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build # 5688 running.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build # 5688 failed.

Copy link
Contributor

@panos-lunarg panos-lunarg left a comment

Choose a reason for hiding this comment

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

I like this change. It abstracts out code that can be reused from both draw calls and dispatch/trace rays classes.
With a first glance one change I would like to request a few high level changes:

  • DrawCallParameters, DispatchParameters and TraceRaysParameters (and other smaller structs such as RenderTargets, VertexInputState, BoundVertexBuffersInfo etc that are closely related to the original classes) to remain where they were (DrawCallsDumpingContext and DispatchTraceRaysDumpingContext)
  • The new functionality to be moved in a new .h + .cpp pair of files (i.e. vulkan_dump_resources_dumping_delegate.[h|cpp] or similar) so that vulkan_replay_dump_resources_json files contain only json related code.
  • Creating a forward declaration of the new VulkanDumpResourcesDelegate and DefaultVulkanDumpResourcesDelegate classes in vulkan_replay_dump_resources_common.h should avoid the cyclic header dependencies and the new header file vulkan_dump_resources_dumping_delegate.h can include vulkan_replay_dump_resources_draw_calls.h and vulkan_replay_dump_resources_compute_ray_tracing.h
  • Rest smaller new structs (DumpResourceType, VulkanDumpResourceInfo, VulkanDumpDrawCallInfo) can also be declared in vulkan_replay_dump_resources_common.h

@davidlunarg
Copy link
Contributor

I built this branch Debug on my Windows box. I tried a replay with dump resources. I immediately got an error: "Run-Time Check Failure #3 - The variable 'quit-frame' is being used without being initialized". I don't think this issue wasn't introduced by this PR, I think it was introduced by a prior commit. Not sure why we haven't encountered this before.

@bradgrantham-lunarg
Copy link
Contributor

I built this branch Debug on my Windows box. I tried a replay with dump resources. I immediately got an error: "Run-Time Check Failure #3 - The variable 'quit-frame' is being used without being initialized". I don't think this issue wasn't introduced by this PR, I think it was introduced by a prior commit. Not sure why we haven't encountered this before.

I fixed this issue last week - #1940 - so this PR branch needs to be updated.

@davidlunarg
Copy link
Contributor

davidlunarg commented Jan 6, 2025

I get a segmentation fault when dumping resources for vkcube. Here's the command I used:

gfxrecon-replay.exe --sfa --remove-unsupported --dump-resources vkcube_1_3.dr-args.txt --dump-resources-before-draw --dump-resources-json-output-per-command vkcube_1_3.gfxr

The file vkcube_1_3.dr-args.txt contains:

BeginCommandBuffer=110,Draw=116,BeginRenderPass=111,EndRenderPass=117,QueueSubmit=147
BeginCommandBuffer=110,Draw=116,BeginRenderPass=111,EndRenderPass=117,QueueSubmit=165

The seg fault occurs on line 934 in vulkan_replay_dump_resources_common.cpp because device_info is null. I also noticed that device_table and instance_table are null.

vkcube_1_3.gfxr is a copy of //nas./smb_traces/vulkan/GFXR/windows-nvidia/vkcube_1_3/gfxrecon_capture_vkcube_1.3.gfxr

Also found that a simple dump resources fails in the same way:

gfxrecon-replay.exe --sfa --remove-unsupported --dump-resources BeginCommandBuffer=110,Draw=116,BeginRenderPass=111,EndRenderPass=117,QueueSubmit=147 vkcube_1_3.gfxr

@ci-tester-lunarg
Copy link

CI gfxreconstruct build queued with queue ID 338832.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build # 5695 running.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build # 5695 failed.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build queued with queue ID 338861.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build # 5696 running.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build # 5696 failed.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build queued with queue ID 338887.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build # 5697 running.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build # 5697 failed.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build queued with queue ID 339507.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build # 5704 running.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build queued with queue ID 339531.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build queued with queue ID 339548.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build queued with queue ID 339574.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build queued with queue ID 339665.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build queued with queue ID 339719.

@locke-lunarg
Copy link
Contributor Author

@panos-lunarg Thank you. I made the changes that you mentioned, except that moving VulkanDumpResourceInfo and VulkanDumpDrawCallInfo to vulkan_replay_dump_resources_common since some structs, like DrawCallsDumpingContext::RenderTargets, couldn't create forward declaration. It could do if I move RenderTargets and some structs out DrawCallsDumpingContext. I could do it if that is your prefer.

@davidlunarg I fixed these two crashes issues. Thank you for test.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build # 5713 running.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build # 5713 failed.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build queued with queue ID 339876.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build # 5715 running.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build queued with queue ID 339892.

Move code about writing data from
vulkan_replay_dump_resources_draw_calls and
vulkan_replay_dump_resources_compute_ray_tracing to vulkan_replay_dump_resources_delegate.
@ci-tester-lunarg
Copy link

CI gfxreconstruct build queued with queue ID 339894.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build # 5717 running.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build # 5717 failed.

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.

Vulkan dump resources - add optional callback for handling dumped resource data
5 participants