Fix AutoExposure and ColorGrading stopping working when Tonemapping is set to None

[momoluna444]

Objective

Fix AutoExposure and ColorGrading stopping working when Tonemapping is set to None.

Solution

Removed the early return in the tonemapping pass.

Testing

Now auto_exposure and color_grading are working even with Tonemapping::None.

[IceSentry]

Could we instead check that color grading and exposure are enabled? I don't like that this pass would now run unconditionally.

[momoluna444]

I just checked, and found AutoExposure is in bevy_post_process. This means we'd have to add it as a dependency for bevy_core_pipeline, would that be okay?

[momoluna444]

Ah, I guess that won't work. It would cause a circular dependency. Maybe the right way is moving tonemapping into bevy_post_process? I wonder why we didn't do that before.

[alice-i-cecile]

Cc @atlv24

[atlv24]

I recall there being a complication when i tried to put tonemapping in bevy_post_process, things may have changed its worth trying again.

[github-actions]

Your PR caused a change in the graphical output of an example or rendering test. This might be intentional, but it could also mean that something broke!
You can review it at https://pixel-eagle.com/project/B04F67C0-C054-4A6F-92EC-F599FEC2FD1D?filter=PR-23522

If it's expected, please add the M-Deliberate-Rendering-Change label.

If this change seems unrelated to your PR, you can consider updating your PR to target the latest main branch, either by rebasing or merging main into it.

[momoluna444]

Currently, tonemapping is located in bevy_core_pipeline, while AutoExposure is in bevy_post_process. Since bevy_post_process already depends on bevy_core_pipeline, we cannot directly add bevy_post_process as a dependency for bevy_core_pipeline without creating a circular dependency.

I can think of the following potential solutions:

• Add a helper marker component in bevy_core_pipeline to indicate whether AutoExposure is enabled.
• Move tonemapping into bevy_post_process (I haven't investigated this fully yet, so I'm not sure what the blockers might be).
• Add a new Linear tonemap variant to explicitly define this behavior.
• Require users to manually remove the TonemappingPlugin or the Tonemapping component when they want to disable tonemapping entirely (as this PR currently does). However, Tonemapping is currently registered as a required component, and prepare_mesh_view_bind_groups assumes it always exists on a view. This approach would require further cleanup.

[alice-i-cecile]

Move tonemapping into bevy_post_process (I haven't investigated this fully yet, so I'm not sure what the blockers might be).

This makes intuitive sense to me, but the rendering folks may have nuanced opinions about why this would be bad. Tonemapping is more common that the other post-processing steps, so we may want to granularly feature-flag that crate at some point.