Follow-ups to the Tonemapping::None change - #25685
Conversation
Camera3d enables DebandDither by default and Camera2d defaulted to Tonemapping::None, so a camera that used None in 0.19 loses dither or color grading without any change of its own. Say which cameras are affected and that Tonemapping::Linear keeps the 0.19 result.
Add a PostUpdate system that logs a warning for a camera that has Tonemapping::None together with DebandDither::Enabled or a non-default ColorGrading. It runs on change detection, so it fires on spawn and on each change to those components. ColorGrading and ColorGradingGlobal derive PartialEq for the default comparison. Update the examples that combine None with dither or grading so they do not log the warning. The testbed white furnace cameras add DebandDither::Disabled, since they want exact output. The headless renderer and the tonemapping example use Linear in place of None. The bloom_2d cycle wraps to Linear. Mention the warning in the migration guide.
Camera2d defaulted to Tonemapping::None, which is a full passthrough. A 2D camera with DebandDither::Enabled or a ColorGrading component lost both unless the user also set a tonemapping method. Linear applies no tone curve but keeps dither, grading, and the clamp, so 2D cameras keep those settings by default. None stays the explicit passthrough on both camera types. A Camera2d with Hdr now runs the tonemapping pass every frame. The pass applies the identity curve, the clamp, and any dither or grading set on the camera. Set Tonemapping::None to skip the pass. Update the migration guide to say so.
cookie1170
left a comment
There was a problem hiding this comment.
i think a warning for this will definitely be helpful and should be kept, but i think a breaking change like that should break old code, rather than silently make its behaviour incorrect. i think calling Tonemapping::None something like Tonemapping::Disabled, so it doesn't conflict with the old Tonemapping::None name would be better.
although maybe people who didn't read the migration guide would just think it's a pointless rename so it might not be that good of an approach 🤔
I debated renaming |
yeah, makes sense |
Objective
In #25499 we made
Tonemapping::Nonea full passthrough and addedTonemapping::Linearfor the identity curve. Almost nobody wants a passthrough outside of HDR display calibration.Nonecombined with dither or grading is almost certainly a bug. The defaults and the docs should reflect that.There was a helpful comment on Discord pointing out that a project upgrading from 0.19 sees the change with no feedback.
Camera3dturns dither on by default andCamera2ddefaulted toNone, so a camera that usedNonein 0.19 quietly loses dither or color grading in 0.20, and nothing tells the user why or what to do about it (besides the migration guide).Solution
Add a warning when a camera combines
Nonewith dither or grading, so the user finds out when their app runs.Change
Camera2d's tonemapping fromNonetoLinear. In 0.19 the 2D default still applied dither and grading, soLinearmatches prior behavior (except that aCamera2dwithHdrnow runs the tonemapping pass, which 0.19 did not).Testing
The warning fires once per camera for bad combinations.
This PR was built by me with the assistance of Claude Code w/ Fable 5.1