nvpro-samples / vk_mini_samples

Collection of Vulkan samples
Apache License 2.0
184 stars 12 forks source link

SER pathtrace sample does nothing, SER doesn't appear to work on Vulkan #8

Closed BattleAxeVR closed 5 months ago

BattleAxeVR commented 5 months ago

Just wanted to add this here as well for visibility, since the issue affects both Path Tracing SDK (in Vulkan mode, DX12 works fine), and this repo as well.

Toggling the SER feature on/off has no effect on the framerate, nor the heatmap:

https://github.com/NVIDIAGameWorks/Path-Tracing-SDK/issues/2

I tried both game ready and latest beta drivers on my RTX 4090, in both code samples by Nvidia, and in both apps, the SER feature doesn't seem to work at all on Vulkan, only DX12 (in Path Tracing SDK). To enable it in that repo I had to add a couple lines, since it appears obviously that Vulkan is a second-class citizen compared to DX12.

BattleAxeVR commented 5 months ago

SER OFF:

image

BattleAxeVR commented 5 months ago

SER ON:

image

NBickford-NV commented 5 months ago

Looks like part of this is a silly bug in ser_pathtrace: the propagation of the specialization constant was accidentally removed 2 weeks ago in https://github.com/nvpro-samples/vk_mini_samples/commit/c5e5d6a550dd1ce96d6e1e62dfae2fdacdc51f9b#diff-6f12885eca6326d135a50a24e87c259021fad71cab5e5f23001e4f89020c78cbL557. (To reproduce, try modifying payload.nrm in the USE_SER branch of traceRay() in pathtrace.rgen.)

This fixes that:

    m_dutil->setObjectName(stages[eClosestHit].module, "Closest Hit");
#endif

+    stages[eRaygen].pSpecializationInfo = specialization.getSpecialization();

Could you try that on the 4090 and see if you see a difference now?

BattleAxeVR commented 5 months ago

YES! That fixed it. Thanks so much!

NBickford-NV commented 5 months ago

Awesome! I'll close this bug once the fix is in the public repo.

BattleAxeVR commented 5 months ago

image

BattleAxeVR commented 5 months ago

Awesome! I'll close this bug once the fix is in the public repo.

Any chance you could help me figure out why it's also broken in the Path Tracing SDK? I'd like to validate VK + DX12 have the same exact perf w/ SER enabled.

BattleAxeVR commented 5 months ago

I already got that repo to show the feature works / enumerated by adding it to the requested device extensions on VK, but the toggle isn't doing anything.

mklefrancois commented 5 months ago

Thanks for reporting, this commit should fix the issue. https://github.com/nvpro-samples/vk_mini_samples/commit/6b70af33aad8bf3c214f77b72357e2350dd02b1c

NBickford-NV commented 5 months ago

@BattleAxeVR For the Path Tracing SDK, let's follow up on the issue at https://github.com/NVIDIAGameWorks/Path-Tracing-SDK/issues/2#issuecomment-1920882049 .