Skip to content

Commit

Permalink
Don't create a zero sized scene buffer (#630)
Browse files Browse the repository at this point in the history
Alternative to #629

Fixes #291
  • Loading branch information
DJMcNab committed Jul 16, 2024
1 parent 55027d8 commit 5a5a638
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 15 deletions.
10 changes: 9 additions & 1 deletion vello/src/render.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -120,12 +122,18 @@ impl Render {
image.0.data.data(),
);
}

let cpu_config =
RenderConfig::new(&layout, params.width, params.height, &params.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::<u32>(), 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)),
Expand Down
65 changes: 51 additions & 14 deletions vello_tests/tests/smoke.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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, &params).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);
}

0 comments on commit 5a5a638

Please sign in to comment.