Skip to content

Make Tonemapping::None a passthrough and add Tonemapping::Linear - #25499

Open
stuartparmenter wants to merge 2 commits into
bevyengine:mainfrom
stuartparmenter:hdr-wave2-tonemapping-linear
Open

Make Tonemapping::None a passthrough and add Tonemapping::Linear#25499
stuartparmenter wants to merge 2 commits into
bevyengine:mainfrom
stuartparmenter:hdr-wave2-tonemapping-linear

Conversation

@stuartparmenter

Copy link
Copy Markdown
Contributor

Objective

Tonemapping::None doesn't actually turn everything off. The SDR in-shader path still applies color grading and dither, and clamps negative color channels to zero. The HDR work needs a real off switch, for example to send exact calibration patterns to the display unmodified.

Solution

Tonemapping::None becomes a full passthrough. Cameras that used it with ColorGrading or DebandDither can switch to the new Tonemapping::Linear, which keeps grading, dither, and the clamp under an identity tone curve. Default cameras render unchanged.

Testing

Check, clippy, and tests pass


This PR was built by me with the assistance of Claude Code w/ Fable 5

@stuartparmenter stuartparmenter added C-Feature A new feature, making something new possible A-Rendering Drawing game state to the screen D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Aug 21, 2026
@github-project-automation github-project-automation Bot moved this to Needs SME Triage in Rendering Aug 21, 2026
@stuartparmenter
stuartparmenter force-pushed the hdr-wave2-tonemapping-linear branch from 14b8ca9 to 84fa137 Compare August 21, 2026 02:00
Comment thread crates/bevy_pbr/src/meshlet/material_pipeline_prepare.rs Outdated

@JMS55 JMS55 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. What code makes Tonemapping:Linear apply color grading/exposure/etc, but not Tonemapping::None?
  2. Not sure how I feel about this PR overall. I get why you need it, but Tonemapping::None not doing color grading and really/especially exposure feels wrong.


match key.tonemapping {
Tonemapping::None => shader_defs.push("TONEMAP_METHOD_NONE".into()),
Tonemapping::None | Tonemapping::Linear => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why linear => None?

@stuartparmenter stuartparmenter Aug 21, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The shader def only picks the tone curve, and TONEMAP_METHOD_NONE means the identity curve. That's exactly what Linear is, so Linear maps to it. None still hits this match because the pipeline is prepared for every view, but the node never dispatches it (so that arm is dead in practice). I can rename the def to TONEMAP_METHOD_LINEAR if that reads better?

@stuartparmenter

stuartparmenter commented Aug 21, 2026

Copy link
Copy Markdown
Contributor Author
  1. What code makes Tonemapping:Linear apply color grading/exposure/etc, but not Tonemapping::None?
  2. Not sure how I feel about this PR overall. I get why you need it, but Tonemapping::None not doing color grading and really/especially exposure feels wrong.
  1. Grading and exposure run inside tone_mapping() here:

    fn tone_mapping(in: vec4<f32>, in_color_grading: ColorGrading) -> vec4<f32> {
    var color = max(in.rgb, vec3(0.0));
    var color_grading = in_color_grading; // So we can take pointers to it.
    // Rotate hue if needed, by converting to and from HSV. Remember that hue is
    // an angle, so it needs to be modulo 2π.
    @if(HUE_ROTATE) {
    var hsv = rgb_to_hsv(color);
    hsv.r = (hsv.r + color_grading.hue) % PI_2;
    color = hsv_to_rgb(hsv);
    }
    // Perform white balance correction. Conveniently, this is a linear
    // transform. The matrix was pre-calculated from the temperature and tint
    // values on the CPU.
    @if(WHITE_BALANCE) {
    color = max(color_grading.balance * color, vec3(0.0));
    }
    // Perform the "sectional" color grading: i.e. the color grading that
    // applies individually to shadows, midtones, and highlights.
    @if(SECTIONAL_COLOR_GRADING) {
    color = sectional_color_grading(color, &color_grading);
    } @else {
    // If we're not doing sectional color grading, the exposure might still need
    // to be applied, for example when using auto exposure.
    color = color * powsafe(vec3(2.0), color_grading.exposure);
    }
    // tone_mapping
    @if(TONEMAP_METHOD_NONE) {
    color = color;
    } @elif(TONEMAP_METHOD_REINHARD) {
    color = tonemapping_reinhard(color.rgb);
    } @elif(TONEMAP_METHOD_REINHARD_LUMINANCE) {
    color = tonemapping_reinhard_luminance(color.rgb);
    } @elif(TONEMAP_METHOD_ACES_FITTED) {
    color = ACESFitted(color.rgb);
    } @elif(TONEMAP_METHOD_AGX) {
    color = applyAgXLog(color);
    color = applyLUT3D(color, 32.0);
    } @elif(TONEMAP_METHOD_SOMEWHAT_BORING_DISPLAY_TRANSFORM) {
    color = somewhat_boring_display_transform(color.rgb);
    } @elif(TONEMAP_METHOD_TONY_MC_MAPFACE) {
    color = sample_tony_mc_mapface_lut(color);
    } @elif(TONEMAP_METHOD_BLENDER_FILMIC) {
    color = sample_blender_filmic_lut(color.rgb);
    } @elif(TONEMAP_METHOD_PBR_NEUTRAL) {
    color = tonemapping_pbr_neutral(color.rgb);
    }
    // Perceptual post tonemapping grading
    color = saturation(color, color_grading.post_saturation);
    return vec4(color, in.a);
    }

    Linear still enters that function and selects the identity curve, so the grading around it runs. None no longer sets TONEMAP_IN_SHADER, so the shader never calls the function at all.

  2. None is already a full passthrough on Hdr cameras:

    if *tonemapping == Tonemapping::None {
    return;
    }
    if !camera.hdr {
    return;
    }
    , so Sdr and Hdr cameras already behave differently -- this helps unify them (for both None and Linear)

The camera's Exposure component is applied in the lighting pass and isn't changed. AutoExposure requires Hdr, where None already bypassed it. The only real change here is an SDR camera that pairs None with a custom grading exposure, and switching to Linear solves that.

@stuartparmenter
stuartparmenter requested a review from JMS55 August 22, 2026 01:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Rendering Drawing game state to the screen C-Feature A new feature, making something new possible D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward

Projects

Status: Needs SME Triage

Development

Successfully merging this pull request may close these issues.

3 participants