From 2962959616eb8fd02275cdfb2912ccda8a89920f Mon Sep 17 00:00:00 2001 From: Arman Uguray Date: Tue, 31 Oct 2023 16:32:31 -0700 Subject: [PATCH] MSAA config: address review comments --- examples/with_winit/src/lib.rs | 38 ++++++++++++++------------------ examples/with_winit/src/stats.rs | 11 ++++++++- src/lib.rs | 4 ++-- src/render.rs | 16 +++++++++----- src/shaders.rs | 24 +++++++++----------- 5 files changed, 51 insertions(+), 42 deletions(-) diff --git a/examples/with_winit/src/lib.rs b/examples/with_winit/src/lib.rs index 218013b58..646f74ec3 100644 --- a/examples/with_winit/src/lib.rs +++ b/examples/with_winit/src/lib.rs @@ -114,7 +114,8 @@ fn run( let mut vsync_on = true; const AA_CONFIGS: [AaConfig; 3] = [AaConfig::Area, AaConfig::Msaa8, AaConfig::Msaa16]; - let mut aa_config_ix = 0; + // We allow cycling through AA configs in either direction, so use a signed index + let mut aa_config_ix: i32 = 0; let mut frame_start_time = Instant::now(); let start = Instant::now(); @@ -134,8 +135,8 @@ fn run( } let mut profile_stored = None; let mut prev_scene_ix = scene_ix - 1; - let mut prev_aa_config_ix = aa_config_ix; let mut profile_taken = Instant::now(); + let mut modifiers = ModifiersState::default(); // _event_loop is used on non-wasm platforms to create new windows event_loop.run(move |event, _event_loop, control_flow| match event { Event::WindowEvent { @@ -150,6 +151,7 @@ fn run( } match event { WindowEvent::CloseRequested => *control_flow = ControlFlow::Exit, + WindowEvent::ModifiersChanged(m) => modifiers = *m, WindowEvent::KeyboardInput { input, .. } => { if input.state == ElementState::Pressed { match input.virtual_keycode { @@ -180,7 +182,11 @@ fn run( stats.clear_min_and_max(); } Some(VirtualKeyCode::M) => { - aa_config_ix = (aa_config_ix + 1) % AA_CONFIGS.len(); + aa_config_ix = if modifiers.shift() { + aa_config_ix.saturating_sub(1) + } else { + aa_config_ix.saturating_add(1) + }; } Some(VirtualKeyCode::P) => { if let Some(renderer) = &renderers[render_state.surface.dev_id] { @@ -333,27 +339,15 @@ fn run( // Allow looping forever scene_ix = scene_ix.rem_euclid(scenes.scenes.len() as i32); + aa_config_ix = aa_config_ix.rem_euclid(AA_CONFIGS.len() as i32); + let example_scene = &mut scenes.scenes[scene_ix as usize]; - let mut reset_title = false; if prev_scene_ix != scene_ix { transform = Affine::IDENTITY; prev_scene_ix = scene_ix; - reset_title = true; - } - if prev_aa_config_ix != aa_config_ix { - prev_aa_config_ix = aa_config_ix; - reset_title = true; - } - if reset_title { - let aa_str = match AA_CONFIGS[aa_config_ix] { - AaConfig::Area => "Analytic Area", - AaConfig::Msaa16 => "16xMSAA", - AaConfig::Msaa8 => "8xMSAA", - }; - render_state.window.set_title(&format!( - "Vello demo - {} - AA method: {}", - example_scene.config.name, aa_str - )); + render_state + .window + .set_title(&format!("Vello demo - {}", example_scene.config.name)); } let mut builder = SceneBuilder::for_fragment(&mut fragment); let mut scene_params = SceneParams { @@ -371,6 +365,7 @@ fn run( // If the user specifies a base color in the CLI we use that. Otherwise we use any // color specified by the scene. The default is black. + let aa_config = AA_CONFIGS[aa_config_ix as usize]; let render_params = vello::RenderParams { base_color: args .args @@ -379,7 +374,7 @@ fn run( .unwrap_or(Color::BLACK), width, height, - antialiasing_method: AA_CONFIGS[aa_config_ix], + antialiasing_method: aa_config, }; let mut builder = SceneBuilder::for_scene(&mut scene); let mut transform = transform; @@ -400,6 +395,7 @@ fn run( stats.samples(), complexity_shown.then_some(scene_complexity).flatten(), vsync_on, + aa_config, ); if let Some(profiling_result) = renderers[render_state.surface.dev_id] .as_mut() diff --git a/examples/with_winit/src/stats.rs b/examples/with_winit/src/stats.rs index 08d8bd457..a93a21a49 100644 --- a/examples/with_winit/src/stats.rs +++ b/examples/with_winit/src/stats.rs @@ -19,7 +19,7 @@ use std::{collections::VecDeque, time::Duration}; use vello::{ kurbo::{Affine, Line, PathEl, Rect, Stroke}, peniko::{Brush, Color, Fill}, - BumpAllocators, SceneBuilder, + AaConfig, BumpAllocators, SceneBuilder, }; use wgpu_profiler::GpuTimerScopeResult; @@ -44,6 +44,7 @@ impl Snapshot { samples: T, bump: Option, vsync: bool, + aa_config: AaConfig, ) where T: Iterator, { @@ -67,6 +68,14 @@ impl Snapshot { format!("Frame Time (min): {:.2} ms", self.frame_time_min_ms), format!("Frame Time (max): {:.2} ms", self.frame_time_max_ms), format!("VSync: {}", if vsync { "on" } else { "off" }), + format!( + "AA method: {}", + match aa_config { + AaConfig::Area => "Analytic Area", + AaConfig::Msaa16 => "16xMSAA", + AaConfig::Msaa8 => "8xMSAA", + } + ), format!("Resolution: {viewport_width}x{viewport_height}"), ]; if let Some(bump) = &bump { diff --git a/src/lib.rs b/src/lib.rs index 8e08918df..6013575fa 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -110,8 +110,8 @@ pub struct RendererOptions { pub timestamp_period: f32, /// If true, run all stages up to fine rasterization on the CPU. - /// TODO: Consider evolving this so that the CPU stages can be configured dynamically via - /// `RenderParams`. + // TODO: Consider evolving this so that the CPU stages can be configured dynamically via + // `RenderParams`. pub use_cpu: bool, /// The anti-aliasing specialization that should be used when creating the pipelines. `None` diff --git a/src/render.rs b/src/render.rs index 5f5f94141..22e0a1341 100644 --- a/src/render.rs +++ b/src/render.rs @@ -420,7 +420,9 @@ impl Render { match fine.aa_config { AaConfig::Area => { recording.dispatch( - shaders.fine_area.expect("unsupported AA mode: area"), + shaders + .fine_area + .expect("shaders not configured to support AA mode: area"), fine_wg_count, [ fine.config_buf, @@ -443,13 +445,17 @@ impl Render { let buf = recording.upload("mask lut", mask_lut); self.mask_buf = Some(buf.into()); } - let shader = match fine.aa_config { - AaConfig::Msaa16 => shaders.fine_msaa16.expect("unsupported AA mode: msaa16"), - AaConfig::Msaa8 => shaders.fine_msaa8.expect("unsupported AA mode: msaa8"), + let fine_shader = match fine.aa_config { + AaConfig::Msaa16 => shaders + .fine_msaa16 + .expect("shaders not configured to support AA mode: msaa16"), + AaConfig::Msaa8 => shaders + .fine_msaa8 + .expect("shaders not configured to support AA mode: msaa8"), _ => unreachable!(), }; recording.dispatch( - shader, + fine_shader, fine_wg_count, [ fine.config_buf, diff --git a/src/shaders.rs b/src/shaders.rs index cd7231394..5423686f9 100644 --- a/src/shaders.rs +++ b/src/shaders.rs @@ -311,23 +311,21 @@ pub fn full_shaders( // Mask LUT buffer, used only when MSAA is enabled. BindType::BufReadOnly, ]; - let fine_variants = { - const AA_MODES: [AaConfig; 3] = [AaConfig::Area, AaConfig::Msaa8, AaConfig::Msaa16]; - const MSAA_INFO: [Option<(&str, &str)>; 3] = [ - None, - Some(("fine_msaa8", "msaa8")), - Some(("fine_msaa16", "msaa16")), + let [fine_area, fine_msaa8, fine_msaa16] = { + const AA_MODES: [(AaConfig, Option<(&str, &str)>); 3] = [ + (AaConfig::Area, None), + (AaConfig::Msaa8, Some(("fine_msaa8", "msaa8"))), + (AaConfig::Msaa16, Some(("fine_msaa16", "msaa16"))), ]; let mut pipelines = [None, None, None]; - for (i, aa_mode) in AA_MODES.iter().enumerate() { + for (i, (aa_mode, msaa_info)) in AA_MODES.iter().enumerate() { if options .preferred_antialiasing_method - .as_ref() - .map_or(false, |m| m != aa_mode) + .map_or(false, |m| m != *aa_mode) { continue; } - let (range_end_offset, label, aa_config) = match MSAA_INFO[i] { + let (range_end_offset, label, aa_config) = match *msaa_info { Some((label, config)) => (0, label, Some(config)), None => (1, "fine_area", None), }; @@ -365,9 +363,9 @@ pub fn full_shaders( coarse, path_tiling_setup, path_tiling, - fine_area: fine_variants[0], - fine_msaa8: fine_variants[1], - fine_msaa16: fine_variants[2], + fine_area, + fine_msaa8, + fine_msaa16, pathtag_is_cpu: false, }) }