From 588b13511fe8b3bd6cfd505365a2906efc30d131 Mon Sep 17 00:00:00 2001 From: Baptiste Parmantier Date: Tue, 29 Sep 2026 10:42:17 +0200 Subject: [PATCH] perf(render): deserialize a scene's children once instead of once per frame prepare_scene ran on the per-frame path and had no cache, although its result depends only on the scene. Every frame cloned the whole serde_json subtree of every child and deserialized it into ChildComponent -- an untagged enum with 57 variants, so serde re-parses the same object once per failed variant before it lands. Measured on two scenarios, timing the call for every frame of a render: mega-showcase, 1236 frames, 9 scenes 144ms -> 46ms 50 nodes in one scene, 1200 frames 388ms -> 3.2ms The 46ms that remain on mega-showcase are the nine first-time deserializations, which is the work that actually has to happen. End-to-end this is within noise on both (9.0s and 8.5s): the redundancy was 1.6% and 4.5% of a render, spread across workers. The issue asked for the order of magnitude before deciding how much to spend, and this is it -- so the fix is the contained one, not the restructuring. The memo is a OnceLock on the Scene rather than a keyed cache, so it has no identity problem and no invalidation to get wrong: it dies with the scenario. rustmotion-core cannot name ChildComponent, which lives a crate above it, so the slot holds Arc and prepare_scene is the typed accessor -- and its only writer, which is what makes the downcast total. The studio rebuilds a whole ResolvedScenario on every optimistic edit and swaps the Arc, so its scenes are new objects with fresh slots; there is no path that mutates scene.children in place after load. deserialize_children's stderr warning for an unreadable child rode on the repetition. It is now emitted once per scene. --- crates/rustmotion-core/src/schema/scenario.rs | 6 ++ crates/rustmotion/src/engine/render/scene.rs | 83 ++++++++++++++++++- 2 files changed, 87 insertions(+), 2 deletions(-) diff --git a/crates/rustmotion-core/src/schema/scenario.rs b/crates/rustmotion-core/src/schema/scenario.rs index c3a5f68..c705ec9 100644 --- a/crates/rustmotion-core/src/schema/scenario.rs +++ b/crates/rustmotion-core/src/schema/scenario.rs @@ -645,6 +645,12 @@ pub struct Scene { pub background: Option, #[serde(default)] pub children: Vec, + /// Filled once, by whoever first deserializes `children` into the component + /// type it owns. `rustmotion-core` cannot name that type, so it is held + /// erased; `Scene::prepared_children` is the typed accessor. + #[serde(skip)] + #[schemars(skip)] + pub prepared_children: std::sync::OnceLock>, #[serde(default)] pub transition: Option, #[serde(default)] diff --git a/crates/rustmotion/src/engine/render/scene.rs b/crates/rustmotion/src/engine/render/scene.rs index fbf42fd..607a8f8 100644 --- a/crates/rustmotion/src/engine/render/scene.rs +++ b/crates/rustmotion/src/engine/render/scene.rs @@ -1,5 +1,6 @@ use crate::error::Result; use skia_safe::{surfaces, Canvas, ClipOp, ColorType, ImageInfo, Paint, Rect}; +use std::sync::Arc; use super::background::draw_animated_background; use super::background::draw_world_bg_with_parallax; @@ -680,8 +681,13 @@ pub fn deserialize_children(scene: &Scene) -> Vec { .collect() } -pub fn prepare_scene(scene: &Scene, _config: &VideoConfig) -> Vec { - deserialize_children(scene) +pub fn prepare_scene(scene: &Scene, _config: &VideoConfig) -> Arc> { + scene + .prepared_children + .get_or_init(|| Arc::new(deserialize_children(scene))) + .clone() + .downcast::>() + .expect("prepare_scene is the only writer of Scene::prepared_children") } pub fn resolve_node_references( @@ -1461,3 +1467,76 @@ pub(super) fn apply_camera_transform( } canvas.translate((-cx - x, -cy - y)); } + +#[cfg(test)] +mod prepared_children_tests { + use super::*; + + fn scenario(children: &str) -> crate::schema::ResolvedScenario { + let json = format!( + r##"{{ + "version": "1.0", + "video": {{ "width": 160, "height": 90, "fps": 30, "background": "#000000" }}, + "scenes": [ + {{ "duration": 1.0, "children": [{children}] }}, + {{ "duration": 1.0, "children": [{children}] }} + ] + }}"## + ); + crate::loader::load_scenario_from_source(None, Some(&json)).expect("load") + } + + const TEXT: &str = r##"{ "type": "text", "content": "hi", "style": { "font-size": 20 } }"##; + + #[test] + fn a_scene_is_deserialized_once_and_every_later_frame_gets_that_same_vec() { + let scenario = scenario(TEXT); + let scene = &scenario.views[0].scenes[0]; + + let first = prepare_scene(scene, &scenario.video); + let second = prepare_scene(scene, &scenario.video); + assert!( + Arc::ptr_eq(&first, &second), + "prepare_scene runs once per frame and its result depends only on the scene — a \ + second call must hand back the same allocation, not re-parse a 57-variant untagged \ + enum for every child" + ); + assert_eq!(first.len(), 1); + } + + #[test] + fn two_scenes_do_not_share_one_anothers_children() { + let scenario = scenario(TEXT); + let first = prepare_scene(&scenario.views[0].scenes[0], &scenario.video); + let second = prepare_scene(&scenario.views[0].scenes[1], &scenario.video); + assert!( + !Arc::ptr_eq(&first, &second), + "the memo lives on the scene, so two scenes must each get their own" + ); + } + + #[test] + fn the_memo_carries_the_children_the_scene_actually_declares() { + let scenario = scenario(TEXT); + let prepared = prepare_scene(&scenario.views[0].scenes[0], &scenario.video); + assert_eq!( + prepared.len(), + scenario.views[0].scenes[0].children.len(), + "every readable child must be there — the memo is a cache, not a filter" + ); + } + + #[test] + fn a_child_that_cannot_be_read_is_reported_once_rather_than_once_per_frame() { + let scenario = scenario(r##"{ "type": "no-such-component" }"##); + let scene = &scenario.views[0].scenes[0]; + let first = prepare_scene(scene, &scenario.video); + let second = prepare_scene(scene, &scenario.video); + assert!(first.is_empty(), "the unreadable child is dropped"); + assert!( + Arc::ptr_eq(&first, &second), + "the stderr warning deserialize_children emits rode on that repetition — one \ + deserialization means one warning per scene, not one per frame" + ); + } +}