From 852e39b85b76bb5542aaf31f9a25e581dde1249e Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Tue, 8 Nov 2022 12:24:16 +0100 Subject: [PATCH] [re_renderer] Enable 4x MSAA (#276) * 4x MSAA always on * alpha to coverage for point rendering * use srgb format for view target and do some renames accordingly also better document the various color conversions and formats * update wgpu --- Cargo.lock | 31 ++-- Cargo.toml | 7 +- .../examples/renderer_standalone.rs | 4 +- .../shader/{tonemap.wgsl => composite.wgsl} | 11 +- .../re_renderer/shader/global_bindings.wgsl | 2 + crates/re_renderer/shader/lines.wgsl | 4 +- crates/re_renderer/shader/point_cloud.wgsl | 43 +++--- crates/re_renderer/shader/test_triangle.wgsl | 2 +- crates/re_renderer/src/global_bindings.rs | 5 +- .../renderer/{tonemapper.rs => compositor.rs} | 52 ++++--- .../src/renderer/generic_skybox.rs | 6 +- crates/re_renderer/src/renderer/lines.rs | 6 +- .../re_renderer/src/renderer/mesh_renderer.rs | 6 +- crates/re_renderer/src/renderer/mod.rs | 2 +- .../re_renderer/src/renderer/point_cloud.rs | 10 +- .../re_renderer/src/renderer/test_triangle.rs | 6 +- .../src/resource_pools/texture_pool.rs | 21 --- crates/re_renderer/src/view_builder.rs | 133 ++++++++++++++---- crates/re_renderer/src/workspace_shaders.rs | 12 +- deny.toml | 1 + 20 files changed, 224 insertions(+), 140 deletions(-) rename crates/re_renderer/shader/{tonemap.wgsl => composite.wgsl} (56%) rename crates/re_renderer/src/renderer/{tonemapper.rs => compositor.rs} (75%) diff --git a/Cargo.lock b/Cargo.lock index 98ebe63a5200..4d5911b05b19 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1029,7 +1029,7 @@ dependencies = [ "egui-wgpu", "egui-winit", "egui_glow", - "glow", + "glow 0.11.2 (registry+https://github.com/rust-lang/crates.io-index)", "glutin", "js-sys", "percent-encoding", @@ -1122,7 +1122,7 @@ dependencies = [ "bytemuck", "egui", "egui-winit", - "glow", + "glow 0.11.2 (registry+https://github.com/rust-lang/crates.io-index)", "memoffset 0.6.5", "puffin", "tracing", @@ -1571,6 +1571,17 @@ dependencies = [ "web-sys", ] +[[package]] +name = "glow" +version = "0.11.2" +source = "git+https://github.com/grovesNL/glow/?rev=c8a011fcd57a5c68cc917ed394baa484bdefc909#c8a011fcd57a5c68cc917ed394baa484bdefc909" +dependencies = [ + "js-sys", + "slotmap", + "wasm-bindgen", + "web-sys", +] + [[package]] name = "gltf" version = "1.0.0" @@ -2314,7 +2325,7 @@ dependencies = [ [[package]] name = "naga" version = "0.10.0" -source = "git+https://github.com/gfx-rs/naga?rev=c52d9102#c52d91023d43092323615fcc746162e478033f26" +source = "git+https://github.com/gfx-rs/naga?rev=e7fc8e6#e7fc8e64f2f23397b149217ecce6e123c5aa5092" dependencies = [ "bit-set", "bitflags", @@ -3197,7 +3208,7 @@ dependencies = [ "egui_glow", "fixed", "glam", - "glow", + "glow 0.11.2 (registry+https://github.com/rust-lang/crates.io-index)", "image", "instant", "itertools", @@ -3841,7 +3852,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f7a5a1017829335f6761bdbae2daf3021a88314a1f8d5f188c9b62ce62e2fd31" dependencies = [ "cgmath", - "glow", + "glow 0.11.2 (registry+https://github.com/rust-lang/crates.io-index)", "instant", "thiserror", "three-d-asset", @@ -4649,7 +4660,7 @@ checksum = "9193164d4de03a926d909d3bc7c30543cecb35400c02114792c2cae20d5e2dbb" [[package]] name = "wgpu" version = "0.14.0" -source = "git+https://github.com/gfx-rs/wgpu.git#cf3ea638ee8b635b1a92b0f1812a8c193c48911e" +source = "git+https://github.com/gfx-rs/wgpu.git#08b160c2932a44adedd9411bd882bc7ef41ea270" dependencies = [ "arrayvec 0.7.2", "js-sys", @@ -4670,7 +4681,7 @@ dependencies = [ [[package]] name = "wgpu-core" version = "0.14.0" -source = "git+https://github.com/gfx-rs/wgpu.git#cf3ea638ee8b635b1a92b0f1812a8c193c48911e" +source = "git+https://github.com/gfx-rs/wgpu.git#08b160c2932a44adedd9411bd882bc7ef41ea270" dependencies = [ "arrayvec 0.7.2", "bit-vec", @@ -4693,7 +4704,7 @@ dependencies = [ [[package]] name = "wgpu-hal" version = "0.14.0" -source = "git+https://github.com/gfx-rs/wgpu.git#cf3ea638ee8b635b1a92b0f1812a8c193c48911e" +source = "git+https://github.com/gfx-rs/wgpu.git#08b160c2932a44adedd9411bd882bc7ef41ea270" dependencies = [ "android_system_properties", "arrayvec 0.7.2", @@ -4705,7 +4716,7 @@ dependencies = [ "d3d12", "foreign-types 0.3.2", "fxhash", - "glow", + "glow 0.11.2 (git+https://github.com/grovesNL/glow/?rev=c8a011fcd57a5c68cc917ed394baa484bdefc909)", "gpu-alloc", "gpu-descriptor", "js-sys", @@ -4731,7 +4742,7 @@ dependencies = [ [[package]] name = "wgpu-types" version = "0.14.0" -source = "git+https://github.com/gfx-rs/wgpu.git#cf3ea638ee8b635b1a92b0f1812a8c193c48911e" +source = "git+https://github.com/gfx-rs/wgpu.git#08b160c2932a44adedd9411bd882bc7ef41ea270" dependencies = [ "bitflags", ] diff --git a/Cargo.toml b/Cargo.toml index a51232fb1ea6..512d4038e0ed 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -40,6 +40,7 @@ egui-wgpu = { git = "https://github.com/emilk/egui", rev = "8c76b8caff32e47e4419 # Because gltf hasn't published a new version: https://github.com/gltf-rs/gltf/issues/357 gltf = { git = "https://github.com/rerun-io/gltf", rev = "3c14ded73755d1ce9e47010edb06db63cb7e2cca" } -# 2022-10-12 - Eq for DepthBiasState and DepthStencilState -wgpu = { git = "https://github.com/gfx-rs/wgpu.git", ref = "aed3b1ae59ee097901b852d934493c7ec167d344" } -wgpu-core = { git = "https://github.com/gfx-rs/wgpu.git", ref = "aed3b1ae59ee097901b852d934493c7ec167d344" } +# 2022-10-12 - Alpha to coverage support for GLES +wgpu = { git = "https://github.com/gfx-rs/wgpu.git", ref = "a377ae2b7fe6c1c9412751166f0917e617164e49" } +wgpu-core = { git = "https://github.com/gfx-rs/wgpu.git", ref = "a377ae2b7fe6c1c9412751166f0917e617164e49" } +#wgpu = { path = "../wgpu/wgpu" } diff --git a/crates/re_renderer/examples/renderer_standalone.rs b/crates/re_renderer/examples/renderer_standalone.rs index 2600bdf6bd36..84081a3fb1bb 100644 --- a/crates/re_renderer/examples/renderer_standalone.rs +++ b/crates/re_renderer/examples/renderer_standalone.rs @@ -507,7 +507,7 @@ struct AppState { impl AppState { fn new(re_ctx: &mut RenderContext) -> Self { let mut rnd = ::seed_from_u64(42); - let random_point_range = -2.0_f32..2.0_f32; + let random_point_range = -5.0_f32..5.0_f32; let random_points = (0..500000) .map(|_| PointCloudPoint { position: glam::vec3( @@ -515,7 +515,7 @@ impl AppState { rnd.gen_range(random_point_range.clone()), rnd.gen_range(random_point_range.clone()), ), - radius: rnd.gen_range(0.005..0.025), + radius: rnd.gen_range(0.005..0.05), srgb_color: [rnd.gen(), rnd.gen(), rnd.gen(), 255], }) .collect_vec(); diff --git a/crates/re_renderer/shader/tonemap.wgsl b/crates/re_renderer/shader/composite.wgsl similarity index 56% rename from crates/re_renderer/shader/tonemap.wgsl rename to crates/re_renderer/shader/composite.wgsl index 6a69a0ffd0f7..6e191bf01ee7 100644 --- a/crates/re_renderer/shader/tonemap.wgsl +++ b/crates/re_renderer/shader/composite.wgsl @@ -8,15 +8,18 @@ struct VertexOutput { }; @group(1) @binding(0) -var hdr_texture: texture_2d; +var input_texture: texture_2d; @fragment fn main(in: VertexOutput) -> @location(0) Vec4 { // 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. - let hdr = textureSample(hdr_texture, nearest_sampler, in.texcoord).rgb; + let input = textureSample(input_texture, nearest_sampler, in.texcoord).rgb; // TODO(andreas): Do something meaningful with values above 1 - let hdr = clamp(hdr, vec3(0.0), vec3(1.0)); - return Vec4(srgb_from_linear(hdr), 1.0); + let input = clamp(input, vec3(0.0), vec3(1.0)); + + // Convert to srgb - this is necessary since the final eframe output does *not* have an srgb format. + // Note that the input here is assumed to be linear - if the input texture was an srgb texture it would have been converted on load. + return Vec4(srgb_from_linear(input), 1.0); } diff --git a/crates/re_renderer/shader/global_bindings.wgsl b/crates/re_renderer/shader/global_bindings.wgsl index 77ed5ac621a6..7ada2b8e36e8 100644 --- a/crates/re_renderer/shader/global_bindings.wgsl +++ b/crates/re_renderer/shader/global_bindings.wgsl @@ -5,6 +5,8 @@ struct FrameUniformBuffer { camera_position: vec3, top_right_screen_corner_in_view: vec2, + /// Multiply this with a camera distance to get a measure of how wide a pixel is in world units. + pixel_world_size_from_camera_distance: f32, }; @group(0) @binding(0) var frame: FrameUniformBuffer; diff --git a/crates/re_renderer/shader/lines.wgsl b/crates/re_renderer/shader/lines.wgsl index 76eb61d3d927..47acf3cb7291 100644 --- a/crates/re_renderer/shader/lines.wgsl +++ b/crates/re_renderer/shader/lines.wgsl @@ -133,6 +133,6 @@ fn vs_main(@builtin(vertex_index) vertex_idx: u32) -> VertexOut { @fragment fn fs_main(in: VertexOut) -> @location(0) Vec4 { // TODO(andreas): Rounded caps, proper shading/lighting, etc. - let shading = 1.2 - length(in.position_world - in.position_world_line) / in.line_radius; - return in.color * shading ; + let shading = max(0.2, 1.2 - length(in.position_world - in.position_world_line) / in.line_radius); + return in.color * shading; } diff --git a/crates/re_renderer/shader/point_cloud.wgsl b/crates/re_renderer/shader/point_cloud.wgsl index febcd0fc2c04..c08a3982ba65 100644 --- a/crates/re_renderer/shader/point_cloud.wgsl +++ b/crates/re_renderer/shader/point_cloud.wgsl @@ -64,8 +64,11 @@ fn vs_main(@builtin(vertex_index) vertex_idx: u32) -> VertexOut { // "unnecessary" overlaps. So instead, we change the size _and_ move the sphere closer (using math!) let radius_sq = point_data.radius * point_data.radius; let camera_offset = radius_sq * distance_to_camera_inv; - let modified_radius = point_data.radius * distance_to_camera_inv * sqrt(distance_to_camera_sq - radius_sq); - let pos = point_data.pos + pos_in_quad * modified_radius + camera_offset * quad_normal; + var modified_radius = point_data.radius * distance_to_camera_inv * sqrt(distance_to_camera_sq - radius_sq); + // We're computing a coverage mask in the fragment shader - make sure the quad doesn't cut off our antialiasing. + // It's fairly subtle but if we don't do this our spheres look slightly squarish + modified_radius += frame.pixel_world_size_from_camera_distance / distance_to_camera_inv; + let pos = point_data.pos + pos_in_quad * modified_radius * 1.0 + camera_offset * quad_normal; // normal billboard (spheres are cut off!): // pos = point_data.pos + pos_in_quad * point_data.radius; // only enlarged billboard (works but requires z care even for non-overlapping spheres): @@ -83,31 +86,39 @@ fn vs_main(@builtin(vertex_index) vertex_idx: u32) -> VertexOut { return out; } -// Return how far the closest intersection point is from ray_origin. -// Returns -1.0 if no intersection happend -fn sphere_intersect(sphere_pos: Vec3, radius_sq: f32, ray_origin: Vec3, ray_dir: Vec3) -> f32 { - let sphere_to_origin = ray_origin - sphere_pos; + +// Returns distance to sphere surface (x) and distance to of closest ray hit (y) +// Via https://iquilezles.org/articles/spherefunctions/ but with more verbose names. +fn sphere_distance(ray_origin: Vec3, ray_dir: Vec3, sphere_origin: Vec3, sphere_radius: f32) -> Vec2 { + let sphere_radius_sq = sphere_radius * sphere_radius; + let sphere_to_origin = ray_origin - sphere_origin; let b = dot(sphere_to_origin, ray_dir); - let c = dot(sphere_to_origin, sphere_to_origin) - radius_sq; - let discriminant = b * b - c; - if (discriminant < 0.0) { - return -1.0; - } - return -b - sqrt(discriminant); + let c = dot(sphere_to_origin, sphere_to_origin) - sphere_radius_sq; + let h = b * b - c; + let d = sqrt(max(0.0, sphere_radius_sq - h)) - sphere_radius; + return Vec2(d, -b - sqrt(max(h, 0.0))); } @fragment fn fs_main(in: VertexOut) -> @location(0) Vec4 { - // TODO(andreas): Pass around squared radius instead. let ray_dir = normalize(in.world_position - frame.camera_position); - if sphere_intersect(in.point_center, in.radius * in.radius, frame.camera_position, ray_dir) < 0.0 { + + // Sphere intersection with anti-aliasing as described by Iq here + // https://www.shadertoy.com/view/MsSSWV + // (but rearranged and labled to it's easier to understand!) + let d = sphere_distance(frame.camera_position, ray_dir, in.point_center, in.radius); + let smallest_distance_to_sphere = d.x; + let closest_ray_dist = d.y; + let pixel_world_size = closest_ray_dist * frame.pixel_world_size_from_camera_distance; + if smallest_distance_to_sphere > pixel_world_size { discard; } + let coverage = 1.0 - clamp(smallest_distance_to_sphere / pixel_world_size, 0.0, 1.0); // TODO(andreas): Do we want manipulate the depth buffer depth to actually render spheres? // TODO(andreas): Proper shading - let shading = min(1.0, 1.2 - distance(in.point_center, in.world_position) / in.radius); // quick and dirty coloring) - return in.color * shading; + let shading = max(0.2, 1.2 - distance(in.point_center, in.world_position) / in.radius); // quick and dirty coloring) + return vec4(in.color.rgb * shading, coverage); } diff --git a/crates/re_renderer/shader/test_triangle.wgsl b/crates/re_renderer/shader/test_triangle.wgsl index c24884c98a31..60f55831105c 100644 --- a/crates/re_renderer/shader/test_triangle.wgsl +++ b/crates/re_renderer/shader/test_triangle.wgsl @@ -22,7 +22,7 @@ var v_colors: array = array( fn vs_main(@builtin(vertex_index) v_idx: u32) -> VertexOut { var out: VertexOut; - out.position = frame.projection_from_world * Vec4(v_positions[v_idx], 0.0, 1.0); + out.position = frame.projection_from_world * Vec4(v_positions[v_idx] * 5.0, 0.0, 1.0); out.color = v_colors[v_idx]; return out; diff --git a/crates/re_renderer/src/global_bindings.rs b/crates/re_renderer/src/global_bindings.rs index e011db0f0661..0936d377ef85 100644 --- a/crates/re_renderer/src/global_bindings.rs +++ b/crates/re_renderer/src/global_bindings.rs @@ -26,7 +26,10 @@ pub(crate) struct FrameUniformBuffer { pub camera_position: wgpu_buffer_types::Vec3, /// View space coordinates of the top right screen corner. - pub top_right_screen_corner_in_view: wgpu_buffer_types::Vec2Padded, + pub top_right_screen_corner_in_view: wgpu_buffer_types::Vec2, + /// Multiply this with a camera distance to get a measure of how wide a pixel is in world units. + pub pixel_world_size_from_camera_distance: f32, + pub _padding: f32, } pub(crate) struct GlobalBindings { diff --git a/crates/re_renderer/src/renderer/tonemapper.rs b/crates/re_renderer/src/renderer/compositor.rs similarity index 75% rename from crates/re_renderer/src/renderer/tonemapper.rs rename to crates/re_renderer/src/renderer/compositor.rs index bd5882f97e6f..8a47bb84ac4f 100644 --- a/crates/re_renderer/src/renderer/tonemapper.rs +++ b/crates/re_renderer/src/renderer/compositor.rs @@ -12,42 +12,42 @@ use super::*; use smallvec::smallvec; -pub struct Tonemapper { +pub struct Compositor { render_pipeline: GpuRenderPipelineHandle, bind_group_layout: GpuBindGroupLayoutHandle, } #[derive(Clone)] -pub struct TonemapperDrawable { - /// [`GpuBindGroup`] pointing at the current HDR source and - /// a uniform buffer for describing a tonemapper configuration. - hdr_target_bind_group: GpuBindGroupHandleStrong, +pub struct CompositorDrawable { + /// [`GpuBindGroup`] pointing at the current image source and + /// a uniform buffer for describing a tonemapper/compositor configuration. + bind_group: GpuBindGroupHandleStrong, } -impl Drawable for TonemapperDrawable { - type Renderer = Tonemapper; +impl Drawable for CompositorDrawable { + type Renderer = Compositor; } -impl TonemapperDrawable { +impl CompositorDrawable { pub fn new( ctx: &mut RenderContext, device: &wgpu::Device, - hdr_target: &GpuTextureHandleStrong, + target: &GpuTextureHandleStrong, ) -> Self { let pools = &mut ctx.resource_pools; - let tonemapper = ctx.renderers.get_or_create::<_, Tonemapper>( + let compositor = ctx.renderers.get_or_create::<_, Compositor>( &ctx.shared_renderer_data, pools, device, &mut ctx.resolver, ); - TonemapperDrawable { - hdr_target_bind_group: pools.bind_groups.alloc( + CompositorDrawable { + bind_group: pools.bind_groups.alloc( device, &BindGroupDesc { - label: "tonemapping".into(), - entries: smallvec![BindGroupEntry::DefaultTextureView(**hdr_target)], - layout: tonemapper.bind_group_layout, + label: "compositor".into(), + entries: smallvec![BindGroupEntry::DefaultTextureView(**target)], + layout: compositor.bind_group_layout, }, &pools.bind_group_layouts, &pools.textures, @@ -58,8 +58,8 @@ impl TonemapperDrawable { } } -impl Renderer for Tonemapper { - type DrawData = TonemapperDrawable; +impl Renderer for Compositor { + type DrawData = CompositorDrawable; fn create_renderer( shared_data: &SharedRendererData, @@ -70,12 +70,12 @@ impl Renderer for Tonemapper { let bind_group_layout = pools.bind_group_layouts.get_or_create( device, &BindGroupLayoutDesc { - label: "tonemapping".into(), + label: "compositor".into(), entries: vec![wgpu::BindGroupLayoutEntry { binding: 0, visibility: wgpu::ShaderStages::FRAGMENT, ty: wgpu::BindingType::Texture { - sample_type: wgpu::TextureSampleType::default(), + sample_type: wgpu::TextureSampleType::Float { filterable: false }, view_dimension: wgpu::TextureViewDimension::D2, multisampled: false, }, @@ -87,11 +87,11 @@ impl Renderer for Tonemapper { let render_pipeline = pools.render_pipelines.get_or_create( device, &RenderPipelineDesc { - label: "tonemapping".into(), + label: "compositor".into(), pipeline_layout: pools.pipeline_layouts.get_or_create( device, &PipelineLayoutDesc { - label: "tonemapping".into(), + label: "compositor".into(), entries: vec![shared_data.global_bindings.layout, bind_group_layout], }, &pools.bind_group_layouts, @@ -111,7 +111,7 @@ impl Renderer for Tonemapper { resolver, &ShaderModuleDesc { label: "tonemap (fragment)".into(), - source: include_file!("../../shader/tonemap.wgsl"), + source: include_file!("../../shader/composite.wgsl"), }, ), vertex_buffers: smallvec![], @@ -124,7 +124,7 @@ impl Renderer for Tonemapper { &pools.shader_modules, ); - Tonemapper { + Compositor { render_pipeline, bind_group_layout, } @@ -134,12 +134,10 @@ impl Renderer for Tonemapper { &self, pools: &'a WgpuResourcePools, pass: &mut wgpu::RenderPass<'a>, - draw_data: &TonemapperDrawable, + draw_data: &CompositorDrawable, ) -> anyhow::Result<()> { let pipeline = pools.render_pipelines.get_resource(self.render_pipeline)?; - let bind_group = pools - .bind_groups - .get_resource(&draw_data.hdr_target_bind_group)?; + let bind_group = pools.bind_groups.get_resource(&draw_data.bind_group)?; pass.set_pipeline(&pipeline.pipeline); pass.set_bind_group(1, &bind_group.bind_group, &[]); diff --git a/crates/re_renderer/src/renderer/generic_skybox.rs b/crates/re_renderer/src/renderer/generic_skybox.rs index ff518abc9cd8..ae5f67f66c1d 100644 --- a/crates/re_renderer/src/renderer/generic_skybox.rs +++ b/crates/re_renderer/src/renderer/generic_skybox.rs @@ -81,10 +81,10 @@ impl Renderer for GenericSkybox { }, ), vertex_buffers: smallvec![], - render_targets: smallvec![Some(ViewBuilder::FORMAT_HDR.into())], + render_targets: smallvec![Some(ViewBuilder::MAIN_TARGET_COLOR_FORMAT.into())], primitive: wgpu::PrimitiveState::default(), depth_stencil: Some(wgpu::DepthStencilState { - format: ViewBuilder::FORMAT_DEPTH, + format: ViewBuilder::MAIN_TARGET_DEPTH_FORMAT, // Pass depth test only if the fragment hasn't been written to. // This allows us to draw the skybox last which is much more efficient than using it as a clear pass! depth_compare: wgpu::CompareFunction::Equal, @@ -92,7 +92,7 @@ impl Renderer for GenericSkybox { stencil: Default::default(), bias: Default::default(), }), - multisample: wgpu::MultisampleState::default(), + multisample: ViewBuilder::MAIN_TARGET_DEFAULT_MSAA_STATE, }, &pools.pipeline_layouts, &pools.shader_modules, diff --git a/crates/re_renderer/src/renderer/lines.rs b/crates/re_renderer/src/renderer/lines.rs index 97a6377408bb..6ca945deac5f 100644 --- a/crates/re_renderer/src/renderer/lines.rs +++ b/crates/re_renderer/src/renderer/lines.rs @@ -473,19 +473,19 @@ impl Renderer for LineRenderer { fragment_entrypoint: "fs_main".into(), fragment_handle: shader_module, vertex_buffers: smallvec![], - render_targets: smallvec![Some(ViewBuilder::FORMAT_HDR.into())], + render_targets: smallvec![Some(ViewBuilder::MAIN_TARGET_COLOR_FORMAT.into())], primitive: wgpu::PrimitiveState { topology: wgpu::PrimitiveTopology::TriangleList, ..Default::default() }, depth_stencil: Some(wgpu::DepthStencilState { - format: ViewBuilder::FORMAT_DEPTH, + format: ViewBuilder::MAIN_TARGET_DEPTH_FORMAT, depth_compare: wgpu::CompareFunction::Greater, depth_write_enabled: true, stencil: Default::default(), bias: Default::default(), }), - multisample: wgpu::MultisampleState::default(), + multisample: ViewBuilder::MAIN_TARGET_DEFAULT_MSAA_STATE, }, &pools.pipeline_layouts, &pools.shader_modules, diff --git a/crates/re_renderer/src/renderer/mesh_renderer.rs b/crates/re_renderer/src/renderer/mesh_renderer.rs index 21139cef0052..9fd61985ccf5 100644 --- a/crates/re_renderer/src/renderer/mesh_renderer.rs +++ b/crates/re_renderer/src/renderer/mesh_renderer.rs @@ -225,20 +225,20 @@ impl Renderer for MeshRenderer { .chain(mesh_vertices::vertex_buffer_layouts()) .collect(), - render_targets: smallvec![Some(ViewBuilder::FORMAT_HDR.into())], + render_targets: smallvec![Some(ViewBuilder::MAIN_TARGET_COLOR_FORMAT.into())], primitive: wgpu::PrimitiveState { topology: wgpu::PrimitiveTopology::TriangleList, cull_mode: None, //Some(wgpu::Face::Back), // TODO(andreas): Need to specify from outside if mesh is CW or CCW? ..Default::default() }, depth_stencil: Some(wgpu::DepthStencilState { - format: ViewBuilder::FORMAT_DEPTH, + format: ViewBuilder::MAIN_TARGET_DEPTH_FORMAT, depth_compare: wgpu::CompareFunction::Greater, depth_write_enabled: true, stencil: Default::default(), bias: Default::default(), }), - multisample: wgpu::MultisampleState::default(), + multisample: ViewBuilder::MAIN_TARGET_DEFAULT_MSAA_STATE, }, &pools.pipeline_layouts, &pools.shader_modules, diff --git a/crates/re_renderer/src/renderer/mod.rs b/crates/re_renderer/src/renderer/mod.rs index 62f7313c1635..2596aa378aa2 100644 --- a/crates/re_renderer/src/renderer/mod.rs +++ b/crates/re_renderer/src/renderer/mod.rs @@ -14,7 +14,7 @@ mod mesh_renderer; pub(crate) use mesh_renderer::MeshRenderer; pub use mesh_renderer::{MeshDrawable, MeshInstance}; -pub mod tonemapper; +pub mod compositor; mod utils; diff --git a/crates/re_renderer/src/renderer/point_cloud.rs b/crates/re_renderer/src/renderer/point_cloud.rs index ff32541c73cd..abc526b9dc47 100644 --- a/crates/re_renderer/src/renderer/point_cloud.rs +++ b/crates/re_renderer/src/renderer/point_cloud.rs @@ -301,19 +301,23 @@ impl Renderer for PointCloudRenderer { // Instance buffer with pairwise overlapping instances! vertex_buffers: smallvec![], - render_targets: smallvec![Some(ViewBuilder::FORMAT_HDR.into())], + render_targets: smallvec![Some(ViewBuilder::MAIN_TARGET_COLOR_FORMAT.into())], primitive: wgpu::PrimitiveState { topology: wgpu::PrimitiveTopology::TriangleList, ..Default::default() }, depth_stencil: Some(wgpu::DepthStencilState { - format: ViewBuilder::FORMAT_DEPTH, + format: ViewBuilder::MAIN_TARGET_DEPTH_FORMAT, depth_compare: wgpu::CompareFunction::Greater, depth_write_enabled: true, stencil: Default::default(), bias: Default::default(), }), - multisample: wgpu::MultisampleState::default(), + multisample: wgpu::MultisampleState { + // We discard pixels to do the round cutout, therefore we need to calculate our own sampling mask. + alpha_to_coverage_enabled: true, + ..ViewBuilder::MAIN_TARGET_DEFAULT_MSAA_STATE + }, }, &pools.pipeline_layouts, &pools.shader_modules, diff --git a/crates/re_renderer/src/renderer/test_triangle.rs b/crates/re_renderer/src/renderer/test_triangle.rs index a9b3765017ab..ed9f9c99ce1a 100644 --- a/crates/re_renderer/src/renderer/test_triangle.rs +++ b/crates/re_renderer/src/renderer/test_triangle.rs @@ -75,16 +75,16 @@ impl Renderer for TestTriangle { }, ), vertex_buffers: smallvec![], - render_targets: smallvec![Some(ViewBuilder::FORMAT_HDR.into())], + render_targets: smallvec![Some(ViewBuilder::MAIN_TARGET_COLOR_FORMAT.into())], primitive: wgpu::PrimitiveState::default(), depth_stencil: Some(wgpu::DepthStencilState { - format: ViewBuilder::FORMAT_DEPTH, + format: ViewBuilder::MAIN_TARGET_DEPTH_FORMAT, depth_compare: wgpu::CompareFunction::Always, depth_write_enabled: true, // writes some depth for testing stencil: Default::default(), bias: Default::default(), }), - multisample: wgpu::MultisampleState::default(), + multisample: ViewBuilder::MAIN_TARGET_DEFAULT_MSAA_STATE, }, &pools.pipeline_layouts, &pools.shader_modules, diff --git a/crates/re_renderer/src/resource_pools/texture_pool.rs b/crates/re_renderer/src/resource_pools/texture_pool.rs index 97f10eb8c78d..0a0f72f9ff75 100644 --- a/crates/re_renderer/src/resource_pools/texture_pool.rs +++ b/crates/re_renderer/src/resource_pools/texture_pool.rs @@ -107,24 +107,3 @@ impl GpuTexturePool { self.pool.get_strong_handle(handle) } } - -pub(crate) fn render_target_2d_desc( - format: wgpu::TextureFormat, - width: u32, - height: u32, - sample_count: u32, -) -> TextureDesc { - TextureDesc { - label: "rendertarget".into(), - size: wgpu::Extent3d { - width, - height, - depth_or_array_layers: 1, - }, - mip_level_count: 1, - sample_count, - dimension: wgpu::TextureDimension::D2, - format, - usage: wgpu::TextureUsages::RENDER_ATTACHMENT | wgpu::TextureUsages::TEXTURE_BINDING, - } -} diff --git a/crates/re_renderer/src/view_builder.rs b/crates/re_renderer/src/view_builder.rs index 98e53126e96f..2f3afe0dfcf4 100644 --- a/crates/re_renderer/src/view_builder.rs +++ b/crates/re_renderer/src/view_builder.rs @@ -5,7 +5,7 @@ use std::sync::Arc; use crate::{ context::*, global_bindings::FrameUniformBuffer, - renderer::{tonemapper::*, Drawable, Renderer}, + renderer::{compositor::*, Drawable, Renderer}, resource_pools::{ bind_group_pool::GpuBindGroupHandleStrong, buffer_pool::BufferDesc, texture_pool::*, }, @@ -30,10 +30,11 @@ pub struct ViewBuilder { } struct ViewTargetSetup { - tonemapping_drawable: TonemapperDrawable, + tonemapping_drawable: CompositorDrawable, bind_group_0: GpuBindGroupHandleStrong, - hdr_render_target: GpuTextureHandleStrong, + hdr_render_target_msaa: GpuTextureHandleStrong, + hdr_render_target_resolved: GpuTextureHandleStrong, depth_buffer: GpuTextureHandleStrong, resolution_in_pixel: [u32; 2], @@ -62,8 +63,36 @@ pub struct TargetConfiguration { } impl ViewBuilder { - pub const FORMAT_HDR: wgpu::TextureFormat = wgpu::TextureFormat::Rgba16Float; - pub const FORMAT_DEPTH: wgpu::TextureFormat = wgpu::TextureFormat::Depth24Plus; + /// Color format used for the main target of the view builder. + /// + /// Eventually we'll want to make this an HDR format and apply tonemapping during composite. + /// However, note that it is easy to run into subtle MSAA quality issues then: + /// Applying MSAA resolve before tonemapping is problematic as it means we're doing msaa in linear. + /// This is especially problematic at bright/dark edges where we may loose "smoothness"! + /// For a nice illustration see [this blog post by MRP](https://therealmjp.github.io/posts/msaa-overview/) + /// We either would need to keep the MSAA target and tonemap it, + /// apply a manual resolve where we inverse-tonemap non-fully-covered pixel before averaging. + /// (an optimized variant of this is described [by AMD here](https://gpuopen.com/learn/optimized-reversible-tonemapper-for-resolve/)) + /// In any case, this gets us onto a potentially much costlier rendering path, especially for tiling GPUs. + pub const MAIN_TARGET_COLOR_FORMAT: wgpu::TextureFormat = wgpu::TextureFormat::Rgba8UnormSrgb; + + /// Depth format used for the main target of the view builder. + pub const MAIN_TARGET_DEPTH_FORMAT: wgpu::TextureFormat = wgpu::TextureFormat::Depth24Plus; + + /// Enable MSAA always. This makes our pipeline less variable as well, as we need MSAA resolve steps if we want any MSAA at all! + /// + /// 4 samples are the only thing `WebGPU` supports, and currently wgpu as well + /// ([tracking issue for more options on native](https://github.com/gfx-rs/wgpu/issues/2910)) + pub const MAIN_TARGET_SAMPLE_COUNT: u32 = 4; + + /// Default multisample state that any [`wgpu::RenderPipeline`] drawing to the main target needs to use. + /// + /// In rare cases, pipelines may want to enable alpha to coverage and/or sample masks. + pub const MAIN_TARGET_DEFAULT_MSAA_STATE: wgpu::MultisampleState = wgpu::MultisampleState { + count: ViewBuilder::MAIN_TARGET_SAMPLE_COUNT, + mask: !0, + alpha_to_coverage_enabled: false, + }; pub fn new_shared() -> SharedViewBuilder { Arc::new(RwLock::new(Some(ViewBuilder::default()))) @@ -77,27 +106,45 @@ impl ViewBuilder { config: &TargetConfiguration, ) -> anyhow::Result<&mut Self> { // TODO(andreas): Should tonemapping preferences go here as well? Likely! - // TODO(andreas): How should we treat multisampling. Once we start it we also need to deal with MSAA resolves - let hdr_render_target = ctx.resource_pools.textures.alloc( + let hdr_render_target_desc = TextureDesc { + label: "hdr rendertarget msaa".into(), + size: wgpu::Extent3d { + width: config.resolution_in_pixel[0], + height: config.resolution_in_pixel[1], + depth_or_array_layers: 1, + }, + mip_level_count: 1, + sample_count: Self::MAIN_TARGET_SAMPLE_COUNT, + dimension: wgpu::TextureDimension::D2, + format: Self::MAIN_TARGET_COLOR_FORMAT, + usage: wgpu::TextureUsages::RENDER_ATTACHMENT, + }; + let hdr_render_target_msaa = ctx + .resource_pools + .textures + .alloc(device, &hdr_render_target_desc); + // Like hdr_render_target, but with MSAA resolved. + let hdr_render_target_resolved = ctx.resource_pools.textures.alloc( device, - &render_target_2d_desc( - Self::FORMAT_HDR, - config.resolution_in_pixel[0], - config.resolution_in_pixel[1], - 1, - ), + &TextureDesc { + label: "hdr rendertarget resolved".into(), + sample_count: 1, + usage: wgpu::TextureUsages::RENDER_ATTACHMENT + | wgpu::TextureUsages::TEXTURE_BINDING, + ..hdr_render_target_desc + }, ); let depth_buffer = ctx.resource_pools.textures.alloc( device, - &render_target_2d_desc( - Self::FORMAT_DEPTH, - config.resolution_in_pixel[0], - config.resolution_in_pixel[1], - 1, - ), + &TextureDesc { + label: "depth buffer".into(), + format: Self::MAIN_TARGET_DEPTH_FORMAT, + ..hdr_render_target_desc + }, ); - let tonemapping_drawable = TonemapperDrawable::new(ctx, device, &hdr_render_target); + let tonemapping_drawable = + CompositorDrawable::new(ctx, device, &hdr_render_target_resolved); // Setup frame uniform buffer let frame_uniform_buffer = ctx.resource_pools.buffers.alloc( @@ -132,6 +179,18 @@ impl ViewBuilder { .truncate() .normalize(); + // Determine how wide a pixel is in world space at unit distance from the camera. + // + // derivation: + // tan(FOV / 2) = (screen_in_world / 2) / distance + // screen_in_world = tan(FOV / 2) * distance * 2 + // + // want: pixels in world per distance, i.e (screen_in_world / resolution / distance) + // => (resolution / screen_in_world / distance) = tan(FOV / 2) * distance * 2 / resolution / distance = + // = tan(FOV / 2) * 2.0 / resolution + let pixel_world_size_from_camera_distance = + (config.fov_y * 0.5).tan() * 2.0 / config.resolution_in_pixel[1] as f32; + queue.write_buffer( &ctx.resource_pools .buffers @@ -145,6 +204,8 @@ impl ViewBuilder { projection_from_world: projection_from_world.into(), camera_position: camera_position.into(), top_right_screen_corner_in_view: top_right_screen_corner_in_view.into(), + pixel_world_size_from_camera_distance, + _padding: 0.0, }), ); @@ -157,7 +218,8 @@ impl ViewBuilder { self.setup = Some(ViewTargetSetup { tonemapping_drawable, bind_group_0, - hdr_render_target, + hdr_render_target_msaa, + hdr_render_target_resolved, depth_buffer, resolution_in_pixel: config.resolution_in_pixel, origin_in_pixel: config.origin_in_pixel, @@ -197,11 +259,16 @@ impl ViewBuilder { .as_ref() .context("ViewBuilder::setup_view wasn't called yet")?; - let color = ctx + let color_msaa = ctx + .resource_pools + .textures + .get_resource(&setup.hdr_render_target_msaa) + .context("hdr render target msaa")?; + let color_resolved = ctx .resource_pools .textures - .get_resource(&setup.hdr_render_target) - .context("hdr render target")?; + .get_resource(&setup.hdr_render_target_resolved) + .context("hdr render target resolved")?; let depth = ctx .resource_pools .textures @@ -211,18 +278,22 @@ impl ViewBuilder { let mut pass = encoder.begin_render_pass(&wgpu::RenderPassDescriptor { label: Some("frame builder hdr pass"), // TODO(andreas): It would be nice to specify this from the outside so we know which view we're rendering color_attachments: &[Some(wgpu::RenderPassColorAttachment { - view: &color.default_view, - resolve_target: None, + view: &color_msaa.default_view, + resolve_target: Some(&color_resolved.default_view), ops: wgpu::Operations { load: wgpu::LoadOp::Clear(wgpu::Color::BLACK), - store: true, + // Don't care about the result, it's going to be resolved to the resolve target. + // This can have be much better perf, especially on tiler gpus. + store: false, }, })], depth_stencil_attachment: Some(wgpu::RenderPassDepthStencilAttachment { view: &depth.default_view, depth_ops: Some(wgpu::Operations { load: wgpu::LoadOp::Clear(0.0), // 0.0 == far since we're using reverse-z - store: false, // discards the depth buffer after use, can be faster + // Don't care about depth results afterwards. + // This can have be much better perf, especially on tiler gpus. + store: false, }), stencil_ops: None, }), @@ -280,10 +351,10 @@ impl ViewBuilder { let tonemapper = ctx .renderers - .get::() - .context("get tonemapper")?; + .get::() + .context("get compositor")?; tonemapper .draw(&ctx.resource_pools, pass, &setup.tonemapping_drawable) - .context("perform tonemapping") + .context("composite into main view") } } diff --git a/crates/re_renderer/src/workspace_shaders.rs b/crates/re_renderer/src/workspace_shaders.rs index d192e9b7bf00..d9dd4996bd49 100644 --- a/crates/re_renderer/src/workspace_shaders.rs +++ b/crates/re_renderer/src/workspace_shaders.rs @@ -11,6 +11,12 @@ pub fn init() { use crate::file_system::FileSystem as _; let fs = crate::MemFileSystem::get(); + { + let virtpath = ::std::path::Path::new("crates/re_renderer/shader/composite.wgsl"); + fs.create_file(&virtpath, include_str!("../shader/composite.wgsl").into()) + .unwrap(); + } + { let virtpath = ::std::path::Path::new("crates/re_renderer/shader/generic_skybox.wgsl"); fs.create_file( @@ -74,12 +80,6 @@ pub fn init() { .unwrap(); } - { - let virtpath = ::std::path::Path::new("crates/re_renderer/shader/tonemap.wgsl"); - fs.create_file(&virtpath, include_str!("../shader/tonemap.wgsl").into()) - .unwrap(); - } - { let virtpath = ::std::path::Path::new("crates/re_renderer/shader/types.wgsl"); fs.create_file(&virtpath, include_str!("../shader/types.wgsl").into()) diff --git a/deny.toml b/deny.toml index 091121dc2e0e..96d5f5e7af62 100644 --- a/deny.toml +++ b/deny.toml @@ -28,6 +28,7 @@ deny = [ skip = [ { name = "ahash" }, # Old version via dark-light { name = "nix" }, # Too many to protect against + { name = "glow" }, # Both wgpu and egui patch glow. Later is going to be removed with old renderer ] skip-tree = [ { name = "criterion" }, # dev-dependency