Skip to content
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 25 additions & 25 deletions crates/bevy_core_pipeline/src/tonemapping/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@ use bevy_render::{
};
use bevy_shader::{load_shader_library, Shader, ShaderDefVal};
use bitflags::bitflags;
#[cfg(not(feature = "tonemapping_luts"))]
use tracing::error;

mod node;

Expand Down Expand Up @@ -111,13 +109,25 @@ pub struct TonemappingPipeline {
}

/// Optionally enables a tonemapping shader that attempts to map linear input stimulus into a perceptually uniform image for a given [`Camera`] entity.
///
#[cfg_attr(
feature = "tonemapping_luts",
doc = "The default tonemapping is [`TonyMcMapface`](Tonemapping::TonyMcMapface)."
)]
#[cfg_attr(
not(feature = "tonemapping_luts"),
doc = "The default tonemapping is [`None`](Tonemapping::None)."
)]
#[derive(
Component, Debug, Hash, Clone, Copy, Reflect, Default, ExtractComponent, PartialEq, Eq,
)]
#[extract_component_filter(With<Camera>)]
#[reflect(Component, Debug, Hash, Default, PartialEq)]
pub enum Tonemapping {
/// Bypass tonemapping.
///
/// Default when `tonemapping_luts` is disabled.
#[cfg_attr(not(feature = "tonemapping_luts"), default)]
None,
/// Suffers from lots hue shifting, brights don't desaturate naturally.
/// Bright primaries and secondaries don't desaturate at all.
Expand All @@ -134,7 +144,7 @@ pub enum Tonemapping {
/// <https://github.com/sobotka/AgX>
/// Very neutral. Image is somewhat desaturated when compared to other tonemappers.
/// Little to no hue shifting. Subtle [Abney shifting](https://en.wikipedia.org/wiki/Abney_effect).
/// NOTE: Requires the `tonemapping_luts` cargo feature.
#[cfg(feature = "tonemapping_luts")]
AgX,
/// By Tomasz Stachowiak
/// Has little hue shifting in the darks and mids, but lots in the brights. Brights desaturate across the spectrum.
Expand All @@ -153,12 +163,14 @@ pub enum Tonemapping {
/// Brightness-equivalent luminance of the input stimulus is compressed. The non-linearity resembles Reinhard.
/// Color hues are preserved during compression, except for a deliberate [Bezold–Brücke shift](https://en.wikipedia.org/wiki/Bezold%E2%80%93Br%C3%BCcke_shift).
/// To avoid posterization, selective desaturation is employed, with care to avoid the [Abney effect](https://en.wikipedia.org/wiki/Abney_effect).
/// NOTE: Requires the `tonemapping_luts` cargo feature.
#[default]
///
/// Default when `tonemapping_luts` is enabled.
#[cfg_attr(feature = "tonemapping_luts", default)]
#[cfg(feature = "tonemapping_luts")]
TonyMcMapface,
/// Default Filmic Display Transform from blender.
/// Somewhat neutral. Suffers from hue shifting. Brights desaturate across the spectrum.
/// NOTE: Requires the `tonemapping_luts` cargo feature.
#[cfg(feature = "tonemapping_luts")]
BlenderFilmic,
}

Expand Down Expand Up @@ -234,34 +246,19 @@ impl SpecializedRenderPipeline for TonemappingPipeline {
shader_defs.push("TONEMAP_METHOD_REINHARD_LUMINANCE".into());
}
Tonemapping::AcesFitted => shader_defs.push("TONEMAP_METHOD_ACES_FITTED".into()),
#[cfg(feature = "tonemapping_luts")]
Tonemapping::AgX => {
#[cfg(not(feature = "tonemapping_luts"))]
error!(
"AgX tonemapping requires the `tonemapping_luts` feature.
Either enable the `tonemapping_luts` feature for bevy in `Cargo.toml` (recommended),
or use a different `Tonemapping` method for your `Camera2d`/`Camera3d`."
);
shader_defs.push("TONEMAP_METHOD_AGX".into());
}
Tonemapping::SomewhatBoringDisplayTransform => {
shader_defs.push("TONEMAP_METHOD_SOMEWHAT_BORING_DISPLAY_TRANSFORM".into());
}
#[cfg(feature = "tonemapping_luts")]
Tonemapping::TonyMcMapface => {
#[cfg(not(feature = "tonemapping_luts"))]
error!(
"TonyMcMapFace tonemapping requires the `tonemapping_luts` feature.
Either enable the `tonemapping_luts` feature for bevy in `Cargo.toml` (recommended),
or use a different `Tonemapping` method for your `Camera2d`/`Camera3d`."
);
shader_defs.push("TONEMAP_METHOD_TONY_MC_MAPFACE".into());
}
#[cfg(feature = "tonemapping_luts")]
Tonemapping::BlenderFilmic => {
#[cfg(not(feature = "tonemapping_luts"))]
error!(
"BlenderFilmic tonemapping requires the `tonemapping_luts` feature.
Either enable the `tonemapping_luts` feature for bevy in `Cargo.toml` (recommended),
or use a different `Tonemapping` method for your `Camera2d`/`Camera3d`."
);
shader_defs.push("TONEMAP_METHOD_BLENDER_FILMIC".into());
}
}
Expand Down Expand Up @@ -389,9 +386,12 @@ pub fn get_lut_bindings<'a>(
| Tonemapping::Reinhard
| Tonemapping::ReinhardLuminance
| Tonemapping::AcesFitted
| Tonemapping::AgX
| Tonemapping::SomewhatBoringDisplayTransform => &tonemapping_luts.agx,
#[cfg(feature = "tonemapping_luts")]
Tonemapping::AgX => &tonemapping_luts.agx,
#[cfg(feature = "tonemapping_luts")]
Tonemapping::TonyMcMapface => &tonemapping_luts.tony_mc_mapface,
#[cfg(feature = "tonemapping_luts")]
Tonemapping::BlenderFilmic => &tonemapping_luts.blender_filmic,
};
let lut_image = images.get(image).unwrap_or(&fallback_image.d3);
Expand Down
1 change: 1 addition & 0 deletions crates/bevy_internal/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ raw_vulkan_init = ["bevy_render/raw_vulkan_init"]
# Include tonemapping LUT KTX2 files.
tonemapping_luts = [
"bevy_core_pipeline?/tonemapping_luts",
"bevy_pbr?/tonemapping_luts",
"ktx2",
"bevy_image/zstd",
]
Expand Down
1 change: 1 addition & 0 deletions crates/bevy_pbr/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ meshlet_processor = [
"dep:itertools",
"dep:bitvec",
]
tonemapping_luts = ["bevy_core_pipeline/tonemapping_luts"]

