Skip to content

Fix: Properly handle scene resets to ensure correct alpha rendering #859

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Mar 18, 2025
Merged

Fix: Properly handle scene resets to ensure correct alpha rendering #859

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0f373b9
Fix: Properly handle scene resets to ensure correct alpha rendering
grebmeg Mar 17, 2025
37f2bd7
Fix: Streamline buffer/texture creation, ensuring size checks & resou…
grebmeg Mar 17, 2025
79cb7a6
Refactor: Move `parsed_svg` out RedrawRequested event
grebmeg Mar 17, 2025
c9235f5
Feat: Add zoom functionality to SVG example
grebmeg Mar 18, 2025
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
30 changes: 14 additions & 16 deletions sparse_strips/vello_hybrid/examples/simple.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,19 +95,6 @@ impl ApplicationHandler for SimpleVelloApp<'_> {
self.renderers[surface.dev_id]
.get_or_insert_with(|| create_vello_renderer(&self.context, &surface));

self.scene.reset();
draw_simple_scene(&mut self.scene);
let device_handle = &self.context.devices[surface.dev_id];
self.renderers[surface.dev_id].as_mut().unwrap().prepare(
&device_handle.device,
&device_handle.queue,
&self.scene,
&RenderParams {
width: surface.config.width,
height: surface.config.height,
},
);

self.state = RenderState::Active {
surface: Box::new(surface),
window,
Expand All @@ -132,9 +119,20 @@ impl ApplicationHandler for SimpleVelloApp<'_> {
.resize_surface(surface, size.width, size.height);
}
WindowEvent::RedrawRequested => {
let width = surface.config.width;
let height = surface.config.height;
self.scene.reset();

draw_simple_scene(&mut self.scene);
let device_handle = &self.context.devices[surface.dev_id];
let render_params = RenderParams {
width: surface.config.width,
height: surface.config.height,
};
self.renderers[surface.dev_id].as_mut().unwrap().prepare(
&device_handle.device,
&device_handle.queue,
&self.scene,
&render_params,
);

let surface_texture = surface
.surface
Expand Down Expand Up @@ -169,7 +167,7 @@ impl ApplicationHandler for SimpleVelloApp<'_> {
self.renderers[surface.dev_id].as_mut().unwrap().render(
&self.scene,
&mut pass,
&RenderParams { width, height },
&render_params,
);
}

Expand Down
97 changes: 78 additions & 19 deletions sparse_strips/vello_hybrid/examples/svg.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,9 @@ use vello_hybrid::{
use wgpu::RenderPassDescriptor;
use winit::{
application::ApplicationHandler,
event::WindowEvent,
event::{ElementState, KeyEvent, MouseScrollDelta, WindowEvent},
event_loop::{ActiveEventLoop, EventLoop},
keyboard::{Key, NamedKey},
window::{Window, WindowId},
};

Expand All @@ -33,6 +34,8 @@ fn main() {
renderers: vec![],
state: RenderState::Suspended(None),
scene: Scene::new(1600, 1200),
render_scale: 5.0,
parsed_svg: None,
};

let event_loop = EventLoop::new().expect("Couldn't create event loop");
Expand All @@ -41,6 +44,11 @@ fn main() {
.expect("Couldn't run event loop");
}

// Constants for zoom behavior
const MIN_SCALE: f64 = 0.1;
const MAX_SCALE: f64 = 20.0;
const ZOOM_STEP: f64 = 0.5;

#[derive(Debug)]
enum RenderState<'s> {
/// `RenderSurface` and `Window` for active rendering.
Expand Down Expand Up @@ -68,6 +76,19 @@ struct SvgVelloApp<'s> {
// description a scene to be drawn (with paths, fills, images, text, etc)
// which is then passed to a renderer for rendering
scene: Scene,

// The scale factor for the rendered SVG
render_scale: f64,

// The parsed SVG
parsed_svg: Option<PicoSvg>,
}

impl SvgVelloApp<'_> {
/// Adjust the render scale by the given delta, clamping to min/max values
fn adjust_scale(&mut self, delta: f64) {
self.render_scale = (self.render_scale + delta).clamp(MIN_SCALE, MAX_SCALE);
}
}

impl ApplicationHandler for SvgVelloApp<'_> {
Expand Down Expand Up @@ -100,23 +121,9 @@ impl ApplicationHandler for SvgVelloApp<'_> {
self.renderers[surface.dev_id]
.get_or_insert_with(|| create_vello_renderer(&self.context, &surface));

self.scene.reset();

let render_scale = 5.0;
let svg_filename = std::env::args().nth(1).expect("svg filename is first arg");
let svg: String = std::fs::read_to_string(svg_filename).expect("error reading file");
let parsed_svg = PicoSvg::load(&svg, 1.0).expect("error parsing SVG");
render_svg(&mut self.scene, render_scale, &parsed_svg.items);
let device_handle = &self.context.devices[surface.dev_id];
self.renderers[surface.dev_id].as_mut().unwrap().prepare(
&device_handle.device,
&device_handle.queue,
&self.scene,
&RenderParams {
width: surface.config.width,
height: surface.config.height,
},
);
self.parsed_svg = Some(PicoSvg::load(&svg, 1.0).expect("error parsing SVG"));

