jkuhlmann / cgltf

:diamond_shape_with_a_dot_inside: Single-file glTF 2.0 loader and writer written in C99
MIT License
1.44k stars 136 forks source link

Add support for KHR_materials_emissive_strength. #170

Closed abwood closed 2 years ago

abwood commented 2 years ago

KHR_materials_emissive_strength is a new extension that recently entered the "Release Candidate" status. This one is pretty simple in that the core specification limits the emissiveFactor to values in the range of [0.0 - 1.0]. This extension provides an additional multiplier to amplify emission beyond 1.0.

After discussion (below) this implementation defines a new flag, has_emissive_strength and an emissive_strength struct that embodies this float property. The original approach is quoted here to preserve the history of this PR conversation, but no longer applies ...

Rather than following the route of other extensions with a has_emissive_strength flag and an emissive_strength struct, I figured it would be easier on the implementers if cgltf took the liberty of factoring in the emissiveStrength into the emissiveFactor internally. My thought is that someday we could see this extension pulled into a future version of glTF, but rather than having two "dials" to control the emissionFactor, the spec editors would instead just relax the schema constraints and allow for emission values to go beyond 1.0.

For confirmation, I locally updated Filament with these changes. Here is an altered DamagedHelmet, with the addition of the KHR_materials_emissive_strength extension that defines a strength of 5.0.

image

And here is the original DamagedHelmet model (i.e., emissiveFactor=[1.0, 1.0, 1.0]) image

TODO

/CC @emackey @prideout

zeux commented 2 years ago

This is definitely easier for loaders but I'm not sure what the processing tools like gltfpack are supposed to do with this. You can decompose the color with components >1 into color+strength but that results in a different output compared to the input. The same question would arise for applications that use fixed-point quantization for material parameters to conserve material structure size.

Also if we ever get the [rather scary] property animation extension it's going to make it impossible to animate strength individually with this approach.

jkuhlmann commented 2 years ago

Thanks for implementing this @abwood! It feels like a great shortcut to do it this way, but I also understand the concerns raised by @zeux. I think if we follow that thought, maybe it would be better to implement this like the other extensions? Ultimately, the change to existing renderers/engines to support it then would be pretty minimal.

abwood commented 2 years ago

Great points, especially when considering the notional property animation extension. I can change it back to the original implementation.

jkuhlmann commented 2 years ago

That would be great! 😃

abwood commented 2 years ago

I think this is good now. I updated cgltf_write and tested it out by loading Box.gltf, cranking up emissive_strength to 5, and then writing it file. I loaded the resulting model into Filament and confirmed the results are as expected. Also ran it through the validator built into the VSCode gltf plugin and the written model came back clean.

Behold! image