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

Add pbr iridescence API #4348

Merged
merged 3 commits into from
Jul 11, 2023
Merged

Add pbr iridescence API #4348

merged 3 commits into from
Jul 11, 2023

Conversation

diegoteran
Copy link
Collaborator

Follows threejs implementation.

elalish
elalish previously approved these changes Jul 8, 2023
Copy link
Collaborator

@elalish elalish left a comment

Choose a reason for hiding this comment

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

Thanks!

return this[$backingThreeMaterial].iridescenceThicknessRange[1];
}

get iridescenceThicknessTexture(): TextureInfo {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I just noticed this function signature doesn't match the API: one returns TextureInfo, while the other returns TextureInfo|null. I noticed because of the ! below. This is probably true of the rest as well - let's make sure we know which is correct and then make them match.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Have these been added to the pbr map with createTextureInfo? Maybe we need something more type-safe than a map...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes, these are all added at $initialize

@diegoteran diegoteran requested a review from elalish July 11, 2023 20:42
Copy link
Collaborator

@elalish elalish left a comment

Choose a reason for hiding this comment

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

Thanks! LGTM.

@diegoteran diegoteran merged commit e020b1b into master Jul 11, 2023
3 checks passed
@diegoteran diegoteran deleted the nxt branch July 11, 2023 21:16
JL-Vidinoti pushed a commit to vidinoti/model-viewer that referenced this pull request Apr 22, 2024
* Add pbr iridescence API

* Make pbr next properties api consistent for TextureInfo

* Add Anisotropy
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants