[HDRP][Path Tracing] Replaced recursive continuation rays with loop in ray generation #7090
Conversation
|
Yamato currently fails with DXR tests for unrelated matters. I tried running all HDRP tests locally, and they are all green. |
|
Preliminary tests shows at least three things:
Will wait for @eturquin to come back to re-evaluate what can be done for speed and fog. |
There was a problem hiding this comment.
@eturquin see with QA for validation, then try to merge before cutoff (with yamato validation)
|
hmm.. need to merge master first |
There was a problem hiding this comment.
Fog issue has been fixed, now approving ✔️
The accumulated images matches between this PR and master. ✔️
This will allow to raise the max depth by default and allow more than 10 for really complex scene (with lots of mirrors or glass etc)
There's is two or three slight differences that can be noted for future reference:
- Perf can a bit a little bit better or a little bit worse (depending on the scene and the setup)
- Intensity clamping is a bit different so there's going to be more fireflies by default (the difference is very light and can be seen only when zooming a lot)
- Noise can be a bit different
f5967f2c2a74150e369a3cc5b6e85e4d.mp4
|
In case there still is a tiny window to merge this, it is ready. |
|
replicated on provate github PR |
…n ray generation (v3) Although user-wise there should not be too much visible change (if at all), other than slightly improved performance (in the 5-10% range after early tests), this PR removes the recursion from the path tracing, with no more continuation rays shot from closesthit(). This allow us to use an arbitrary number of bounces (still caped to 32 in the UI, but could be increased to any value now), without adding to the recursion level. This also makes aspects of the code cleaner and easier to read (no more do we have to deal with different miss() functions, for instance). This will also hopefully open the door to other optimizations, like returning a BSDFData-like payload from closesthit() and do all shading computations in the raygen(), or even return a materialID and compute BSDFData or equivalent in the raygen() too. Note that this PR existed on the public repository already, and was validated by all reviewers, but slightly too late to be merged before the repo change: #7090
Although user-wise there should not be too much visible change (if at all), other than slightly improved performance (in the 5-10% range after early tests), this PR removes the recursion from the path tracing, with no more continuation rays shot from closesthit().
This allow us to use an arbitrary number of bounces (still caped to 10 in the UI, but could be increased to any value now), without adding to the recursion level. This also makes aspects of the code cleaner and easier to read (no more do we have to deal with different miss() functions, for instance).
This will also hopefully open the door to other optimizations, like returning a BSDFData-like payload from closesthit() and do all shading computations in the raygen(), or even return a materialID and compute BSDFData or equivalent in the raygen() too.
TO BE TESTED: general path tracing usage, making sure nothing is blatantly broken. 5 of the tests have changed slightly, because of a Russian roulette adjustment. Refs have been updated. Also, the old intensity clamping scheme cannot be applied anymore, without "faking" recursion, so a slightly different one is now in place, that may produce a bit more fireflies.