shifathosn
/
photocraft
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

perf(raster): moving a layer no longer stores tiles of only the default pixel (#4844) 

Surface::translated, under every layer move and the Move drag preview, allocated
every target tile that a moved source tile reached. For a move that isn't a whole
number of tiles, the transparent margins of a layer's edge tiles landed on target
tiles of their own, and those were stored though they hold only the default pixel
(256 KB each at 8-bit RGBA). #4414's shared uniform tiles were also stored when
their value was the default. Moving two 300 px blocks by (100, 56) left 10 of 18
stored tiles empty; nudging back and forth kept adding more.

A target that ends up holding only the default pixel is no longer stored, as
prune() would drop it. A copied tile is compared row by row with a row of the
default pixel after the copy (one memcmp per row; a tile with content stops at
its first row), and a shared uniform tile of the default value is skipped. Moves
by whole tiles still re-share the existing tiles unchanged.

Release, 24 cores, cargo xtask perf --quick (perf_scenarios, p50, HEAD vs this,
two runs each): move layer on 16-bit 15000x10000 + refresh 5.3 -> 3.9 ms; move
layer on 32-bit 20 layers + refresh 11.9 -> 5.9 ms; the other Move rows within
noise. translated() alone on 36 MP: full layer unchanged (27.0 -> 27.6 ms RGBA8);
40 sparse blocks 11.8 -> 10.6 ms RGBA8, 44.0 -> 39 ms RGBA16, 308 -> 190 tiles.

The new test covers RGBA8, RGBA16, RGBA32F and GRAY8 over a zero and a non-zero
default pixel, and both paths; it fails on the old code.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
deadmeu 2026-10-13 16:07:20 +00:00 committed by GitHub
parent f1233bb43a
commit 533b9a1c62
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
1 changed files with 50 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

53
crates/raster/src/lib.rs
View File

@ -747,7 +747,8 @@ impl Surface {
/// The surface moved by whole pixels: its encoded samples copied tile to tile (no float
/// round trip, no intermediate buffer); moves by whole tiles share the tiles themselves, and
/// a target tile read entirely from tiles of one single pixel value shares one such tile (the
/// inside of a solid fill stays one copy-on-write tile however often it moves).
/// inside of a solid fill stays one copy-on-write tile however often it moves). A tile it
/// copies that ends up holding only the default pixel isn't stored.
/// `content` must hold every pixel that isn't the default one (`content_bounds`, say): only
/// tiles it reaches are kept.
pub fn translated(&self, dx: i32, dy: i32, content: Rect) -> Surface {
@ -763,6 +764,8 @@ impl Surface {
let targets: std::collections::BTreeSet<TileCoord> = sources().flat_map(|(c, _)| c.rect().translate(dx, dy).intersect(&moved).tiles()).collect();
let mut uniform = UniformTiles::default();
let zero = vec![0u8; bpp];
// One tile row of the default pixel, to compare a finished tile with a row at a time.
let default_row = self.default_pixel.repeat(TILE_SIZE.unsigned_abs() as usize);
for dc in targets {
let dr = dc.rect();
let sr = dr.translate(dx.saturating_neg(), dy.saturating_neg());
@ -774,7 +777,10 @@ impl Surface {
&& content.contains_rect(&sr)
&& sr.tiles().all(|sc| self.tiles.contains_key(&sc));
if covered && let Some(shared) = uniform.window(self, sr) {
out.tiles.insert(dc, shared);
// A tile of only the default pixel is left out, as `prune` would.
if shared.data.get(..bpp) != Some(&*self.default_pixel) {
out.tiles.insert(dc, shared);
}
continue;
}
// A covered target is overwritten whole, so skip filling it with the default pixel.
@ -795,7 +801,11 @@ impl Surface {
}
}
}
out.tiles.insert(dc, Arc::new(tile));
// A target that got only default pixels isn't stored, as `prune` would drop it. Checked
// after the copy, a row at a time: a tile with content stops at its first such row.
if tile.data.chunks(default_row.len()).any(|row| row != default_row) {
out.tiles.insert(dc, Arc::new(tile));
}
}
out
}
@ -1427,6 +1437,43 @@ mod tests {
let _ = s.translated(i32::MIN + 1, 3, all);
}
#[test]
fn translated_stores_no_tiles_of_only_the_default_pixel() {
let only_default = |t: &Surface| {
let dp = t.default_pixel();
t.tiles().filter(|(c, _)| t.read_region(c.rect()).chunks_exact(dp.len()).all(|p| p == dp.as_slice())).count()
};
// Two blocks a few tiles apart, moved off the tile grid and back: the default-pixel edges
// of their tiles land on tiles of their own, which aren't stored.
for fmt in [PixelFormat::RGBA8, PixelFormat::RGBA16, PixelFormat::RGBA32F, PixelFormat::GRAY8] {
for default in [0.0, 0.25] {
let mut s = Surface::with_default(fmt, &vec![default; fmt.channels()]);
s.fill_rect(Rect::new(0, 0, 300, 300), &vec![1.0; fmt.channels()]);
s.fill_rect(Rect::new(2000, 1000, 2300, 1300), &vec![0.5; fmt.channels()]);
let all = Rect::new(-600, -600, 3000, 2000);
let mut t = s.clone();
for (dx, dy) in [(100, 56), (-100, -56), (10, 7), (-3, 40), (130, -90), (-137, 43)] {
let moved = t.translated(dx, dy, t.content_bounds());
assert_eq!(moved.read_region(all), t.read_region(all.translate(-dx, -dy)), "{fmt:?} over {default} by ({dx}, {dy})");
assert_eq!(only_default(&moved), 0, "{fmt:?} over {default} by ({dx}, {dy})");
t = moved;
}
assert_eq!(t.read_region(all), s.read_region(all), "{fmt:?} over {default}: back where it started");
assert_eq!(t.tile_count(), s.tile_count(), "{fmt:?} over {default}: back where it started");
}
}
// Stored tiles of the default pixel inside `content`: the targets that would share one of
// them (#4414's uniform windows) are left out too.
let mut s = Surface::with_default(PixelFormat::GRAY8, &[0.25]);
s.fill_rect(Rect::new(0, 0, 1024, 1024), &[0.25]);
s.write_pixel(600, 600, &[1.0]);
let content = Rect::new(0, 0, 1024, 1024);
let t = s.translated(10, 7, content);
assert_eq!(only_default(&t), 0);
assert_eq!(t.tile_count(), 1);
assert_eq!(t.read_region(Rect::new(-20, -20, 1100, 1100)), s.read_region(Rect::new(-30, -27, 1090, 1093)));
}
proptest! {
#[test]
fn region_write_read_roundtrip(x in -600i32..600, y in -600i32..600, w in 1u32..90, h in 1u32..90, seed in any::<u32>()) {