From 4d1a949cb335fe8e2c53f0b87643d7c121b3ada7 Mon Sep 17 00:00:00 2001 From: Bruce Mitchener Date: Sat, 23 May 2026 15:16:02 +0700 Subject: [PATCH] fix: preserve image atlas across patchless resolves Vello's renderer keeps a persistent GPU image atlas, while the resolver tracks logical atlas residency and dirty image uploads. Patchless resolves used to return Images::default(), which made the renderer treat an image-free frame as a 0x0 atlas and replace a previously populated atlas with a fresh 1x1 texture. The resolver still kept resident image entries marked clean. If a later scene reused the same image, it returned cached atlas coordinates but scheduled no upload. When the renderer grew the atlas back to its previous size, that new GPU texture did not contain the resident image, so image brushes, including glyph image brushes, sampled blank content. Patchless resolves now begin an image-cache resolve and return the cache's resident atlas state when there are resident images. That clears the per-resolve upload list while preserving the GPU atlas those clean images sample from. Fresh patchless resolves with no resident images still report no atlas, so image-free frames do not allocate the default image atlas just to draw solid content. Shader hot reload had the same class of failure: it dropped the GPU atlas by replacing the engine while keeping clean resolver residency. Reset resolver state when reloading shaders, and carry image overrides into the new engine so the next image frame uploads into the fresh atlas. Add focused coverage for the root invariant and the failing GPU sequence: image brush glyphs rendered before and after an image-free frame with the same renderer. --- vello/src/lib.rs | 5 + vello_encoding/src/image_cache.rs | 8 ++ vello_encoding/src/resolve.rs | 63 +++++++++- vello_tests/tests/regression.rs | 193 +++++++++++++++++++++++++++++- 4 files changed, 266 insertions(+), 3 deletions(-) diff --git a/vello/src/lib.rs b/vello/src/lib.rs index 92cd9f9b5d..c7781334f9 100644 --- a/vello/src/lib.rs +++ b/vello/src/lib.rs @@ -616,7 +616,12 @@ impl Renderer { if let Some(error) = error { return Err(error.into()); } + engine.image_overrides = std::mem::take(&mut self.engine.image_overrides); self.engine = engine; + // The new engine has no GPU resources, including the persistent image atlas. + // Start a fresh resolver so the next image frame uploads resident images into + // the new atlas instead of treating them as clean cache hits. + self.resolver = Resolver::new(); self.image_atlas = None; self.shaders = shaders; #[cfg(feature = "debug_layers")] diff --git a/vello_encoding/src/image_cache.rs b/vello_encoding/src/image_cache.rs index 8b827d89dc..163509e4f6 100644 --- a/vello_encoding/src/image_cache.rs +++ b/vello_encoding/src/image_cache.rs @@ -98,6 +98,14 @@ impl ImageCache { } } + pub(crate) fn images_for_patchless_resolve(&self) -> Images<'_> { + if self.map.is_empty() { + Images::default() + } else { + self.images() + } + } + pub(crate) fn bump_size(&mut self) -> bool { let mut new_size = self.atlas.size().width * 2; while new_size <= self.max_size { diff --git a/vello_encoding/src/resolve.rs b/vello_encoding/src/resolve.rs index 2513a2168c..ea2b3fc472 100644 --- a/vello_encoding/src/resolve.rs +++ b/vello_encoding/src/resolve.rs @@ -187,8 +187,17 @@ impl Resolver { ) -> (Layout, Ramps<'a>, Images<'a>) { let resources = &encoding.resources; if resources.patches.is_empty() { + // This resolve has no late-bound image patches, so it has no image uploads. + // If previous resolves left resident images in the cache, keep reporting the + // atlas size so the renderer can reuse the GPU atlas those clean images sample + // from. + self.image_cache.begin_resolve(); let layout = resolve_solid_paths_only(encoding, packed); - return (layout, Ramps::default(), Images::default()); + return ( + layout, + Ramps::default(), + self.image_cache.images_for_patchless_resolve(), + ); } let patch_sizes = self.resolve_patches(encoding); self.resolve_pending_images(); @@ -650,3 +659,55 @@ fn size_to_words(byte_size: usize) -> u32 { fn align_up(len: usize, alignment: u32) -> usize { len + (len.wrapping_neg() & (alignment as usize - 1)) } + +#[cfg(test)] +mod tests { + use super::*; + use peniko::{Blob, ImageAlphaType, ImageBrush, ImageFormat}; + use std::sync::Arc; + + fn image(id_byte: u8, width: u32, height: u32) -> ImageData { + let len = (width * height * 4) as usize; + ImageData { + data: Blob::new(Arc::new(vec![id_byte; len])), + format: ImageFormat::Rgba8, + width, + height, + alpha_type: ImageAlphaType::Alpha, + } + } + + fn image_encoding(image: ImageData) -> Encoding { + let mut encoding = Encoding::new(); + encoding.encode_image(&ImageBrush::new(image), 1.0); + encoding + } + + #[test] + fn patchless_resolve_preserves_image_atlas_size() { + let image = image(7, 8, 8); + let image_encoding = image_encoding(image); + + let mut resolver = Resolver::new(); + let mut packed = Vec::new(); + let patchless_encoding = Encoding::new(); + + let (_layout, _ramps, images) = resolver.resolve(&patchless_encoding, &mut packed); + assert_eq!((images.width, images.height), (0, 0)); + assert!(images.images.is_empty()); + + let atlas_size = { + let (_layout, _ramps, images) = resolver.resolve(&image_encoding, &mut packed); + assert_eq!(images.images.len(), 1); + (images.width, images.height) + }; + + let (_layout, _ramps, images) = resolver.resolve(&patchless_encoding, &mut packed); + assert_eq!((images.width, images.height), atlas_size); + assert!(images.images.is_empty()); + + let (_layout, _ramps, images) = resolver.resolve(&image_encoding, &mut packed); + assert_eq!((images.width, images.height), atlas_size); + assert!(images.images.is_empty()); + } +} diff --git a/vello_tests/tests/regression.rs b/vello_tests/tests/regression.rs index 2e330c96e6..b0ff5404e1 100644 --- a/vello_tests/tests/regression.rs +++ b/vello_tests/tests/regression.rs @@ -3,13 +3,21 @@ //! Tests to ensure that certain issues which don't deserve a test scene don't regress +use std::sync::Arc; + use scenes::ImageCache; use scenes::SimpleText; use vello::{ - AaConfig, Scene, + AaConfig, RendererOptions, Scene, kurbo::{Affine, Rect, RoundedRect, Stroke}, peniko::{ - Color, ColorStop, Extend, Gradient, ImageQuality, InterpolationAlphaSpace, color::palette, + Blob, Brush, Color, ColorStop, Extend, Gradient, ImageAlphaType, ImageBrush, ImageData, + ImageFormat, ImageQuality, InterpolationAlphaSpace, color::palette, + }, + util::{RenderContext, block_on_wgpu}, + wgpu::{ + self, BufferDescriptor, BufferUsages, CommandEncoderDescriptor, Extent3d, + TexelCopyBufferInfo, TextureDescriptor, TextureFormat, TextureUsages, }, }; use vello_tests::{TestParams, smoke_snapshot_test_sync, snapshot_test_sync}; @@ -144,6 +152,187 @@ fn text_stroke_width_zero() { .assert_mean_less_than(0.001); } +const GLYPH_IMAGE_BACKGROUND: [u8; 4] = [247, 243, 236, 255]; + +fn glyph_image_background() -> Color { + Color::from_rgba8( + GLYPH_IMAGE_BACKGROUND[0], + GLYPH_IMAGE_BACKGROUND[1], + GLYPH_IMAGE_BACKGROUND[2], + GLYPH_IMAGE_BACKGROUND[3], + ) +} + +fn glyph_image_data() -> ImageData { + let width = 96_u32; + let height = 96_u32; + let mut bytes = Vec::with_capacity(usize::try_from(width * height * 4).unwrap()); + for y in 0..height { + for x in 0..width { + let r = 32 + u8::try_from((223 * x) / (width - 1)).unwrap(); + let g = 36 + u8::try_from((170 * y) / (height - 1)).unwrap(); + let stripe = if ((x / 8) + (y / 8)) % 2 == 0 { 34 } else { 0 }; + let b = 82 + stripe; + bytes.extend_from_slice(&[r, g, b, 255]); + } + } + ImageData { + data: Blob::new(Arc::new(bytes)), + format: ImageFormat::Rgba8, + width, + height, + alpha_type: ImageAlphaType::Alpha, + } +} + +fn glyph_image_brush(image: &ImageData) -> Brush { + Brush::Image( + ImageBrush::new(image.clone()) + .with_quality(ImageQuality::Medium) + .with_extend(Extend::Repeat), + ) +} + +fn glyph_image_brush_scene(image: &ImageData) -> Scene { + let mut scene = Scene::new(); + let mut text = SimpleText::new(); + scene.fill( + vello::peniko::Fill::NonZero, + Affine::IDENTITY, + glyph_image_background(), + None, + &Rect::new(0.0, 0.0, 256.0, 256.0), + ); + let brush = glyph_image_brush(image); + text.add_var_run( + &mut scene, + None, + 42.0, + &[], + &brush, + Affine::translate((14.0, 116.0)), + None, + None, + vello::peniko::Fill::NonZero, + "texture", + true, + ); + scene +} + +fn image_free_scene() -> Scene { + let mut scene = Scene::new(); + scene.fill( + vello::peniko::Fill::NonZero, + Affine::IDENTITY, + Color::from_rgb8(30, 30, 34), + None, + &Rect::new(0.0, 0.0, 256.0, 256.0), + ); + scene +} + +#[test] +#[cfg_attr(skip_gpu_tests, ignore)] +fn glyph_image_brush_survives_image_free_render() { + pollster::block_on(async { + let width = 256; + let height = 256; + let image = glyph_image_data(); + let mut context = RenderContext::new(); + let device_id = context.device(None).await.expect("compatible device"); + let device_handle = &mut context.devices[device_id]; + let device = &device_handle.device; + let queue = &device_handle.queue; + let mut renderer = vello::Renderer::new( + device, + RendererOptions { + num_init_threads: std::num::NonZeroUsize::new(1), + antialiasing_support: std::iter::once(AaConfig::Area).collect(), + ..RendererOptions::default() + }, + ) + .expect("create renderer"); + let size = Extent3d { + width, + height, + depth_or_array_layers: 1, + }; + let target = device.create_texture(&TextureDescriptor { + label: Some("glyph image brush target"), + size, + mip_level_count: 1, + sample_count: 1, + dimension: wgpu::TextureDimension::D2, + format: TextureFormat::Rgba8Unorm, + usage: TextureUsages::STORAGE_BINDING + | TextureUsages::TEXTURE_BINDING + | TextureUsages::COPY_SRC, + view_formats: &[], + }); + let view = target.create_view(&wgpu::TextureViewDescriptor::default()); + let params = vello::RenderParams { + base_color: Color::from_rgba8(0, 0, 0, 0), + width, + height, + antialiasing_method: AaConfig::Area, + }; + + for scene in [ + glyph_image_brush_scene(&image), + image_free_scene(), + glyph_image_brush_scene(&image), + ] { + renderer + .render_to_texture(device, queue, &scene, &view, ¶ms) + .expect("render scene"); + } + + let padded_byte_width = (width * 4).next_multiple_of(256); + let buffer_size = u64::from(padded_byte_width) * u64::from(height); + let buffer = device.create_buffer(&BufferDescriptor { + label: Some("glyph image brush readback"), + size: buffer_size, + usage: BufferUsages::MAP_READ | BufferUsages::COPY_DST, + mapped_at_creation: false, + }); + let mut encoder = device.create_command_encoder(&CommandEncoderDescriptor { + label: Some("glyph image brush copy"), + }); + encoder.copy_texture_to_buffer( + target.as_image_copy(), + TexelCopyBufferInfo { + buffer: &buffer, + layout: wgpu::TexelCopyBufferLayout { + offset: 0, + bytes_per_row: Some(padded_byte_width), + rows_per_image: None, + }, + }, + size, + ); + queue.submit([encoder.finish()]); + let buf_slice = buffer.slice(..); + let (sender, receiver) = futures_intrusive::channel::shared::oneshot_channel(); + buf_slice.map_async(wgpu::MapMode::Read, move |v| sender.send(v).unwrap()); + block_on_wgpu(device, receiver.receive()) + .expect("map callback") + .expect("map readback"); + let data = buf_slice.get_mapped_range(); + let unpadded_byte_width = usize::try_from(width * 4).unwrap(); + let padded_byte_width = usize::try_from(padded_byte_width).unwrap(); + let non_background_pixels = data + .chunks(padded_byte_width) + .flat_map(|row| row[..unpadded_byte_width].chunks_exact(4)) + .filter(|pixel| **pixel != GLYPH_IMAGE_BACKGROUND) + .count(); + assert!( + non_background_pixels > 0, + "image brush glyph run rendered as a blank image after an image-free render" + ); + }); +} + /// /// See . #[test]