From d4dbcc9fd4dc5c71cddc4c4c3e495b6333bdb136 Mon Sep 17 00:00:00 2001 From: Baptiste Parmantier Date: Thu, 13 Aug 2026 12:23:55 +0200 Subject: [PATCH] fix(studio): render frames on a stack deep enough to deserialise them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Opening a deeply nested scenario aborted the whole process with a stack overflow, while the same file rendered fine from the CLI. The depth cost is deserialisation, not painting: `prepare_scene` calls `deserialize_children` on every frame, and `ChildComponent` -> `Component` -> `Container`/`Card` recurses once per level. Those enums are untagged, so serde buffers each level through `ContentDeserializer` — a level is expensive in stack as well as deep, and a debug build's frames are several times fatter. Measured on a 28-level scenario: between 2 and 4 MiB needed in debug. Rust gives a spawned thread 2 MiB, so the prefetch workers overflowed; the CLI survived only because it happens to render on the 8 MiB main thread. Both webview asset handlers were exposed for the same reason. All three render sites now get an explicit stack. `render_frame_deep` is scoped, so the scenario and tasks are borrowed rather than cloned. Adds a probe test that reports the stack a given scenario needs. --- crates/rustmotion-studio/src/editor/frames.rs | 38 ++++++++++++++++++ .../rustmotion-studio/src/editor/prefetch.rs | 7 +++- crates/rustmotion-studio/src/editor/view.rs | 4 +- crates/rustmotion-studio/src/library/data.rs | 2 +- .../rustmotion/tests/deep_scenario_stack.rs | 40 +++++++++++++++++++ 5 files changed, 87 insertions(+), 4 deletions(-) create mode 100644 crates/rustmotion/tests/deep_scenario_stack.rs diff --git a/crates/rustmotion-studio/src/editor/frames.rs b/crates/rustmotion-studio/src/editor/frames.rs index 64b369ed..a734fd2e 100644 --- a/crates/rustmotion-studio/src/editor/frames.rs +++ b/crates/rustmotion-studio/src/editor/frames.rs @@ -5,6 +5,44 @@ use std::sync::{Arc, Mutex, OnceLock}; use rustmotion::encode::video::FrameTask; use rustmotion::schema::ResolvedScenario; +/// Stack for any thread that renders a frame. +/// +/// The depth cost is **deserialisation**, not painting: `prepare_scene` calls +/// `deserialize_children` on every frame, and `ChildComponent` → `Component` → +/// `Container`/`Card` recurses once per level of nesting. Because those enums +/// are untagged, serde buffers each level through `ContentDeserializer`, so a +/// level is expensive in stack as well as deep — and a debug build's frames are +/// several times fatter than a release build's. +/// +/// Measured: a scenario nested ~28 levels — which a UI composed from small +/// helper functions reaches without trying — needs between 2 and 4 MiB in debug. +/// Rust's default for a spawned thread is 2 MiB, so the prefetch workers +/// overflowed and aborted the process; the same file rendered fine from the CLI, +/// which happens to run on the 8 MiB main thread. The two webview asset handlers +/// get whatever stack the platform hands their callback, which is no better. +/// +/// Address space is cheap and committed lazily; take a wide margin. +pub const RENDER_STACK: usize = 32 * 1024 * 1024; + +/// [`render_frame`] on a thread with [`RENDER_STACK`], for callers that are not +/// on the main thread. Scoped, so the scenario and tasks are borrowed rather +/// than cloned. +pub fn render_frame_deep( + scenario: &ResolvedScenario, + tasks: &[rustmotion::encode::video::FrameTask], + frame: u32, + scale: f32, +) -> Vec { + std::thread::scope(|scope| { + std::thread::Builder::new() + .stack_size(RENDER_STACK) + .spawn_scoped(scope, || render_frame(scenario, tasks, frame, scale)) + .expect("spawn render thread") + .join() + .unwrap_or_default() + }) +} + /// Render one frame and encode it to JPEG bytes (preview-only; the final video /// render path does not use this). JPEG keeps the encode cost low enough for /// the webview transport to keep up with playback. diff --git a/crates/rustmotion-studio/src/editor/prefetch.rs b/crates/rustmotion-studio/src/editor/prefetch.rs index 462896d2..4d68d6b0 100644 --- a/crates/rustmotion-studio/src/editor/prefetch.rs +++ b/crates/rustmotion-studio/src/editor/prefetch.rs @@ -299,7 +299,12 @@ pub fn ensure_prefetcher() { .map(|n| n.get()) .unwrap_or(4); for _ in 0..worker_count(cores) { - std::thread::spawn(prefetch_loop); + // Explicit stack: these workers render, and the 2 MiB default is + // not enough for a deeply nested scenario in a debug build. Sized + // once here rather than per frame — see `frames::RENDER_STACK`. + let _ = std::thread::Builder::new() + .stack_size(crate::editor::frames::RENDER_STACK) + .spawn(prefetch_loop); } }); } diff --git a/crates/rustmotion-studio/src/editor/view.rs b/crates/rustmotion-studio/src/editor/view.rs index 9ee60522..75362516 100644 --- a/crates/rustmotion-studio/src/editor/view.rs +++ b/crates/rustmotion-studio/src/editor/view.rs @@ -11,7 +11,7 @@ use crate::scenario::{ use super::annotations::AnnotationsPanel; use super::diff_panel::{DiffPanel, DiffSide}; use super::export::ExportToast; -use super::frames::{baseline_arcs, frame_hits, render_frame, scene_prefix, HitPct}; +use super::frames::{baseline_arcs, frame_hits, render_frame_deep, scene_prefix, HitPct}; use super::inspector::InspectorPanel; use super::playback::{ playback_action, use_hot_reload, use_playback_clock, PlaybackAction, PlaybackBar, @@ -386,7 +386,7 @@ fn serve_or_render( return Err(()); } let rendered = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { - render_frame(scenario, tasks, idx, scale_factor(key.scale_pct)) + render_frame_deep(scenario, tasks, idx, scale_factor(key.scale_pct)) })) .map_err(|_| { fail_ledger() diff --git a/crates/rustmotion-studio/src/library/data.rs b/crates/rustmotion-studio/src/library/data.rs index 11205f45..f62482cb 100644 --- a/crates/rustmotion-studio/src/library/data.rs +++ b/crates/rustmotion-studio/src/library/data.rs @@ -132,7 +132,7 @@ pub fn render_thumbnail(path: &Path) -> Option> { if tasks.is_empty() { return None; } - Some(crate::editor::frames::render_frame( + Some(crate::editor::frames::render_frame_deep( &scenario, &tasks, 0, 0.25, )) } diff --git a/crates/rustmotion/tests/deep_scenario_stack.rs b/crates/rustmotion/tests/deep_scenario_stack.rs new file mode 100644 index 00000000..a4e5b4b4 --- /dev/null +++ b/crates/rustmotion/tests/deep_scenario_stack.rs @@ -0,0 +1,40 @@ +//! A deeply nested scenario must render on the stacks the app actually uses. +//! +//! The CLI renders on the main thread (8 MB on macOS) and never noticed. The +//! studio renders thumbnails inside a webview asset-handler callback, which runs +//! on an OS-provided thread with a far smaller stack — so a scenario that is +//! merely *deep* aborts the whole process with a stack overflow there while +//! rendering fine from the terminal. +use std::path::PathBuf; + +fn render_on(stack_kb: usize, path: PathBuf) -> bool { + std::thread::Builder::new() + .stack_size(stack_kb * 1024) + .spawn(move || { + let scenario = rustmotion::loader::load_input(&path).expect("load"); + let tasks = rustmotion::encode::build_frame_tasks(&scenario); + rustmotion::encode::render_frame_task_scaled( + &scenario.video, + &scenario, + &tasks[0], + 0.25, + ) + .expect("render"); + }) + .unwrap() + .join() + .is_ok() +} + +#[test] +#[ignore = "probe: needs PROBE_FILE="] +fn report_stack_needed() { + // One size per process: a stack overflow aborts, it cannot be caught, so a + // loop inside a single run would stop at the first failure. + let p = PathBuf::from(std::env::var("PROBE_FILE").expect("PROBE_FILE")); + let kb: usize = std::env::var("PROBE_STACK_KB").unwrap().parse().unwrap(); + println!( + "stack {kb} KiB -> {}", + if render_on(kb, p) { "ok" } else { "panic" } + ); +}