Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

KHR_lights_punctual extension proposal #1223

Merged
merged 42 commits into from
Oct 31, 2018
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
31ca6c2
First draft of khr_lights
MiiBond Jan 10, 2018
524fd6b
Updates to clarify units
MiiBond Jan 18, 2018
8cb0c17
Removing hemisphere light
MiiBond Jan 18, 2018
bd8938e
Update spotlight property description
MiiBond Jan 24, 2018
07a8fec
Adjustments to light descriptions for directional and spotlights
MiiBond Jan 25, 2018
2113228
Clarifying usage of ambient lights and colour space of lights
MiiBond Jan 29, 2018
e7ae552
Adding base node properties to lights
MiiBond Feb 5, 2018
0610717
Removing name properites from lights in KHR_lights example.
MiiBond Feb 5, 2018
72096c4
Adding light's name property back in for consistency
MiiBond Feb 6, 2018
e495b9e
Fix typos in schemas
MiiBond Feb 13, 2018
625b28e
Remove redundant bounds specification for inner and outer cone angles
MiiBond Mar 14, 2018
dbc99db
Fix inconsistency in spotlight angle bounds
MiiBond Mar 14, 2018
8326130
Updates to light schemas
MiiBond Mar 15, 2018
26f376f
Merge branch 'master' of github.com:KhronosGroup/glTF into khr_lights_v1
MiiBond Apr 3, 2018
0a79dc7
Move KHR_lights to match new folder structure
MiiBond Apr 3, 2018
6aff0cc
Few tweaks to address comments
MiiBond Apr 11, 2018
6e871f6
Adding range
MiiBond May 24, 2018
c605724
Add range property to schema
MiiBond May 24, 2018
766ba65
Remove reference to aesthetic reasons for using light range.
MiiBond May 30, 2018
b5dff39
Merge branch 'master' of github.com:KhronosGroup/glTF into khr_lights_v1
MiiBond Jun 27, 2018
8ae54ee
Rename extension and remove ambient lights
MiiBond Jun 27, 2018
3dcdfb0
Format exponents.
Jul 2, 2018
bd047d4
Update readme title.
Jul 3, 2018
34ae110
Update inner and outer angle description for spotlights
MiiBond Jul 6, 2018
382d3d6
Merge branch 'master' of github.com:KhronosGroup/glTF into khr_lights_v1
MiiBond Jul 6, 2018
be36985
Merge branch 'khr_lights_v1' of github.com:MiiBond/glTF into khr_ligh…
MiiBond Jul 6, 2018
2945499
Remove reference to UE4 and Frostbite
MiiBond Jul 6, 2018
08bc3c7
Remove more references to ambient lights
MiiBond Jul 12, 2018
28d2bac
Make it clearer that the spotlight code is for the attenuation betwee…
MiiBond Jul 12, 2018
78a93ae
Updating the list of contributors
MiiBond Aug 9, 2018
d9a6811
Updating the description for range
MiiBond Aug 9, 2018
50050e6
Merge branch 'master' of github.com:KhronosGroup/glTF into khr_lights_v1
MiiBond Aug 24, 2018
66dfb38
Change default light direction from pointing down +Z to pointing down -Z
MiiBond Aug 24, 2018
1048d16
Address several comments
MiiBond Sep 5, 2018
10ed0c6
Scale affects light orientation, not properties.
Oct 2, 2018
d9313cf
Remove space in filename.
Oct 3, 2018
fa6d985
Remove space in filename.
Oct 3, 2018
2d240ec
+z -> -z
Oct 3, 2018
93ae7df
Make 'light' required on node.
Oct 15, 2018
e4989c1
Make 'lights' required on top-level extension.
Oct 15, 2018
03f63e0
Use default values when 'spot' property is omitted.
Oct 15, 2018
f49c51e
Make spot property required when type is 'spot'.
Oct 23, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
143 changes: 143 additions & 0 deletions extensions/Khronos/KHR_lights/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,143 @@
# KHR\_lights

## Contributors

* Norbert Nopper, UX3D, <mailto:nopper@ux3d.io>
* Thomas Kress, UX3D, <mailto:kress@ux3d.io>
* Mike Bond, Adobe, <mailto:mbond@adobe.com>

## Status

Draft (not ratified yet)

## Dependencies

Written against the glTF 2.0 spec.

## Overview

This extension defines a set of lights for use with glTF 2.0.

Many 3D tools and engines support built-in implementations of light types. Using this extension, tools can export and engines can import these lights.

This extension defines five light types: `ambient`, `directional`, `point` and `spot`.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should read four now that hemispherical lights have been removed.


## Lights

Lights define light sources within a scene.

`ambient` lights are not transformable and, thus, can only be defined on the scene. All other lights are contained in nodes and inherit the transform of that node.
Copy link
Contributor

@donmccurdy donmccurdy Feb 25, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking through implications here:

  1. In the current syntax, this also means a scene may contain no more than 1 ambient light.
  2. If we were to support animating light properties in the future (e.g. intensity, color) ambient lights will be in a slightly awkward position, such that they have an index in the lights array, whereas all other animated properties are targeted by node. If the same ambient light is reused in multiple scenes the animation syntax must clarify which instance of the ambient light is targeted.
  3. Given issues above, and for consistency, would it be better to allow ambient lights' use in nodes like other lights, but say (either a strict requirement or implementation note + validation warning) that the node must be an immediate child of the scene and have the identity transform?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

^IMO these concerns are resolved.


A conforming implementation of this extension must be able to load light data defined in the asset and has to render the asset using those lights.

### Defining Lights

Lights are defined within an dictionary property in the glTF manifest file, by adding an `extensions` property to the top-level glTF 2.0 object and defining a `KHR_lights` property with a `lights` array inside it.

