Skip to content

[diedsoon] Transparent QoF (part 2): RemapAlpha, Node Scene Depth Difference, #6201

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 22 commits into from

Conversation

skhiat
Copy link
Contributor

@skhiat skhiat commented Nov 4, 2021

Remap Transparent Channel of the BaseColor/Color texture for {Lit, Unlit, LayeredLit, LitTesselation, LayeredLitTesselation}
Lit:
image

Unlit:
image

LayeredLit:
image

LitTesselation:
image

LayeredLitTesselation:
image

Add Scene Depth Difference for 3 format: {Linear01, Raw, Eye}:
image

Approved by UX - Maddelena

UX review notes: changes the order of parameters

https://jira.unity3d.com/browse/HDRP-1600
https://jira.unity3d.com/browse/HDRP-1602
https://jira.unity3d.com/browse/HDRP-1603

@github-actions
Copy link

github-actions bot commented Nov 4, 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

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 changed the title Transparent QoF (part 1): RemapAlpha, Node Scene Depth Difference, Transparent QoF (part 2): RemapAlpha, Node Scene Depth Difference, Nov 4, 2021
@skhiat skhiat requested review from a team November 4, 2021 11:33
Copy link
Contributor

@sebastienlagarde sebastienlagarde left a comment

Choose a reason for hiding this comment

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

This PR have been built on top of the other PR, so it include 2 change. (for reviewvers)

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.

Double checked the remapping sliders on all compatible shaders ✔️

The new node works well but users may benefit from understanding how the node is made. Maybe it can be interesting to have it a subgraph since it's mostly subtracting SceneDepth and Pixeldepth ?
EDIT: Not possible since we can't do enum as input in subgraph.

Also there's no documentation so we should document it.
And adding a test using the node in the shadergraph project would be great ! :)

@skhiat skhiat force-pushed the HDRP/transparent_quality_of_life_part2 branch from 26b4025 to b0e5ac5 Compare November 30, 2021 15:21
@skhiat skhiat requested a review from remi-chapelain December 2, 2021 14:13
@esmelusina
Copy link
Contributor

Can you separate SG specific changes into a separate PR? It doesn't seem like the alpha remapping for HDRP and SceneDepthDifference need to be coupled.

@skhiat
Copy link
Contributor Author

skhiat commented Dec 2, 2021

Can you separate SG specific changes into a separate PR? It doesn't seem like the alpha remapping for HDRP and SceneDepthDifference need to be coupled.

checkout the PR here:
#6467

@skhiat skhiat changed the title Transparent QoF (part 2): RemapAlpha, Node Scene Depth Difference, [diedsoon] Transparent QoF (part 2): RemapAlpha, Node Scene Depth Difference, Dec 8, 2021
@esmelusina
Copy link
Contributor

Can this be closed?

@skhiat
Copy link
Contributor Author

skhiat commented Feb 9, 2022

Can this be closed?

I'll close them when we merge the other linked one:
#7013
#6924

@skhiat skhiat closed this Apr 22, 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.

4 participants