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 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/bevy_core_pipeline/src/core_2d/camera_2d.rs
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
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 {
deband_dither: true,
},
camera: Default::default(),
projection: Default::default(),
visible_entities: Default::default(),
Expand Down
101 changes: 75 additions & 26 deletions crates/bevy_core_pipeline/src/tonemapping/mod.rs
Expand Up @@ -12,7 +12,7 @@ use bevy_render::camera::Camera;
use bevy_render::extract_component::{ExtractComponent, ExtractComponentPlugin};
use bevy_render::renderer::RenderDevice;
use bevy_render::view::ViewTarget;
use bevy_render::{render_resource::*, RenderApp};
use bevy_render::{render_resource::*, RenderApp, RenderStage};

const TONEMAPPING_SHADER_HANDLE: HandleUntyped =
HandleUntyped::weak_from_u64(Shader::TYPE_UUID, 17015368199668024512);
Expand Down Expand Up @@ -42,15 +42,51 @@ impl Plugin for TonemappingPlugin {
app.add_plugin(ExtractComponentPlugin::<Tonemapping>::default());

if let Ok(render_app) = app.get_sub_app_mut(RenderApp) {
render_app.init_resource::<TonemappingPipeline>();
render_app
.init_resource::<TonemappingPipeline>()
.init_resource::<SpecializedRenderPipelines<TonemappingPipeline>>()
.add_system_to_stage(RenderStage::Queue, queue_view_tonemapping_pipelines);
}
}
}

#[derive(Resource)]
pub struct TonemappingPipeline {
texture_bind_group: BindGroupLayout,
tonemapping_pipeline_id: CachedRenderPipelineId,
}

#[derive(Copy, Clone, PartialEq, Eq, Hash)]
pub struct TonemappingPipelineKey {
deband_dither: bool,
}

impl SpecializedRenderPipeline for TonemappingPipeline {
type Key = TonemappingPipelineKey;

fn specialize(&self, key: Self::Key) -> RenderPipelineDescriptor {
let mut shader_defs = Vec::new();
if key.deband_dither {
shader_defs.push("DEBAND_DITHER".to_string());
}
RenderPipelineDescriptor {
label: Some("tonemapping pipeline".into()),
layout: Some(vec![self.texture_bind_group.clone()]),
vertex: fullscreen_shader_vertex_state(),
fragment: Some(FragmentState {
shader: TONEMAPPING_SHADER_HANDLE.typed(),
shader_defs,
entry_point: "fragment".into(),
targets: vec![Some(ColorTargetState {
format: ViewTarget::TEXTURE_FORMAT_HDR,
blend: None,
write_mask: ColorWrites::ALL,
})],
}),
primitive: PrimitiveState::default(),
depth_stencil: None,
multisample: MultisampleState::default(),
}
}
}

impl FromWorld for TonemappingPipeline {
Expand Down Expand Up @@ -79,37 +115,50 @@ impl FromWorld for TonemappingPipeline {
],
});

let tonemap_descriptor = RenderPipelineDescriptor {
label: Some("tonemapping pipeline".into()),
layout: Some(vec![tonemap_texture_bind_group.clone()]),
vertex: fullscreen_shader_vertex_state(),
fragment: Some(FragmentState {
shader: TONEMAPPING_SHADER_HANDLE.typed(),
shader_defs: vec![],
entry_point: "fragment".into(),
targets: vec![Some(ColorTargetState {
format: ViewTarget::TEXTURE_FORMAT_HDR,
blend: None,
write_mask: ColorWrites::ALL,
})],
}),
primitive: PrimitiveState::default(),
depth_stencil: None,
multisample: MultisampleState::default(),
};

let mut cache = render_world.resource_mut::<PipelineCache>();
TonemappingPipeline {
texture_bind_group: tonemap_texture_bind_group,
tonemapping_pipeline_id: cache.queue_render_pipeline(tonemap_descriptor),
}
}
}

#[derive(Component)]
pub struct ViewTonemappingPipeline(CachedRenderPipelineId);

pub fn queue_view_tonemapping_pipelines(
mut commands: Commands,
mut pipeline_cache: ResMut<PipelineCache>,
mut pipelines: ResMut<SpecializedRenderPipelines<TonemappingPipeline>>,
upscaling_pipeline: Res<TonemappingPipeline>,
view_targets: Query<(Entity, &Tonemapping)>,
) {
for (entity, tonemapping) in view_targets.iter() {
if let Tonemapping::Enabled { deband_dither } = tonemapping {
let key = TonemappingPipelineKey {
deband_dither: *deband_dither,
};
let pipeline = pipelines.specialize(&mut pipeline_cache, &upscaling_pipeline, key);

commands
.entity(entity)
.insert(ViewTonemappingPipeline(pipeline));
}
}
}

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

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

