From 60ac6907a726ed666ee2b65d795b83d8b502c321 Mon Sep 17 00:00:00 2001 From: Daniel McNab <36049421+DJMcNab@users.noreply.github.com> Date: Fri, 27 Sep 2024 10:46:02 +0100 Subject: [PATCH 1/4] Remove the async pipeline from the public API --- examples/headless/src/main.rs | 1 - examples/simple/src/main.rs | 3 +-- examples/simple_sdl2/src/main.rs | 3 +-- examples/with_winit/src/lib.rs | 4 +++- vello/src/debug/renderer.rs | 12 +++++----- vello/src/lib.rs | 38 ++++++++++++++++++++++++-------- vello/src/render.rs | 10 --------- vello_tests/src/lib.rs | 1 - 8 files changed, 40 insertions(+), 32 deletions(-) diff --git a/examples/headless/src/main.rs b/examples/headless/src/main.rs index 98d00c3f..c5cb8b06 100644 --- a/examples/headless/src/main.rs +++ b/examples/headless/src/main.rs @@ -139,7 +139,6 @@ async fn render(mut scenes: SceneSet, index: usize, args: &Args) -> Result<()> { width, height, antialiasing_method: vello::AaConfig::Area, - debug: vello::DebugLayers::none(), }; let mut scene = Scene::new(); scene.append(&fragment, Some(transform)); diff --git a/examples/simple/src/main.rs b/examples/simple/src/main.rs index d08e7d2d..d0e997d0 100644 --- a/examples/simple/src/main.rs +++ b/examples/simple/src/main.rs @@ -7,7 +7,7 @@ use std::sync::Arc; use vello::kurbo::{Affine, Circle, Ellipse, Line, RoundedRect, Stroke}; use vello::peniko::Color; use vello::util::{RenderContext, RenderSurface}; -use vello::{AaConfig, DebugLayers, Renderer, RendererOptions, Scene}; +use vello::{AaConfig, Renderer, RendererOptions, Scene}; use winit::application::ApplicationHandler; use winit::dpi::LogicalSize; use winit::event::*; @@ -147,7 +147,6 @@ impl<'s> ApplicationHandler for SimpleVelloApp<'s> { width, height, antialiasing_method: AaConfig::Msaa16, - debug: DebugLayers::none(), }, ) .expect("failed to render to surface"); diff --git a/examples/simple_sdl2/src/main.rs b/examples/simple_sdl2/src/main.rs index 5adacf92..9db667ed 100644 --- a/examples/simple_sdl2/src/main.rs +++ b/examples/simple_sdl2/src/main.rs @@ -16,7 +16,7 @@ use std::num::NonZeroUsize; use vello::kurbo::{Affine, Circle, Ellipse, Line, RoundedRect, Stroke}; use vello::peniko::Color; use vello::util::{RenderContext, RenderSurface}; -use vello::{AaConfig, DebugLayers, Renderer, RendererOptions, Scene}; +use vello::{AaConfig, Renderer, RendererOptions, Scene}; use vello::wgpu; @@ -84,7 +84,6 @@ pub fn main() { width, height, antialiasing_method: AaConfig::Msaa16, - debug: DebugLayers::none(), }, ) .expect("failed to render to surface"); diff --git a/examples/with_winit/src/lib.rs b/examples/with_winit/src/lib.rs index 576452d2..86489795 100644 --- a/examples/with_winit/src/lib.rs +++ b/examples/with_winit/src/lib.rs @@ -487,7 +487,6 @@ impl<'s> ApplicationHandler for VelloApp<'s> { width, height, antialiasing_method, - debug: self.debug, }; self.scene.reset(); let mut transform = self.transform; @@ -547,6 +546,8 @@ impl<'s> ApplicationHandler for VelloApp<'s> { // Note: we don't run the async/"robust" pipeline, as // it requires more async wiring for the readback. See // [#gpu > async on wasm](https://xi.zulipchat.com/#narrow/stream/197075-gpu/topic/async.20on.20wasm) + #[allow(deprecated)] + // #[expect(deprecated, reason = "This deprecation is not targeted at us.")] // Our MSRV is too low to use `expect` if self.async_pipeline && cfg!(not(target_arch = "wasm32")) { self.scene_complexity = vello::block_on_wgpu( &device_handle.device, @@ -559,6 +560,7 @@ impl<'s> ApplicationHandler for VelloApp<'s> { &self.scene, &surface_texture, &render_params, + self.debug, ), ) .expect("failed to render to surface"); diff --git a/vello/src/debug/renderer.rs b/vello/src/debug/renderer.rs index adc8380e..03b4f753 100644 --- a/vello/src/debug/renderer.rs +++ b/vello/src/debug/renderer.rs @@ -215,13 +215,13 @@ impl DebugRenderer { bump: &BumpAllocators, params: &RenderParams, downloads: &DebugDownloads, + layers: DebugLayers, ) { - if params.debug.is_empty() { + if layers.is_empty() { return; } - let (unpaired_pts_len, unpaired_pts_buf) = if params.debug.contains(DebugLayers::VALIDATION) - { + let (unpaired_pts_len, unpaired_pts_buf) = if layers.contains(DebugLayers::VALIDATION) { // TODO: have this write directly to a GPU buffer? let unpaired_pts: Vec = validate_line_soup(bytemuck::cast_slice(&downloads.lines.get_mapped_range())); @@ -266,7 +266,7 @@ impl DebugRenderer { target, clear_color: None, }); - if params.debug.contains(DebugLayers::BOUNDING_BOXES) { + if layers.contains(DebugLayers::BOUNDING_BOXES) { recording.draw(DrawParams { shader_id: self.bboxes, instance_count: captured.sizes.path_bboxes.len(), @@ -277,7 +277,7 @@ impl DebugRenderer { clear_color: None, }); } - if params.debug.contains(DebugLayers::LINESOUP_SEGMENTS) { + if layers.contains(DebugLayers::LINESOUP_SEGMENTS) { recording.draw(DrawParams { shader_id: self.linesoup, instance_count: bump.lines, @@ -288,7 +288,7 @@ impl DebugRenderer { clear_color: None, }); } - if params.debug.contains(DebugLayers::LINESOUP_POINTS) { + if layers.contains(DebugLayers::LINESOUP_POINTS) { recording.draw(DrawParams { shader_id: self.linesoup_points, instance_count: bump.lines, diff --git a/vello/src/lib.rs b/vello/src/lib.rs index fc6dbd72..53039714 100644 --- a/vello/src/lib.rs +++ b/vello/src/lib.rs @@ -89,6 +89,7 @@ mod shaders; #[cfg(feature = "wgpu")] mod wgpu_engine; +use std::sync::atomic::AtomicBool; #[cfg(feature = "wgpu")] use std::{num::NonZeroUsize, sync::Arc}; @@ -282,13 +283,6 @@ pub struct RenderParams { /// The anti-aliasing algorithm. The selected algorithm must have been initialized while /// constructing the `Renderer`. pub antialiasing_method: AaConfig, - - /// Options for debug layer rendering. - /// - /// This only has an effect when the `debug_layers` feature is enabled. - // This is exposed publicly as a least-effort to avoid changing the API when features change. - // We expect the API to change here in the near future. - pub debug: DebugLayers, } #[cfg(feature = "wgpu")] @@ -518,7 +512,9 @@ impl Renderer { Ok(()) } - /// Renders a scene to the target texture. + /// Renders a scene to the target texture using an async pipeline. + /// + /// Almost all consumers should prefer [`Self::render_to_texture`]. /// /// The texture is assumed to be of the specified dimensions and have been created with /// the [`wgpu::TextureFormat::Rgba8Unorm`] format and the [`wgpu::TextureUsages::STORAGE_BINDING`] @@ -529,6 +525,10 @@ impl Renderer { /// /// This return type is not stable, and will likely be changed when a more principled way to access /// relevant statistics is implemented + #[cfg_attr(docsrs, doc(hidden))] + #[deprecated( + note = "render_to_texture should be preferred, as the _async version has no stability guarantees" + )] pub async fn render_to_texture_async( &mut self, device: &Device, @@ -630,7 +630,15 @@ impl Renderer { }) } - /// See [`Self::render_to_surface`] + /// This is a version of [`render_to_surface`](Self::render_to_surface) which uses an async pipeline + /// to allow improved debugging of Vello itself. + /// Most users should prefer `render_to_surface`. + /// + /// See [`render_to_texture_async`](Self::render_to_texture_async) for more details. + #[cfg_attr(docsrs, doc(hidden))] + #[deprecated( + note = "render_to_surface should be preferred, as the _async version has no stability guarantees" + )] pub async fn render_to_surface_async( &mut self, device: &Device, @@ -638,7 +646,18 @@ impl Renderer { scene: &Scene, surface: &SurfaceTexture, params: &RenderParams, + debug_layers: DebugLayers, ) -> Result> { + if cfg!(not(feature = "debug_layers")) && !debug_layers.is_empty() { + static HAS_WARNED: AtomicBool = AtomicBool::new(false); + if !HAS_WARNED.swap(true, std::sync::atomic::Ordering::Release) { + log::warn!( + "Requested debug layers {debug:?} but `debug_layers` feature is not enabled.", + debug = debug_layers + ); + } + } + let width = params.width; let height = params.height; let mut target = self @@ -691,6 +710,7 @@ impl Renderer { bump, params, &downloads, + debug_layers, ); // TODO: this sucks. better to release everything in a helper diff --git a/vello/src/render.rs b/vello/src/render.rs index 9529bea5..cd26c369 100644 --- a/vello/src/render.rs +++ b/vello/src/render.rs @@ -4,7 +4,6 @@ //! Take an encoded scene and create a graph to render it use std::mem::size_of; -use std::sync::atomic::AtomicBool; use crate::recording::{BufferProxy, ImageFormat, ImageProxy, Recording, ResourceProxy}; use crate::shaders::FullShaders; @@ -149,15 +148,6 @@ impl Render { data, )) }; - if cfg!(not(feature = "debug_layers")) && !params.debug.is_empty() { - static HAS_WARNED: AtomicBool = AtomicBool::new(false); - if !HAS_WARNED.swap(true, std::sync::atomic::Ordering::Release) { - log::warn!( - "Requested debug layers {debug:?} but `debug_layers` feature is not enabled.", - debug = params.debug - ); - } - } let image_atlas = if images.images.is_empty() { ImageProxy::new(1, 1, ImageFormat::Rgba8) } else { diff --git a/vello_tests/src/lib.rs b/vello_tests/src/lib.rs index f9b3c1c1..42b78ea0 100644 --- a/vello_tests/src/lib.rs +++ b/vello_tests/src/lib.rs @@ -100,7 +100,6 @@ pub async fn get_scene_image(params: &TestParams, scene: &Scene) -> Result Date: Fri, 27 Sep 2024 10:56:25 +0100 Subject: [PATCH 2/4] =?UTF-8?q?=F0=9F=93=8E?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- vello/src/debug/renderer.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/vello/src/debug/renderer.rs b/vello/src/debug/renderer.rs index 03b4f753..3be8f4d1 100644 --- a/vello/src/debug/renderer.rs +++ b/vello/src/debug/renderer.rs @@ -207,6 +207,8 @@ impl DebugRenderer { } } + #[allow(clippy::too_many_arguments)] + // #[expect(clippy::too_many_arguments, reason="This function is internal, so the argument count doesn't cause issues for consumers.")] pub fn render( &self, recording: &mut Recording, From 5f4e8db3de498950b9b28ac3b1e9f08b53db3743 Mon Sep 17 00:00:00 2001 From: Daniel McNab <36049421+DJMcNab@users.noreply.github.com> Date: Fri, 27 Sep 2024 11:12:04 +0100 Subject: [PATCH 3/4] Correct import granularity --- vello/src/lib.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/vello/src/lib.rs b/vello/src/lib.rs index 53039714..ff4995fe 100644 --- a/vello/src/lib.rs +++ b/vello/src/lib.rs @@ -89,9 +89,11 @@ mod shaders; #[cfg(feature = "wgpu")] mod wgpu_engine; -use std::sync::atomic::AtomicBool; #[cfg(feature = "wgpu")] -use std::{num::NonZeroUsize, sync::Arc}; +use std::{ + num::NonZeroUsize, + sync::{atomic::AtomicBool, Arc}, +}; /// Styling and composition primitives. pub use peniko; From 39438739dc5cfdf430bec1b2282b6ed86f688523 Mon Sep 17 00:00:00 2001 From: Daniel McNab <36049421+DJMcNab@users.noreply.github.com> Date: Thu, 3 Oct 2024 10:27:36 +0100 Subject: [PATCH 4/4] Hide some more items which are only useful with the async pipeline --- vello/src/debug.rs | 1 + vello/src/lib.rs | 1 + vello/src/util.rs | 3 ++- 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/vello/src/debug.rs b/vello/src/debug.rs index 5ddf89e1..918fd0dd 100644 --- a/vello/src/debug.rs +++ b/vello/src/debug.rs @@ -14,6 +14,7 @@ pub(crate) use renderer::*; /// Bitflags for enabled debug operations. /// /// Currently, all layers additionally require the `debug_layers` feature. +#[cfg_attr(docsrs, doc(hidden))] #[derive(Copy, Clone)] pub struct DebugLayers(u8); diff --git a/vello/src/lib.rs b/vello/src/lib.rs index ff4995fe..d665f77b 100644 --- a/vello/src/lib.rs +++ b/vello/src/lib.rs @@ -113,6 +113,7 @@ pub use render::Render; pub use scene::{DrawGlyphs, Scene}; use thiserror::Error; #[cfg(feature = "wgpu")] +#[cfg_attr(docsrs, doc(hidden))] pub use util::block_on_wgpu; pub use recording::{ diff --git a/vello/src/util.rs b/vello/src/util.rs index 466448f2..5a508dbb 100644 --- a/vello/src/util.rs +++ b/vello/src/util.rs @@ -190,9 +190,10 @@ impl std::task::Wake for NullWake { /// Block on a future, polling the device as needed. /// /// This will deadlock if the future is awaiting anything other than GPU progress. +#[cfg_attr(docsrs, doc(hidden))] pub fn block_on_wgpu(device: &Device, mut fut: F) -> F::Output { if cfg!(target_arch = "wasm32") { - panic!("Blocking can't work on WASM, so"); + panic!("Blocking can't work on WASM, so don't try"); } let waker = std::task::Waker::from(std::sync::Arc::new(NullWake)); let mut context = std::task::Context::from_waker(&waker);