Each light defines a mandatory `type` property that designates the type of light (`ambient`, `directional`, `point` or `spot`). The following example defines a white-colored directional light.

```javascript
"extensions": {
"KHR_lights" : {
"lights": [
{
"color": [
1.0,
1.0,
1.0
],
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this mean the extension will only support RGB color for lights (not including RGBA)? I don't know if alpha channel has a mean for lights but I used vec4 (RGBA) to specify light color in my shaders :-S

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I'm not aware of a use case (or, at least not a common use case) for alpha in a light's colour.

"type": "directional"
}
]
}
}
```

### Adding Light Instances to Nodes

`directional`, `point`, `spot` lights have position and/or orientation, which can be animated. These lights are attached to a node by defining the `extensions.KHR_lights` property and, within that, an index into the `lights` array using the `light` property.

```javascript
"nodes" : [
{
"extensions" : {
"KHR_lights" : {
"light" : 0
}
}
}
]
```

For light types that have a position (`point` and `spot` lights), the light's position is defined as the node's world location.
For light types that have a direction (`directional` and `spot` lights), the light's direction is defined as the 3-vector `(0.0, 0.0, 1.0)` and the rotation of the node orients the light accordingly.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this mean the direction in a world space will be "light node's world rotation matrix * Vector3(0.0, 0.0, 1.0)"?


### Adding Light Instances to Scenes

`ambient` lights have no position and no orientation. These lights must be attached to the scene by defining the `extensions.KHR_lights` property and, within that, an index into the `lights` array using the `light` property.

```javascript
"scenes" : [
{
"extensions" : {
"KHR_lights" : {
"light" : 0
}
}
}
]
```

### Light Types

All light types share the common set of properties listed below.

#### Light Shared Properties

| Property | Description | Required |
|:-----------------------|:------------------------------------------| :--------------------------|
| `color` | RGB value for light's color. | No, Default: `[1.0, 1.0, 1.0]` |
| `intensity` | Brightness of light in. The units that this is defined in depend on the type of light. `point` and `spot` lights use luminous intensity in candela (lm/sr) while `directional` lights use illuminance in lux (lm/m^2) | No, Default: `1.0` |
| `type` | Declares the type of the light. | :white_check_mark: Yes |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps you're missing name property here?
In examples/lights.gltf light has name property.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, yes I am. Thank you.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait, actually, I think this is just a problem with my example. My intension was for lights to use the name of the node that they are attached to so the name in the examples shouldn't be there.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

About removing name entirely. I wish it weren't there on meshes or cameras, because in my mind the node is a mesh or a camera, it doesn't contain them, and so one of the two names must be thrown away in loading. But since both do have names in the current spec, it would be more consistent to keep one here. I'm undecided on this..

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, you're right. I guess that's why I had it there. It is pretty odd to have a name on both the node and the light objects but, as it's consistent, I guess I'll put it back.


#### Ambient

Ambient lights define constant lighting throughout the scene.

#### Directional

Directional lights are light sources that emit from infinitely far away in the direction of the -z axis. This light type inherits the orientation of the node that it belongs to. Because it is at an infinite distance, the light is not attenuated. It's intensity is defined in lumens per metre squared, or lux (lm/m^2).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... are light sources that emit from infinitely far away in the direction of the -z axis.

I think this could be phrased more clearly, but struggling with it. What about: "... are light sources infinitely far away in the direction of the -z axis, emitting light in the direction of the +z axis".

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adjusted.


#### Point

Point lights emit light in all directions from a position in space. The brightness of the light attenuates in a physically correct manner as distance increases from the light's position (i.e. brightness goes like the inverse square of the distance). Point light intensity is defined in candela, which is lumens per square radian (lm/sr).

#### Spot

Spot lights emit light in a cone over a distance. The angle and falloff of the cone is defined using two numbers, the `innerConeAngle` and `outerConeAngle`. As with point lights, the brightness also attenuates in a physically correct manner as distance increases from the light's position (i.e. brightness goes like the inverse square of the distance). Spot light intensity refers to the brightness inside the `innerConeAngle` and is defined in candela, which is lumens per square radian (lm/sr). Engines that don't support two angles for spotlights should use `outerConeAngle` as the spotlight angle (leaving `innerConeAngle` to implicitly be `0`).

| Property | Description | Required |
|:-----------------------|:------------------------------------------| :--------------------------|
| `innerConeAngle` | Angle from centre of spotlight where falloff begins. | No, Default: `0` |
| `outerConeAngle` | Angle from centre of spotlight where falloff ends. Must be greater than innerConeAngle. | No, Default: `PI / 2.0` |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's specify explicitly that each angle is in radians.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In three.js, neither angle may exceed PI / 2. I only have some old screenshots to go by, but Unreal appears to have the same requirement. So probably we should specify a min/max here.

In that case, maybe the default outerConeAngle value should be less extreme than 90º?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call.


```javascript
"extensions": {
"KHR_lights" : {
"lights": [
{
"spot": {
"innerConeAngle": 0.785398163397448,
"outerConeAngle": 1.57079632679,
},
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since type specifies it is "spot" why it used here again? Obviously it simplifies to access the data after getting type but it could be better name : "cone" (because we already know spot has cone shape):

"cone": {
  "innerAngle": 0.785398163397448,
  "outerAngle": 1.57079632679,
}

it also reduces a few bytes :) it is just feedback.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think keeping the name spot for the options related to "spot" lights is clearest, and probably consistent with what we'd want for future light types. No preference on keeping Cone in the name or not, though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I've debated removing "Cone" from the property names too but I kind of like the explicit description.

"color": [
1.0,
1.0,
1.0
],
"type": "spot"
}
]
}
}
```
Binary file not shown.
Loading