From 02a875721d4ae3dad05616ee21e865bcbe96aacc Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Mon, 16 Dec 2024 21:17:34 +0100 Subject: [PATCH] Fix 2D/3D view artifacts on view's border when using fractional zoom (#8369) ### Related * Fixes https://github.com/rerun-io/rerun/issues/8330 ### What Wasn't able to repro the issue in the first place though - tried fractional zoom and a contrast heavy background to no avail * [x] confirm this fixes it --------- Co-authored-by: Antoine Beyeler --- .../viewer/re_renderer/shader/composite.wgsl | 4 +-- .../re_renderer/shader/copy_texture.wgsl | 2 +- .../re_renderer/shader/debug_overlay.wgsl | 2 +- .../re_renderer/shader/global_bindings.wgsl | 7 ++-- .../re_renderer/shader/instanced_mesh.wgsl | 2 +- .../shader/outlines/jumpflooding_init.wgsl | 26 ++++++++------ .../outlines/jumpflooding_init_msaa.wgsl | 26 ++++++++------ .../viewer/re_renderer/src/global_bindings.rs | 35 ++++++++++++++----- 8 files changed, 68 insertions(+), 36 deletions(-) diff --git a/crates/viewer/re_renderer/shader/composite.wgsl b/crates/viewer/re_renderer/shader/composite.wgsl index 53b4bd1ec692..270e1549cc72 100644 --- a/crates/viewer/re_renderer/shader/composite.wgsl +++ b/crates/viewer/re_renderer/shader/composite.wgsl @@ -26,7 +26,7 @@ fn main(in: FragmentInput) -> @location(0) vec4f { // Note that we can't use a simple textureLoad using @builtin(position) here despite the lack of filtering. // The issue is that positions provided by @builtin(position) are not dependent on the set viewport, // but are about the location of the texel in the target texture. - var color = textureSample(color_texture, nearest_sampler, in.texcoord); + var color = textureSample(color_texture, nearest_sampler_clamped, in.texcoord); // TODO(andreas): We assume that the color from the texture does *not* have pre-multiplied alpha. @@ -42,7 +42,7 @@ fn main(in: FragmentInput) -> @location(0) vec4f { // Outlines { - let closest_positions = textureSample(outline_voronoi_texture, nearest_sampler, in.texcoord); + let closest_positions = textureSample(outline_voronoi_texture, nearest_sampler_clamped, in.texcoord); let distance_pixel_a = distance(pixel_coordinates, closest_positions.xy); let distance_pixel_b = distance(pixel_coordinates, closest_positions.zw); diff --git a/crates/viewer/re_renderer/shader/copy_texture.wgsl b/crates/viewer/re_renderer/shader/copy_texture.wgsl index 82a41f0959c3..02fa06dbe0ca 100644 --- a/crates/viewer/re_renderer/shader/copy_texture.wgsl +++ b/crates/viewer/re_renderer/shader/copy_texture.wgsl @@ -11,5 +11,5 @@ var tex: texture_2d; @fragment fn main(in: FragmentInput) -> @location(0) vec4f { - return textureSample(tex, nearest_sampler, in.texcoord); + return textureSample(tex, nearest_sampler_clamped, in.texcoord); } diff --git a/crates/viewer/re_renderer/shader/debug_overlay.wgsl b/crates/viewer/re_renderer/shader/debug_overlay.wgsl index 17e09a51ff2a..250938a744b6 100644 --- a/crates/viewer/re_renderer/shader/debug_overlay.wgsl +++ b/crates/viewer/re_renderer/shader/debug_overlay.wgsl @@ -53,7 +53,7 @@ fn main_vs(@builtin(vertex_index) vertex_index: u32) -> VertexOutput { @fragment fn main_fs(in: VertexOutput) -> @location(0) vec4f { if uniforms.mode == ShowFloatTexture { - return vec4f(textureSample(debug_texture_float, nearest_sampler, in.texcoord).rgb, 1.0); + return vec4f(textureSample(debug_texture_float, nearest_sampler_clamped, in.texcoord).rgb, 1.0); } else if uniforms.mode == ShowUintTexture { let coords = vec2i(in.texcoord * vec2f(textureDimensions(debug_texture_uint).xy)); let raw_values = textureLoad(debug_texture_uint, coords, 0); diff --git a/crates/viewer/re_renderer/shader/global_bindings.wgsl b/crates/viewer/re_renderer/shader/global_bindings.wgsl index 5f28525af381..5c896b2f9276 100644 --- a/crates/viewer/re_renderer/shader/global_bindings.wgsl +++ b/crates/viewer/re_renderer/shader/global_bindings.wgsl @@ -31,10 +31,11 @@ struct FrameUniformBuffer { var frame: FrameUniformBuffer; @group(0) @binding(1) -var nearest_sampler: sampler; - +var nearest_sampler_repeat: sampler; @group(0) @binding(2) -var trilinear_sampler: sampler; +var nearest_sampler_clamped: sampler; +@group(0) @binding(3) +var trilinear_sampler_repeat: sampler; // See config.rs#DeviceTier const DEVICE_TIER_GLES = 0u; diff --git a/crates/viewer/re_renderer/shader/instanced_mesh.wgsl b/crates/viewer/re_renderer/shader/instanced_mesh.wgsl index a5638ee3d1f0..1c5d022ffb2d 100644 --- a/crates/viewer/re_renderer/shader/instanced_mesh.wgsl +++ b/crates/viewer/re_renderer/shader/instanced_mesh.wgsl @@ -64,7 +64,7 @@ fn vs_main(in_vertex: VertexIn, in_instance: InstanceIn) -> VertexOut { @fragment fn fs_main_shaded(in: VertexOut) -> @location(0) vec4f { - let texture = linear_from_srgb(textureSample(albedo_texture, trilinear_sampler, in.texcoord).rgb); + let texture = linear_from_srgb(textureSample(albedo_texture, trilinear_sampler_repeat, in.texcoord).rgb); let albedo = texture * in.color.rgb * material.albedo_factor.rgb diff --git a/crates/viewer/re_renderer/shader/outlines/jumpflooding_init.wgsl b/crates/viewer/re_renderer/shader/outlines/jumpflooding_init.wgsl index 9e4698a24ead..d7ec782a208c 100644 --- a/crates/viewer/re_renderer/shader/outlines/jumpflooding_init.wgsl +++ b/crates/viewer/re_renderer/shader/outlines/jumpflooding_init.wgsl @@ -3,8 +3,13 @@ @group(0) @binding(0) var mask_texture: texture_2d; -fn has_edge(closest_center_sample: vec2u, sample_coord: vec2i) -> vec2f { - let mask_neighbor = textureLoad(mask_texture, sample_coord, 0).xy; +fn has_edge(max_coord: vec2i, closest_center_sample: vec2u, sample_coord: vec2i) -> vec2f { + // Note that `textureLoad` calls with out-of-bounds coordinates are allowed to return *any* + // value on the texture or "transparent"/"opaque" zero. + // See https://www.w3.org/TR/WGSL/#textureload + // Therefore, if we want consistent behavior, we have to do the clamp ourselves. + let clamped_coord = clamp(sample_coord, vec2i(0), max_coord); + let mask_neighbor = textureLoad(mask_texture, clamped_coord, 0).xy; return vec2f(closest_center_sample != mask_neighbor); } @@ -15,6 +20,7 @@ fn has_edge(closest_center_sample: vec2u, sample_coord: vec2i) -> vec2f { fn main(in: FragmentInput) -> @location(0) vec4f { let resolution = textureDimensions(mask_texture).xy; let center_coord = vec2i(vec2f(resolution) * in.texcoord); + let max_coord = vec2i(resolution) - vec2i(1); let mask_center = textureLoad(mask_texture, center_coord, 0).xy; @@ -25,25 +31,25 @@ fn main(in: FragmentInput) -> @location(0) vec4f { // Sample closest neighbors top/bottom/left/right { // right - let edge = has_edge(mask_center, center_coord + vec2i(1, 0)); + let edge = has_edge(max_coord, mask_center, center_coord + vec2i(1, 0)); let edge_pos = vec2f(1.0, 0.5); edge_pos_a_and_b += vec4f(edge_pos, edge_pos) * edge.xxyy; num_edges_a_and_b += edge; } { // bottom - let edge = has_edge(mask_center, center_coord + vec2i(0, 1)); + let edge = has_edge(max_coord, mask_center, center_coord + vec2i(0, 1)); let edge_pos = vec2f(0.5, 1.0); edge_pos_a_and_b += vec4f(edge_pos, edge_pos) * edge.xxyy; num_edges_a_and_b += edge; } { // left - let edge = has_edge(mask_center, center_coord + vec2i(-1, 0)); + let edge = has_edge(max_coord, mask_center, center_coord + vec2i(-1, 0)); let edge_pos = vec2f(0.0, 0.5); edge_pos_a_and_b += vec4f(edge_pos, edge_pos) * edge.xxyy; num_edges_a_and_b += edge; } { // top - let edge = has_edge(mask_center, center_coord + vec2i(0, -1)); + let edge = has_edge(max_coord, mask_center, center_coord + vec2i(0, -1)); let edge_pos = vec2f(0.5, 0.0); edge_pos_a_and_b += vec4f(edge_pos, edge_pos) * edge.xxyy; num_edges_a_and_b += edge; @@ -51,25 +57,25 @@ fn main(in: FragmentInput) -> @location(0) vec4f { // Sample closest neighbors diagonally. { // top-right - let edge = has_edge(mask_center, center_coord + vec2i(1, -1)); + let edge = has_edge(max_coord, mask_center, center_coord + vec2i(1, -1)); let edge_pos = vec2f(1.0, 0.0); edge_pos_a_and_b += vec4f(edge_pos, edge_pos) * edge.xxyy; num_edges_a_and_b += edge; } { // bottom-right - let edge = has_edge(mask_center, center_coord + vec2i(1, 1)); + let edge = has_edge(max_coord, mask_center, center_coord + vec2i(1, 1)); let edge_pos = vec2f(1.0, 1.0); edge_pos_a_and_b += vec4f(edge_pos, edge_pos) * edge.xxyy; num_edges_a_and_b += edge; } { // bottom-left - let edge = has_edge(mask_center, center_coord + vec2i(-1, 1)); + let edge = has_edge(max_coord, mask_center, center_coord + vec2i(-1, 1)); let edge_pos = vec2f(0.0, 1.0); edge_pos_a_and_b += vec4f(edge_pos, edge_pos) * edge.xxyy; num_edges_a_and_b += edge; } { // top-left - let edge = has_edge(mask_center, center_coord + vec2i(-1, -1)); + let edge = has_edge(max_coord, mask_center, center_coord + vec2i(-1, -1)); let edge_pos = vec2f(0.0, 0.0); //edge_pos_a_and_b += vec4f(edge_pos, edge_pos) * edge.xxyy; // multiplied by zero, optimize out num_edges_a_and_b += edge; diff --git a/crates/viewer/re_renderer/shader/outlines/jumpflooding_init_msaa.wgsl b/crates/viewer/re_renderer/shader/outlines/jumpflooding_init_msaa.wgsl index fb504e4f70aa..71b3405038ef 100644 --- a/crates/viewer/re_renderer/shader/outlines/jumpflooding_init_msaa.wgsl +++ b/crates/viewer/re_renderer/shader/outlines/jumpflooding_init_msaa.wgsl @@ -3,8 +3,13 @@ @group(0) @binding(0) var mask_texture: texture_multisampled_2d; -fn has_edge(closest_center_sample: vec2u, sample_coord: vec2i, sample_idx: i32) -> vec2f { - let mask_neighbor = textureLoad(mask_texture, sample_coord, sample_idx).xy; +fn has_edge(max_coord: vec2i, closest_center_sample: vec2u, sample_coord: vec2i, sample_idx: i32) -> vec2f { + // Note that `textureLoad` calls with out-of-bounds coordinates are allowed to return *any* + // value on the texture or "transparent"/"opaque" zero. + // See https://www.w3.org/TR/WGSL/#textureload + // Therefore, if we want consistent behavior, we have to do the clamp ourselves. + let clamped_coord = clamp(sample_coord, vec2i(0), max_coord); + let mask_neighbor = textureLoad(mask_texture, clamped_coord, sample_idx).xy; return vec2f(closest_center_sample != mask_neighbor); } @@ -49,6 +54,7 @@ fn has_edge(closest_center_sample: vec2u, sample_coord: vec2i, sample_idx: i32) fn main(in: FragmentInput) -> @location(0) vec4f { let resolution = textureDimensions(mask_texture).xy; let center_coord = vec2i(vec2f(resolution) * in.texcoord); + let max_coord = vec2i(resolution) - vec2i(1); //let num_samples = textureNumSamples(mask_texture); // TODO(andreas): Should we assert somehow on textureNumSamples here? @@ -73,25 +79,25 @@ fn main(in: FragmentInput) -> @location(0) vec4f { // Sample closest neighbors top/bottom/left/right { // right - let edge = has_edge(mask_right_top, center_coord + vec2i(1, 0), 2); + let edge = has_edge(max_coord, mask_right_top, center_coord + vec2i(1, 0), 2); let edge_pos = vec2f(1.0, 0.5); edge_pos_a_and_b += vec4f(edge_pos, edge_pos) * edge.xxyy; num_edges_a_and_b += edge; } { // bottom - let edge = has_edge(mask_bottom_right, center_coord + vec2i(0, 1), 0); + let edge = has_edge(max_coord, mask_bottom_right, center_coord + vec2i(0, 1), 0); let edge_pos = vec2f(0.5, 1.0); edge_pos_a_and_b += vec4f(edge_pos, edge_pos) * edge.xxyy; num_edges_a_and_b += edge; } { // left - let edge = has_edge(mask_left_bottom, center_coord + vec2i(-1, 0), 1); + let edge = has_edge(max_coord, mask_left_bottom, center_coord + vec2i(-1, 0), 1); let edge_pos = vec2f(0.0, 0.5); edge_pos_a_and_b += vec4f(edge_pos, edge_pos) * edge.xxyy; num_edges_a_and_b += edge; } { // top - let edge = has_edge(mask_top_left, center_coord + vec2i(0, -1), 3); + let edge = has_edge(max_coord, mask_top_left, center_coord + vec2i(0, -1), 3); let edge_pos = vec2f(0.5, 0.0); edge_pos_a_and_b += vec4f(edge_pos, edge_pos) * edge.xxyy; num_edges_a_and_b += edge; @@ -100,25 +106,25 @@ fn main(in: FragmentInput) -> @location(0) vec4f { // Sample closest neighbors diagonally. // This is not strictly necessary, but empirically the result looks a lot better! { // top-right - let edge = has_edge(mask_right_top, center_coord + vec2i(1, -1), 2); + let edge = has_edge(max_coord, mask_right_top, center_coord + vec2i(1, -1), 2); let edge_pos = vec2f(1.0, 0.0); edge_pos_a_and_b += vec4f(edge_pos, edge_pos) * edge.xxyy; num_edges_a_and_b += edge; } { // bottom-right - let edge = has_edge(mask_bottom_right, center_coord + vec2i(1, 1), 0); + let edge = has_edge(max_coord, mask_bottom_right, center_coord + vec2i(1, 1), 0); let edge_pos = vec2f(1.0, 1.0); edge_pos_a_and_b += vec4f(edge_pos, edge_pos) * edge.xxyy; num_edges_a_and_b += edge; } { // bottom-left - let edge = has_edge(mask_left_bottom, center_coord + vec2i(-1, 1), 1); + let edge = has_edge(max_coord, mask_left_bottom, center_coord + vec2i(-1, 1), 1); let edge_pos = vec2f(0.0, 1.0); edge_pos_a_and_b += vec4f(edge_pos, edge_pos) * edge.xxyy; num_edges_a_and_b += edge; } { // top-left - let edge = has_edge(mask_top_left, center_coord + vec2i(-1, -1), 3); + let edge = has_edge(max_coord, mask_top_left, center_coord + vec2i(-1, -1), 3); let edge_pos = vec2f(0.0, 0.0); //edge_pos_a_and_b += vec4f(edge_pos, edge_pos) * edge.xxyy; // multiplied by zero, optimize out num_edges_a_and_b += edge; diff --git a/crates/viewer/re_renderer/src/global_bindings.rs b/crates/viewer/re_renderer/src/global_bindings.rs index 287b55243074..ae06425c0e79 100644 --- a/crates/viewer/re_renderer/src/global_bindings.rs +++ b/crates/viewer/re_renderer/src/global_bindings.rs @@ -45,8 +45,9 @@ pub struct FrameUniformBuffer { pub(crate) struct GlobalBindings { pub(crate) layout: GpuBindGroupLayoutHandle, - nearest_neighbor_sampler: GpuSamplerHandle, - trilinear_sampler: GpuSamplerHandle, + nearest_neighbor_sampler_repeat: GpuSamplerHandle, + nearest_neighbor_sampler_clamped: GpuSamplerHandle, + trilinear_sampler_repeat: GpuSamplerHandle, } impl GlobalBindings { @@ -73,24 +74,31 @@ impl GlobalBindings { }, count: None, }, - // Sampler without any filtering. + // Sampler without any filtering, repeat. wgpu::BindGroupLayoutEntry { binding: 1, visibility: wgpu::ShaderStages::FRAGMENT, ty: wgpu::BindingType::Sampler(wgpu::SamplerBindingType::NonFiltering), count: None, }, - // Trilinear sampler. + // Sampler without any filtering, clamped. wgpu::BindGroupLayoutEntry { binding: 2, visibility: wgpu::ShaderStages::FRAGMENT, + ty: wgpu::BindingType::Sampler(wgpu::SamplerBindingType::NonFiltering), + count: None, + }, + // Trilinear sampler, repeat. + wgpu::BindGroupLayoutEntry { + binding: 3, + visibility: wgpu::ShaderStages::FRAGMENT, ty: wgpu::BindingType::Sampler(wgpu::SamplerBindingType::Filtering), count: None, }, ], }, ), - nearest_neighbor_sampler: pools.samplers.get_or_create( + nearest_neighbor_sampler_repeat: pools.samplers.get_or_create( device, &SamplerDesc { label: "GlobalBindings::nearest_neighbor_sampler".into(), @@ -100,7 +108,17 @@ impl GlobalBindings { ..Default::default() }, ), - trilinear_sampler: pools.samplers.get_or_create( + nearest_neighbor_sampler_clamped: pools.samplers.get_or_create( + device, + &SamplerDesc { + label: "GlobalBindings::nearest_neighbor_sampler_clamped".into(), + address_mode_u: wgpu::AddressMode::ClampToEdge, + address_mode_v: wgpu::AddressMode::ClampToEdge, + address_mode_w: wgpu::AddressMode::ClampToEdge, + ..Default::default() + }, + ), + trilinear_sampler_repeat: pools.samplers.get_or_create( device, &SamplerDesc { label: "GlobalBindings::trilinear_sampler".into(), @@ -131,8 +149,9 @@ impl GlobalBindings { label: "GlobalBindings::create_bind_group".into(), entries: smallvec![ frame_uniform_buffer_binding, - BindGroupEntry::Sampler(self.nearest_neighbor_sampler), - BindGroupEntry::Sampler(self.trilinear_sampler), + BindGroupEntry::Sampler(self.nearest_neighbor_sampler_repeat), + BindGroupEntry::Sampler(self.nearest_neighbor_sampler_clamped), + BindGroupEntry::Sampler(self.trilinear_sampler_repeat), ], layout: self.layout, },