Tresjs / tres

Declarative ThreeJS using Vue Components
https://tresjs.org
MIT License
1.93k stars 86 forks source link

THREE material `defines` can't be set via Tres props #605

Open andretchen0 opened 3 months ago

andretchen0 commented 3 months ago

Expected

<MyMaterial defines-STANDARD="myValue" />

... should patch defines.STANDARD.

Bug

It patches defines.standard.

Cause

When patching, src/nodeOps.ts lowercases pierced props.

https://github.com/Tresjs/tres/blob/98109af7d501da1ae5f817e7dc61c6d6ad902891/src/core/nodeOps.ts#L227

Context

THREE materials have a defines property. It's an object whose key/value pairs are injected into the material's shader during compilation.

By convention, the keys are written in ALL_CAPS case. This is the case for all THREE materials and for many external materials for use in the THREE ecosystem.

For example, here's MeshStandardMaterial's defines:

                this.defines = { 'STANDARD': '' };

https://github.com/mrdoob/three.js/blob/ef80ac74e6716a50104a57d8add6c8a950bff8d7/src/materials/MeshStandardMaterial.js#L73

Workaround

I'm working on porting a material to Cientos. It extends THREE.MeshStandardMaterial and adds 3 defines. The extended material follows THREE's convention and uses ALL_CAPS case for the keys.

The workaround I'm currently using modifies the material's source. That would otherwise be unnecessary and means that if the original material is updated in the future, we can't simply grab a copy and insert it into Tres.

This is possible in this case, because I'm extending MeshStandardMaterial. However, under the current Tres setup, one can't meaningfully use :defines-X as a prop on built-in materials, as far as I can see.

Reproduction

https://stackblitz.com/edit/tresjs-basic-akdmgw?file=src%2Fcomponents%2FTheExperience.vue

Steps to reproduce

See StackBlitz.

System Info

All systems affected

Used Package Manager

npm

Code of Conduct