From 5a5a638a58a524230484db110151fa7591904a88 Mon Sep 17 00:00:00 2001 From: Daniel McNab <36049421+DJMcNab@users.noreply.github.com> Date: Tue, 16 Jul 2024 10:56:21 +0100 Subject: [PATCH] Don't create a zero sized scene buffer (#630) Alternative to #629 Fixes #291 --- vello/src/render.rs | 10 +++++- vello_tests/tests/smoke.rs | 65 ++++++++++++++++++++++++++++++-------- 2 files changed, 60 insertions(+), 15 deletions(-) diff --git a/vello/src/render.rs b/vello/src/render.rs index a248de311..af6afda50 100644 --- a/vello/src/render.rs +++ b/vello/src/render.rs @@ -3,6 +3,8 @@ //! Take an encoded scene and create a graph to render it +use std::mem::size_of; + use crate::recording::{BufferProxy, ImageFormat, ImageProxy, Recording, ResourceProxy}; use crate::shaders::FullShaders; use crate::{AaConfig, RenderParams}; @@ -120,12 +122,18 @@ impl Render { image.0.data.data(), ); } - let cpu_config = RenderConfig::new(&layout, params.width, params.height, ¶ms.base_color); let buffer_sizes = &cpu_config.buffer_sizes; let wg_counts = &cpu_config.workgroup_counts; + if packed.is_empty() { + // HACK: wgpu doesn't allow empty buffers, so we make sure that the scene buffer we upload + // can contain at least one array item. + // The values passed here should never be read, because the scene size in config + // is zero. + packed.resize(size_of::(), u8::MAX); + } let scene_buf = ResourceProxy::Buffer(recording.upload("scene", packed)); let config_buf = ResourceProxy::Buffer( recording.upload_uniform("config", bytemuck::bytes_of(&cpu_config.gpu)), diff --git a/vello_tests/tests/smoke.rs b/vello_tests/tests/smoke.rs index 08487f9a4..1826a8d19 100644 --- a/vello_tests/tests/smoke.rs +++ b/vello_tests/tests/smoke.rs @@ -6,20 +6,6 @@ use vello::peniko::{Brush, Color, Format}; use vello::Scene; use vello_tests::TestParams; -#[test] -#[cfg_attr(skip_gpu_tests, ignore)] -fn simple_square_gpu() { - simple_square(false); -} - -#[test] -// The fine shader still requires a GPU, and so we still get a wgpu device -// skip this for now -#[cfg_attr(skip_gpu_tests, ignore)] -fn simple_square_cpu() { - simple_square(true); -} - fn simple_square(use_cpu: bool) { let mut scene = Scene::new(); scene.fill( @@ -54,3 +40,54 @@ fn simple_square(use_cpu: bool) { assert_eq!(red_count, 50 * 50); assert_eq!(black_count, 150 * 150 - 50 * 50); } + +fn empty_scene(use_cpu: bool) { + let scene = Scene::new(); + + // Adding an alpha factor here changes the resulting colour *slightly*, + // presumably due to pre-multiplied alpha. + // We just assume that alpha scenarios work fine + let color = Color::PLUM; + let params = TestParams { + use_cpu, + base_colour: Some(color), + ..TestParams::new("simple_square", 150, 150) + }; + let image = vello_tests::render_then_debug_sync(&scene, ¶ms).unwrap(); + assert_eq!(image.format, Format::Rgba8); + for pixel in image.data.data().chunks_exact(4) { + let &[r, g, b, a] = pixel else { unreachable!() }; + let image_color = Color::rgba8(r, g, b, a); + if image_color != color { + panic!("Got {image_color:?}, expected clear colour {color:?}"); + } + } +} + +#[test] +#[cfg_attr(skip_gpu_tests, ignore)] +fn simple_square_gpu() { + simple_square(false); +} + +#[test] +// The fine shader still requires a GPU, and so we still get a wgpu device +// skip this for now +#[cfg_attr(skip_gpu_tests, ignore)] +fn simple_square_cpu() { + simple_square(true); +} + +#[test] +#[cfg_attr(skip_gpu_tests, ignore)] +fn empty_scene_gpu() { + empty_scene(false); +} + +#[test] +// The fine shader still requires a GPU, and so we still get a wgpu device +// skip this for now +#[cfg_attr(skip_gpu_tests, ignore)] +fn empty_scene_cpu() { + empty_scene(true); +}