G5: a layer rendered by two workers at once is shared from the cache (#1812)

Preview workers that start together miss the layer cache together, and each kept its own copy
of every layer; on the GPU each copy was uploaded and packed into its own Classic 3D atlas, which
ran the device out of memory on a cold start. LayerCache::insert now keeps the first buffer of a
key and returns it, and the renderer goes on with the returned buffer, so the workers share one.
Renders don't wait for each other: on the shared frame pool a blocking wait could end up beneath
the work it waits for (tried, it stalled playback for the wait's timeout).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

crates/render/src/cache.rs

@@ -168,15 +168,23 @@ impl LayerCache {
                 // Disk cache: a hit comes back into memory.
                 let (dc, dk) = self.disk_key(key)?;
                 let buf = dc.get_layer(dk)?;
-                self.insert_mem(key, buf.clone());
-                Some(buf)
+                Some(self.insert_mem(key, buf))
             }
         }
     }
 
-    pub fn insert(&self, key: u64, buf: Arc<Buf>) {
+    /// Keep `buf` under `key`, and return the buffer the cache holds for it: the one kept first
+    /// when another render of the same layer got there before (#1812: preview workers starting
+    /// together each rendered every layer; sharing the first buffer, the GPU compositor uploads
+    /// and packs it once instead of once per worker, which ran it out of memory). Renders don't
+    /// wait for each other: on the frame workers' shared thread pool a wait can end up under
+    /// the very work it waits for.
+    pub fn insert(&self, key: u64, buf: Arc<Buf>) -> Arc<Buf> {
         if self.gated() {
-            return;
+            return buf;
+        }
+        if let Some(kept) = self.inner.lock().ok().and_then(|g| g.map.get(&key).map(|e| e.buf.clone())) {
+            return kept;
         }
         let started = self.inner.lock().ok().and_then(|mut g| g.missed_at.remove(&key));
         if let Some((dc, dk)) = self.disk_key(key) {
@@ -185,20 +193,24 @@ impl LayerCache {
                 dc.put_layer(dk, &buf);
             }
         }
-        self.insert_mem(key, buf);
+        self.insert_mem(key, buf)
     }
 
-    fn insert_mem(&self, key: u64, buf: Arc<Buf>) {
+    /// See [`LayerCache::insert`]: the buffer kept under `key` (`buf`, unless one was kept first
+    /// or `buf` is too large to keep).
+    fn insert_mem(&self, key: u64, buf: Arc<Buf>) -> Arc<Buf> {
         let bytes = buf.img.data.len() * std::mem::size_of::<effectcraft_raster::Px>();
-        let Ok(mut g) = self.inner.lock() else { return };
+        let Ok(mut g) = self.inner.lock() else { return buf };
         if bytes > g.budget / 4 {
-            return;
+            return buf;
         }
         g.clock += 1;
         let now = g.clock;
-        if let Some(old) = g.map.insert(key, Entry { buf, bytes, last_use: now }) {
-            g.bytes -= old.bytes;
+        if let Some(e) = g.map.get_mut(&key) {
+            e.last_use = now;
+            return e.buf.clone();
         }
+        g.map.insert(key, Entry { buf: buf.clone(), bytes, last_use: now });
         g.bytes += bytes;
         if g.bytes > g.budget {
             // Evict least-recently used entries down to 3/4 of the budget.
@@ -214,6 +226,7 @@ impl LayerCache {
                 }
             }
         }
+        buf
     }
 
     /// The memory budget in bytes.
@@ -668,3 +681,23 @@ pub fn derive(key: u64, i: u64) -> u64 {
     i.hash(&mut h);
     h.finish()
 }
+
+#[cfg(test)]
+mod shared_insert_tests {
+    use std::sync::Arc;
+
+    use super::LayerCache;
+    use effectcraft_effects::Buf;
+    use effectcraft_raster::Image;
+
+    /// Two renders of one layer kept under one key: both use the buffer kept first.
+    #[test]
+    fn the_buffer_kept_first_is_shared() {
+        let cache = LayerCache::default();
+        let buf = || Arc::new(Buf { img: Image::new(4, 4), offset: [0.0; 2], scale: 1.0 });
+        let (a, b) = (buf(), buf());
+        assert!(Arc::ptr_eq(&cache.insert(3, a.clone()), &a));
+        assert!(Arc::ptr_eq(&cache.insert(3, b), &a), "the later render gets the first buffer");
+        assert!(Arc::ptr_eq(&cache.get(3).unwrap(), &a));
+    }
+}

crates/render/src/lib.rs

@@ -1478,10 +1478,11 @@ impl<'a> Renderer<'a> {
         let mut nb = (*b).clone();
         color::convert(&mut nb.img, &c);
         let nb = Arc::new(nb);
-        if let (Some(cache), Some(k)) = (self.cache_to_keep(), key) {
-            cache.insert(k, nb.clone());
+        // The buffer the cache keeps (another worker's, if it kept one first).
+        match (self.cache_to_keep(), key) {
+            (Some(cache), Some(k)) => cache.insert(k, nb),
+            _ => nb,
         }
-        nb
     }
 
     fn layer_buf_keyed(&self, ctx: &EvalCtx, layer: &Layer) -> Option<(Arc<Buf>, Option<u64>)> {
@@ -1496,9 +1497,11 @@ impl<'a> Renderer<'a> {
             return Some((b, key));
         }
         let flat = Arc::new(styles::Styled { passes: st.passes.iter().map(|(b, m)| ((**b).clone(), *m)).collect(), body: (*st.body).clone() }.flatten());
-        if let (Some(c), Some(k)) = (self.cache_to_keep(), key) {
-            c.insert(k, flat.clone());
-        }
+        // The buffer the cache keeps (another worker's, if it kept one first).
+        let flat = match (self.cache_to_keep(), key) {
+            (Some(c), Some(k)) => c.insert(k, flat),
+            _ => flat,
+        };
         Some((flat, key))
     }
 
@@ -1527,9 +1530,11 @@ impl<'a> Renderer<'a> {
         }
         let buf = self.masked_source(ctx, layer)?;
         let buf = Arc::new(self.apply_effects_timed(ctx, layer, buf, false, usize::MAX, timing.map(|t| &mut t.effects)));
-        if let (Some(c), Some(k)) = (self.cache_to_keep(), key) {
-            c.insert(k, buf.clone());
-        }
+        // The buffer the cache keeps (another worker's, if it kept one first).
+        let buf = match (self.cache_to_keep(), key) {
+            (Some(c), Some(k)) => c.insert(k, buf),
+            _ => buf,
+        };
         Some((buf, key))
     }
 
@@ -1552,9 +1557,11 @@ impl<'a> Renderer<'a> {
             buf = self.effect_space_stack(ctx, layer, buf, false, effects, None, origin);
         }
         let buf = Arc::new(buf);
-        if let (Some(c), Some(k)) = (self.cache_to_keep(), key) {
-            c.insert(k, buf.clone());
-        }
+        // The buffer the cache keeps (another worker's, if it kept one first).
+        let buf = match (self.cache_to_keep(), key) {
+            (Some(c), Some(k)) => c.insert(k, buf),
+            _ => buf,
+        };
         Some(buf)
     }
 
@@ -2215,9 +2222,11 @@ impl<'a> Renderer<'a> {
         let mut foot = self.source(ctx, layer)?;
         let _ = masks::apply(ctx, layer, &mut foot, self.mask_blur(ctx, layer));
         let foot = Arc::new(foot);
-        if let (Some(c), Some(k)) = (self.cache_to_keep(), key) {
-            c.insert(k, foot.clone());
-        }
+        // The buffer the cache keeps (another worker's, if it kept one first).
+        let foot = match (self.cache_to_keep(), key) {
+            (Some(c), Some(k)) => c.insert(k, foot),
+            _ => foot,
+        };
         Some(foot)
     }
 

crates/render/src/tests.rs

@@ -1010,3 +1010,32 @@ fn a_selected_layer_reading_the_comp_scene_invalidates_its_owner() {
     let (a, b) = (particles_x(&here), particles_x(&panned));
     assert!(b < a - 20.0, "the blended particles follow the camera: {a} -> {b}");
 }
+
+/// #1812: preview workers that render a layer at once end up with one buffer (the one kept
+/// first), so the GPU compositor uploads and packs it once, not once per worker.
+#[test]
+fn workers_rendering_a_layer_at_once_share_one_buffer() {
+    let (mut p, cid, comp) = setup();
+    let mut l = solid(&mut p, &comp, [0.2, 0.5, 0.8], 1600, 1200);
+    add_effect(&mut p, &mut l, "ec.blur.gaussian", [1600.0, 1200.0], &[("blurriness", Value::Scalar(60.0))]);
+    p.comp_mut(cid).unwrap().layers.push(l);
+    let cache = crate::LayerCache::default();
+    let start = std::sync::Barrier::new(8);
+    let bufs: Vec<_> = std::thread::scope(|s| {
+        let hs: Vec<_> = (0..8)
+            .map(|_| {
+                s.spawn(|| {
+                    let mut r = crate::Renderer::new(&p, &crate::NoFootage, crate::RenderOpts::default());
+                    r.cache = Some(&cache);
+                    let c = p.comp(cid).unwrap();
+                    let ctx = r.eval_ctx(cid, Tick::ZERO).unwrap();
+                    start.wait();
+                    r.layer_buf(&ctx, &c.layers[0]).unwrap()
+                })
+            })
+            .collect();
+        hs.into_iter().map(|h| h.join().unwrap()).collect()
+    });
+    assert!(bufs.iter().all(|b| std::sync::Arc::ptr_eq(b, &bufs[0])), "one shared buffer");
+    assert_eq!(cache.stats().entries, 1);
+}