HYDRA-955 : Pass normals and tangents to Hydra from render item and mesh adapters #116
Conversation
| } | ||
|
|
||
| if (key == HdTokens->normals) { | ||
| MStatus status; |
There was a problem hiding this comment.
I am doing a change in this file to standardize the methods and avoid passing sometimes a MfnMesh and sometimes not.
| return uma; | ||
| } | ||
|
|
||
| TF_DEFINE_ENV_SETTING(MAYA_HYDRA_PASS_NORMALS_TO_HYDRA, true, |
There was a problem hiding this comment.
With passing normal as true by default, is there any test case to be rebaselined?
also, it's better to have a test case for normal if there's no one.
There was a problem hiding this comment.
With the new normals, all the tests should pass without being rebaselined. I haven't added any tests since I think it will be difficult to differentiate between the generated normals and the passed normals, there are only a few details which are different.
There was a problem hiding this comment.
Is there no way to generate a small synthetic scene (e.g. a mesh cube maybe?) with passed normals that would be completely different from generated normals (e.g. the opposite sign), and look at the normal values in the Hydra scene index scene, i.e. without image comparison? Don't know if this is valid / realistic, just thinking out loud.
There was a problem hiding this comment.
You're right, I can use some scene from the bugs we have when not using the normals from OGS.
I'll add this as a test. Thanks.
| mvb->unmap(); | ||
| } | ||
| break; | ||
| case MHWRender::MGeometry::kTangent:{ |
There was a problem hiding this comment.
Didn't see notion for tangent type in Hydra, is mikkt tangent not supported in Hydra, right?
Also, it's good to have a test case to cover tangent.
There was a problem hiding this comment.
Yes only Mikkt is supported for tangents, but it's not explicitly mentioned in Hydra.
I left the code for tangents for a later PR but we need to modify the translation of materials to support tangents. At this time the tangents won't be used so I cannot add a test for them.
lilike-adsk
assigned lilike-adsk and lanierd-adsk and unassigned lanierd-adsk and lilike-adsk
lilike-adsk
added
the
ready-for-merge
No description provided.