[dependencies]
# bevy
Expand Down
3 changes: 3 additions & 0 deletions crates/bevy_pbr/src/deferred/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -503,11 +503,14 @@ pub fn prepare_deferred_lighting_pipelines(
MeshPipelineKey::TONEMAP_METHOD_REINHARD_LUMINANCE
}
Tonemapping::AcesFitted => MeshPipelineKey::TONEMAP_METHOD_ACES_FITTED,
#[cfg(feature = "tonemapping_luts")]
Tonemapping::AgX => MeshPipelineKey::TONEMAP_METHOD_AGX,
Tonemapping::SomewhatBoringDisplayTransform => {
MeshPipelineKey::TONEMAP_METHOD_SOMEWHAT_BORING_DISPLAY_TRANSFORM
}
#[cfg(feature = "tonemapping_luts")]
Tonemapping::TonyMcMapface => MeshPipelineKey::TONEMAP_METHOD_TONY_MC_MAPFACE,
#[cfg(feature = "tonemapping_luts")]
Tonemapping::BlenderFilmic => MeshPipelineKey::TONEMAP_METHOD_BLENDER_FILMIC,
};
}
Expand Down
3 changes: 3 additions & 0 deletions crates/bevy_pbr/src/material.rs
Original file line number Diff line number Diff line change
Expand Up @@ -632,11 +632,14 @@ pub const fn tonemapping_pipeline_key(tonemapping: Tonemapping) -> MeshPipelineK
Tonemapping::Reinhard => MeshPipelineKey::TONEMAP_METHOD_REINHARD,
Tonemapping::ReinhardLuminance => MeshPipelineKey::TONEMAP_METHOD_REINHARD_LUMINANCE,
Tonemapping::AcesFitted => MeshPipelineKey::TONEMAP_METHOD_ACES_FITTED,
#[cfg(feature = "tonemapping_luts")]
Tonemapping::AgX => MeshPipelineKey::TONEMAP_METHOD_AGX,
Tonemapping::SomewhatBoringDisplayTransform => {
MeshPipelineKey::TONEMAP_METHOD_SOMEWHAT_BORING_DISPLAY_TRANSFORM
}
#[cfg(feature = "tonemapping_luts")]
Tonemapping::TonyMcMapface => MeshPipelineKey::TONEMAP_METHOD_TONY_MC_MAPFACE,
#[cfg(feature = "tonemapping_luts")]
Tonemapping::BlenderFilmic => MeshPipelineKey::TONEMAP_METHOD_BLENDER_FILMIC,
}
}
Expand Down
3 changes: 3 additions & 0 deletions crates/bevy_pbr/src/render/mesh.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2107,9 +2107,12 @@ bitflags::bitflags! {
const TONEMAP_METHOD_REINHARD = 1 << Self::TONEMAP_METHOD_SHIFT_BITS;
const TONEMAP_METHOD_REINHARD_LUMINANCE = 2 << Self::TONEMAP_METHOD_SHIFT_BITS;
const TONEMAP_METHOD_ACES_FITTED = 3 << Self::TONEMAP_METHOD_SHIFT_BITS;
#[cfg(feature = "tonemapping_luts")]
const TONEMAP_METHOD_AGX = 4 << Self::TONEMAP_METHOD_SHIFT_BITS;
const TONEMAP_METHOD_SOMEWHAT_BORING_DISPLAY_TRANSFORM = 5 << Self::TONEMAP_METHOD_SHIFT_BITS;
#[cfg(feature = "tonemapping_luts")]
const TONEMAP_METHOD_TONY_MC_MAPFACE = 6 << Self::TONEMAP_METHOD_SHIFT_BITS;
#[cfg(feature = "tonemapping_luts")]
const TONEMAP_METHOD_BLENDER_FILMIC = 7 << Self::TONEMAP_METHOD_SHIFT_BITS;
Comment on lines 2109 to 2116
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should this MeshPipelineKeys be gated?

@DGriffin91

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i am moving forwards with the asumption that yes

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not really sure of all the implications. Who/what all might depend on them existing. I would guess if they have the feature disabled they're being conscious about things like this missing. Not sure how it would affect plugins that expect them to be there or something. Might be better to resolve at runtime.

Copy link
Contributor

Choose a reason for hiding this comment

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

It would at least be nice to use #[doc(cfg)] to make the fact that these are gated behind a feature apparent on docs.rs

const SHADOW_FILTER_METHOD_RESERVED_BITS = Self::SHADOW_FILTER_METHOD_MASK_BITS << Self::SHADOW_FILTER_METHOD_SHIFT_BITS;
const SHADOW_FILTER_METHOD_HARDWARE_2X2 = 0 << Self::SHADOW_FILTER_METHOD_SHIFT_BITS;
Expand Down
7 changes: 7 additions & 0 deletions release-content/migration-guides/gated-tonemapping.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
title: `Tonemapping` modes `TonyMcMapface`, `BlenderFilmic`, and `AgX` are now gated behind `tonemapping_luts`
pull_requests: [20924]
---

`Tonemapping` mode `TonyMcMapface`, `BlenderFilmic`, and `AgX` are now only present with the `tonemapping_luts`
instead of having a notice on the documentation and logging an error during runtime.
8 changes: 8 additions & 0 deletions release-content/migration-guides/tonemapping-defaults.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
title: Different defaults for `Tonemapping` based on `tonemapping_luts` feature
pull_requests: [20924]
---

`Tonemapping` component now has a different defaults based on `tonemappint_luts` feature.
When `tonemapping_luts` is present the default remains `TonyMcMapface`, but when it is off
the default is now `None`.
Loading