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

Cycles : Correct tagging of tangent primitive variables to allow normal maps to render properly. #5269

Closed

Conversation

boberfly
Copy link
Collaborator

Generally describe what this PR will do, and why it is needed

  • Tangents need to exist and be tagged correctly for Cycles to use these with normal maps
  • For these to function properly the user needs to do a few steps:
  1. Create a MeshTangents node for a given UV, and name it with the name of the UV with .tangent on the end eg. uv.tangent
  2. Create a primitive variable of type float + FaceVarying and name it based on the UV name with .tangent_sign on the end eg. uv.tangent_sign (I use PrimitiveVariables + ResamplePrimitiveVariables nodes)
  3. Plug a tangent-space cyclesImageTexture into a cyclesNormalMap node, and plug that into a cyclesPrincipledShader's normal socket.

I am not sure exactly on the specifics of tangent_sign but it is needed for it to render.
Old
New

Related issues

  • It came up again in the Discord from a user

Dependencies

  • NA

Breaking changes

  • NA

Checklist

  • I have read the contribution guidelines.
  • I have updated the documentation, if applicable.
  • I have tested my change(s) in the test suite, and added new test cases where necessary.
  • My code follows the Gaffer project's prevailing coding style and conventions.

Todo: Get the proper MikkTSpace code into place again, now that it is bundled with Cycles 3.5 and have it implicitly be created in the absence of tangent and tangent_sign primitive variables. This will be targetting for Gaffer 1.3.x probably.

@boberfly
Copy link
Collaborator Author

Ok a quick study, the biTangent is calculated in-kernel via the signage:
vector B = tangent_sign * cross(ninterp, tangent);

I'm not sure if we could just scrape this from the existing biTangent somehow or we should extend MeshAlgoTangents to also output tangent_sign as an option?

We also have MikkTSpace now in third_party: https://github.com/blender/cycles/tree/main/third_party/mikktspace perhaps we should add this too for other renderers or workflows to benefit?

@johnhaddon johnhaddon deleted the branch GafferHQ:1.1_maintenance July 13, 2023 16:22
@johnhaddon johnhaddon closed this Jul 13, 2023
johnhaddon pushed a commit to johnhaddon/gaffer that referenced this pull request Apr 23, 2024
This allows normal maps to be rendered using the procedure described in GafferHQ#5269. It's really just a stop-gap measure until we (or ideally Cycles) can generate tangents on demand based on the needs of the assigned shaders. It also differs from GafferHQ#5269 in that we require the names to be exactly `uv.tangent` and `uv.tangent_sign` rather than just ending with `tangent_sign` or `tangent`.
johnhaddon pushed a commit to johnhaddon/gaffer that referenced this pull request Apr 24, 2024
This allows normal maps to be rendered using the procedure described in GafferHQ#5269. It's really just a stop-gap measure until we (or ideally Cycles) can generate tangents on demand based on the needs of the assigned shaders. It also differs from GafferHQ#5269 in that we require the names to be exactly `uv.tangent` and `uv.tangent_sign` rather than just ending with `tangent_sign` or `tangent`.
johnhaddon pushed a commit to johnhaddon/gaffer that referenced this pull request Apr 25, 2024
This allows normal maps to be rendered using the procedure described in GafferHQ#5269. It's really just a stop-gap measure until we (or ideally Cycles) can generate tangents on demand based on the needs of the assigned shaders. It also differs from GafferHQ#5269 in that we require the names to be exactly `uv.tangent` and `uv.tangent_sign` rather than just ending with `tangent_sign` or `tangent`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

2 participants