Skip to content

Commit 6f22a89

Browse files
authored
Fix: Properly handle scene resets to ensure correct alpha rendering (#859)
### Overview This PR addresses the visual artifacts (dotted/dashed lines) that appear when rendering the same scene multiple times using the `vello_hybrid` renderer. <img width="532" alt="image" src="https://github.com/user-attachments/assets/da632491-987e-4f6f-a4dc-2ad4fb730282" /> ### Issue Description Previously, when resetting the scene and re-rendering repeatedly, strange visual artifacts would appear at the boundaries between shapes - specifically dotted or dashed lines along what should be properly alpha sampled boundaries. These artifacts only appeared when: - The scene was reset and re-rendered in `RedrawRequested` event handler - The `prepare` method was called after each re-render ### Root Cause The root cause was a misalignment between the alpha values stored in the scene and the GPU alpha texture: - When `Scene::reset` was called, it correctly cleared the alphas array in the scene (reset added as a separate commit) - When `render_svg` was called, it populated the scene with new alpha values - However, `Renderer::prepare` method only created a new alpha texture when the strips buffer size increased - This led to a situation where new alpha values were being written to a texture sized for previous frames, or, if alphas weren’t properly reset, they retained stale data from previous frames, causing textures to grow larger each frame, meantime alpha data copying size was restricted to the current texture size. - The shader was calculating texture coordinates based on strip `col` values, but these values pointed to positions that might exceed the texture dimensions - Due to GPU texture addressing behavior, these out-of-bounds accesses "wrapped around" silently, causing the shader to sample completely unrelated alpha values The critical issue was that we were only checking if the strips buffer needed to be recreated, but not independently verifying if the alpha texture needed to be recreated when only the alpha values changed. ### Solution Fixed the issue by modifying `prepare` method to: - Separately check if the strips buffer or alpha texture needs to be recreated - Create a new alpha texture whenever the required texture size exceeds the current texture size - Ensure both resources are properly sized for the current frame's data before writing to them - Add assertions to verify the alpha texture is large enough to hold all the alpha values This ensures that the alpha texture always matches the current frame's alpha values, maintaining the correct relationship between strip `col` indices and texture coordinates, eliminating the visual artifacts. ### Notes - `Scene::reset` method has been updated to properly (fully) reset the scene state by clearing all data
1 parent 654e47c commit 6f22a89

File tree

4 files changed

+230
-113
lines changed

4 files changed

+230
-113
lines changed

sparse_strips/vello_hybrid/examples/simple.rs

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -95,19 +95,6 @@ impl ApplicationHandler for SimpleVelloApp<'_> {
9595
self.renderers[surface.dev_id]
9696
.get_or_insert_with(|| create_vello_renderer(&self.context, &surface));
9797

98-
self.scene.reset();
99-
draw_simple_scene(&mut self.scene);
100-
let device_handle = &self.context.devices[surface.dev_id];
101-
self.renderers[surface.dev_id].as_mut().unwrap().prepare(
102-
&device_handle.device,
103-
&device_handle.queue,
104-
&self.scene,
105-
&RenderParams {
106-
width: surface.config.width,
107-
height: surface.config.height,
108-
},
109-
);
110-
11198
self.state = RenderState::Active {
11299
surface: Box::new(surface),
113100
window,
@@ -132,9 +119,20 @@ impl ApplicationHandler for SimpleVelloApp<'_> {
132119
.resize_surface(surface, size.width, size.height);
133120
}
134121
WindowEvent::RedrawRequested => {
135-
let width = surface.config.width;
136-
let height = surface.config.height;
122+
self.scene.reset();
123+
124+
draw_simple_scene(&mut self.scene);
137125
let device_handle = &self.context.devices[surface.dev_id];
126+
let render_params = RenderParams {
127+
width: surface.config.width,
128+
height: surface.config.height,
129+
};
130+
self.renderers[surface.dev_id].as_mut().unwrap().prepare(
131+
&device_handle.device,
132+
&device_handle.queue,
133+
&self.scene,
134+
&render_params,
135+
);
138136

139137
let surface_texture = surface
140138
.surface
@@ -169,7 +167,7 @@ impl ApplicationHandler for SimpleVelloApp<'_> {
169167
self.renderers[surface.dev_id].as_mut().unwrap().render(
170168
&self.scene,
171169
&mut pass,
172-
&RenderParams { width, height },
170+
&render_params,
173171
);
174172
}
175173

sparse_strips/vello_hybrid/examples/svg.rs

Lines changed: 78 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,9 @@ use vello_hybrid::{
2020
use wgpu::RenderPassDescriptor;
2121
use winit::{
2222
application::ApplicationHandler,
23-
event::WindowEvent,
23+
event::{ElementState, KeyEvent, MouseScrollDelta, WindowEvent},
2424
event_loop::{ActiveEventLoop, EventLoop},
25+
keyboard::{Key, NamedKey},
2526
window::{Window, WindowId},
2627
};
2728

@@ -33,6 +34,8 @@ fn main() {
3334
renderers: vec![],
3435
state: RenderState::Suspended(None),
3536
scene: Scene::new(1600, 1200),
37+
render_scale: 5.0,
38+
parsed_svg: None,
3639
};
3740

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

47+
// Constants for zoom behavior
48+
const MIN_SCALE: f64 = 0.1;
49+
const MAX_SCALE: f64 = 20.0;
50+
const ZOOM_STEP: f64 = 0.5;
51+
4452
#[derive(Debug)]
4553
enum RenderState<'s> {
4654
/// `RenderSurface` and `Window` for active rendering.
@@ -68,6 +76,19 @@ struct SvgVelloApp<'s> {
6876
// description a scene to be drawn (with paths, fills, images, text, etc)
6977
// which is then passed to a renderer for rendering
7078
scene: Scene,
79+
80+
// The scale factor for the rendered SVG
81+
render_scale: f64,
82+
83+
// The parsed SVG
84+
parsed_svg: Option<PicoSvg>,
85+
}
86+
87+
impl SvgVelloApp<'_> {
88+
/// Adjust the render scale by the given delta, clamping to min/max values
89+
fn adjust_scale(&mut self, delta: f64) {
90+
self.render_scale = (self.render_scale + delta).clamp(MIN_SCALE, MAX_SCALE);
91+
}
7192
}
7293

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