self.state = RenderState::Active {
surface: Box::new(surface),
Expand Down Expand Up @@ -154,10 +161,62 @@ impl ApplicationHandler for SvgVelloApp<'_> {
.resize_surface(surface, size.width, size.height);
}

WindowEvent::MouseWheel {
delta: MouseScrollDelta::PixelDelta(pos),
..
} => {
// Convert pixel delta to a scale adjustment
// Divide by a factor to make the zoom less sensitive
self.adjust_scale(pos.y * ZOOM_STEP / 50.0);
}

WindowEvent::PinchGesture { delta, .. } => {
self.adjust_scale(delta * ZOOM_STEP);
}

WindowEvent::KeyboardInput {
event:
KeyEvent {
logical_key,
state: ElementState::Pressed,
..
},
..
} => {
match logical_key {
Key::Character(c) => match c.as_str() {
"+" | "=" => self.adjust_scale(ZOOM_STEP),
"-" | "_" => self.adjust_scale(-ZOOM_STEP),
// Reset to original scale
"0" => {
self.render_scale = 5.0;
}
_ => {}
},
Key::Named(NamedKey::Escape) => event_loop.exit(),
_ => {}
}
}

WindowEvent::RedrawRequested => {
let width = surface.config.width;
let height = surface.config.height;
self.scene.reset();

if let Some(parsed_svg) = &self.parsed_svg {
render_svg(&mut self.scene, self.render_scale, &parsed_svg.items);
}

let device_handle = &self.context.devices[surface.dev_id];
let render_params = RenderParams {
width: surface.config.width,
height: surface.config.height,
};
self.renderers[surface.dev_id].as_mut().unwrap().prepare(
&device_handle.device,
&device_handle.queue,
&self.scene,
&render_params,
);

let surface_texture = surface
.surface
.get_current_texture()
Expand Down Expand Up @@ -190,7 +249,7 @@ impl ApplicationHandler for SvgVelloApp<'_> {
self.renderers[surface.dev_id].as_mut().unwrap().render(
&self.scene,
&mut pass,
&RenderParams { width, height },
&render_params,
);
}
device_handle.queue.submit([encoder.finish()]);
Expand Down
156 changes: 95 additions & 61 deletions sparse_strips/vello_hybrid/src/render.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,80 +204,110 @@ impl Renderer {
let render_data = scene.prepare_render_data();
let required_strips_size = size_of::<GpuStrip>() as u64 * render_data.strips.len() as u64;

// Check if we need to create new resources or resize existing ones
let needs_new_resources = match &self.resources {
Some(resources) => required_strips_size > resources.strips_buffer.size(),
None => true,
};
let (needs_new_strips_buffer, needs_new_alpha_texture) = match &self.resources {
Copy link
Member

@DJMcNab DJMcNab Mar 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some of this logic is quite hard to follow; there's a lot more unwrap/expect than I'd normally expect (ha). That is, I feel like the three stages of "check if anything needs to be recreated, then recreate them, then access them", could be combined better.

That being said, I don't think that refactoring that now is helpful, and so I'm happy for this to land.

Copy link
Collaborator Author

@grebmeg grebmeg Mar 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a similar feeling but haven’t yet come up with a better approach to managing resource preparation from None to Some and subsequently validating whether recreation is necessary. One idea is to precreate all buffers/textures in new, eliminating the need to check for None and only requiring checks for resizing buffers/textures. I think I just need to spend more time refining this and will revisit it a bit later.

Some(resources) => {
let strips_too_small = required_strips_size > resources.strips_buffer.size();

if needs_new_resources {
let strips_buffer = device.create_buffer(&wgpu::BufferDescriptor {
label: Some("Strips Buffer"),
size: required_strips_size,
usage: wgpu::BufferUsages::VERTEX | wgpu::BufferUsages::COPY_DST,
mapped_at_creation: false,
});
let max_texture_dimension_2d = device.limits().max_texture_dimension_2d;
let alpha_len = render_data.alphas.len();
// 4 alpha values u32 each per texel
let required_alpha_height =
(u32::try_from(alpha_len).unwrap()).div_ceil(max_texture_dimension_2d * 4);
let required_alpha_size = max_texture_dimension_2d * required_alpha_height * 4;

let max_texture_dimension_2d = device.limits().max_texture_dimension_2d;
let alpha_len = render_data.alphas.len();
// 4 alpha values u32 each per texel
let alpha_texture_height =
(u32::try_from(alpha_len).unwrap()).div_ceil(max_texture_dimension_2d * 4);
let current_alpha_size =
resources.alphas_texture.width() * resources.alphas_texture.height() * 4;
let alpha_too_small = required_alpha_size > current_alpha_size;

// Ensure dimensions don't exceed WebGL2 limits
assert!(
alpha_texture_height <= max_texture_dimension_2d,
"Alpha texture height exceeds WebGL2 limit"
);
(strips_too_small, alpha_too_small)
}
None => (true, true),
};

let alphas_texture = device.create_texture(&wgpu::TextureDescriptor {
label: Some("Alpha Texture"),
size: wgpu::Extent3d {
width: max_texture_dimension_2d,
height: alpha_texture_height,
depth_or_array_layers: 1,
},
mip_level_count: 1,
sample_count: 1,
dimension: wgpu::TextureDimension::D2,
format: wgpu::TextureFormat::Rgba32Uint,
usage: wgpu::TextureUsages::TEXTURE_BINDING | wgpu::TextureUsages::COPY_DST,
view_formats: &[],
});
let alphas_texture_view =
alphas_texture.create_view(&wgpu::TextureViewDescriptor::default());
if needs_new_strips_buffer || needs_new_alpha_texture {
let strips_buffer = if needs_new_strips_buffer {
device.create_buffer(&wgpu::BufferDescriptor {
label: Some("Strips Buffer"),
size: required_strips_size,
usage: wgpu::BufferUsages::VERTEX | wgpu::BufferUsages::COPY_DST,
mapped_at_creation: false,
})
} else {
// Reuse existing buffer if it's big enough
self.resources
.as_ref()
.expect("Strips buffer not found")
.strips_buffer
.clone()
};
let (alphas_texture, render_bind_group) = if needs_new_alpha_texture {
let max_texture_dimension_2d = device.limits().max_texture_dimension_2d;
let alpha_len = render_data.alphas.len();
// 4 alpha values u32 each per texel
let alpha_texture_height =
(u32::try_from(alpha_len).unwrap()).div_ceil(max_texture_dimension_2d * 4);

let config_buf = device.create_buffer_init(&wgpu::util::BufferInitDescriptor {
label: Some("Config Buffer"),
contents: bytemuck::bytes_of(&Config {
width: render_params.width,
height: render_params.height,
strip_height: Tile::HEIGHT.into(),
}),
usage: wgpu::BufferUsages::UNIFORM,
});
// Ensure dimensions don't exceed WebGL2 limits
assert!(
alpha_texture_height <= max_texture_dimension_2d,
"Alpha texture height exceeds WebGL2 limit"
);

let render_bind_group = device.create_bind_group(&wgpu::BindGroupDescriptor {
label: Some("Render Bind Group"),
layout: &self.render_bind_group_layout,
entries: &[
wgpu::BindGroupEntry {
binding: 0,
resource: wgpu::BindingResource::TextureView(&alphas_texture_view),
let alphas_texture = device.create_texture(&wgpu::TextureDescriptor {
label: Some("Alpha Texture"),
size: wgpu::Extent3d {
width: max_texture_dimension_2d,
height: alpha_texture_height,
depth_or_array_layers: 1,
},
wgpu::BindGroupEntry {
binding: 1,
resource: config_buf.as_entire_binding(),
},
],
});
mip_level_count: 1,
sample_count: 1,
dimension: wgpu::TextureDimension::D2,
format: wgpu::TextureFormat::Rgba32Uint,
usage: wgpu::TextureUsages::TEXTURE_BINDING | wgpu::TextureUsages::COPY_DST,
view_formats: &[],
});
let alphas_texture_view =
alphas_texture.create_view(&wgpu::TextureViewDescriptor::default());

let config_buf = device.create_buffer_init(&wgpu::util::BufferInitDescriptor {
label: Some("Config Buffer"),
contents: bytemuck::bytes_of(&Config {
width: render_params.width,
height: render_params.height,
strip_height: Tile::HEIGHT.into(),
}),
usage: wgpu::BufferUsages::UNIFORM,
});

let render_bind_group = device.create_bind_group(&wgpu::BindGroupDescriptor {
label: Some("Render Bind Group"),
layout: &self.render_bind_group_layout,
entries: &[
wgpu::BindGroupEntry {
binding: 0,
resource: wgpu::BindingResource::TextureView(&alphas_texture_view),
},
wgpu::BindGroupEntry {
binding: 1,
resource: config_buf.as_entire_binding(),
},
],
});
(alphas_texture, render_bind_group)
} else {
let resources = self.resources.as_ref().unwrap();
(
resources.alphas_texture.clone(),
resources.render_bind_group.clone(),
)
};
self.resources = Some(GpuResources {
strips_buffer,
alphas_texture,
render_bind_group,
});
}
};

// Now that we have resources, we can update the data
if let Some(resources) = &self.resources {
Expand All @@ -291,6 +321,10 @@ impl Renderer {
// Prepare alpha data for the texture with 4 alpha values per texel
let texture_width = resources.alphas_texture.width();
let texture_height = resources.alphas_texture.height();
assert!(
render_data.alphas.len() <= (texture_width * texture_height * 4) as usize,
"Alpha texture dimensions are too small to fit the alpha data"
);
let mut alpha_data = vec![0_u32; render_data.alphas.len()];
alpha_data[..].copy_from_slice(&render_data.alphas);
alpha_data.resize((texture_width * texture_height * 4) as usize, 0);
Expand Down
Loading