Fix off-by-one issue with tiles that are to the right of the viewport (#1189)
Maybe we can at some point figure out what was wrong with the original
solution, but for now using the original fix which should be less error
prone is a good start I think.
---------
Co-authored-by: Tom Churchman <thomas@churchman.nl>
diff --git a/sparse_strips/vello_common/src/tile.rs b/sparse_strips/vello_common/src/tile.rs
index 97a7548..2a2ee72 100644
--- a/sparse_strips/vello_common/src/tile.rs
+++ b/sparse_strips/vello_common/src/tile.rs
@@ -262,17 +262,18 @@
// For ease of logic, special-case purely vertical tiles.
if line_left_x == line_right_x {
- // Do not emit tiles that are strictly on the right of the viewport. They do not
- // impact winding, and if we don't do this, we might end up with too big tile
- // coordinates, which will cause overflows in strip rendering.
- if line_left_x as u16 >= tile_columns {
- continue;
- }
-
let y_top_tiles = (line_top_y as u16).min(tile_rows);
let y_bottom_tiles = (line_bottom_y.ceil() as u16).min(tile_rows);
- let x = line_left_x as u16;
+ // Clamp all tiles that are strictly on the right of the viewport to the tile x coordinate
+ // right next to the outside of the viewport. If we don't do this, we might end up
+ // with too big tile coordinates, which will cause overflows in strip rendering.
+ // TODO: in principle it is possible to cull right-of-viewport tiles, but it was causing some
+ // issues, and we are choosing to do the less efficient but working thing for now.
+ // See <https://github.com/linebender/vello/pull/1189> and
+ // <https://github.com/linebender/vello/issues/1126>.
+ let x = (line_left_x as u16).min(tile_columns + 1);
+
for y_idx in y_top_tiles..y_bottom_tiles {
let y = f32::from(y_idx);
@@ -569,6 +570,7 @@
let mut tiles = Tiles::new(Level::try_detect().unwrap_or(Level::fallback()));
tiles.make_tiles(&line_buf, 10, 10);
- assert!(tiles.is_empty());
+ assert_eq!(tiles.tile_buf[0].x, 4);
+ assert_eq!(tiles.tile_buf[1].x, 4);
}
}
diff --git a/sparse_strips/vello_sparse_tests/snapshots/tile_clamped_off_by_one.png b/sparse_strips/vello_sparse_tests/snapshots/tile_clamped_off_by_one.png
new file mode 100644
index 0000000..068ae07
--- /dev/null
+++ b/sparse_strips/vello_sparse_tests/snapshots/tile_clamped_off_by_one.png
@@ -0,0 +1,3 @@
+version https://git-lfs.github.com/spec/v1
+oid sha256:8213b9c80b0420d773f103303275b617d264f66fa76e05a5f72dec1f696e26f1
+size 71
diff --git a/sparse_strips/vello_sparse_tests/tests/issues.rs b/sparse_strips/vello_sparse_tests/tests/issues.rs
index 792633b..041588d 100644
--- a/sparse_strips/vello_sparse_tests/tests/issues.rs
+++ b/sparse_strips/vello_sparse_tests/tests/issues.rs
@@ -8,7 +8,7 @@
use vello_common::kurbo::{BezPath, Rect, Shape, Stroke};
use vello_common::peniko::{Color, ColorStop, Fill, Gradient};
use vello_common::pixmap::Pixmap;
-use vello_cpu::color::palette::css::RED;
+use vello_cpu::color::palette::css::{BLACK, RED};
use vello_cpu::peniko::Compose;
use vello_cpu::{Level, RenderContext, RenderMode, RenderSettings};
use vello_dev_macros::vello_test;
@@ -394,3 +394,14 @@
ctx.flush();
ctx.render_to_pixmap(&mut pixmap);
}
+
+/// See <https://github.com/linebender/vello/issues/1181>.
+#[vello_test(width = 556, height = 8)]
+fn tile_clamped_off_by_one(ctx: &mut impl Renderer) {
+ let rect = Rect::new(0.0, 0.0, 556.0, 8.0);
+
+ ctx.set_paint(BLACK);
+ ctx.push_layer(Some(&rect.to_path(0.1)), None, None, None);
+ ctx.fill_path(&rect.to_path(0.1));
+ ctx.pop_layer();
+}