Skip to content

Conversation

@JulienIgnace-Unity
Copy link
Contributor

@JulienIgnace-Unity JulienIgnace-Unity commented Jun 14, 2021

Purpose of this PR

This PR improves depth pyramid visualization.

  • Controls for remapping are now hidden if remapping is not enabled.
  • Remapping as been reworked to have the same behavior as in render doc.
    Previously it would remap [near, far] to [minRange, maxRange] (the debug params) which meant that we were compressing all
    world space values into a tiny range making effectively all values near the camera appear black. Changing max value would just
    make far values appear grey instead of white.
    Now we remap [nearminValue, farmaxValue] to [0, 1] which means that users can choose what range they want to display in
    shades of grey. To make the UI more consistent we use the [0, 1] range instead of [near, far] and use them as multipliers
    (because the near/far values are camera dependent).

Fix for https://fogbugz.unity3d.com/f/cases/1342238/


Testing status

  • Verified that remap controls were hidden if not enabled.
  • Tested the new remap scheme
  • Tested that disabling remap would still display the full range of depth (as required by AOV)

@JulienIgnace-Unity JulienIgnace-Unity requested review from a team and sebastienlagarde June 14, 2021 09:32
@github-actions github-actions bot added the HDRP label Jun 14, 2021
@TomasKiniulis
Copy link
Contributor

TomasKiniulis commented Jun 14, 2021

Thanks for the fix! It works well, one question though, shouldn't this fix also keep the visualization results same between "Enable Depth Remap" enabled and disabled? I guess that means changing default depth remapping values to the ones that are used when checkbox is disabled? But in my opinion that would still make more sense than having remap set to min 0 and max 1 as this way barely any depth gradient is actually visible even with near infinite scale meshes.

Edit: Discussing with Julien seems we cannot directly control default remap values since they are camera dependant

2021-06-14.12-51-27.mp4

@JulienIgnace-Unity JulienIgnace-Unity marked this pull request as ready for review June 14, 2021 12:36
@JulienIgnace-Unity JulienIgnace-Unity requested review from a team as code owners June 15, 2021 09:26
…phics into hd/fix-depth-debug-remap

# Conflicts:
#	com.unity.render-pipelines.high-definition/CHANGELOG.md
@JulienIgnace-Unity JulienIgnace-Unity force-pushed the hd/fix-depth-debug-remap branch from fbc7e2b to 9dd025f Compare June 15, 2021 09:31
@theopnv theopnv removed the request for review from a team June 15, 2021 09:50
…phics into hd/fix-depth-debug-remap

# Conflicts:
#	com.unity.render-pipelines.high-definition/CHANGELOG.md
#	com.unity.render-pipelines.high-definition/Runtime/Debug/DebugDisplay.cs
@sebastienlagarde sebastienlagarde removed request for a team June 17, 2021 17:53
@sebastienlagarde sebastienlagarde merged commit 7f5b564 into hd/bugfix Jun 17, 2021
@sebastienlagarde sebastienlagarde deleted the hd/fix-depth-debug-remap branch June 17, 2021 17:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants