Skip to content

Commit

Permalink
Hide the async pipeline from the public API (#706)
Browse files Browse the repository at this point in the history
In doing ports to Vello 0.3 in our other repositories, I kept running
into that adding the `debug_layers` field needed to be added, and I
don't think it will be clear to users what it's for.

In addition, there is no reason to use the async pipeline for end users
(at least as things stand). In #694, the debug layers feature was marked
as internal, so that isn't a good reason.
  • Loading branch information
DJMcNab authored Oct 3, 2024
1 parent 30c5e8e commit 88e59c3
Show file tree
Hide file tree
Showing 10 changed files with 49 additions and 34 deletions.
1 change: 0 additions & 1 deletion examples/headless/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
3 changes: 1 addition & 2 deletions examples/simple/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::*;
Expand Down Expand Up @@ -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");
Expand Down
3 changes: 1 addition & 2 deletions examples/simple_sdl2/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -84,7 +84,6 @@ pub fn main() {
width,
height,
antialiasing_method: AaConfig::Msaa16,
debug: DebugLayers::none(),
},
)
.expect("failed to render to surface");
Expand Down
4 changes: 3 additions & 1 deletion examples/with_winit/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -487,7 +487,6 @@ impl<'s> ApplicationHandler<UserEvent> for VelloApp<'s> {
width,
height,
antialiasing_method,
debug: self.debug,
};
self.scene.reset();
let mut transform = self.transform;
Expand Down Expand Up @@ -547,6 +546,8 @@ impl<'s> ApplicationHandler<UserEvent> 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,
Expand All @@ -559,6 +560,7 @@ impl<'s> ApplicationHandler<UserEvent> for VelloApp<'s> {
&self.scene,
&surface_texture,
&render_params,
self.debug,
),
)
.expect("failed to render to surface");
Expand Down
1 change: 1 addition & 0 deletions vello/src/debug.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
14 changes: 8 additions & 6 deletions vello/src/debug/renderer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -215,13 +217,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<LineEndpoint> =
validate_line_soup(bytemuck::cast_slice(&downloads.lines.get_mapped_range()));
Expand Down Expand Up @@ -266,7 +268,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(),
Expand All @@ -277,7 +279,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,
Expand All @@ -288,7 +290,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,
Expand Down
43 changes: 33 additions & 10 deletions vello/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,10 @@ mod shaders;
mod wgpu_engine;

#[cfg(feature = "wgpu")]
use std::{num::NonZeroUsize, sync::Arc};
use std::{
num::NonZeroUsize,
sync::{atomic::AtomicBool, Arc},
};

/// Styling and composition primitives.
pub use peniko;
Expand All @@ -110,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::{
Expand Down Expand Up @@ -282,13 +286,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")]
Expand Down Expand Up @@ -518,7 +515,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`]
Expand All @@ -529,6 +528,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,
Expand Down Expand Up @@ -630,15 +633,34 @@ 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,
queue: &Queue,
scene: &Scene,
surface: &SurfaceTexture,
params: &RenderParams,
debug_layers: DebugLayers,
) -> Result<Option<BumpAllocators>> {
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
Expand Down Expand Up @@ -691,6 +713,7 @@ impl Renderer {
bump,
params,
&downloads,
debug_layers,
);

// TODO: this sucks. better to release everything in a helper
Expand Down
10 changes: 0 additions & 10 deletions vello/src/render.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand Down
3 changes: 2 additions & 1 deletion vello/src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<F: Future>(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);
Expand Down
1 change: 0 additions & 1 deletion vello_tests/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,6 @@ pub async fn get_scene_image(params: &TestParams, scene: &Scene) -> Result<Image
width,
height,
antialiasing_method: params.anti_aliasing,
debug: vello::DebugLayers::none(),
};
let size = Extent3d {
width,
Expand Down

0 comments on commit 88e59c3

Please sign in to comment.