From 30736471c4001740c9898f1717d930d625f2107b Mon Sep 17 00:00:00 2001
From: Daniel McNab <36049421+DJMcNab@users.noreply.github.com>
Date: Fri, 20 Sep 2024 17:00:27 +0100
Subject: [PATCH] Add a failing test and warning for #680 (#681)

See #680 for details.

This test will currently pass, but only because the behaviour is
incorrect.

This adds a warning for when this bug would apply, for users
---
 vello/src/render.rs               | 13 ++++++
 vello_tests/src/snapshot.rs       | 15 ++++++-
 vello_tests/tests/known_issues.rs | 67 +++++++++++++++++++++++++++++++
 3 files changed, 94 insertions(+), 1 deletion(-)
 create mode 100644 vello_tests/tests/known_issues.rs

diff --git a/vello/src/render.rs b/vello/src/render.rs
index 4e2c4c484..9529bea5b 100644
--- a/vello/src/render.rs
+++ b/vello/src/render.rs
@@ -168,6 +168,19 @@ impl Render {
         }
         let cpu_config =
             RenderConfig::new(&layout, params.width, params.height, &params.base_color);
+        // HACK: The coarse workgroup counts is the number of active bins.
+        if (cpu_config.workgroup_counts.coarse.0
+            * cpu_config.workgroup_counts.coarse.1
+            * cpu_config.workgroup_counts.coarse.2)
+            > 256
+        {
+            log::warn!(
+                "Trying to paint too large image. {}x{}.\n\
+                See https://github.com/linebender/vello/issues/680 for details",
+                params.width,
+                params.height
+            );
+        }
         let buffer_sizes = &cpu_config.buffer_sizes;
         let wg_counts = &cpu_config.workgroup_counts;
 
diff --git a/vello_tests/src/snapshot.rs b/vello_tests/src/snapshot.rs
index fe32aa667..eaefd42fe 100644
--- a/vello_tests/src/snapshot.rs
+++ b/vello_tests/src/snapshot.rs
@@ -147,14 +147,27 @@ pub fn smoke_snapshot_test_sync(scene: Scene, params: &TestParams) -> Result<Sna
     pollster::block_on(snapshot_test(scene, params, SnapshotDirectory::Smoke))
 }
 
-/// Run a snapshot test of the given scene.
+/// Run an snapshot test of the given scene.
+///
+/// In most cases, you should use [`snapshot_test_sync`] or [`smoke_snapshot_test_sync`].
 pub async fn snapshot_test(
     scene: Scene,
     params: &TestParams,
     directory: SnapshotDirectory,
 ) -> Result<Snapshot> {
     let raw_rendered = render_then_debug(&scene, params).await?;
+    snapshot_test_image(raw_rendered, params, directory)
+}
 
+/// Evaluate a snapshot test on the given image.
+///
+/// This is useful if a post-processing step needs to happen
+/// in-between running Vello and the image.
+pub fn snapshot_test_image(
+    raw_rendered: Image,
+    params: &TestParams,
+    directory: SnapshotDirectory,
+) -> Result<Snapshot> {
     let reference_path = snapshot_dir(directory)
         .join(&params.name)
         .with_extension("png");
diff --git a/vello_tests/tests/known_issues.rs b/vello_tests/tests/known_issues.rs
new file mode 100644
index 000000000..2bc16e101
--- /dev/null
+++ b/vello_tests/tests/known_issues.rs
@@ -0,0 +1,67 @@
+// Copyright 2024 the Vello Authors
+// SPDX-License-Identifier: Apache-2.0 OR MIT
+
+//! Reproductions for known bugs, to allow test driven development
+
+use vello::{
+    kurbo::{Affine, Rect},
+    peniko::{Color, Format},
+    Scene,
+};
+use vello_tests::TestParams;
+
+/// A reproduction of <https://github.com/linebender/vello/issues/680>
+fn many_bins(use_cpu: bool) {
+    let mut scene = Scene::new();
+    scene.fill(
+        vello::peniko::Fill::NonZero,
+        Affine::IDENTITY,
+        Color::RED,
+        None,
+        &Rect::new(-5., -5., 256. * 20., 256. * 20.),
+    );
+    let params = TestParams {
+        use_cpu,
+        ..TestParams::new("many_bins", 256 * 17, 256 * 17)
+    };
+    // To view, use VELLO_DEBUG_TEST=many_bins
+    let image = vello_tests::render_then_debug_sync(&scene, &params).unwrap();
+    assert_eq!(image.format, Format::Rgba8);
+    let mut red_count = 0;
+    let mut black_count = 0;
+    for pixel in image.data.data().chunks_exact(4) {
+        let &[r, g, b, a] = pixel else { unreachable!() };
+        let is_red = r == 255 && g == 0 && b == 0 && a == 255;
+        let is_black = r == 0 && g == 0 && b == 0 && a == 255;
+        if !is_red && !is_black {
+            panic!("{pixel:?}");
+        }
+        match (is_red, is_black) {
+            (true, true) => unreachable!(),
+            (true, false) => red_count += 1,
+            (false, true) => black_count += 1,
+            (false, false) => panic!("Got unexpected pixel {pixel:?}"),
+        }
+    }
+    // When #680 is fixed, this will become:
+    // let drawn_bins = 17 /* x bins */ * 17 /* y bins*/;
+
+    // The current maximum number of bins.
+    let drawn_bins = 256;
+    let expected_red_count = drawn_bins * 256 /* tiles per bin */ * 256 /* Pixels per tile */;
+    assert_eq!(red_count, expected_red_count);
+    assert!(black_count > 0);
+}
+
+#[test]
+#[cfg_attr(skip_gpu_tests, ignore)]
+fn many_bins_gpu() {
+    many_bins(false);
+}
+
+#[test]
+#[cfg_attr(skip_gpu_tests, ignore)]
+#[should_panic]
+fn many_bins_cpu() {
+    many_bins(true);
+}