KhronosGroup / glTF

glTF – Runtime 3D Asset Delivery
Other
7.19k stars 1.14k forks source link

glTF 2.0: New KHR_lights extension #945

Closed McNopper closed 6 years ago

McNopper commented 7 years ago

As the lights have to be extracted from the KHR_materials_common extension, here is a first try by example:

{
  "lights": [
    {
      "ambient": {
        "color": [
          1,
          1,
          1
        ]
      }
    },
    {
      "directional": {
        "color": [
          1,
          1,
          1
        ]
      }
    },
    {
      "point": {
        "color": [
          1,
          1,
          1
        ],
        "constantAttenuation": 1,
        "linearAttenuation": 0.5,
        "quadraticAttenuation": 0.25
      }
    },
    {
      "spot": {
        "color": [
          1,
          1,
          1
        ],
        "constantAttenuation": 1,
        "fallOffAngle": 40,
        "fallOffExponent": 0,
        "linearAttenuation": 0.5,
        "quadraticAttenuation": 0.25
      }
    }
  ]
}

Directional, point and spot are obvious for PBR and non-PBR materials.

Regarding ambient and for PBR, this needs to be further discussed and explained. But I suggest to treat the value as if an environment map is used, which does have just this color.

MiiBond commented 6 years ago

I suppose maybe we should discuss whether flags for casting shadows should be included as part of a light. Certainly I'm not suggesting that we include details about shadow techniques or settings but what about a simple boolean? i.e. castShadow: true

There will be a large amount of variance in the final rendering between engines but this would at least allow the glTF to inform the engine whether or not to render a shadow.

The default would be false.

MiiBond commented 6 years ago

