GLTF LightsPunctualExtensionLoader misses most of the light definitions

[zzuegg]

The imported model looks fine in blender but fails when importing to jme.

LightColors are set to NaN Lightpostions are set to 0,0,0 Radius is set to Infinity

I will investigate one i have free time

Reference model is intel sponza

[Ali-RS]

Pinging @tlf30 just in case, as he is the author of the extension.

[zzuegg]

Could also be that the model definition is broken. Don't know yet.

The color issue is that intensity is set to 0 and the lumens calculator returns NaN. not sure how blender handles that.

[Ali-RS]

Could also be that the model definition is broken. Don't know yet.

By the way, you can check this in an online gltf validator/viewer.

https://github.khronos.org/glTF-Validator/

https://gltf-viewer.donmccurdy.com/

https://sandbox.babylonjs.com/

[zzuegg]

Its a 3.7gb file. Anything downloadable?

Nervermind. i see its doing all offline

[zzuegg]

Beside a bunch of "unused *" things there are no errors.

Regarding the colors, in blender the lights are set to 0 watt. I think in that case we should not return NaN but ColorRgba(0,0,0,0) . The bigger problem is that we actually loose the light information color so there is no way to increase the light power later in your application. If it was my call i would add some additional properties to the jme lights

[zzuegg]

Ok, the position issue can also be considered as fixed. It is a result of the way lights get added trough the LightControl, when processing a loaded model right after loading the information about the lightposition is not yet set. You have to call updateLogicalState first

[Ali-RS]

Regarding the colors, in blender the lights are set to 0 watt. I think in that case we should not return NaN but ColorRgba(0,0,0,0) .

So, are you saying blender gltf exporter does not export light colors correctly?

Edit: Tested with blender 3.0 and it seems fine. Here is an empty scene with a point light and directional light and light colors are exported correctly.

{
    "asset" : {
        "generator" : "Khronos glTF Blender I/O v1.7.33",
        "version" : "2.0"
    },
    "extensionsUsed" : [
        "KHR_lights_punctual"
    ],
    "extensionsRequired" : [
        "KHR_lights_punctual"
    ],
    "extensions" : {
        "KHR_lights_punctual" : {
            "lights" : [
                {
                    "color" : [
                        0.03215407952666283,
                        0.027881082147359848,
                        1
                    ],
                    "intensity" : 1,
                    "type" : "directional",
                    "name" : "Sun"
                },
                {
                    "color" : [
                        0.004941803403198719,
                        1,
                        0.0242206659168005
                    ],
                    "intensity" : 10,
                    "type" : "point",
                    "name" : "Point"
                }
            ]
        }
    },
    "scene" : 0,
    "scenes" : [
        {
            "name" : "Scene",
            "nodes" : [
                1,
                3
            ]
        }
    ],
    "nodes" : [
        {
            "extensions" : {
                "KHR_lights_punctual" : {
                    "light" : 0
                }
            },
            "name" : "Sun_Orientation",
            "rotation" : [
                -0.7071067690849304,
                0,
                0,
                0.7071067690849304
            ]
        },
        {
            "children" : [
                0
            ],
            "name" : "Sun"
        },
        {
            "extensions" : {
                "KHR_lights_punctual" : {
                    "light" : 1
                }
            },
            "name" : "Point_Orientation",
            "rotation" : [
                -0.7071067690849304,
                0,
                0,
                0.7071067690849304
            ]
        },
        {
            "children" : [
                2
            ],
            "name" : "Point",
            "translation" : [
                -0.5107669830322266,
                1.0932204723358154,
                0
            ]
        }
    ]
}

[Ali-RS]

Radius is set to Infinity

It seems you should set the range in the "Distance" field, not in the "Radius" field. See https://github.com/KhronosGroup/glTF-Blender-IO/issues/618

[zzuegg]

Yeah, but when in blender you set light power to 0 jme translates that to ColorRGBA(NaN,NaN,NaN,NaN). Also we loose the information about the light color when it is zeroed out on import since jme does not support setting luminosity/power

