Skip to content
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

Add Eye Graph Preset to HDRP Shader List #2431

Merged
merged 1 commit into from
Oct 29, 2020

Conversation

johnpars
Copy link
Contributor

@johnpars johnpars commented Oct 28, 2020

Purpose of this PR

This PR copies the Eye ShaderGraph preset in the HDRP Test Projects, and places it into HDRP Resources.
This makes it available to users as a quick starting point for creating eye materials.
Note that we have accompanying documentation for this on the way, in #2424

image

image


Testing status

Locally, I have created a new material, switched the shader to HDRP/Eye in the shader list, and quickly authored an eye to test for parity with eye materials in the original Graphics Tests.


Comments to reviewers

  • Currently, there is no Eye sample in the HDRP Material Sample. Do we want one for 10.2?
  • Would it make sense to update the graphics tests to point to this graph in HDRP Resources?
  • Slightly depends on a small change in Changed the names of some of the parameters for the Eye Utils SG Nodes. #2413, which renames some util nodes inputs. I have used the eye graph from that PR branch, so that when this merges it will update automatically (tested locally).

@github-actions github-actions bot added the HDRP label Oct 28, 2020
@sebastienlagarde sebastienlagarde requested review from remi-chapelain and removed request for a team October 28, 2020 21:47
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.

Approving the PR since the graph is known and used in some DXR and HDRP Test ! ✔️
YES, I think it would be a great addition to our material sample to have this and/or multiple version of this as I already stated when the HDRP test have been made ! :)
The same way we have our material sample for hair / decals / fabric, we certainly need one with Eye especially how hard it is to setup it and have a proper result ! And all the sample in the HDRP/DXR test looks great and can be integrated a wide range of use.

As for linking the HDRP test with this one, My two cents, is I don't think it's a good idea since any slight modification on this graph would make the test fail and need to rebuild the capture. I think the test graph and the sample graph should remain different.

@sebastienlagarde sebastienlagarde marked this pull request as ready for review October 29, 2020 20:23
@sebastienlagarde sebastienlagarde merged commit fe73f69 into master Oct 29, 2020
@sebastienlagarde sebastienlagarde deleted the HDRP/add-eye-graph-preset branch October 29, 2020 20:24
sebastienlagarde added a commit that referenced this pull request Oct 30, 2020
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.

3 participants