Skip to content

Commit 0f3ef03

Browse files
LaurenzVtomcur
andauthored
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>
1 parent 8d1bfe9 commit 0f3ef03

File tree

3 files changed

+26
-10
lines changed

3 files changed

+26
-10
lines changed

sparse_strips/vello_common/src/tile.rs

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -262,17 +262,18 @@ impl Tiles {
262262

263263
// For ease of logic, special-case purely vertical tiles.
264264
if line_left_x == line_right_x {
265-
// Do not emit tiles that are strictly on the right of the viewport. They do not
266-
// impact winding, and if we don't do this, we might end up with too big tile
267-
// coordinates, which will cause overflows in strip rendering.
268-
if line_left_x as u16 >= tile_columns {
269-
continue;
270-
}
271-
272265
let y_top_tiles = (line_top_y as u16).min(tile_rows);
273266
let y_bottom_tiles = (line_bottom_y.ceil() as u16).min(tile_rows);
274267

275-
let x = line_left_x as u16;
268+
// Clamp all tiles that are strictly on the right of the viewport to the tile x coordinate
269+
// right next to the outside of the viewport. If we don't do this, we might end up
270+
// with too big tile coordinates, which will cause overflows in strip rendering.
271+
// TODO: in principle it is possible to cull right-of-viewport tiles, but it was causing some
272+
// issues, and we are choosing to do the less efficient but working thing for now.
273+
// See <https://github.com/linebender/vello/pull/1189> and
274+
// <https://github.com/linebender/vello/issues/1126>.
275+
let x = (line_left_x as u16).min(tile_columns + 1);
276+
276277
for y_idx in y_top_tiles..y_bottom_tiles {
277278
let y = f32::from(y_idx);
278279

@@ -569,6 +570,7 @@ mod tests {
569570

570571
let mut tiles = Tiles::new(Level::try_detect().unwrap_or(Level::fallback()));
571572
tiles.make_tiles(&line_buf, 10, 10);
572-
assert!(tiles.is_empty());
573+
assert_eq!(tiles.tile_buf[0].x, 4);
574+
assert_eq!(tiles.tile_buf[1].x, 4);
573575
}
574576
}
Lines changed: 3 additions & 0 deletions
Loading

sparse_strips/vello_sparse_tests/tests/issues.rs

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use vello_common::color::palette::css::{DARK_BLUE, LIME, REBECCA_PURPLE};
88
use vello_common::kurbo::{BezPath, Rect, Shape, Stroke};
99
use vello_common::peniko::{Color, ColorStop, Fill, Gradient};
1010
use vello_common::pixmap::Pixmap;
11-
use vello_cpu::color::palette::css::RED;
11+
use vello_cpu::color::palette::css::{BLACK, RED};
1212
use vello_cpu::peniko::Compose;
1313
use vello_cpu::{Level, RenderContext, RenderMode, RenderSettings};
1414
use vello_dev_macros::vello_test;
@@ -394,3 +394,14 @@ fn multi_threading_oob_access() {
394394
ctx.flush();
395395
ctx.render_to_pixmap(&mut pixmap);
396396
}
397+
398+
/// See <https://github.com/linebender/vello/issues/1181>.
399+
#[vello_test(width = 556, height = 8)]
400+
fn tile_clamped_off_by_one(ctx: &mut impl Renderer) {
401+
let rect = Rect::new(0.0, 0.0, 556.0, 8.0);
402+
403+
ctx.set_paint(BLACK);
404+
ctx.push_layer(Some(&rect.to_path(0.1)), None, None, None);
405+
ctx.fill_path(&rect.to_path(0.1));
406+
ctx.pop_layer();
407+
}

0 commit comments

Comments
 (0)