Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions crates/rustmotion-studio/src/editor/audio.rs
Original file line number Diff line number Diff line change
Expand Up @@ -165,9 +165,7 @@ pub fn prepare(scenario: Arc<ResolvedScenario>, total_duration: f64) {
match mixed {
Ok(Some(pcm_bytes)) => {
let pcm: Vec<f32> = pcm_bytes
.as_chunks::<2>()
.0
.iter()
.chunks_exact(2)
.map(|b| i16::from_le_bytes([b[0], b[1]]) as f32 / 32768.0)
.collect();
let _ = tx.send(Cmd::Load(Arc::new(pcm), SAMPLE_RATE));
Expand Down
11 changes: 8 additions & 3 deletions crates/rustmotion-studio/src/editor/frames.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,20 +26,25 @@ 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.
/// than cloned. `Err` means the render thread panicked (a Skia panic, a
/// dimension mismatch, a JPEG encode failure): the caller must not treat that
/// as a successful empty frame.
// The panic payload carries no information callers act on; they only branch
// on success vs. failure (retry-budget ledger, cache, HTTP status).
#[allow(clippy::result_unit_err)]
pub fn render_frame_deep(
scenario: &ResolvedScenario,
tasks: &[rustmotion::encode::video::FrameTask],
frame: u32,
scale: f32,
) -> Vec<u8> {
) -> Result<Vec<u8>, ()> {
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()
.map_err(|_| ())
})
}

Expand Down
24 changes: 16 additions & 8 deletions crates/rustmotion-studio/src/editor/view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -361,6 +361,10 @@ pub fn StudioApp(view: Signal<View>) -> Element {
/// into the cache for the next request. `gen_b` is `Some(model generation)`
/// when serving side B (drives stale-generation eviction); side A passes the
/// baseline hash inside `key.generation` and `None` here.
///
/// `render_frame_deep` already isolates a render panic inside its own scoped
/// thread and reports it as `Err`; the `catch_unwind` here only guards the
/// (rarer) case of the OS failing to spawn that thread.
fn serve_or_render(
key: FrameKey,
idx: u32,
Expand All @@ -385,15 +389,19 @@ fn serve_or_render(
{
return Err(());
}
let rendered = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
let outer = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
render_frame_deep(scenario, tasks, idx, scale_factor(key.scale_pct))
}))
.map_err(|_| {
fail_ledger()
.lock()
.unwrap_or_else(|e| e.into_inner())
.record_failure(key);
})?;
}));
let rendered = match outer {
Ok(Ok(bytes)) => bytes,
_ => {
fail_ledger()
.lock()
.unwrap_or_else(|e| e.into_inner())
.record_failure(key);
return Err(());
}
};
let (gen_b, gen_a) = match key.side {
DiffSide::B => (gen_b, None),
DiffSide::A => (None, Some(key.generation)),
Expand Down
2 changes: 1 addition & 1 deletion crates/rustmotion-studio/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
mod app;
mod components;
mod editor;
pub mod editor;
mod library;
pub mod scenario;

Expand Down
4 changes: 1 addition & 3 deletions crates/rustmotion-studio/src/library/data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,9 +132,7 @@ pub fn render_thumbnail(path: &Path) -> Option<Vec<u8>> {
if tasks.is_empty() {
return None;
}
Some(crate::editor::frames::render_frame_deep(
&scenario, &tasks, 0, 0.25,
))
crate::editor::frames::render_frame_deep(&scenario, &tasks, 0, 0.25).ok()
}

/// Cheap check: is this JSON a Rustmotion scenario? Avoids the heavier
Expand Down
4 changes: 1 addition & 3 deletions crates/rustmotion-studio/src/scenario/optimistic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -382,9 +382,7 @@ mod tests {
.expect("render")
};
let count_red = |rgba: &[u8]| {
rgba.as_chunks::<4>()
.0
.iter()
rgba.chunks_exact(4)
.filter(|p| p[0] > 180 && p[1] < 90 && p[2] < 90)
.count()
};
Expand Down
65 changes: 56 additions & 9 deletions crates/rustmotion-studio/tests/audit_ws_g.rs
Original file line number Diff line number Diff line change
@@ -1,23 +1,27 @@
//! Regression tests for the studio's file-write pipeline: a debounced write
//! Regression tests for the studio's file-write pipeline — a debounced write
//! that must rebase onto the current disk instead of replaying a stale
//! in-memory snapshot, coalesced edits inside one debounce window that must
//! all survive (not just the last), and undo/redo cancelling a still-pending
//! write before it can clobber the just-restored state.
//! write before it can clobber the just-restored state — plus a render-thread
//! panic that must surface as an error instead of being cached as an empty
//! JPEG.
//!
//! `rustmotion-studio` has no `[dev-dependencies]` and cannot gain one in
//! this change, so every test below drives the crate's existing public
//! surface (`scenario::*`) against real temp files with plain synchronous
//! `#[test]`s — no Dioxus runtime, no async executor. That public surface is
//! itself the fix for the crate having no integration tests: the defects
//! lived in a debounce timer and Dioxus event handlers that cannot be driven
//! from a test, so each one was reduced to a pure decision over plain data
//! (`resolve_flush`, the pending-write queue) and the handler calls that
//! instead of deciding inline.
//! surface (`scenario::*`, `editor::frames`) against real temp files with
//! plain synchronous `#[test]`s — no Dioxus runtime, no async executor. That
//! public surface is itself the fix for the crate having no integration
//! tests: the defects lived in a debounce timer and Dioxus event handlers
//! that cannot be driven from a test, so each one was reduced to a pure
//! decision over plain data (`resolve_flush`, the pending-write queue,
//! `render_frame_deep` returning `Result`) and the handler calls that instead
//! of deciding inline.

use std::fs;
use std::path::PathBuf;
use std::sync::{Arc, Mutex};

use rustmotion_studio::editor::frames::render_frame_deep;
use rustmotion_studio::scenario::{
apply_optimistic, empty_scenario, pending_write_slot, queue_mutation, record_edit,
resolve_flush, take_pending, undo, Mutation, Shared, SharedHistory, StudioModel,
Expand Down Expand Up @@ -219,3 +223,46 @@ fn the_write_pipeline_round_trips_an_edit_through_a_real_file() {
);
let _ = fs::remove_file(&path);
}

// ── A render-thread panic surfaces as an error, never a cached empty JPEG ──

#[test]
fn a_render_thread_panic_is_reported_as_an_error_not_an_empty_jpeg() {
let big = rustmotion::loader::load_scenario_from_source(
None,
Some(r##"{ "video": { "width": 64, "height": 64 }, "scenes": [ { "duration": 0.1 }, { "duration": 0.1 } ] }"##),
)
.unwrap();
let tasks = rustmotion::encode::build_frame_tasks(&big);
assert!(
tasks.len() >= 2,
"need frames spanning both scenes to reach scene_idx 1"
);

// Deliberately mismatched: `tasks` reference a second scene this smaller
// scenario does not have, which panics inside the render thread.
let small = rustmotion::loader::load_scenario_from_source(
None,
Some(r##"{ "video": { "width": 64, "height": 64 }, "scenes": [ { "duration": 0.1 } ] }"##),
)
.unwrap();

let last_frame = (tasks.len() - 1) as u32;
let result = render_frame_deep(&small, &tasks, last_frame, 1.0);
assert!(
result.is_err(),
"a scenario/task mismatch panics inside the render thread and must surface as Err"
);
}

#[test]
fn a_normal_render_returns_nonempty_jpeg_bytes() {
let scenario = rustmotion::loader::load_scenario_from_source(
None,
Some(r##"{ "video": { "width": 64, "height": 64 }, "scenes": [ { "duration": 0.1 } ] }"##),
)
.unwrap();
let tasks = rustmotion::encode::build_frame_tasks(&scenario);
let jpeg = render_frame_deep(&scenario, &tasks, 0, 1.0).expect("a normal render succeeds");
assert!(!jpeg.is_empty());
}
Loading