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

Fix a crash on replay for trimmed trace #1375

Conversation

naomipappe
Copy link
Contributor

When the descriptor sets are updated using vkUpdateDescriptorSetWithTemplate in the captured application, and the update in question involves updating acceleration structures, the trimmed trace would crash on replay.

This is caused by the fact that state recreation uses vkUpdateDescriptorSets and VkWriteDescriptorSet, which requires the information for the update of acceleration structures be chained in the pNext field of relevant VkWriteDescriptorSet with VkWriteDescriptorSetAccelerationStructureKHR structure filled appropriately. As the vkUpdateDescriptorSetWithTemplate does not share this structure, the pNext ends up empty on state recreation, and thus causes a crash.

This change fixes that, by filling out the required structure and copying it.

This change also depends on #1363 to properly copy the structure data.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build queued with queue ID 91877.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build # 3576 running.

@naomipappe naomipappe changed the title Fixes a crash on replaying a fastforwarded trace Fix a crash on replay for trimmed trace Nov 29, 2023
@ci-tester-lunarg
Copy link

CI gfxreconstruct build # 3576 failed.

@naomipappe naomipappe marked this pull request as draft November 29, 2023 16:36
@bradgrantham-lunarg
Copy link
Contributor

I think if you make your branch for this PR to be the #1363 branch plus these changes, then CI won't fail because of missing functions, and after #1363 is merged the commits in this PR that are also in #1363's branch will not be duplicated when this PR is merged.

@naomipappe naomipappe force-pushed the fix-trimming-with-acceleration-structures branch from ae41810 to 7a5681d Compare December 4, 2023 11:45
@ci-tester-lunarg
Copy link

CI gfxreconstruct build queued with queue ID 94610.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build # 3587 running.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build # 3587 failed.

@lunarpapillo
Copy link
Contributor

The LunarG CI failure appears to be due to an unrelated GPU crash. I've restored the machine and will restart the test.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build queued with queue ID 94960.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build # 3590 running.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build # 3590 passed.

@naomipappe naomipappe marked this pull request as ready for review December 5, 2023 10:32
@MarkY-LunarG MarkY-LunarG self-requested a review January 19, 2024 16:12
If acceleration structure is updated with template, the pNext was not filled, as it is not used for update with template. The state recreation, however, uses ordinary descriptor update call, so pNext must be filled. This changes creates relevant struct and copies it's contents to pNext for state recreation.

Change-Id: If42324ff887db6d591b55c10072f5f5ad81d6f2e
@MarkY-LunarG
Copy link
Contributor

I wrote a test to verify this fix in my fork of Sascha Willem's examples. I'm also going to create a separate issue to not use the pnext chains like we are (as you mentioned) but to eventually fix this correctly.

@MarkY-LunarG MarkY-LunarG force-pushed the fix-trimming-with-acceleration-structures branch from 7a5681d to aeb1292 Compare February 7, 2024 19:31
@ci-tester-lunarg
Copy link

CI gfxreconstruct build queued with queue ID 130149.

@MarkY-LunarG
Copy link
Contributor

Rebased on dev (now that #1363 was integrated it through this into chaos.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build # 3723 running.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build # 3723 passed.

@MarkY-LunarG MarkY-LunarG merged commit 03b4aef into LunarG:dev Feb 7, 2024
8 checks passed
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.

6 participants