Allow users to bring their own lights

[jsantell]

While some content publishers want full control over the scene, there are specifically perf costs for things like shadows and lights. Do we want something like allow-model-lights to import dynamic lights that are found in a glTF model? Would this enable shadows as well? Should this be disabled, even if set, on mobile?

[jsantell]

I don't think dynamic lights is a good idea for several reasons (more in #13) and it seems that the 2.0 extension for lights is in flux (#35). Pinging Don for more info.

Edit: 2.0 light extension is supported by GLTFLoader

[cdata]

My take on this is that we should have two settings. The primary (and default) setting causes lights in the GLTF to be removed, and replaced with whatever built-in lighting / envmap scheme we come up with. The alternative (user configurable) setting retains the lights in the GLTF.

The rationale here is that the default setting puts users on the good path that is likely to lead to a good performance and consistency outcome, while the user configurable alternative allows a user to say, "no, really, I know what I am doing, please give me the lights that are in the GLTF." In other words, we give users a good default, plus an unconstrained escape hatch.

[donmccurdy]

+1 for ignoring lights by default (even though the extension is pretty close to finalized).

Also consider logging a warning when punctual lights are ignored — perhaps even going so far as to say that including lights in an asset is "generally not recommended" or some such thing. I think (but would be willing to discuss) that env maps should similarly be ignored by default.

In general, I am hoping that DCC tool exports and model libraries like Poly will not include lights or environment maps by default, and these defaults can give a slight nudge in that direction.

[jsantell]

@donmccurdy is there envmap support in gltf now?

[donmccurdy]

No, only as a vendor extension: https://github.com/KhronosGroup/glTF/tree/master/extensions/2.0/Vendor/EXT_lights_image_based. I think we would want to discuss further before implementing it at this stage.

[elalish]

Done ages ago