@sebevan mentioned in my Babylon.js pull request (https://github.com/BabylonJS/Babylon.js/pull/3579) that spotlights that don't include an outerConeAngle could default to a gaussian falloff, starting from the innerConeAngle. Anyone have any thoughts on this?

donmccurdy commented 6 years ago

As far as I can tell from docs, Unity doesn't have any concept of an innerConeAngle: https://docs.unity3d.com/ScriptReference/Light.html ... anyone know differently? If Unity can't support it, is that enough of a problem to consider excluding it?

spotlights that don't include an outerConeAngle could default to a gaussian falloff

The comment on BabylonJS suggests inner should be optional, not outer, right? Do any engines implement gaussian falloff for spot lights? Asking engines to support two different falloff modes seems like a stretch if we want consistent implementations.

I think I'd vote to leave shadows out of scope on this extension.

MiiBond commented 6 years ago

Does Unity not have a property to adjust the way the light falls off? All I can see in the docs is the spotAngle and Unity is crashing on startup for me right now...

I'm okay with simplifying spotlights to only have a single angle property if it helps this extension get accepted sooner. However, being able to control the falloff a bit more explicitly would be my preference, even if a couple of engines just approximate the behaviour.

MiiBond commented 6 years ago

Okay, so after talking on the call this morning, it sounds like we're okay with innerConeAngle and outerConeAngle as two separate properties. Babylon.js is interested in adding support for the extra angle, leaving just Unity as being a major engine that doesn't support it.

For engines that don't support two angles, it would be expected that they would use outerConeAngle as the principal angle, leaving innerConeAngle to implicitly be 0.

donmccurdy commented 6 years ago

+1 for innerConeAngle.

Also, we plan to omit area lights from this extension, to ensure that the extension will have broad support. Area lights can be added in a future extension, perhaps alongside hemisphere lights or IBL, for more advanced lighting. Community members are welcome to initiate that discussion in a new PR or use custom vendor extensions for your needs in the meantime.

aras-p commented 6 years ago

leaving just Unity as being a major engine that doesn't support it.

New rendering pipelines (both "HD" and "Lightweight") in Unity 2018.1 (beta out now) support inner & outer spotlight cone angles.

Unity does, but says: "Since the lighting calculation is quite processor-intensive, area lights are not available at runtime and can only be baked into lightmaps."

The "HD" rendering pipeline in Unity 2018.1 does support realtime area lights (shadows from them would still be "punctual" though), largely implemented via Linearly Transfomed Cosines approximation.

awefers commented 6 years ago

+1 for innerConeAngle.

@MiiBond As for the "castShadow" property, it would be nice to extend this beyond lights to meshes.

pjcozzi commented 6 years ago

Thanks everyone for the great discussion here! Reviews are very welcome on the pull request for this by @MiiBond.

recp commented 6 years ago

I implemented COLLADA 1.4 and 1.5 lights in AssetKit I want to provide single interface for both COLLADA (1.4 and 1.5+) and glTF (2.0+) so I need to convert the extension to existing data structure which is (speaking for spot light for now):

typedef struct AkSpotLight {
  AkLightBase base;
  float       constAttn;
  float       linearAttn;
  float       quadAttn;
  float       falloffAngle;
  float       falloffExp;
} AkSpotLight;

or as alternative I can update/upgrade this structure for new glTF design if it is possible to convert COLLADA structure to glTF's inner / outer angle or vice-versa. I couldn't see attenuation properties in proposed PR (https://github.com/KhronosGroup/glTF/pull/1223).

Actually it explicitly says:

Engines that don't support two angles for spotlights should use outerConeAngle as the spotlight angle (leaving innerConeAngle to implicitly be 0).

this solves the problem of converting falloffAngleto inner/outer angle design; I will upgrade my importer and render engine to use inner/outer angle, but what about other properties? e.g. attenuations (linear, quadratic, constant), fallOfExponent...

I found a good tutorial which explains dual cone spot: http://ngreen.org/articles/quick-n-easy-dual-cone-spotlights-glsl.html

and this: https://msdn.microsoft.com/en-us/library/windows/desktop/bb174697(v=vs.85).aspx

seems like attenuations (linear, quadratic, constant) and fallOfExponent properties are not used here too if I didn't miss them.

@McNopper's first proposal (or following ones) fits with my importer and renderer, if glTF will use new design I would like to know how to convert old design/structure ( single cone ) to new design/structure ( dual cone ) to provide single and better interface to users.

I need to convert between them because some tools may use single cone (with linear, quadratic and constant attenuations) and some tools may use dual cone (inner / outer angle), is it possible to do that without losing any lighting information?

donmccurdy commented 6 years ago

The current KHR_lights proposal would remove those attenuation properties and (implicitly) expect physically-correct values to be used. So it is possible to convert glTF lights to your representation with explicit attenuation values, but physically impossible attenuations could not be represented in the glTF KHR_lights proposal.

I implemented COLLADA 1.4 and 1.5 lights in AssetKit I want to provide single interface for both COLLADA (1.4 and 1.5+) and glTF (2.0+)

The lights extension is still open for feedback, so we're glad to have more opinions on lights. But in general, beyond just lights, note that COLLADA will have many features that will never be included in glTF, and that is by design. So keeping a single interface may be difficult long-term.

recp commented 6 years ago

@donmccurdy thanks for your comments,

I'm updating my data structures for glTF if necessary and sometimes this makes API more clear and fantastic. So I'll implement PBR in my importer and renderer after OIT transparency stuff finished. And I want to update my lights structures too for new design. Dual cone seems better alternative but I never used it before.

So after updated my structures to glTF I need to convert COLLADA values to glTF structures. Or I need to create another Spot light type which I'm reluctant to do so.

You said "it is possible to convert glTF lights to your representation", is it possible to do the opposite? Also how can I convert glTF to my representation as alternative, any formula?

--

note that COLLADA will have many features that will never be included in glTF, and that is by design.

I'm aware and happy for that, I like both specs.

So keeping a single interface may be difficult long-term.

You are right, but single interface makes things easier for tools and engines. Maintaining single interface also makes codes re-usable in importer. For instance I'm not sure how many tools support COLLADA 1.5 because some libraries provides two interface... With single interface, tools/engines can support new version of COLLADA and glTF more quickly I think. Also I'm considering to provide some sub tools e.g. mesh optimizer... single interface will make things more easier for that, I hope.

Currently while writing new render engine I'm only using single interface to load COLLADA 1.4, 1.5 and glTF 2.0 (not all features for now), so I can focus on render stuff instead of implementing different APIs/interfaces in render side.

In the future some conflicts may occurs but COLLADA (and AssetKit) has technique/profile/extra elements which makes it very extensible. In the future any conflicts can be resolved with technique="gltf or collada" or profile="gltf or collada"... These are my thoughts, I hope I can achieve what I think :)

DRx3D commented 6 years ago

I know I am very late to this discussion, but I just recently found out about adding lights to glTF. I looked through this issue but did not find anything related to an answer. Perhaps it is in this issue or another issue, if so, please point me that way.

My question is what is the use case for including lighting in a glTF file. If the glTF file is to contain an efficient delivery of the model to the GPU, adding lights increases the size of the model to the limit of the lights and the scene manager needs to deal with something not directly under its control.

If the glTF file contains an entire scene (or subscene), then isn't the nice partitioning effect of having interchangeable and reusable model files lost?

dakom commented 6 years ago

Can I put in a request for documentation on how to use the lights data in a webgl shader? Especially helpful would be if there was a webgl reference implementation, like these but to handle lights:

MiiBond commented 6 years ago

Updated three.js implementation of KHR_lights. https://github.com/mrdoob/three.js/pull/13341

donmccurdy commented 6 years ago

Can I put in a request for documentation on how to use the lights data in a webgl shader?

@dakom Good suggestion, I think that would be put in glTF-WebGL-PBR if we add it. Filed an issue.

If the glTF file contains an entire scene (or subscene), then isn't the nice partitioning effect of having interchangeable and reusable model files lost?

I don't think any determination has yet been made that glTF assets should represent a single model, rather than a scene, or (if so) where that line should be drawn. This might be better to raise in a new issue, perhaps. Lights existed in glTF 1.0 (not sure whether this issue was raised then), this PR is primarily updating the syntax for glTF 2.0 and PBR models.

donmccurdy commented 6 years ago

Also — I'm sure it's mentioned somewhere in this thread already, but a proposal for KHR_lights is now open, and it would be great to have feedback directed there: https://github.com/KhronosGroup/glTF/pull/1223

UX3D-nopper commented 6 years ago

range should be default -1.0. If -1.0, there should be no range restrictions. If range >= 0.0, the the range restriction should apply. If we keep it like this, we have a division by zero.