Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hide the async pipeline from the public API #706

Merged
merged 4 commits into from
Oct 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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