google / model-viewer

Easily display interactive 3D models on the web and in AR!
https://modelviewer.dev
Apache License 2.0
6.96k stars 820 forks source link

Materials belonging to a variant will not update in the viewer when changed through the API #4881

Closed literallylara closed 1 month ago

literallylara commented 1 month ago

Description

When a variant is selected, any update on materials belonging to the variant seem to have no effect in the viewer. Is this intended behavior? If so I would expect the API to throw an error or a warning. My client offers customizable products which are set up with variants but certain parts on the model need to change (e.g. a customer-provided artwork). I tried to track down the issue in the code and think it might be related to this line but I am not sure: https://github.com/google/model-viewer/blob/2edbce11fecee779a8f5abd46b2f796512fe7014/packages/model-viewer/src/three-components/GLTFInstance.ts#L169

Steps to reproduce:

  1. Go to: https://modelviewer.dev/examples/scenegraph/#variants
  2. Select the beach variant
  3. Get a reference to the \: $el = document.querySelector('#variants model-viewer') (or via inspector)
  4. Find the corresponding material: m = $el.model.materials.find(v => v.hasVariant('beach'))
  5. Ensure that it is active: m.isActive (should report true)
  6. Inspect the current roughness factor: m.pbrMetallicRoughness.roughnessFactor (will report 1)
  7. Update it: m.pbrMetallicRoughness.setRoughnessFactor(0)
  8. Ensure that the update succeeded: m.pbrMetallicRoughness.roughnessFactor (will report 0)
  9. Notice that the material did not change in the viewer. (seems to affect any material property)

Version

Browser Affected

OS

elalish commented 1 month ago

Thank you for the detailed repro steps - this is clearly a bug. It even repros on the editor; only the first variant is editable. I swear this used to work, so this may be a regression - time to add some more tests.

literallylara commented 1 month ago

Gotcha, do you happen to know which versions are likely to be unaffected or where it's best to look if I want to give fixing it a try? 🙂

elalish commented 1 month ago

Upon further investigation, this was missed because it only repros on models with certain combinations of properties. Basically we're using GLTFLoader's applyFinalMaterial() incorrectly - it's a bit ugly, so I'll go ahead and take care of refactoring it. I really appreciate the offer though!