bevyengine / bevy

A refreshingly simple data-driven game engine built in Rust
https://bevyengine.org
Apache License 2.0
36.08k stars 3.56k forks source link

default point lights seems wrong after exposure #11577

Closed mockersf closed 8 months ago

mockersf commented 9 months ago

Bevy version

Main after https://github.com/bevyengine/bevy/pull/11347

What went wrong

Looking at examples, most point lights are set to 150000 lumens, which is more than direct sunlight. Some are even up to 1000000 lumens, which seems an absurd value to set.

The default value of a point light is 800 lumens, which is now useless.

Bevy default values should make sense, and examples should not use values that don't make sense.

GitGhillie commented 9 months ago

Is the issue that the numbers are too big (i.e. you'd prefer a different scale/unit) or that they are not outputting the correct amount of light?

most point lights are set to 150000 lumens, which is more than direct sunlight

Bit of a nitpick but that sounds like you're comparing lumens and lux which you can't do without a distance for reference. The sun outputs 36 octillion lumens...

mockersf commented 9 months ago

We should use lux and lumens, and I don't know if the output is correct. My issue is that the defaults and the examples don't make sense

GitGhillie commented 9 months ago

Maybe good to note that in the 3d_scene for example we are trying to light the cube from almost 10m away, so that would require a powerful light. But yeah still not sure about the output either

MAG-AdrianMeredith commented 9 months ago

In layman's terms, lumens is the input energy, the others lux,candela etc is how much light reaches a particular surface. If it helps I found the GLTF spec quite handy for under standing whats what for light intensities https://github.com/KhronosGroup/glTF/blob/main/extensions/2.0/Khronos/KHR_lights_punctual/README.md

tbillington commented 8 months ago

Would it be worth having something similar to from_visibility for setting up the light ?

cart commented 8 months ago

Did some digging. Blender's default light is 1000 watts, their exporter exports this as 54,351 candelas (682,994 lumens). Blender's only exposure that I'm aware of / could find is the Color Management exposure value, which is calculated using input_color * 2^exposure. Note that this is the same formula Bevy uses for its Color Grading exposure. Both Blender and Bevy default these exposure values to 0.0.

This also means that #11347 added a second exposure applied before the Color Grading exposure. This was not mentioned explicitly in that PR from what I've seen, and it does feel a bit strange to have two in two separate places. We should discuss / defend the existence of two exposure properties that exist in two different components that are functionally very similar in terms of what they do to output colors.

I exported a scene from Blender to GLTF and tried to align everything I could:

Blender is on the left, Bevy is on the right:

image

Bevy is significantly brighter / fully washed out. Something isn't being accounted for. Some ideas:

  1. Maybe Blender actually has an additional hidden "camera exposure" that is darkening things (similar to our hard coded exposure prior to #11347). We would then just need to calibrate accordingly (aka we do need a second exposure as introduced in #11347). This does make sense conceptually / is consistent with Filament and our previous hard-coded exposure.
  2. It is possible that the watts to candelas conversion done by blender is wrong (or their units are wrong somewhere else). Note that they used to write watts directly to the gltf without conversion and then they corrected for that here: https://github.com/KhronosGroup/glTF-Blender-IO/issues/564. But I've seen a lot of conflicting conversations about converting Blender's watts to lumens (ex: multiply by 683 (Edit: the previous link actually makes sense after converting from candelas to lumens / they line up well)
  3. Maybe our lighting calculations are just very out of sync / don't agree on how a light of a particular power behaves.

Something that does feel a bit strange: if I do write the watts value directly to the Bevy point light (1000 as the "intensity in lumens") with no other changes to the settings above, things line up pretty nicely:

image

(blender left, bevy right)

Lines up quite well in terms of brightness!

Likewise if I hard code Bevy's ColorGrading::exposure to `(-7.0f32).exp2() and set Blender's exposure to -7 (note the minus), we get this:

image

They line up pretty much exactly! Maybe just coincidence. But it is interesting. If we plug our ears and pretend Watts == Lumens and that we don't need ev100 exposure settings, then we are roughly in sync with blender and we get lumen values that line up with expectations from the real world (~1000 lumens for an indoor lamp). Weird.

Another comparison: if we take the first case (using the exported Watts->candelas->lumens), but also set our new ExposureSettings::ev100 to 10.0 (more than the new default of 7.0, less than the old hard-coded default of ~12), we get this:

image

Essentially identical!

And here is the current default ev100 (7.0):

image

Note that Bevy's lighting is oversaturated relative to Blender's.

My current bet is:

  1. The Blender GLTF exporter is probably correct about its watts -> candelas conversion
  2. Eevee does have some sort of internal exposure setting that is close to ev100 10.0 (which is a very reasonable number)
  3. For some reason inherent to the approach both we (and blender) are using, we need exorbitantly high / non-real-world lumens (on the order of hundreds of thousands) to emulate an indoor lamp (which should be closer to 1,000)

In that case, we should just calibrate our default ev100 to 10.0, accept the crazy numbers required, and call it a day. It would be reassuring if we could reproduce these results in at least one other engine (blender, unity, unreal, etc) with aligned settings.

My "long shot" conspiracy theory bet (based on the weird agreement in the second test above) is that Eevee and Bevy are using roughly the same algorithms internally, they agree on lumens as units (and blender is just wrong about it being watts), blender doesn't have a hidden exposure roughly 10.0 ev100 (and therefore we should remove ours / default it to something that produces 1.0 as the factor), and we can therefore use "real world" lumen values.

GitGhillie commented 8 months ago

This also means that https://github.com/bevyengine/bevy/pull/11347 added a second exposure applied before the Color Grading exposure. This was not mentioned explicitly in that PR from what I've seen, and it does feel a bit strange to have two in two separate places. We should discuss / defend the existence of two exposure properties that exist in two different components that are functionally very similar in terms of what they do to output colors.

There was a bit of discussion about that in the original PR: https://github.com/bevyengine/bevy/pull/8407#issuecomment-1871653253

fintelia commented 8 months ago

Please also see https://github.com/bevyengine/bevy/pull/8407#issuecomment-1858639312. The Watts -> lumens conversion is an oversimplification if not outright wrong

I think the reason both Blender and Bevy need seemingly huge values for light brightness is because CGI scenes use fewer lights and often place them further away from the objects they are lighting:

There's also the lack of indirect light from point lights in scenes without walls (Blender) or any scene (Bevy), which I don't know how to quantify off-hand, but probably is a pretty significant impact

cart commented 8 months ago

Thanks for the links! I definitely missed the conversation that happened in #8407. Accidentally retread some ground :smile:

I do agree that those points are relevant / play a role. But I find it pretty hard to believe that the scene above (a 2 meter cube with a light 2 meters away) requires 682,994 lumens (341 times the power of my "lights up the whole room at night" 2000 lumen led bulb) to be reasonably lit from 2 meters away.

There's also the lack of indirect light from point lights in scenes without walls (Blender) or any scene (Bevy), which I don't know how to quantify off-hand, but probably is a pretty significant impact

Hmmm yeah light bounces probably play a significant role here.

That being said, I think "agreement with Blender" is our highest priority. For 0.13 it probably makes sense to dial in the default ev100 to whatever gives us parity, which is essentially why @GitGhillie chose ev100 = 7 in 8407. Weirdly that value was very out of sync for me (see my above experiments). @GitGhillie, did you switch to AgX as your default tonemapper (and do things like remove ambient light)?

cart commented 8 months ago

I'll run some more tests

GitGhillie commented 8 months ago

Yeah I forgot about tonemapping back then but judging from the pictures I did remove the ambient light. Since I don't have the test setup from back then I made a new one (based on PR 11581 but should be the same) and interestingly I'm now also landing on an ev100 between 9 and 10 to match the lighting: image

(ofc tonemapping is not going to make such a significant difference so I'm now checking if I missed anything else)

cart commented 8 months ago

Yup 10 seems like the sweet spot to me with the default 1000w light:

image

Our reds desaturate way faster than Blender's for some reason. But everything else is pretty in sync.

cart commented 8 months ago

9.5 ev100 for reference (maybe slightly closer): image

cart commented 8 months ago

10w light, 9.5 ev100 (we end up being "too bright" in some cases) image

cart commented 8 months ago

10w light, 10 ev100 (better, but green still shows "too much" relative to blender) image

cart commented 8 months ago

10w light, 10.5 ev100 (we start seriously losing the white cube and blue sphere relative to blender, green is still slightly too visible) image

My take is 10 ev100 is the best for point lights

cart commented 8 months ago

Directional light 1 W/m^2, ev100 10: image

Blender is slightly brighter across the board

cart commented 8 months ago

Directional light 1 W/m^2, ev100 9.5: image

Now we're slightly brighter than blender. 9.5 seems closer than 10, sweet spot at this brightness is probably 9.7 or something.

cart commented 8 months ago

Directional light 0.05 W/m^2, ev100 9.5

image

We are significantly darker. ev100 9.2 seems like the sweet spot:

image

cart commented 8 months ago

10 W/m^2, ev100 9.2

image

We are slightly brighter (and significantly more desaturated).

ev100 10.0

image

We are slightly darker.

ev100 9.5 feels close to the sweet spot image

fintelia commented 8 months ago

I'm going to guess that the real value is log2(1000 / 1.2) = 9.703.

cart commented 8 months ago

My takeaway is that there isn't a perfect value to achieve parity, but that the ideal constraint solve is in fact approximately 9.7 :)

cart commented 8 months ago

For 0.13, I think we set the default to 9.7 (with a link to this issue in the docs for posterity), then adjust brightness of the lights in the examples accordingly with their end-of-series DragonballZ-tier power levels.

GitGhillie commented 8 months ago

@doonv how do you feel about changing the default in #11581 again? 😅 Otherwise I can also make a PR to your PR tomorrow (I'm on EU time, need to go now). Edit: This can actually be its own PR now that I think of it

doonv commented 8 months ago

@doonv how do you feel about changing the default in #11581 again? 😅 Otherwise I can also make a PR to your PR tomorrow (I'm on EU time, need to go now)

I'd rather not, I already went through the examples multiple times and I don't wanna do that again.

GitGhillie commented 8 months ago

Understandable! I'm willing to do it but first I want to see a go/no-go decision on your PR before I also start going back and forth on the examples

fintelia commented 8 months ago

Blender seems to be making a bunch of errors that all cancel each other out if the true exposure is ev100=9.7:

Perhaps the thing to do is to add a constant of BLENDER_DEFAULT = 9.7 and not even pretend it is physically based?

cart commented 8 months ago

Given that the purpose of the constant is to sync with blender, I'm cool with that (although it seems premature to say if / how blender is getting things wrong). 9.7 seems like a pretty reasonable "middle of the road" exposure value.

cart commented 8 months ago

(but for the record, it certainly seems like we're missing something here either on the Blender side or the Bevy side)

cart commented 8 months ago

Strategy-wise, lets finish reviewing and merging #11581 and then add EV100_BLENDER to the constants, use it as default, and in the same PR re-calibrate the examples

fintelia commented 8 months ago

Is that going to work with Blender exported GLTFs? Or does EV100_BLENDER actually need to be 9.7 + log2(683) = 19.118?

DGriffin91 commented 8 months ago

It's also worth noting that blender uses a different version of AgX than bevy does. Bevy added AgX before blender did. Bevy uses the original AgX getting it's LUT in a easier format from https://github.com/MrLixm/AgXc. Note on AgXc github about blender's version.

Subjectively, they look a bit different, it's been a while since I compared so I don't remember what the differences were.

Blender uses https://github.com/EaryChow/AgX

Blender AgX release notes: https://wiki.blender.org/wiki/Reference/Release_Notes/4.0/Color_Management

Blender PR that added AgX https://projects.blender.org/blender/blender/pulls/106355

It would probably be good to get blender's version in bevy at some point. Idk about the licensing, AgX, AgXc and Eary's AgX all don't have any license documents of any kind.

If you turn off tonemapping or use blender filmic in both they should match a bit more closely.

cart commented 8 months ago

Is that going to work with Blender exported GLTFs? Or does EV100_BLENDER actually need to be 9.7 + log2(683) = 19.118?

EV100_BLENDER would be 9.7 because thats what I set ExposureSettings::ev100 to when I was calibrating.

cart commented 8 months ago

(and yes that calibration was done with Blender-exported gltfs)

cart commented 8 months ago

It's also worth noting that blender uses a different version of AgX than bevy does. Bevy added AgX before blender did. Bevy uses the original AgX getting it's LUT in a easier format from https://github.com/MrLixm/AgXc. Note on AgXc github about blender's version.

Ahhh gotcha that would explain it

cart commented 8 months ago

Blender Filmic, ev100 9.7, directional light 10 W/m^2 image

(basically identical)

directional light 0.03 W/m^2

image

(basically identical)

point light 10w

image

(big divergence)

point light 500w

image

(basically identical)

Consensus: basically identical with the exception of low-light point lights.

cart commented 8 months ago

image

The big "DragonBallZ" units are actually probably correct.

cart commented 8 months ago

Just kicked off a merge of #11581. I'm doing the follow up work we discussed above now.

cart commented 8 months ago

Reopening this because it isn't resolved yet

Bcompartment commented 8 months ago

IMO point light should have physical control values instead of arbitrary "intensity". Wiki: _https://en.wikipedia.org/wiki/Inverse-square_law_ intensity = 1/d^2 So, what bevy's "intensity" even mean ? There isn't frame of reference. Using common words like "brightness", "intensity" is essentially the same as measuring distance in burgers or football fields instead of meters. It may fell right first but only causes confusion in long run.

My proposal: 1 Only physically based measurement units 2 Only real world scales, without hidden adjustments, if value feels big, just use e notation ( 3.1415e0, 10e32, 4.021e-5)

Q "What about new users ?" A "We should create examples that explains physical light model and camera internals work."