103-
self.scene.reset();
104-
105-
let render_scale = 5.0;
106124
let svg_filename = std::env::args().nth(1).expect("svg filename is first arg");
107125
let svg: String = std::fs::read_to_string(svg_filename).expect("error reading file");
108-
let parsed_svg = PicoSvg::load(&svg, 1.0).expect("error parsing SVG");
109-
render_svg(&mut self.scene, render_scale, &parsed_svg.items);
110-
let device_handle = &self.context.devices[surface.dev_id];
111-
self.renderers[surface.dev_id].as_mut().unwrap().prepare(
112-
&device_handle.device,
113-
&device_handle.queue,
114-
&self.scene,
115-
&RenderParams {
116-
width: surface.config.width,
117-
height: surface.config.height,
118-
},
119-
);
126+
self.parsed_svg = Some(PicoSvg::load(&svg, 1.0).expect("error parsing SVG"));
120127

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

164+
WindowEvent::MouseWheel {
165+
delta: MouseScrollDelta::PixelDelta(pos),
166+
..
167+
} => {
168+
// Convert pixel delta to a scale adjustment
169+
// Divide by a factor to make the zoom less sensitive
170+
self.adjust_scale(pos.y * ZOOM_STEP / 50.0);
171+
}
172+
173+
WindowEvent::PinchGesture { delta, .. } => {
174+
self.adjust_scale(delta * ZOOM_STEP);
175+
}
176+
177+
WindowEvent::KeyboardInput {
178+
event:
179+
KeyEvent {
180+
logical_key,
181+
state: ElementState::Pressed,
182+
..
183+
},
184+
..
185+
} => {
186+
match logical_key {
187+
Key::Character(c) => match c.as_str() {
188+
"+" | "=" => self.adjust_scale(ZOOM_STEP),
189+
"-" | "_" => self.adjust_scale(-ZOOM_STEP),
190+
// Reset to original scale
191+
"0" => {
192+
self.render_scale = 5.0;
193+
}
194+
_ => {}
195+
},
196+
Key::Named(NamedKey::Escape) => event_loop.exit(),
197+
_ => {}
198+
}
199+
}
200+
157201
WindowEvent::RedrawRequested => {
158-
let width = surface.config.width;
159-
let height = surface.config.height;
202+
self.scene.reset();
203+
204+
if let Some(parsed_svg) = &self.parsed_svg {
205+
render_svg(&mut self.scene, self.render_scale, &parsed_svg.items);
206+
}
207+
160208
let device_handle = &self.context.devices[surface.dev_id];
209+
let render_params = RenderParams {
210+
width: surface.config.width,
211+
height: surface.config.height,
212+
};
213+
self.renderers[surface.dev_id].as_mut().unwrap().prepare(
214+
&device_handle.device,
215+
&device_handle.queue,
216+
&self.scene,
217+
&render_params,
218+
);
219+
161220
let surface_texture = surface
162221
.surface
163222
.get_current_texture()
@@ -190,7 +249,7 @@ impl ApplicationHandler for SvgVelloApp<'_> {
190249
self.renderers[surface.dev_id].as_mut().unwrap().render(
191250
&self.scene,
192251
&mut pass,
193-
&RenderParams { width, height },
252+
&render_params,
194253
);
195254
}
196255
device_handle.queue.submit([encoder.finish()]);

