Add new sample cubemap node with world space direction #2311
Conversation
|
It appears that you made a non-draft PR! |
alindmanUnity
left a comment
alindmanUnity
left a comment
There was a problem hiding this comment.
Did you update the result images for the input node tests? I don't see any png file changes
| @@ -0,0 +1,24 @@ | |||
| # Sample Cubemap Node | |||
There was a problem hiding this comment.
should be Sample Reflected Cubemap
| ## Description | ||
|
|
||
| Samples a **Cubemap** with reflected vector and returns a **Vector 4** color value for use in the shader. Requires **View Direction** and **Normal** inputs to sample the **Cubemap**. You can achieve a blurring effect by sampling at a different Level of Detail using the **LOD** input. You can also define a custom **Sampler State** using the **Sampler** input. | ||
|
|
There was a problem hiding this comment.
probably worth noting that it was previously Sample Cubemap?
There was a problem hiding this comment.
I will let the reply to @sharlenet . I don't know. I think this is the purpose of the upgrade guide to say that?
I think the doc should be what is the current state of node. But open to change if needed.
Nope, waiting for Yamato to generate all screenshots on all platforms |
|
|
||
| ## Renamed Sample Cubemap | ||
|
|
||
| Shader Graph has renamed the previous Sample Cubemap Node in Sample Reflected Cubemap and have added a new Sample Cubemap which used world space direction. |
There was a problem hiding this comment.
little grammar fix here,
"Shader Graph has renamed the previous Sample Cubemap Node to Sample Reflected Cubemap**,** and has added a new Sample Cubemap**,** which uses world space direction."
There was a problem hiding this comment.
thanks fixed
| ## Description | ||
|
|
||
| Samples a **Cubemap** and returns a **Vector 4** color value for use in the shader. Requires **View Direction** and **Normal** inputs to sample the **Cubemap**. You can achieve a blurring effect by sampling at a different Level of Detail using the **LOD** input. You can also define a custom **Sampler State** using the **Sampler** input. | ||
| Samples a **Cubemap** and returns a **Vector 4** color value for use in the shader. Requires **Direction** inputs in world space to sample the **Cubemap**. You can achieve a blurring effect by sampling at a different Level of Detail using the **LOD** input. You can also define a custom **Sampler State** using the **Sampler** input. |
There was a problem hiding this comment.
"inputs" should be "input" since there's only one
There was a problem hiding this comment.
Thanks for pointing this out! I'll fix it.
ghost
self-requested a review
ghost
left a comment
ghost
left a comment
There was a problem hiding this comment.
So happy to see this!
Only tests I think this needed were 1. basic use case and 2. upgrade test with shadergraph containing old Sample Cubemap node (which passed). Aside from the minor grammar fixes, this is good to go!
| ## Texture | ||
|
|
||
| |[Cubemap Asset](Cubemap-Asset-Node.md)|[Sample Cubemap](Sample-Cubemap-Node.md)| | ||
| |[Cubemap Asset](Cubemap-Asset-Node.md)|[Sample Cubemap](Sample-Cubemap-Node.md)|[Sample Cubemap](Sample-Reflected-Cubemap-Node.md)| |
There was a problem hiding this comment.
I don't understand why a third column was added here. Should it just replace the existing [Sample Cubemap](Sample-Cubemap-Node.md)?
There was a problem hiding this comment.
I just don't know what this stuff was :), will revert
|
Yamato is green |
* Add a new Sample Cubemap node * Add new cubemap sample node to input test * fix issue in test * update reference screenshots * Edited Sample Cubemap Node page * adress PR feedback on doc Co-authored-by: Sharlene Tan <[email protected]>
…ion #2311 (#2348) * Add new sample cubemap node with world space direction (#2311) * Add a new Sample Cubemap node * Add new cubemap sample node to input test * fix issue in test * update reference screenshots * Edited Sample Cubemap Node page * adress PR feedback on doc Co-authored-by: Sharlene Tan <[email protected]> * Updated documentation * Updated Input-Nodes.md * Updated images Co-authored-by: Sharlene Tan <[email protected]>
Purpose of this PR
Added a new Sample Cubemap Node which take the world space direction to sample it.
(High demanding change from artists)
Rename the old Sample Cubemap to Sample Reflected Cubemap to highlight that it is performing a reflection along from the view direction and object normal. Functionnality is unchanged.
Testing status
I have added a new input node test in existing test of shader graph project
I have tested the new node with world space normal and a provided vector3 and a sky cubemap. All this to works well.
Note: I am not sure about what is a cubemap asset, maybe there is something to check here? But my guess is that it is independent of this change.
Comments to reviewers
Notes for the reviewers you have assigned.