address review: audio detection by breadth-first search, private cache folder resolved before the temp check, oversized layers not written, solid cap after rounding, selector static amount, viewport i32 clamp

crates/render/src/audio.rs

@@ -44,34 +44,29 @@ pub fn mix_comp(project: &Project, footage: &dyn FootageSource, expr: Option<&dy
 /// Whether the comp has anything audible (footage with audio, a sound generator, or a precomp
 /// that has).
 pub fn comp_has_audio(project: &Project, comp: ItemId) -> bool {
-    // Each comp is answered once: a comp used by many layers at each of several levels was
-    // walked again for every use.
-    fn walk(project: &Project, comp: ItemId, depth: usize, seen: &mut std::collections::HashMap<ItemId, bool>) -> bool {
-        if let Some(&r) = seen.get(&comp) {
-            return r;
+    // Breadth first, so each comp is looked at once and at its shallowest level: a comp reached
+    // first through a long chain (past MAX_DEPTH) is still found by a shorter route, and a cycle
+    // cannot hide a result (nothing is cached as "silent" while it is being explored).
+    let mut seen = std::collections::HashSet::from([comp]);
+    let mut level = vec![comp];
+    for _ in 0..MAX_DEPTH {
+        let mut next = vec![];
+        for id in level {
+            let Some(c) = project.comp(id) else { continue };
+            for l in c.layers.iter().filter(|l| l.switches.audio) {
+                match &l.source {
+                    _ if generates(l) => return true,
+                    LayerSource::Footage { item } if matches!(project.item(*item).map(|i| &i.kind), Some(ItemKind::Footage(f)) if f.has_audio && !f.missing) => {
+                        return true;
+                    }
+                    LayerSource::Comp { item } if seen.insert(*item) => next.push(*item),
+                    _ => {}
+                }
+            }
         }
-        // In progress: a cycle back into it adds nothing.
-        seen.insert(comp, false);
-        let r = walk_once(project, comp, depth, seen);
-        seen.insert(comp, r);
-        r
+        level = next;
     }
-    fn walk_once(project: &Project, comp: ItemId, depth: usize, seen: &mut std::collections::HashMap<ItemId, bool>) -> bool {
-        let Some(c) = project.comp(comp) else { return false };
-        depth < MAX_DEPTH
-            && c.layers.iter().any(|l| {
-                l.switches.audio
-                    && (generates(l)
-                        || match &l.source {
-                            LayerSource::Footage { item } => {
-                                matches!(project.item(*item).map(|i| &i.kind), Some(ItemKind::Footage(f)) if f.has_audio && !f.missing)
-                            }
-                            LayerSource::Comp { item } => walk(project, *item, depth + 1, seen),
-                            _ => false,
-                        })
-            })
-    }
-    walk(project, comp, 0, &mut std::collections::HashMap::new())
+    false
 }
 
 /// Enabled audio effects of a layer (in stack order) with their effect ids.
@@ -419,6 +414,33 @@ mod tests {
         assert!(!rx.recv_timeout(std::time::Duration::from_secs(20)).expect("detection finishes"));
     }
 
+    #[test]
+    fn audio_found_by_a_short_route_is_not_hidden_by_a_deep_one() {
+        let mut p = Project::default();
+        let base = Comp::new(8, 8, FrameRate::new(25, 1), Tick::from_seconds_f64(1.0));
+        let mk = |p: &mut Project, layers: Vec<Layer>| {
+            let mut c = base.clone();
+            c.layers = layers.into();
+            p.add_item("C", effectcraft_color::Label::Sandstone, None, ItemKind::Comp(c.into()))
+        };
+        let f = Footage { kind: FootageKind::Audio, has_audio: true, ..Default::default() };
+        let fid = p.add_item("x.wav", effectcraft_color::Label::SeaFoam, None, ItemKind::Footage(f));
+        let sound = build::layer(&mut p, &base, "s", LayerSource::Footage { item: fid }, (0, 0), None);
+        let audible = mk(&mut p, vec![sound]);
+        // A chain of MAX_DEPTH comps ends in `audible` one level past what is searched; the root
+        // reaches `audible` directly as well, after the chain.
+        let mut chain = audible;
+        for _ in 0..MAX_DEPTH {
+            let l = build::layer(&mut p, &base, "c", LayerSource::Comp { item: chain }, (0, 0), None);
+            chain = mk(&mut p, vec![l]);
+        }
+        assert!(!comp_has_audio(&p, chain), "too deep to be found");
+        let via_chain = build::layer(&mut p, &base, "chain", LayerSource::Comp { item: chain }, (0, 0), None);
+        let direct = build::layer(&mut p, &base, "direct", LayerSource::Comp { item: audible }, (0, 0), None);
+        let root = mk(&mut p, vec![via_chain, direct]);
+        assert!(comp_has_audio(&p, root));
+    }
+
     #[test]
     fn mixes_layers_with_levels_and_in_out() {
         let mut p = Project::default();

crates/render/src/disk_cache.rs

@@ -310,7 +310,13 @@ pub fn default_folder() -> PathBuf {
 #[cfg(unix)]
 fn make_private(folder: &Path) -> std::io::Result<()> {
     use std::os::unix::fs::PermissionsExt;
-    if !folder.starts_with(std::env::temp_dir()) {
+    // Judge the folder as written and as it resolves (a relative path, a link or an alias such as
+    // /tmp -> /private/tmp can all land in the shared temp folder).
+    let tmp = std::env::temp_dir();
+    let tmp_real = tmp.canonicalize().unwrap_or_else(|_| tmp.clone());
+    let given = std::path::absolute(folder).unwrap_or_else(|_| folder.to_path_buf());
+    let real = folder.canonicalize().unwrap_or_else(|_| given.clone());
+    if ![&given, &real].iter().any(|p| p.starts_with(&tmp) || p.starts_with(&tmp_real)) {
         return Ok(());
     }
     if std::fs::symlink_metadata(folder)?.file_type().is_symlink() {
@@ -634,7 +640,7 @@ impl DiskCache {
 
     /// Queue a layer buffer for writing.
     pub fn put_layer(&self, key: u128, buf: &Buf) {
-        if self.contains(Kind::Layer, key) {
+        if !layer_fits(buf.img.data.len()) || self.contains(Kind::Layer, key) {
             return;
         }
         self.enqueue(Kind::Layer, key, encode_layer(buf));
@@ -642,6 +648,9 @@ impl DiskCache {
 
     /// Write a layer buffer synchronously (tests).
     pub fn put_layer_now(&self, key: u128, buf: &Buf) {
+        if !layer_fits(buf.img.data.len()) {
+            return;
+        }
         self.write_now(Kind::Layer, key, &encode_layer(buf));
     }
 
@@ -827,6 +836,12 @@ fn decode_frame(p: &[u8]) -> Option<Frame8> {
     (rgba.len() == w as usize * h as usize * 4).then_some(Frame8 { width: w, height: h, rgba })
 }
 
+/// Whether a layer of `pixels` pixels (16 bytes each, decompressed) is within what a cache entry
+/// may hold: a larger one could never be read back, so it is not compressed or written.
+fn layer_fits(pixels: usize) -> bool {
+    pixels.checked_mul(16).is_some_and(|b| b <= MAX_ENTRY_BYTES)
+}
+
 fn encode_layer(b: &Buf) -> Vec<u8> {
     let (w, h) = (b.img.width, b.img.height);
     let n = b.img.data.len();
@@ -1034,6 +1049,32 @@ mod tests {
         let _ = std::fs::remove_dir_all(&d);
     }
 
+    #[cfg(unix)]
+    #[test]
+    fn a_folder_that_resolves_into_the_temp_folder_is_made_private() {
+        use std::os::unix::fs::PermissionsExt;
+        // `<cwd>/link` names a folder in the temp folder only through the link.
+        let real = tmpdir("alias-real");
+        std::fs::create_dir_all(&real).unwrap();
+        let link = std::env::current_dir().unwrap().join(format!("effectcraft-alias-link-{}", std::process::id()));
+        let _ = std::fs::remove_file(&link);
+        std::os::unix::fs::symlink(&real, &link).unwrap();
+        let dc = DiskCache::open(link.join("cache"), 1 << 20);
+        let mode = std::fs::metadata(real.join("cache")).map(|m| m.permissions().mode() & 0o777);
+        drop(dc);
+        let _ = std::fs::remove_file(&link);
+        let _ = std::fs::remove_dir_all(&real);
+        assert_eq!(mode.unwrap(), 0o700);
+    }
+
+    #[test]
+    fn layers_too_big_to_read_back_are_not_written() {
+        assert!(layer_fits(MAX_ENTRY_BYTES / 16));
+        assert!(!layer_fits(MAX_ENTRY_BYTES / 16 + 1));
+        assert!(!layer_fits(10_000 * 10_000));
+        assert!(!layer_fits(usize::MAX));
+    }
+
     #[test]
     fn an_entry_stating_a_size_its_dimensions_do_not_need_is_not_decompressed() {
         // A 2×2 frame whose payload states 3 GiB: refused before any allocation.

crates/render/src/pipeline.rs

@@ -39,6 +39,26 @@ pub struct MediaOverride {
 /// The most pixels a solid layer's buffer holds (as the other layer buffers are held to).
 const MAX_SOLID_PIXELS: f64 = 16.0e6;
 
+/// The scale and whole-pixel size of a `width`×`height` solid rendered at `scale`, lowered until
+/// the rounded size (each side at least one pixel) is within [`MAX_SOLID_PIXELS`].
+pub(crate) fn solid_size(width: u32, height: u32, mut scale: f64) -> (f64, u32, u32) {
+    let size = |s: f64| (((width as f64 * s).ceil() as u32).max(1), ((height as f64 * s).ceil() as u32).max(1));
+    // Rounding up leaves an elongated solid over the cap after one step; each step shrinks the
+    // overshoot geometrically, so a few are enough.
+    for i in 0..64 {
+        let (w, h) = size(scale);
+        let area = w as f64 * h as f64;
+        if area <= MAX_SOLID_PIXELS {
+            break;
+        }
+        // The first step is the exact area ratio; later ones undershoot a little so rounding up
+        // cannot leave the size stuck just over the cap.
+        scale *= (MAX_SOLID_PIXELS / area).sqrt() * if i == 0 { 1.0 } else { 0.99 };
+    }
+    let (w, h) = size(scale);
+    (scale, w, h)
+}
+
 impl MediaOverride {
     /// A Media Replacement of `layer`'s footage by `item`.
     pub fn new(layer: LayerId, item: ItemId) -> Self {
@@ -695,10 +715,7 @@ impl<'a> Renderer<'a> {
                 let ItemKind::Solid(sol) = &self.project.item(*item)?.kind else { return None };
                 // One colour: a solid larger than MAX_SOLID_PIXELS is made at a lower scale (the
                 // buffer carries it) instead of allocating gigabytes for a uniform plane.
-                let area = (sol.width as f64 * s) * (sol.height as f64 * s);
-                let s = if area > MAX_SOLID_PIXELS { s * (MAX_SOLID_PIXELS / area).sqrt() } else { s };
-                let w = ((sol.width as f64 * s).ceil() as u32).max(1);
-                let h = ((sol.height as f64 * s).ceil() as u32).max(1);
+                let (s, w, h) = solid_size(sol.width, sol.height, s);
                 if layer.switches.adjustment {
                     return Some(Buf { img: Image::filled(w, h, [1.0; 4]), offset: [0.0; 2], scale: s });
                 }

crates/render/src/tests.rs

@@ -1759,3 +1759,17 @@ fn a_copy_of_an_overridden_clip_is_not_served_the_keyed_clips_pixels() {
     assert_same(&render(&keyed, Some(&cache)), &render(&keyed, None), "the keyed clip's copy");
     assert_same(&render(&overridden, Some(&cache)), &render(&overridden, None), "the overridden clip's copy");
 }
+
+#[test]
+fn elongated_solids_stay_within_the_pixel_cap_after_rounding() {
+    use crate::pipeline::solid_size;
+    // 1 × 100 million at full scale: rounding up used to leave 40 million pixels.
+    for (w, h) in [(1u32, 100_000_000u32), (100_000_000, 1), (u32::MAX, u32::MAX), (1, u32::MAX)] {
+        let (s, pw, ph) = solid_size(w, h, 1.0);
+        assert!(s > 0.0 && f64::from(pw) * f64::from(ph) <= 16.0e6, "{w}x{h} -> {pw}x{ph} at {s}");
+    }
+    // A solid within the cap keeps its scale; an over-cap square is lowered by the exact ratio.
+    assert_eq!(solid_size(4000, 4000, 1.0), (1.0, 4000, 4000));
+    let (s, pw, ph) = solid_size(5000, 5000, 1.0);
+    assert!((s - 0.8).abs() < 0.01 && pw * ph <= 16_000_000);
+}

crates/render/src/text.rs

@@ -243,7 +243,9 @@ pub fn animator_selection(ctx: &EvalCtx, layer: &Layer, anim: &PropGroup, lay: &
             }
             "expressionSelector" => {
                 let Some(prop) = sel.get("amount") else { continue };
-                let stat = ctx.v3(layer, sel, "amount", [100.0; 3]);
+                // The keyframed amount, read without running its expression (the fallback when the
+                // expression cannot run); an Essential Property override still applies.
+                let stat = if prop.has_expression() { prop.value_at(layer.layer_time(ctx.time)) } else { ctx.value(layer, prop) }.as_vec3();
                 // One allowance for the whole layer: the expression runs per character, each run
                 // with its own time limit, so a slow expression on a long text is stopped once it
                 // has failed or the layer's evaluations pass MAX_SELECTOR_TIME.

crates/render/src/three_d/adv/raster.rs

@@ -127,8 +127,8 @@ fn setup(s: &Scene, clip: &[[f32; 4]], range: std::ops::Range<usize>) -> Vec<STr
                 y1 = y1.max(v.0[1]);
             }
             // Clamped on both sides to the viewport before narrowing to i32, so a far-off
-            // coordinate cannot wrap into a huge pixel range.
-            let (w, h) = (s.width as i64, s.height as i64);
+            // coordinate cannot wrap into a huge pixel range (a viewport past i32::MAX is capped there).
+            let (w, h) = (i64::from(s.width.min(i32::MAX as u32)), i64::from(s.height.min(i32::MAX as u32)));
             let bbox = [
                 (x0.floor() as i64).clamp(0, w) as i32,
                 (y0.floor() as i64).clamp(0, h) as i32,