Blender exporter seems to work fine. At least the other viewers i tried showed the correct result

[Ali-RS]

but when in blender you set light power to 0 jme translates that to ColorRGBA(NaN,NaN,NaN,NaN)

It seems that is because of Math.log(0) when intensity/lumens is 0: https://github.com/jMonkeyEngine/jmonkeyengine/blob/52568d76ffec063355bcff515066cb75887baf34/jme3-plugins/src/gltf/java/com/jme3/scene/plugins/gltf/LightsPunctualExtensionLoader.java#L250

Probably we should add a check for intensity=0.

By the way, why do you need to set the power to 0? Probably you can just set the color to (0,0,0,0) in Blender if you want to zero it out.

[zzuegg]

This is a model i downloaded from intel. Other engines probably support setting the light power in your editor/game. The color would still be preserved

[pspeed42]

We should look into how JME treats alpha in the light color. Perhaps we can coopt it.

[zzuegg]

Internally i don't know if there is something happening with alpha. on the shader side color.a is used to define the light type.

It does also not make much sense to have a radius as well as luminosity power defined.

[tlf30]

It has been a while since I dove into the GLTF spec on lights, so I cannot comment on the radius at this time. But, for the issue at hand with NaN color values, a couple things:

1. As mentioned, a "quick fix" would be to check for 0 and return a 0 color. And, as mentioned this will cause the color value of the light to be lost. In addition, someone will need to verify, but if using PBR I believe that having a 0 color will cause other calcs in the shaders to break. An example is the /Common/ShaderLib/Hdr.glsllib, which is where the lumen conversion code in question originally came from, has the same issue but at the shader level.
2. The main issue is JME's lights. There are many drawbacks to using the color as both the color and intensity. Ideally these would be separate parameters. But at this point that would be a major breaking change, perhaps one that should be done still though and added to the next major JME version. It would require rewriting many (if not all) of the lighting shaders.
3. Right now, there is no direct way to change light intensity in JME. It requires calculating a new color value that will preserve the color with a higher intensity (due to lights not having a separate intensity parameter). Due to this, until JME has a separate parameter for light intensity, I do not think that we should hack a solution together that preserves the light color just for this particular instance as it would behave differently than the rest of the engine. Although I agree it would be nice, we need a proper fix and not a large hack.

With all of that said, I think this is a very good reason to advocate for a reform of the JME lights (and not just for adding an intensity parameter). There are several improvements that could be made to them to make them more "modern" in terms of features supported by other engines, but at the cost of a major break in compatibility and a lot of up-front work on rewriting parts of the engine.
So really in the short term adding a check for a value of 0 and returning a 0 color might work, but it needs to be verified that this does not break at some other level like in the shader when a light has a 0 color value.

[zzuegg]

I looked over the code that setups the light data for the shaders. Color.A is not passed to the shader currently. So this could be used to store light intensity. We could also simply move the intensity calculation to the XXXXLightingLogic. We could then, with minimal changes support luminosity.

I checked also with blender how "wrong" radii / luminosity values are processed. Even blender does behave super strange when using high power and small radii. (Light gets super bright with hard cut of) I think jme's lighting would behave similar.

I propose adding a setLuminosity or setPower method to light which will be stored in the alpha value of the light color. When passing data to the shader the luminosity gets applied and passed as final value to the shader. The actual light color will stay untouched

Default setting for luminosity should be one that does not affect the current light intesity. (maybe -1 for not used?)

Issues: -Currently stored and loaded lights will probably have alpha set to 1. -Decision if the setColor method on the lights should only update rgb and leave a untouched

[Ali-RS]

When passing data to the shader the luminosity gets applied and passed as final value to the shader.

Is the math simply a color * intensity or it must proceed through a similar process like in here? would it happen inside fragment shader?

[zzuegg]

I would use the calculation from the gltf importer. That way the outcome would be the same.

No changes to the shaders needed.

Not sure on the other importers yet.