Skip to content

[DeadSoon] Transparent QoF (part 1): RefractNode and Default DistortionBlur to 0 #6032

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 38 commits into from

Conversation

skhiat
Copy link
Contributor

@skhiat skhiat commented Oct 15, 2021

Transparent Quality Of Life (part 1)

  • RefractNode:
    Parametrize with eta which is the ratio of both IOR (Index Of Refraction) from incoming_medium (n_i) and outgoing_medium (n_o) eta == n_i/n_i. For instance from Air to Glass that would be 1.0/1.3333.
    If it's air-to-air interaction eta == 1:
    image
    Otherwise:
    Air-glass interaction:
    image

Note: the parameter incident is a vector which is from "source-to-surface" (for instance from eye-to-pixel):
image

We have 2 mode:

  • Safe: Protect against NaN when a sqrt of a negative number which happen when we have glass-air interaction, at a specific angle we have no refraction but pure reflection which should be handle by Fresnel.

  • CriticalAngle: Not protected the user should care of the negative case.

  • Default DistortionBlur to 0:
    Double check:

  • "Check what's is happening with existing project, does it change their value as well"

https://jira.unity3d.com/browse/HDRP-1599

@github-actions
Copy link

github-actions bot commented Oct 15, 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://unity-ci.cds.internal.unity3d.com/project/902/
Search for your PR branch using the search bar at the top, 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
/jobDefinition/.yamato%2Fall-hdrp.yml%23PR_HDRP_trunk
With changes to HDRP packages, you should also run
/jobDefinition/.yamato%2Fall-lightmapping.yml%23PR_Lightmapping_trunk

Shader Graph
/jobDefinition/.yamato%252Fall-shadergraph.yml%2523PR_ShaderGraph_trunk
Depending on your PR, you may also want
/jobDefinition/.yamato%252Fall-shadergraph_builtin_foundation.yml%2523PR_ShaderGraph_BuiltIn_Foundation_trunk
/jobDefinition/.yamato%252Fall-shadergraph_builtin_lighting.yml%2523PR_ShaderGraph_BuiltIn_Lighting_trunk

SRP Core
You could run ABV on your branch before merging your PR, but it will start A LOT of jobs. Please be responsible about it and run it only when you feel the PR is ready:
/jobDefinition/.yamato%252F_abv.yml%2523all_project_ci_trunk
Be aware that any modifications to the Core package impacts everyone in the Graphics repo so please discuss the PR with your lead.

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.

@skhiat skhiat requested review from a team and Vic-Cooper October 27, 2021 07:14
This reverts commit 38fd991.
@skhiat skhiat marked this pull request as ready for review October 27, 2021 07:50
@skhiat skhiat requested a review from kecho October 29, 2021 11:07
@skhiat skhiat marked this pull request as draft October 29, 2021 11:10
@skhiat skhiat requested review from sarah-welton and removed request for Vic-Cooper November 2, 2021 12:30
Copy link
Contributor

@sarah-welton sarah-welton left a comment

Choose a reason for hiding this comment

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

Please address these comments before final submission. Feel free to reach out to me on Slack @sarah.welton to discuss :)

@skhiat skhiat requested a review from sarah-welton November 5, 2021 10:01
Copy link
Contributor

@sarah-welton sarah-welton left a comment

Choose a reason for hiding this comment

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

If I've mis-stated anything in the text I've provided, please let me know - otherwise, please make the requested changes :) Thank you!

@skhiat skhiat requested a review from sarah-welton November 5, 2021 14:48
Copy link
Contributor

@bencloward bencloward left a comment

Choose a reason for hiding this comment

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

Looks good - thanks for making this!

@skhiat skhiat changed the title Transparent QoF (part 1): RefractNode and Default DistortionBlur to 0 [hold] Transparent QoF (part 1): RefractNode and Default DistortionBlur to 0 Nov 28, 2021
@skhiat skhiat changed the title [hold] Transparent QoF (part 1): RefractNode and Default DistortionBlur to 0 Transparent QoF (part 1): RefractNode and Default DistortionBlur to 0 Nov 30, 2021
@skhiat skhiat marked this pull request as ready for review November 30, 2021 17:05
@skhiat skhiat changed the title Transparent QoF (part 1): RefractNode and Default DistortionBlur to 0 [DeadSoon] Transparent QoF (part 1): RefractNode and Default DistortionBlur to 0 Dec 3, 2021
@skhiat skhiat changed the title [DeadSoon] Transparent QoF (part 1): RefractNode and Default DistortionBlur to 0 [DeadSoon] [DeadSoon] Transparent QoF (part 1): RefractNode and Default DistortionBlur to 0 Dec 3, 2021
@skhiat skhiat changed the title [DeadSoon] [DeadSoon] Transparent QoF (part 1): RefractNode and Default DistortionBlur to 0 [DeadSoon] Transparent QoF (part 1): RefractNode and Default DistortionBlur to 0 Dec 3, 2021
@esmelusina
Copy link
Contributor

Looks like this PR was refactored to be HDRP only and merged, and this new one will move it into ShaderGraph proper: #6924.

As there is now a replacement PR, I'm going to go ahead and close this.

@esmelusina esmelusina closed this Feb 1, 2022
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.

7 participants