Replace Ambient Lights with Environment Map Lights

[SparkyPotato]

Objective

Transparently uses simple EnvironmentMapLights to mimic AmbientLights. Implements the first part of #17468, but I can implement hemispherical lights in this PR too if needed.

Solution

• A function EnvironmentMapLight::solid_color(&mut Assets<Image>, Color) is provided to make an environment light with a solid color.
• A new system is added to SimulationLightSystems that maps AmbientLights on views or the world to a corresponding EnvironmentMapLight.

I have never worked with (or on) Bevy before, so nitpicky comments on how I did things are appreciated :).

Testing

Testing was done on a modified version of the 3d/lighting example, where I removed all lights except the ambient light. I have not included the example, but can if required.

---

[github-actions]

Welcome, new contributor!

Please make sure you've read our contributing guide and we look forward to reviewing your pull request shortly ✨

[IceSentry]

I think we may want to add a deprecation notice to the AmbientLight component and remove it after 0.16. For this PR I think this is fine as is though.

[IceSentry]

This is great, thank you!

[IceSentry]

I can implement hemispherical lights in this PR too if needed

The PR is simple enough that you could add this in too I think. Up to you, but I would approve it either way.

[SparkyPotato]

I can implement hemispherical lights in this PR too if needed

The PR is simple enough that you could add this in too I think. Up to you, but I would approve it either way.

Might as well add it in then.

[robtfm]

code looks good.

simplifying the shaders is nice, but is the cost of binding an extra texture per view and looking up for every pixel really worth the saving of 7-8 lines of shader code?

[SparkyPotato]

Found a bug with user-provided environment maps being overridden by the default ambient light, so I had to complicate the sync system a bit. Not sure if what I did was the most optimal.

code looks good.

simplifying the shaders is nice, but is the cost of binding an extra texture per view and looking up for every pixel really worth the saving of 7-8 lines of shader code?

The lighting example does go from about 0.51 ms to 0.55 ms on my machine (and enabling deferred makes that 0.46 ms to 0.49 ms), but I do doubt the usage of ambient light will be widespread enough that this matters, especially after the new procedural sky rolls around.

[IceSentry]

simplifying the shaders is nice, but is the cost of binding an extra texture per view and looking up for every pixel really worth the saving of 7-8 lines of shader code?

I guess what we could do is keep the constructor thing in this PR and do the ambient light thing separately? I can see why that part could be a bit more controversial and that people might still want that around.

[pcwalton]

It's not just about simplifying the shader code: it's that the interaction between ambient and environment map light never made any sense. They should be considered basically mutually exclusive (strictly speaking, ambient should be considered a type of light probe, which this PR moves us toward).

[mate-h]

This does not need to be a HDR texture right, since the user is defining the color in sRGB and an intensity?

[SparkyPotato]

Yeah, it's an 8-bit sRGB texture.

[robtfm]

The lighting example does go from about 0.51 ms to 0.55 ms on my machine (and enabling deferred makes that 0.46 ms to 0.49 ms)

is that in release? if so then i'd argue a ~10% fps reduction is not justified, but it's surprising how large the impact is.

They should be considered basically mutually exclusive (strictly speaking, ambient should be considered a type of light probe, which this PR moves us toward).

makes sense. if the impact is really this big then perhaps we need some investigation into env map perf before going further though?