Conversation
Missing: iridescene, next commit anisotropy, not supported by three.js yet.
|
Hi Emmett, |
elalish
left a comment
elalish
left a comment
There was a problem hiding this comment.
I just realized you haven't added any of your earlier APIs into our editor yet. You may want to do that first to verify everything is working well before you get too far along with the rest.
| onUpdate, | ||
| TextureUsage.SheenColor, | ||
| null, | ||
| correlatedMaterials, |
There was a problem hiding this comment.
Maybe a helper function to removed some duplication?
| case TextureUsage.ClearcoatNormal: | ||
| material.clearcoatNormalMap = threeTexture; | ||
| break; | ||
| case TextureUsage.SheenColor: |
There was a problem hiding this comment.
I don't love this huge switch statement - if you can think of a cleaner way to architect this, I'm open to it, but only if you're feeling inspired.
| color.fromArray(rgb); | ||
| } else { | ||
| color.set(rgb as ColorRepresentation); | ||
| } |
There was a problem hiding this comment.
If this reused, consider a function.
| private[$transmissionTexture]!: TextureInfo; | ||
| private[$thicknessTexture]!: TextureInfo; | ||
| private[$specularTexture]!: TextureInfo; | ||
| private[$specularColorTexture]!: TextureInfo; |
There was a problem hiding this comment.
I really hate symbols, but they're only necessary directly in a publicly-accessible object. Maybe we can assemble these into a single object and just have one symbol instead?
There was a problem hiding this comment.
Maybe this map approach is better?
| private[$thicknessTexture]!: TextureInfo; | ||
| private[$specularTexture]!: TextureInfo; | ||
| private[$specularColorTexture]!: TextureInfo; | ||
| private[$pbrTextures]!: Map<TextureUsage, TextureInfo>; |
There was a problem hiding this comment.
You can skip the ! by creating it right here, see $variantSet above.
| textureInfoFromUsage(TextureUsage.Transmission); | ||
| textureInfoFromUsage(TextureUsage.Thickness); | ||
| textureInfoFromUsage(TextureUsage.Specular); | ||
| textureInfoFromUsage(TextureUsage.SpecularColor); |
There was a problem hiding this comment.
💯
nit: maybe createTextureInfo? FromUsage is already clear from the input type.
* Add PBR Next properties API Missing: iridescene, next commit anisotropy, not supported by three.js yet. * Cleanup code * Fix for cleanup bug * nits
2 participants
Missing:
iridescene, next commit
anisotropy, not supported by three.js yet.
Reference Issue