sparse_strips/vello_hybrid/src/render.rs

Lines changed: 95 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -204,80 +204,110 @@ impl Renderer {
204204
let render_data = scene.prepare_render_data();
205205
let required_strips_size = size_of::<GpuStrip>() as u64 * render_data.strips.len() as u64;
206206

207-
// Check if we need to create new resources or resize existing ones
208-
let needs_new_resources = match &self.resources {
209-
Some(resources) => required_strips_size > resources.strips_buffer.size(),
210-
None => true,
211-
};
207+
let (needs_new_strips_buffer, needs_new_alpha_texture) = match &self.resources {
208+
Some(resources) => {
209+
let strips_too_small = required_strips_size > resources.strips_buffer.size();
212210

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

221-
let max_texture_dimension_2d = device.limits().max_texture_dimension_2d;
222-
let alpha_len = render_data.alphas.len();
223-
// 4 alpha values u32 each per texel
224-
let alpha_texture_height =
225-
(u32::try_from(alpha_len).unwrap()).div_ceil(max_texture_dimension_2d * 4);
218+
let current_alpha_size =
219+
resources.alphas_texture.width() * resources.alphas_texture.height() * 4;
220+
let alpha_too_small = required_alpha_size > current_alpha_size;
226221

227-
// Ensure dimensions don't exceed WebGL2 limits
228-
assert!(
229-
alpha_texture_height <= max_texture_dimension_2d,
230-
"Alpha texture height exceeds WebGL2 limit"
231-
);
222+
(strips_too_small, alpha_too_small)
223+
}
224+
None => (true, true),
225+
};
232226

233-
let alphas_texture = device.create_texture(&wgpu::TextureDescriptor {
234-
label: Some("Alpha Texture"),
235-
size: wgpu::Extent3d {
236-
width: max_texture_dimension_2d,
237-
height: alpha_texture_height,
238-
depth_or_array_layers: 1,
239-
},
240-
mip_level_count: 1,
241-
sample_count: 1,
242-
dimension: wgpu::TextureDimension::D2,
243-
format: wgpu::TextureFormat::Rgba32Uint,
244-
usage: wgpu::TextureUsages::TEXTURE_BINDING | wgpu::TextureUsages::COPY_DST,
245-
view_formats: &[],
246-
});
247-
let alphas_texture_view =
248-
alphas_texture.create_view(&wgpu::TextureViewDescriptor::default());
227+
if needs_new_strips_buffer || needs_new_alpha_texture {
228+
let strips_buffer = if needs_new_strips_buffer {
229+
device.create_buffer(&wgpu::BufferDescriptor {
230+
label: Some("Strips Buffer"),
231+
size: required_strips_size,
232+
usage: wgpu::BufferUsages::VERTEX | wgpu::BufferUsages::COPY_DST,
233+
mapped_at_creation: false,
234+
})
235+
} else {
236+
// Reuse existing buffer if it's big enough
237+
self.resources
238+
.as_ref()
239+
.expect("Strips buffer not found")
240+
.strips_buffer
241+
.clone()
242+
};
243+
let (alphas_texture, render_bind_group) = if needs_new_alpha_texture {
244+
let max_texture_dimension_2d = device.limits().max_texture_dimension_2d;
245+
let alpha_len = render_data.alphas.len();
246+
// 4 alpha values u32 each per texel
247+
let alpha_texture_height =
248+
(u32::try_from(alpha_len).unwrap()).div_ceil(max_texture_dimension_2d * 4);
249249

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

260-
let render_bind_group = device.create_bind_group(&wgpu::BindGroupDescriptor {
261-
label: Some("Render Bind Group"),
262-
layout: &self.render_bind_group_layout,
263-
entries: &[
264-
wgpu::BindGroupEntry {
265-
binding: 0,
266-
resource: wgpu::BindingResource::TextureView(&alphas_texture_view),
256+
let alphas_texture = device.create_texture(&wgpu::TextureDescriptor {
257+
label: Some("Alpha Texture"),
258+
size: wgpu::Extent3d {
259+
width: max_texture_dimension_2d,
260+
height: alpha_texture_height,
261+
depth_or_array_layers: 1,
267262
},
268-
wgpu::BindGroupEntry {
269-
binding: 1,
270-
resource: config_buf.as_entire_binding(),
271-
},
272-
],
273-
});
263+
mip_level_count: 1,
264+
sample_count: 1,
265+
dimension: wgpu::TextureDimension::D2,
266+
format: wgpu::TextureFormat::Rgba32Uint,
267+
usage: wgpu::TextureUsages::TEXTURE_BINDING | wgpu::TextureUsages::COPY_DST,
268+
view_formats: &[],
269+
});
270+
let alphas_texture_view =
271+
alphas_texture.create_view(&wgpu::TextureViewDescriptor::default());
272+
273+
let config_buf = device.create_buffer_init(&wgpu::util::BufferInitDescriptor {
274+
label: Some("Config Buffer"),
275+
contents: bytemuck::bytes_of(&Config {
276+
width: render_params.width,
277+
height: render_params.height,
278+
strip_height: Tile::HEIGHT.into(),
279+
}),
280+
usage: wgpu::BufferUsages::UNIFORM,
281+
});
274282

283+
let render_bind_group = device.create_bind_group(&wgpu::BindGroupDescriptor {
284+
label: Some("Render Bind Group"),
285+
layout: &self.render_bind_group_layout,
286+
entries: &[
287+
wgpu::BindGroupEntry {
288+
binding: 0,
289+
resource: wgpu::BindingResource::TextureView(&alphas_texture_view),
290+
},
291+
wgpu::BindGroupEntry {
292+
binding: 1,
293+
resource: config_buf.as_entire_binding(),
294+
},
295+
],
296+
});
297+
(alphas_texture, render_bind_group)
298+
} else {
299+
let resources = self.resources.as_ref().unwrap();
300+
(
301+
resources.alphas_texture.clone(),
302+
resources.render_bind_group.clone(),
303+
)
304+
};
275305
self.resources = Some(GpuResources {
276306
strips_buffer,
277307
alphas_texture,
278308
render_bind_group,
279309
});
280-
}
310+
};
281311

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

0 commit comments

Comments
 (0)