impl ExtractComponent for Tonemapping {
Expand Down
11 changes: 4 additions & 7 deletions crates/bevy_core_pipeline/src/tonemapping/node.rs
@@ -1,6 +1,6 @@
use std::sync::Mutex;

use crate::tonemapping::{Tonemapping, TonemappingPipeline};
use crate::tonemapping::{TonemappingPipeline, ViewTonemappingPipeline};
use bevy_ecs::prelude::*;
use bevy_ecs::query::QueryState;
use bevy_render::{
Expand All @@ -15,7 +15,7 @@ use bevy_render::{
};

pub struct TonemappingNode {
query: QueryState<(&'static ViewTarget, Option<&'static Tonemapping>), With<ExtractedView>>,
query: QueryState<(&'static ViewTarget, &'static ViewTonemappingPipeline), With<ExtractedView>>,
cached_texture_bind_group: Mutex<Option<(TextureViewId, BindGroup)>>,
}

Expand Down Expand Up @@ -54,14 +54,11 @@ impl Node for TonemappingNode {
Err(_) => return Ok(()),
};

let tonemapping_enabled = tonemapping.map_or(false, |t| t.is_enabled);
if !tonemapping_enabled || !target.is_hdr() {
if !target.is_hdr() {
return Ok(());
}

let pipeline = match pipeline_cache
.get_render_pipeline(tonemapping_pipeline.tonemapping_pipeline_id)
{
let pipeline = match pipeline_cache.get_render_pipeline(tonemapping.0) {
Some(pipeline) => pipeline,
None => return Ok(()),
};
Expand Down
12 changes: 11 additions & 1 deletion crates/bevy_core_pipeline/src/tonemapping/tonemapping.wgsl
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);
}
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
}
}

12 changes: 6 additions & 6 deletions crates/bevy_core_pipeline/src/upscaling/mod.rs
Expand Up @@ -31,7 +31,7 @@ impl Plugin for UpscalingPlugin {
render_app
.init_resource::<UpscalingPipeline>()
.init_resource::<SpecializedRenderPipelines<UpscalingPipeline>>()
.add_system_to_stage(RenderStage::Queue, queue_upscaling_bind_groups);
.add_system_to_stage(RenderStage::Queue, queue_view_upscaling_pipelines);
}
}
}
Expand Down Expand Up @@ -110,11 +110,9 @@ impl SpecializedRenderPipeline for UpscalingPipeline {
}

#[derive(Component)]
pub struct UpscalingTarget {
pub pipeline: CachedRenderPipelineId,
}
pub struct ViewUpscalingPipeline(CachedRenderPipelineId);

fn queue_upscaling_bind_groups(
fn queue_view_upscaling_pipelines(
mut commands: Commands,
mut pipeline_cache: ResMut<PipelineCache>,
mut pipelines: ResMut<SpecializedRenderPipelines<UpscalingPipeline>>,
Expand All @@ -128,6 +126,8 @@ fn queue_upscaling_bind_groups(
};
let pipeline = pipelines.specialize(&mut pipeline_cache, &upscaling_pipeline, key);

commands.entity(entity).insert(UpscalingTarget { pipeline });
commands
.entity(entity)
.insert(ViewUpscalingPipeline(pipeline));
}
}
6 changes: 3 additions & 3 deletions crates/bevy_core_pipeline/src/upscaling/node.rs
Expand Up @@ -13,10 +13,10 @@ use bevy_render::{
view::{ExtractedView, ViewTarget},
};

use super::{UpscalingPipeline, UpscalingTarget};
use super::{UpscalingPipeline, ViewUpscalingPipeline};

pub struct UpscalingNode {
query: QueryState<(&'static ViewTarget, &'static UpscalingTarget), With<ExtractedView>>,
query: QueryState<(&'static ViewTarget, &'static ViewUpscalingPipeline), With<ExtractedView>>,
cached_texture_bind_group: Mutex<Option<(TextureViewId, BindGroup)>>,
}

Expand Down Expand Up @@ -89,7 +89,7 @@ impl Node for UpscalingNode {
}
};

let pipeline = match pipeline_cache.get_render_pipeline(upscaling_target.pipeline) {
let pipeline = match pipeline_cache.get_render_pipeline(upscaling_target.0) {
Some(pipeline) => pipeline,
None => return Ok(()),
};
Expand Down
8 changes: 6 additions & 2 deletions crates/bevy_pbr/src/material.rs
Expand Up @@ -363,9 +363,13 @@ 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 { deband_dither }) = tonemapping {
if !view.hdr {
view_key |= MeshPipelineKey::TONEMAP_IN_SHADER;

if *deband_dither {
view_key |= MeshPipelineKey::DEBAND_DITHER;
}
}
}
let rangefinder = view.rangefinder3d();
Expand Down
6 changes: 6 additions & 0 deletions crates/bevy_pbr/src/render/mesh.rs
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
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
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

8 changes: 6 additions & 2 deletions crates/bevy_sprite/src/mesh2d/material.rs
Expand Up @@ -328,9 +328,13 @@ 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 { deband_dither }) = tonemapping {
if !view.hdr {
view_key |= Mesh2dPipelineKey::TONEMAP_IN_SHADER;

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

Expand Down
6 changes: 6 additions & 0 deletions crates/bevy_sprite/src/mesh2d/mesh.rs
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