Imported Spotlight is incorrect

[hybridherbst]

Description

When importing a glTF file containing spotlights, those spotlights are incorrectly placed/rotated in model-viewer. This seems to stem from an underlying threejs issue, where spotlights break when a scene is serialized and then deserialized again. Not sure why model-viewer does a full serialize/deserialize pass though even when just loading in a file (which I think is a bug).

Live Demo

• Download LightRoundtrip.zip
• Drop in modelviewer.dev/editor
• Drop in this black pixel as environment map to see the light better

Result

Three.js Editor (expected result)

Unity Editor (expected result)

Three.js Editor (after page refresh which does a serialization/deserialization pass - known issue - matches model-viewer result) This is a known issue in three: https://github.com/mrdoob/three.js/issues/9508 with an old-ish PR related to it: https://github.com/mrdoob/three.js/pull/14658#issuecomment-666051811

I think the problem here is that model-viewer does that serialize/deserialize pass, not sure why that's needed.

Browser Affected

• [X] Chrome, version: xx.x.xxxx.xx
• [X] Edge
• [X] Firefox
• [X] IE
• [X] Safari

OS

• [X] Android
• [X] iOS
• [X] Linux
• [X] MacOS
• [X] Windows

[elalish]

We don't serialize/deserialize, but we do make a copy of the scene. I guess that also somehow triggers this bug? It sounds like the light targets are not being hooked up properly.

[hybridherbst]

I think three in some places implements "copy" with interally literally converting to/from JSON, maybe it's hitting one of those here when doing the copy (is the copy then used for showing the scene instead of the original?)

[hybridherbst]

Just ran into this again so I thought I'd bump this issue :) LightRig.zip

Expected:

Actual:

[elalish]

Yes, fair. I don't suppose you'd like to try a PR to fix it? I think you know more about three's directional lights than I do and as far as I can tell you're the only one using them... 🥲

[hybridherbst]

I think there would be two places: one is that three should remove the target properties from lights altogether (see https://github.com/mrdoob/three.js/issues/5079 and https://github.com/mrdoob/three.js/pull/14658), the other would be that model-viewer shouldn't serialize/deserialize the scene (which causes the targets to break).

[elalish]

That sounds reasonable. When those three.js issues get fixed, let's come back to this. We have a few workarounds to reassign lost info from the serialization process that we can probably add this to.