Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Merged by Bors] - Fix color banding by dithering image before quantization #5264

Closed
wants to merge 7 commits into from
Closed
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
2 changes: 1 addition & 1 deletion crates/bevy_core_pipeline/src/core_2d/camera_2d.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ impl Camera2dBundle {
global_transform: Default::default(),
camera: Camera::default(),
camera_2d: Camera2d::default(),
tonemapping: Tonemapping { is_enabled: false },
tonemapping: Tonemapping::Disabled,
}
}
}
4 changes: 3 additions & 1 deletion crates/bevy_core_pipeline/src/core_3d/camera_3d.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,9 @@ impl Default for Camera3dBundle {
fn default() -> Self {
Self {
camera_render_graph: CameraRenderGraph::new(crate::core_3d::graph::NAME),
tonemapping: Tonemapping { is_enabled: true },
tonemapping: Tonemapping::Enabled {
is_deband_dither_enabled: true,
},
camera: Default::default(),
projection: Default::default(),
visible_entities: Default::default(),
Expand Down
14 changes: 12 additions & 2 deletions crates/bevy_core_pipeline/src/tonemapping/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,18 @@ impl FromWorld for TonemappingPipeline {

#[derive(Component, Clone, Reflect, Default)]
#[reflect(Component)]
pub struct Tonemapping {
pub is_enabled: bool,
pub enum Tonemapping {
#[default]
Disabled,
Enabled {
is_deband_dither_enabled: bool,
},
}

impl Tonemapping {
pub fn is_enabled(&self) -> bool {
matches!(self, Tonemapping::Enabled { .. })
}
}

impl ExtractComponent for Tonemapping {
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_core_pipeline/src/tonemapping/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ impl Node for TonemappingNode {
Err(_) => return Ok(()),
};

let tonemapping_enabled = tonemapping.map_or(false, |t| t.is_enabled);
let tonemapping_enabled = tonemapping.map_or(false, |t| t.is_enabled());
if !tonemapping_enabled || !target.is_hdr() {
return Ok(());
}
Expand Down
12 changes: 11 additions & 1 deletion crates/bevy_core_pipeline/src/tonemapping/tonemapping.wgsl
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,15 @@ var hdr_sampler: sampler;
fn fragment(in: FullscreenVertexOutput) -> @location(0) vec4<f32> {
let hdr_color = textureSample(hdr_texture, hdr_sampler, in.uv);

return vec4<f32>(reinhard_luminance(hdr_color.rgb), hdr_color.a);
var output_rgb = vec3<f32>(reinhard_luminance(hdr_color.rgb));
aevyrie marked this conversation as resolved.
Show resolved Hide resolved

#ifdef DEBAND_DITHER
output_rgb = pow(output_rgb.rgb, vec3<f32>(1.0 / 2.2));
output_rgb = output_rgb + screen_space_dither(in.position.xy);
// This conversion back to linear space is required because our output texture format is
// SRGB; the GPU will assume our output is linear and will apply an SRGB conversion.
output_rgb = pow(output_rgb.rgb, vec3<f32>(2.2));
#endif

return vec4<f32>(output_rgb, hdr_color.a);
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,11 @@ fn reinhard_luminance(color: vec3<f32>) -> vec3<f32> {
let l_new = l_old / (1.0 + l_old);
return tonemapping_change_luminance(color, l_new);
}

// Source: Advanced VR Rendering, GDC 2015, Alex Vlachos, Valve, Slide 49
// https://media.steampowered.com/apps/valve/2015/Alex_Vlachos_Advanced_VR_Rendering_GDC2015.pdf
fn screen_space_dither(frag_coord: vec2<f32>) -> vec3<f32> {
var dither = vec3<f32>(dot(vec2<f32>(171.0, 231.0), frag_coord)).xxx;
dither = fract(dither.rgb / vec3<f32>(103.0, 71.0, 97.0));
return (dither - 0.5) / 255.0;
}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
}
}

11 changes: 9 additions & 2 deletions crates/bevy_pbr/src/material.rs
Original file line number Diff line number Diff line change
Expand Up @@ -363,9 +363,16 @@ pub fn queue_material_meshes<M: Material>(
let mut view_key =
MeshPipelineKey::from_msaa_samples(msaa.samples) | MeshPipelineKey::from_hdr(view.hdr);

if let Some(tonemapping) = tonemapping {
if tonemapping.is_enabled && !view.hdr {
if let Some(Tonemapping::Enabled {
is_deband_dither_enabled,
}) = tonemapping
{
if !view.hdr {
view_key |= MeshPipelineKey::TONEMAP_IN_SHADER;

if *is_deband_dither_enabled {
view_key |= MeshPipelineKey::DEBAND_DITHER;
}
}
}
let rangefinder = view.rangefinder3d();
Expand Down
6 changes: 6 additions & 0 deletions crates/bevy_pbr/src/render/mesh.rs
Original file line number Diff line number Diff line change
Expand Up @@ -518,6 +518,7 @@ bitflags::bitflags! {
const TRANSPARENT_MAIN_PASS = (1 << 0);
const HDR = (1 << 1);
const TONEMAP_IN_SHADER = (1 << 2);
const DEBAND_DITHER = (1 << 3);
const MSAA_RESERVED_BITS = Self::MSAA_MASK_BITS << Self::MSAA_SHIFT_BITS;
const PRIMITIVE_TOPOLOGY_RESERVED_BITS = Self::PRIMITIVE_TOPOLOGY_MASK_BITS << Self::PRIMITIVE_TOPOLOGY_SHIFT_BITS;
}
Expand Down Expand Up @@ -636,6 +637,11 @@ impl SpecializedMeshPipeline for MeshPipeline {

if key.contains(MeshPipelineKey::TONEMAP_IN_SHADER) {
shader_defs.push("TONEMAP_IN_SHADER".to_string());

// Debanding is tied to tonemapping in the shader, cannot run without it.
if key.contains(MeshPipelineKey::DEBAND_DITHER) {
shader_defs.push("DEBAND_DITHER".to_string());
}
}

let format = match key.contains(MeshPipelineKey::HDR) {
Expand Down
3 changes: 3 additions & 0 deletions crates/bevy_pbr/src/render/pbr.wgsl
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,9 @@ fn fragment(in: FragmentInput) -> @location(0) vec4<f32> {

#ifdef TONEMAP_IN_SHADER
output_color = tone_mapping(output_color);
#endif
#ifdef DEBAND_DITHER
output_color = dither(output_color, in.frag_coord.xy);
#endif
return output_color;
}
6 changes: 6 additions & 0 deletions crates/bevy_pbr/src/render/pbr_functions.wgsl
Original file line number Diff line number Diff line change
Expand Up @@ -262,3 +262,9 @@ fn tone_mapping(in: vec4<f32>) -> vec4<f32> {
}
#endif

#ifdef DEBAND_DITHER
fn dither(color: vec4<f32>, pos: vec2<f32>) -> vec4<f32> {
return vec4<f32>(color.rgb + screen_space_dither(pos.xy), color.a);
}
#endif

11 changes: 9 additions & 2 deletions crates/bevy_sprite/src/mesh2d/material.rs
Original file line number Diff line number Diff line change
Expand Up @@ -328,9 +328,16 @@ pub fn queue_material2d_meshes<M: Material2d>(
let mut view_key = Mesh2dPipelineKey::from_msaa_samples(msaa.samples)
| Mesh2dPipelineKey::from_hdr(view.hdr);

if let Some(tonemapping) = tonemapping {
if tonemapping.is_enabled && !view.hdr {
if let Some(Tonemapping::Enabled {
is_deband_dither_enabled,
}) = tonemapping
{
if !view.hdr {
view_key |= Mesh2dPipelineKey::TONEMAP_IN_SHADER;

if *is_deband_dither_enabled {
view_key |= Mesh2dPipelineKey::DEBAND_DITHER;
}
}
}

Expand Down
6 changes: 6 additions & 0 deletions crates/bevy_sprite/src/mesh2d/mesh.rs
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,7 @@ bitflags::bitflags! {
const NONE = 0;
const HDR = (1 << 0);
const TONEMAP_IN_SHADER = (1 << 1);
const DEBAND_DITHER = (1 << 2);
const MSAA_RESERVED_BITS = Self::MSAA_MASK_BITS << Self::MSAA_SHIFT_BITS;
const PRIMITIVE_TOPOLOGY_RESERVED_BITS = Self::PRIMITIVE_TOPOLOGY_MASK_BITS << Self::PRIMITIVE_TOPOLOGY_SHIFT_BITS;
}
Expand Down Expand Up @@ -376,6 +377,11 @@ impl SpecializedMeshPipeline for Mesh2dPipeline {

if key.contains(Mesh2dPipelineKey::TONEMAP_IN_SHADER) {
shader_defs.push("TONEMAP_IN_SHADER".to_string());

// Debanding is tied to tonemapping in the shader, cannot run without it.
if key.contains(Mesh2dPipelineKey::DEBAND_DITHER) {
shader_defs.push("DEBAND_DITHER".to_string());
}
}

let vertex_buffer_layout = layout.get_layout(&vertex_attributes)?;
Expand Down
17 changes: 15 additions & 2 deletions crates/bevy_sprite/src/render/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ bitflags::bitflags! {
const COLORED = (1 << 0);
const HDR = (1 << 1);
const TONEMAP_IN_SHADER = (1 << 2);
const DEBAND_DITHER = (1 << 3);
const MSAA_RESERVED_BITS = Self::MSAA_MASK_BITS << Self::MSAA_SHIFT_BITS;
}
}
Expand Down Expand Up @@ -212,6 +213,11 @@ impl SpecializedRenderPipeline for SpritePipeline {

if key.contains(SpritePipelineKey::TONEMAP_IN_SHADER) {
shader_defs.push("TONEMAP_IN_SHADER".to_string());

// Debanding is tied to tonemapping in the shader, cannot run without it.
if key.contains(SpritePipelineKey::DEBAND_DITHER) {
shader_defs.push("DEBAND_DITHER".to_string());
}
}

let format = match key.contains(SpritePipelineKey::HDR) {
Expand Down Expand Up @@ -508,9 +514,16 @@ pub fn queue_sprites(

for (mut transparent_phase, visible_entities, view, tonemapping) in &mut views {
let mut view_key = SpritePipelineKey::from_hdr(view.hdr) | msaa_key;
if let Some(tonemapping) = tonemapping {
if tonemapping.is_enabled && !view.hdr {
if let Some(Tonemapping::Enabled {
is_deband_dither_enabled,
}) = tonemapping
{
if !view.hdr {
view_key |= SpritePipelineKey::TONEMAP_IN_SHADER;

if *is_deband_dither_enabled {
view_key |= SpritePipelineKey::DEBAND_DITHER;
}
}
}
let pipeline = pipelines.specialize(
Expand Down