Skip to content

Conversation

@anisunity
Copy link
Contributor

Original problem described here:
https://forum.unity.com/threads/feature-request-better-denoiser-for-gi.1132984/

Major fixes of this PR
Exposure is now properly handeled as well as exposure weight for materials.
Sky is always clamped in HSV space
All materials are clamped in RGB space for RTR
All materials are clamped in HSV for RTGI

A regression was introduced in this PR #4340
image

Added new tests to cover properly all the cases.

Testing status
The new tests pass locally.
Pair tested with @remi-chapelain

@anisunity anisunity requested review from a team, remi-chapelain and sebastienlagarde July 6, 2021 12:49
@anisunity anisunity self-assigned this Jul 6, 2021
@github-actions
Copy link

github-actions bot commented Jul 6, 2021

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_2021.2

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

github-actions bot commented Jul 6, 2021

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.

Has already been pair tested extensively.
Thanks for extending the coverage of the previous tests (added RTGI and high exposure that makes intensity values go over the buffer limit maximum value)

@remi-chapelain remi-chapelain self-requested a review July 6, 2021 13:22
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Update this

@anisunity
Copy link
Contributor Author

Fixed comment and launched tests

@anisunity anisunity force-pushed the HDRP/emissive-rt-perf branch from 8d19c10 to d7d2773 Compare July 8, 2021 06:33
@sebastienlagarde sebastienlagarde merged commit 3b79056 into master Jul 9, 2021
@sebastienlagarde sebastienlagarde deleted the HDRP/emissive-rt-perf branch July 9, 2021 14:03
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.

4 participants