Skip to content

Conversation

Aqil-Ahmad
Copy link
Contributor

Description

Implements planar texture alignment for PBR materials, allowing checkbox to work with PBR texture transforms.

Related Issues

Fixes #2975

Issue Link:


Checklist

Please ensure the following before requesting review:

  • I have provided a clear title and detailed description for this pull request.
  • If useful, I have included media such as screenshots and video to show off my changes.
  • The PR is linked to a relevant issue with sufficient context.
  • I have tested the changes locally and verified they work as intended.
  • All new and existing tests pass.
  • Code follows the project's style guidelines.
  • Documentation has been updated if needed.
  • Any dependent changes have been merged and published in downstream modules
  • I have reviewed the contributing guidelines.

Additional Notes

Currently:
Using Blinn-Phong Values for Alignment

Alternatives (seeking input):
Modify calcAlignedPlanarTE() to accept PBR transforms directly, OR
Create a new calcAlignedPlanarPBR() function

@github-actions github-actions bot added the c/cpp label Oct 18, 2025
@Aqil-Ahmad
Copy link
Contributor Author

Aqil-Ahmad commented Oct 18, 2025

/claim #2975


center_obj->setTEScale(center_te, pbr_scale_u, pbr_scale_v);
center_obj->setTERotation(center_te, pbr_rot);
center_obj->setTEOffset(center_te, pbr_offset_u, pbr_offset_v);
Copy link
Contributor

@akleshchev akleshchev Oct 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks wrong. Calling setTEScale not only sets values, but also marks object for a rebuild so should be avoided when possible. This code looks like either a workaround or a hack.

Seems like calcAlignedPlanarTE should be split into pbr/blinn-phong variants or LLMaterial::calcAlignedPlanarTE should be immplemented or calcAlignedPlanarTE should get a 'bool for_pbr' argument.

Copy link
Contributor Author

@Aqil-Ahmad Aqil-Ahmad Oct 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the feedback! Yes, I figured this might not be the right approach and wanted to get input on whether to implement a new one or modify the current calcAlignedPlanarTE()

working on it.

@Aqil-Ahmad Aqil-Ahmad force-pushed the pbr-planar-alignment branch from 7893b2b to 64ea4f7 Compare October 21, 2025 15:05
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.

PBR Align Planar Faces Not Functioning

2 participants