-
Notifications
You must be signed in to change notification settings - Fork 801
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
[SG2]Imposter nodes and documents #7865
base: sg2/main
Are you sure you want to change the base?
Conversation
It appears that you made a non-draft PR! |
This reverts commit dba3cd2.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a better place to put this hlsl file?
...aphDeltaRegistry/FunctionDefinitions/StandardDefinitions/Input/MeshDeformation/Imposter.hlsl
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I need to do a bit more work on this to test our the actual nodes themselves but I won't be able to do that until tomorrow, so I'm just going to submit this bit now and then look at it some more tomorrow when I can switch to your branch and try it.
|
||
## Description | ||
|
||
The Imposter UV Node calculates the billboard positon and the virtual UVs for sampling. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about we say: "The Imposter UV Node calculates the billboard position (fix the typo in this word) and the UV coordinates needed by the Imposter Sample node." We need to indicate that data output here is intended specifically for the Imposter Sample node.
...Registry/FunctionDefinitions/StandardDefinitions/Input/MeshDeformation/ImposterSampleNode.cs
Outdated
Show resolved
Hide resolved
...eltaRegistry/FunctionDefinitions/StandardDefinitions/Input/MeshDeformation/ImposterUVNode.cs
Outdated
Show resolved
Hide resolved
...eltaRegistry/FunctionDefinitions/StandardDefinitions/Input/MeshDeformation/ImposterUVNode.cs
Outdated
Show resolved
Hide resolved
@"ImposterSample(HeightMapChannel, ViewDirectionTS, Parallax, Frames, Texture.tex, Texture.texelSize, Clip, Grid, UV0, UV1, UV2, Sampler.samplerstate, RGBA);", | ||
new ParameterDescriptor[] | ||
{ | ||
new ParameterDescriptor("Texture", TYPE.Texture2D, Usage.In), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be an array texture?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in the current set up it needs to be a single texture split into frames.
...Registry/FunctionDefinitions/StandardDefinitions/Input/MeshDeformation/ImposterSampleNode.cs
Outdated
Show resolved
Hide resolved
...Registry/FunctionDefinitions/StandardDefinitions/Input/MeshDeformation/ImposterSampleNode.cs
Outdated
Show resolved
Hide resolved
This reverts commit 7aadd3d.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Making great progress on this!
...tor/GraphDeltaRegistry/FunctionDefinitions/StandardDefinitions/Utility/ImposterSampleNode.cs
Outdated
Show resolved
Hide resolved
...tor/GraphDeltaRegistry/FunctionDefinitions/StandardDefinitions/Utility/ImposterSampleNode.cs
Outdated
Show resolved
Hide resolved
.../Editor/GraphDeltaRegistry/FunctionDefinitions/StandardDefinitions/Utility/ImposterUVNode.cs
Show resolved
Hide resolved
....sg2/Editor/GraphDeltaRegistry/FunctionDefinitions/StandardDefinitions/Utility/Imposter.hlsl
Outdated
Show resolved
Hide resolved
....sg2/Editor/GraphDeltaRegistry/FunctionDefinitions/StandardDefinitions/Utility/Imposter.hlsl
Outdated
Show resolved
Hide resolved
...tor/GraphDeltaRegistry/FunctionDefinitions/StandardDefinitions/Utility/ImposterSampleNode.cs
Outdated
Show resolved
Hide resolved
.../Editor/GraphDeltaRegistry/FunctionDefinitions/StandardDefinitions/Utility/ImposterUVNode.cs
Outdated
Show resolved
Hide resolved
.../Editor/GraphDeltaRegistry/FunctionDefinitions/StandardDefinitions/Utility/ImposterUVNode.cs
Outdated
Show resolved
Hide resolved
.../Editor/GraphDeltaRegistry/FunctionDefinitions/StandardDefinitions/Utility/ImposterUVNode.cs
Outdated
Show resolved
Hide resolved
....sg2/Editor/GraphDeltaRegistry/FunctionDefinitions/StandardDefinitions/Utility/Imposter.hlsl
Outdated
Show resolved
Hide resolved
....sg2/Editor/GraphDeltaRegistry/FunctionDefinitions/StandardDefinitions/Utility/Imposter.hlsl
Outdated
Show resolved
Hide resolved
...tor/GraphDeltaRegistry/FunctionDefinitions/StandardDefinitions/Utility/ImposterSampleNode.cs
Show resolved
Hide resolved
.../Editor/GraphDeltaRegistry/FunctionDefinitions/StandardDefinitions/Utility/ImposterUVNode.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I reviewed with Tracy live. Main discoveries are:
- Align heightmap port naming with the existing Parallax node.
- Texture size allows user input though would only work with square of 2s, something we might want to handle more elegantly, i.g. change the input field to dropdown selections. And user may have difference resolution on base color vs mask textures for example.
This is super valuable work and useful for users, so great job Tracy! LGTM!
Purpose of this PR
Add imposter nodes and related documents to the node library.
Testing status
Connect the outputs of the nodes to the context node in an sg2 graph, and no error is shown.
Comments to reviewers