Found while re-scoring the Remotion differential. Pre-existing defect, independent of the audit chantier.
Fixed by PR #168. Kept for the record.
The finding
prefetch_icons exists so that N render threads do not hit the network in parallel for the same icon. It served no purpose: its cache key and the painter's could never coincide.
The painter oversamples by 2 and keys on the render size:
// crates/rustmotion-components/src/icon.rs:44-50
const OVERSAMPLE: u32 = 2;
let render_w = target_w * OVERSAMPLE;
let render_h = target_h * OVERSAMPLE;
let cache_key = format!("icon:{}:{}:{}x{}", self.icon, color, render_w, render_h);The preloader keys on the target size, without the factor:
// crates/rustmotion/src/engine/preload.rs:81
let cache_key = format!("icon:{}:{}:{}x{}", icon, color, w, h);For a 40x40 icon the preloader wrote under icon:x:#fff:40x40 and the painter read icon:x:#fff:80x80. No collision was possible at any size.
Consequences
- The preload never helped. Every render thread made its own HTTP request on first paint of each icon — exactly what
prefetch_icons was meant to prevent.
- It cost extra. The preload downloaded, rasterized (
fetch_icon_svg + usvg + tiny_skia::Pixmap, preload.rs:85-95) and stored under a key nobody read. Wasted network and CPU on every render.
- The preloader rasterized at the wrong resolution:
render_w = (*w).max(1) (preload.rs:91), without OVERSAMPLE. Fixing the key alone would have served the painter a bitmap at half size.
Root cause
OVERSAMPLE was a constant local to icon.rs, and the cache key was built by format! in two independent places. Nothing tied them together — the same class of defect as the audio sample rate fixed by PR #151, where two sites declared a rate without consulting each other.
Resolution
PR #168 extracted a single icon_cache_key(icon, color, target_w, target_h) that applies the oversample and builds the key, called by both sites. The oversample is applied to the preloader's rasterization too.
Fixing the key without fixing the rasterization would have turned a useless preload into a harmful one: the painter would find a bitmap at half size and display it blurred. The two had to go together.
Found while re-scoring the Remotion differential. Pre-existing defect, independent of the audit chantier.
The finding
prefetch_iconsexists so that N render threads do not hit the network in parallel for the same icon. It served no purpose: its cache key and the painter's could never coincide.The painter oversamples by 2 and keys on the render size:
The preloader keys on the target size, without the factor:
For a 40x40 icon the preloader wrote under
icon:x:#fff:40x40and the painter readicon:x:#fff:80x80. No collision was possible at any size.Consequences
prefetch_iconswas meant to prevent.fetch_icon_svg+usvg+tiny_skia::Pixmap,preload.rs:85-95) and stored under a key nobody read. Wasted network and CPU on every render.render_w = (*w).max(1)(preload.rs:91), withoutOVERSAMPLE. Fixing the key alone would have served the painter a bitmap at half size.Root cause
OVERSAMPLEwas a constant local toicon.rs, and the cache key was built byformat!in two independent places. Nothing tied them together — the same class of defect as the audio sample rate fixed by PR #151, where two sites declared a rate without consulting each other.Resolution
PR #168 extracted a single
icon_cache_key(icon, color, target_w, target_h)that applies the oversample and builds the key, called by both sites. The oversample is applied to the preloader's rasterization too.Fixing the key without fixing the rasterization would have turned a useless preload into a harmful one: the painter would find a bitmap at half size and display it blurred. The two had to go together.