Skip to content

Fix gpu driver crash caused by pixel history #3729

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

Open
wants to merge 1 commit into
base: v1.x
Choose a base branch
from
Open

Fix gpu driver crash caused by pixel history #3729

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions renderdoc/driver/vulkan/wrappers/vk_cmd_funcs.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1572,6 +1572,7 @@ bool WrappedVulkan::Serialise_vkBeginCommandBuffer(SerialiserType &ser, VkComman

if(m_ActionCallback && m_ActionCallback->ForceLoadRPs())
{
unwrappedInheritInfo.subpass = 0;
Copy link
Owner

@baldurk baldurk Nov 12, 2025

Choose a reason for hiding this comment

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

If I understand right I think this should happen below, after this value is used to look up the right element in loadFBs and loadRPs?

Without knowing exactly what goes wrong it's hard to say.

Copy link
Author

@nwpuCfy nwpuCfy Nov 12, 2025

Choose a reason for hiding this comment

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

when calling Pixel History, m_ActionCallback is called too in vkBeginCommandBuffer; however, in the case when vkCmdExecuteCommands is executed in 1-indexed subpass, the subpass passed to driver is 1, but the actual subpass
is 0 (because renderdoc change original rp to force load rp); this is the real case in the game ‘Star Resonance’ and causes
crash in our driver(innosilicon gpu).

Copy link
Author

@nwpuCfy nwpuCfy Nov 12, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

API call goes like this:

vkCmdBeginRenderpass
......
vkCmdExecuteCommands
......
vkCmdNextSubpass
vkCmdExecuteCommands (crash)
......
driver may need the right subpass info

Copy link
Owner

@baldurk baldurk Nov 12, 2025

Choose a reason for hiding this comment

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

OK if I understand right then, the change I mentioned should be made to ensure the right load RP is selected but otherwise this sounds good.

Copy link
Author

@nwpuCfy nwpuCfy Nov 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

OK if I understand right then, the change I mentioned should be made to ensure the right load RP is selected but otherwise this sounds good.

Yes, choose the subpass of the load pr instead of the subpass of the original pr.

Copy link
Owner

@baldurk baldurk Nov 15, 2025

Choose a reason for hiding this comment

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

Just to clarify that I am saying you should make the change I asked for above as currently this code is not correct.

Copy link
Author

@nwpuCfy nwpuCfy Nov 15, 2025

Choose a reason for hiding this comment

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

ok

if(unwrappedInheritInfo.framebuffer != VK_NULL_HANDLE)
{
const VulkanCreationInfo::Framebuffer &fbinfo =
Expand Down
Loading