Skip to content

Conversation

eturquin
Copy link
Contributor

When rescaling a sample for later reuse (in path tracing), there could be situations where, due to numerical imprecision, the sample value would reach 1.0, potentially causing issues in subsequent computations (NaNs, zeros, etc.). Such issues were spotted for instance in volumetric scattering. This PR addresses that problem.

@github-actions
Copy link

Hi! This comment will help you figure out which jobs to run before merging your PR. The suggestions are dynamic based on what files you have changed.
Link to Yamato: https://yamato.cds.internal.unity3d.com/jobs/902-Graphics
Search for your PR branch using the sidebar on the left, then add the following segment(s) to the end of the URL (you may need multiple tabs depending on how many packages you change)

HDRP
/.yamato%252Fall-hdrp.yml%2523PR_HDRP_trunk
With changes to HDRP packages, you should also run
/.yamato%252Fall-lightmapper.yml%2523PR_LightMapper_trunk

Depending on the scope of your PR, you may need to run more jobs than what has been suggested. Please speak to your lead or a Graphics SDET (#devs-graphics-automation) if you are unsure.

@github-actions
Copy link

It appears that you made a non-draft PR!
Please convert your PR to draft (button on the right side of the page).
See the PR template for more information.
Thank you!

Copy link
Contributor

@remi-chapelain remi-chapelain left a comment

Choose a reason for hiding this comment

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

For the fix in the PR, couln't find any issues after clamping the value ✔️

Using this PR to give feedback on top of theses fixes.
Theses improvements helps a lot what happens with volumetric fog in raster !

There is still some discrepencies, not linked to specific issues but just unsuported features of the volumetric fog override by the pathtracer.
What I would like is to include those in the documentation. (Adding a Volumetric Fog section, under sub surface scattering section for example, here)

Here's a list of what is currently not supported.
Some are linked to how they are rendered in raster so would be expected (like the quality stuff) but others are just unsupported but could make sense in pathtracing pipeline:

  • Density volume, now called Local Volumetric Fog (plays a large role is discrepencies in project where there is one setup)
  • Shadow dimmer in Volumetric options in lights is ignored
  • Ambient light probe dimmer
  • Volumetric fog distance
  • Slice deformation uniformity
  • Denoising mode
  • All the quality controls
  • Directionnal light only checkbox
  • Anisotropy

image

image

image

@eturquin
Copy link
Contributor Author

@sebastienlagarde, @remi-chapelain, I'll add a couple of these missing features that are easy to add (light and shadow dimmers, directional-only mode) to this PR, do not merge it just yet.

@eturquin
Copy link
Contributor Author

eturquin commented Sep 29, 2021

I added support for

  • light/shadow dimmers (volumetric and non-volumetric)
  • directional lights only volumetric scattering

The PR is now ready for review.

@remi-chapelain remi-chapelain self-requested a review September 29, 2021 13:03
Copy link
Contributor

@remi-chapelain remi-chapelain left a comment

Choose a reason for hiding this comment

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

Thanks for the additions, theses are great to better match what happens in raster ! 🥳
I tested and couldn't find any issues ✔️

However, would be great to just add a section in the doc of the pathtracer just to note the rest of the things unsupported for now :

  • Local volumetric fog (aka old density volume)
  • Ambient light probe dimmer
  • Volumetric fog distance
  • Anisotropy
  • And raster quality settings (denoising, quality control, slice deformation uniformity.. etc)

@eturquin
Copy link
Contributor Author

@remi-chapelain, the doc will be updated in another PR.

@eturquin eturquin closed this Sep 30, 2021
@eturquin eturquin reopened this Sep 30, 2021
@remi-chapelain remi-chapelain self-requested a review October 1, 2021 08:05
sebastienlagarde added a commit that referenced this pull request Oct 3, 2021
* Add missing DisallowMultipleComponent attribute in AdditionalData components #5859

* [HDRP][Path Tracing] Slight robustness improvement for sample rescaling #5800

* recaptured reflection probes, updated normals on 2nd room's floor (#5873)

* Hair Documentation Pass (#5868)

* Hair Documentation Pass

* Add the images

* Apply various feedback to the documentation

* Fix small typo

* Update the OSX reference image (forgot to add it previous PR, causing failure) and rerun Yamato

* Update the tooltips

* [HDRP] Remove Fake GTAO bounce on the occlusion when used for direct lighting #5836

* Update 5014_VolumetricCloudsBanding.png

* Update 2307_Shadow_VeryHigh.png

Co-authored-by: Arttu Peltonen <77337829+arttu-peltonen@users.noreply.github.com>
Co-authored-by: Emmanuel Turquin <emmanuel@turquin.org>
Co-authored-by: pierre-unity <39901544+pierre-unity@users.noreply.github.com>
Co-authored-by: John Parsaie <johnpa@unity3d.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants