Fix incorrect light values on blend import #108356
Conversation
|
Ah, nevermind. Changed into a draft, noticed that the MRP in the issue doesn't actually match the Blender render when using physical light units. It might just be the background, which is not really fixable in this PR, but it does need further investigation 😄 |
|
It was just the background, all good! Added a fourth row of comparison screenshots, again in the same project with the same exposure settings, but using the MRP blend file (minus the background). |
| // Non-physical light units seem need an entirely separate set of multipliers to match | ||
| // Blender at the default exposure. Unsure why these specific multipliers look right, | ||
| // but they sure do, and they're oddly round numbers, so maybe they're correct. | ||
| if (cast_to<OmniLight3D>(light)) { | ||
| light->set_param(Light3D::PARAM_ENERGY, blender_light_units / 50.0); | ||
| } else if (cast_to<SpotLight3D>(light)) { | ||
| light->set_param(Light3D::PARAM_ENERGY, blender_light_units / 10.0); | ||
| } else { | ||
| light->set_param(Light3D::PARAM_ENERGY, blender_light_units / 3.0); | ||
| } |
There was a problem hiding this comment.
I simply eyeballed some different lighting setups, and got all of them to look right using these factors
Lights match perfectly between Blender and Godot in non-physical mode if you export in Unitless mode, there's no need to eyeball.
There was a problem hiding this comment.
So they do! I tried that, but I suppose there was something else wrong in that version, haha. This is much better, fixed in d3dfcea.
There was a problem hiding this comment.
Turns out they ended up a bit brighter than Blender when comparing default exposure to default exposure (see updated screenshots), but I think that's very acceptable given how simple of a fix this was. (The MRP screenshot looks the worst, but maybe it's fine, if this is a common workflow anyway.)
| // Blender has physically based attenuation, so this is needed to get comparable lighting results. | ||
| light->set_param(Light3D::PARAM_ATTENUATION, 2.0); |
There was a problem hiding this comment.
I agree with this but there are compatibility considerations discussed here
This should be changed in both blend and gltf importer simultaneously because those two are basically the same and should behave identically on import (I mean all changes to light import, not just attenuation)
There was a problem hiding this comment.
Ah, yeah. That's probably better to do in another PR then, rather keep this one focused on the very obvious issue of the light energy/intensity values.
There was a problem hiding this comment.
Not sure if it is actually, the discussion in that proposal was about changing the default value in nodes which would immediately break existing projects. But this PR changes import logic, not the default. Strictly speaking it's already breaking import compatibility by changing light intensity, so maybe it makes sense to fix the attenuation as well
There was a problem hiding this comment.
I moved the attenuation fix to apply to all glTF imports, that seems like an obvious enough fix (assuming that changing import results isn't breaking): ee9ebe4
I would rather not generalize the light brightness fix to all glTFs, because the luminous efficacy constant used to convert Blender watts into glTF lumens is a less clear specification issue (see #73624), which as far as I know, used to be (is?) 683, and is now in the process of being changed to 177.82 (which has a better logic behind it, I think). This PR's intention is to try and fix this specifically for Blender files, because Blender is not a moving target unlike the glTF specification, we can just look at what Blender uses for luminous efficacy.
Though I suppose that specifically for projects using non-physical light units, all glTFs could be imported using the Unitless lighting mode, now that that's apparently all that's needed. Nevermind, that's too late, it's a Blender option, not a glTF option, it's just up to the glTF author to export using Unitless and it'll work fine with non-physical light units 😄
This commit changes the Blender glTF export process to output light intensities in watts, and adds a separate conversion step to convert those into appropriate units for Godot. Blender's watts result in correct relative brightnesses for the lights when using use_physical_light_units, but unfortunately only then. Due to this discrepancy, the conversion process has separate branches depending on that project setting, which also means that blend files need to be reimported after changing that setting. When using physical light units, Blender's light values are already in the correct quantities, the values are just in the wrong "scale" (radiometric vs photometric). To account for this, the conversion process converts the watts (and watts/m^2) to lumens (and lux) by using a luminous efficacy constant picked up from Blender's IES profile parser, an existing case of Blender converting lumens (well, candelas) into watts. The luminous efficacy value used (177.83) is also currently being proposed to be used in glTF-Blender-IO#2554. When using non-physical light units, using Blender's watts directly as the energy ends up with way too bright lights, though some light types more than others. This branch is not grounded in any real numbers, I simply eyeballed some different lighting setups, and got all of them to look right using these factors, using the default exposure settings and the AgX tonemapper for both Godot and Blender. Much less sure about this branch, but I think this is better than what's in master, at least. Fixes godotengine#92168.
neonmoe
force-pushed
the
fix-incorrect-light-values-for-blender-imports
branch
from
ee9ebe4 to
2b3b6f0
Compare
|
Rebased after making changes based on reviews, and updated the screenshots to match. |
Thanks for the mention, I always appreciate it :) Should we wait until KhronosGroup/glTF-Blender-IO#2554 is merged before locking in the luminous efficacy value used (177.83)? Also, in the comment where this constant is described, it might be good to link this source PR or related source code in other repositories. I'd like to hear thoughts from @will-ca and @RevoluPowered who have already discussed some of this in #73624 I would love to dive deep into this, but I don't have the time in the next while to familiarize myself with Blender and its standards. |
|
After writing out this comment on the glTF-specific issue, I think this might actually be best fixed for physical light units by letting Blender export using the "spec" lighting mode, passing the glTF light intensity values to our intensity parameter (and multiplying spot and omnilights by 4*pi), and leaving the energy parameter as 1. Then Godot would be working according to the glTF spec, and any luminous efficacy related decisions would be up to Blender. Currently it'd export lights using 683lm/W, which would be a little bright compared to 177.82lm/W, but still definitely in a usable range, compared to passing the 683-scale values into the energy parameter (which gets multiplied by 1000lm or 100000lx due to the default intensity), as Godot is currently doing. That'd also fix #73624. And once Blender switches to 177.83lm/W, Godot wouldn't need any changes to reflect that. |
This commit changes the Blender glTF export process to output light intensities in watts, and adds a separate conversion step to convert those into appropriate units for Godot.
Blender's watts result in correct relative brightnesses for the lights when using use_physical_light_units, but unfortunately only then. Due to this discrepancy, the conversion process has separate branches depending on that project setting, which also means that blend files need to be reimported after changing that setting.
When using physical light units, Blender's light values are already in the correct quantities, the values are just in the wrong "scale" (radiometric vs photometric). To account for this, the conversion process converts the watts (and watts/m^2) to lumens (and lux) by using a luminous efficacy constant picked up from Blender's IES profile parser, an existing case of Blender converting lumens (well, candelas) into watts. The luminous efficacy value used (177.83) is also currently being proposed to be used in glTF-Blender-IO#2554.
When using non-physical light units, using Blender's watts directly as the energy ends up with way too bright lights. Turns out, Blender's "Unitless" export option gets pretty close to Blender with the default Godot exposure, so that is used when physical light units are disabled, and no further post-processing is applied.
Additionally, this sets the light attenuation for glTF (and thus Blender) imports to 2.0 by default, to match the attenuation seen in Cycles and EEVEE.
Fixes #92168.
Screenshots
The screenshots using physical light units use camera attributes tuned for indoor scenes (2.4 f-stop, 100 Hz shutter speed, 400 ISO sensitivity), fine-tuned to match Blender. I think indoor levels of exposure make sense, because adding a sun into Blender's default cube scene ends up being way overexposed. Note that the exposure being eyeballed means that these screenshots aren't really helpful for validating if 177.83 is the correct luminous efficacy, but I do think 177.83 results in appropriate lumens, and the exposure used to match Blender here feels about right to me.
The non-physical light units screenshots are using the default camera attributes' exposure.
(The Godot screenshots reflect commit 2b3b6f0.)
The first scene is just the default Blender cube with the background taken out to ease calibration. The second one has all the three light types, to validate that they look appropriate relative to each other. The third one has a single directional light, which is The Sun levels of bright, except it has an almost black color to avoid overexposing the default ("indoors") exposure. The fourth one is the Blender scene included in the issue's MRP, with the background set to black (since that can't be accounted for by this import process, and the default Godot background is too bright for this exposure with physical light units).
The point of the three scenes is to move around the lights, use different intensities, and then see if the same exposure settings lead to matching results with Blender.
Here's the project including all the scenes that those screenshots are from: light-tests.zip