From b2cf845624a7065cc18ce26e505f6a3abf669967 Mon Sep 17 00:00:00 2001 From: redonion23 <199118131+redonion23@users.noreply.github.com> Date: Tue, 13 Oct 2026 01:35:49 +0200 Subject: [PATCH] Scripting: key calls fetch only the key they need, so removeKey stays fast (#814) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every key call (removeKey, keyTime, keyValue, setInterpolationTypeAtKey…) looked its key up through `__keys()`, which serialised the whole key list, eases and tangents included, on each call. Removing keys one by one was therefore quadratic: about 100 ms per removeKey on a 2,000-key property. The `keys` query now takes an optional `index` and returns just that key with the count; `__key` uses it. removeKey on 2,000 keys drops from 103.7 ms to 0.4 ms (release, Windows 11 aarch64). Co-Authored-By: Claude Opus 5.5 --- crates/script/src/model.rs | 41 ++++++++++++++++++------------------ crates/script/src/prelude.js | 7 +++--- crates/script/src/tests.rs | 18 ++++++++++++++++ 3 files changed, 43 insertions(+), 23 deletions(-) diff --git a/crates/script/src/model.rs b/crates/script/src/model.rs index 4445fbbf..419be0c1 100644 --- a/crates/script/src/model.rs +++ b/crates/script/src/model.rs @@ -502,27 +502,28 @@ fn keys(s: &Session, a: &J) -> R { .collect(); J::Array(v) }; - let out: Vec = p - .keys - .iter() - .enumerate() - .map(|(i, k)| { - // Spatial tangents as the motion path plays (auto-Bezier and continuous keys too). - let (spatial_in, spatial_out) = if p.spatial { effectcraft_keyframe::spatial_tangents(&p.keys, i) } else { (k.spatial_in, k.spatial_out) }; - json!({ - "time": l.comp_time(k.time).seconds(), - "layerTime": k.time.seconds(), - "value": js_value(c, &k.value), - "inInterp": interp(k.in_interp), "outInterp": interp(k.out_interp), - "inEase": ease(i, false), "outEase": ease(i, true), - "continuous": k.continuous, "autoBezier": k.auto_bezier, "roving": k.roving, - "spatialIn": spatial_in, "spatialOut": spatial_out, - "spatialContinuous": k.spatial_continuous, "spatialAutoBezier": k.spatial_auto, - "selected": sel.contains(&k.time), - }) + let key = |i: usize, k: &effectcraft_keyframe::Keyframe| -> J { + // Spatial tangents as the motion path plays (auto-Bezier and continuous keys too). + let (spatial_in, spatial_out) = if p.spatial { effectcraft_keyframe::spatial_tangents(&p.keys, i) } else { (k.spatial_in, k.spatial_out) }; + json!({ + "time": l.comp_time(k.time).seconds(), + "layerTime": k.time.seconds(), + "value": js_value(c, &k.value), + "inInterp": interp(k.in_interp), "outInterp": interp(k.out_interp), + "inEase": ease(i, false), "outEase": ease(i, true), + "continuous": k.continuous, "autoBezier": k.auto_bezier, "roving": k.roving, + "spatialIn": spatial_in, "spatialOut": spatial_out, + "spatialContinuous": k.spatial_continuous, "spatialAutoBezier": k.spatial_auto, + "selected": sel.contains(&k.time), }) - .collect(); - Ok(J::Array(out)) + }; + // `index` (zero-based): only that key, as `{count, key}` (`key` null when out of range), so + // looking keys up one at a time doesn't serialise the whole list each time (#814). + if let Some(i) = u(a, "index") { + let k = usize::try_from(i).ok().and_then(|i| p.keys.get(i).map(|k| key(i, k))); + return Ok(json!({"count": p.keys.len(), "key": k})); + } + Ok(J::Array(p.keys.iter().enumerate().map(|(i, k)| key(i, k)).collect())) } /// Apply `layer.setText` attribute keys to the Source Text document at `time` and return the new diff --git a/crates/script/src/prelude.js b/crates/script/src/prelude.js index 9daac6ab..b5095715 100644 --- a/crates/script/src/prelude.js +++ b/crates/script/src/prelude.js @@ -828,10 +828,11 @@ Property.prototype.property = function () { return null; }; }; P.__keys = function () { return __get("keys", { comp: this.__layer.__comp, layer: this.__layer.__id, uid: this.__uid }); }; P.__key = function (i) { - var k = this.__keys(); i = __num(i, "keyIndex"); - if (i < 1 || i > k.length || Math.floor(i) !== i) throw __err("keyIndex " + i + " is out of range 1.." + k.length); - return k[i - 1]; + // Only the key asked for: fetching the whole list made every key call linear (#814). + var r = i >= 1 && Math.floor(i) === i ? __get("keys", { comp: this.__layer.__comp, layer: this.__layer.__id, uid: this.__uid, index: i - 1 }) : null; + if (!r || !r.key) throw __err("keyIndex " + i + " is out of range 1.." + (r ? r.count : this.numKeys)); + return r.key; }; // Script value → engine JSON for `prop.set` / `prop.addKey`. P.__toEngine = function (v, info, time) { diff --git a/crates/script/src/tests.rs b/crates/script/src/tests.rs index 303bd820..2cccbf78 100644 --- a/crates/script/src/tests.rs +++ b/crates/script/src/tests.rs @@ -828,6 +828,24 @@ fn script_remove_keys_batches_a_range_as_one_edit() { assert!(invalid.error.is_some()); } +/// #814: one key is looked up without serialising the whole list (removeKey on a property with +/// thousands of keys was linear in the key count), and the key calls behave as before. +#[test] +fn script_key_lookup_fetches_only_that_key() { + let mut s = session(); + let o = ok( + &mut s, + "var c = app.project.items.addComp('Keys', 100, 100, 1, 100, 25); var p = c.layers.addNull().transform.rotation;\ + var ts = [], vs = []; for (var i = 0; i < 2000; i++) { ts.push(i / 25); vs.push(i); } p.setValuesAtTimes(ts, vs);\ + var q = { comp: p.__layer.__comp, layer: p.__layer.__id, uid: p.__uid };\ + q.index = 2; var one = __get('keys', q); q.index = 2000; var none = __get('keys', q);\ + p.removeKey(p.numKeys); p.removeKey(1);\ + var err = ''; try { p.keyTime(1999); } catch (e) { err = e.message; }\ + [one.count, one.key.time, none.key, p.numKeys, p.keyTime(1), p.keyValue(1998), err]", + ); + assert_eq!(o.result, json!([2000, 0.08, null, 1998, 0.04, 1998, "keyIndex 1999 is out of range 1..1998"])); +} + #[test] fn script_layer_comment_and_unnamed_marker_params_round_trip() { let mut s = session();