Prepare AnimationCHOP for public release (v0.4.0) - #17
Merged
Conversation
Update the anim submodule to v0.1.2 and adopt the TouchDesigner headers that expose per-node save/load of arbitrary byte data. - Bump ext/anim 61e6168 -> v0.1.2. The previous pin was the pre-squash form of the same work and is not reachable from anim's main, so a recursive clone would have failed once the prep branch is deleted. v0.1.2 also carries two fixes: an out-of-bounds iterator dereference in Channel::evaluate() at the last keyframe, and insert_keyframe() falling through after an in-place replace and inserting a moved-from keyframe. - Replace ext/td/include with the headers adding saveData()/loadData(). CHOP_PluginInfo::apiVersion is now private, so both FillCHOPPluginInfo functions call setAPIVersion() and return early when the running build does not support the version. This raises the minimum TouchDesigner version, as the new virtuals change the vtable layout. - Stop tracking td/Plugins. The binaries are published as release assets, and Plugins.json is a per-machine TouchDesigner trust file holding a host name and absolute paths.
v0.3.0 makes the Id constructor private and returns references rather than pointers from the Animation Id lookups. Both reach py_channel.cpp: - Build the PY_Channel id by copying Channel::id() instead of rebuilding an Id from its raw value, which the library no longer permits. - Take the address of the channel(Id) result now that it returns a reference. Also translate a missed Id lookup into a null ChannelData. channel(Id) has always thrown std::out_of_range on a miss, but getChannelData() never caught it while every one of its callers tests for a null channel, so a Python Channel outliving a remove_channel() would unwind a C++ exception through the CPython boundary instead of raising. The two renames in v0.2.0 (Id::isValid to is_valid, and the GrabbedHandle enumerators to PascalCase) need no changes here; neither symbol is used.
The bindings had no automated coverage: the only tests were scripts that have to be run by hand inside TouchDesigner. tests/python builds a small CPython extension that compiles the real operator sources and stands in for TouchDesigner with a fake PY_Context. That is the whole TouchDesigner surface the bindings touch -- they cast self to a PY_Struct, read ->context, and call getNodeInstance()/makeNodeDirty() -- so the module under test is the real binding code rather than a reimplementation of it. The operator's own method and getset tables are bound to the stand-in node object, which is why they are now externally linked. 77 tests cover Point, Keyframe, Channel and the animation as a whole: construction, evaluation, extend behaviour, state round-trips, error handling and node invalidation. Run them with `cmake --workflow --preset dev`, which configures, builds and runs the suite through ctest. uv supplies the Python 3.11 and pytest when present, so there is nothing to install first. Three behaviours are pinned by tests because they surprise: - keyframe(i) returns a detached copy, so assigning to its properties updates only that copy and never the channel; - the operator's range is its own setting rather than the span of its channels, and clear() does not reset it; - Channel.num_samples takes a sample rate, unlike the operator's property. No Catch2 suite yet: anim carries its own, and what remains here is bound to either TouchDesigner or CPython. The saveData()/loadData() codec is the natural subject and should bring tests/cpp with it.
…y and error handling
Keyframes and Points come out of a channel as detached copies, so setting a property on one leaves the channel untouched. This is forced by anim: keyframes have no id and are exposed only as const&, because every edit has to clamp the time between its neighbours, re-solve their handles and invalidate the eval cache. Channel is the opposite -- a live handle that re-resolves by id on each access. Nothing said so, and channel[0].value = x reads like an assignment while doing nothing. Document the split in the type docstrings, on each keyframe accessor and in docs/, and add channel[index] = keyframe as a synonym for update_keyframe so the read-modify-write round trip is discoverable from the subscript a user already reached for. Deletion by subscript raises rather than being added silently. Also pin the sequence protocol, which had no coverage, and correct the assumption that a time edit re-sorts the channel: it clamps, so keyframes never reorder and an index stays valid until a create or delete.
Adds the local pre-release gate: run_td_tests.ps1 (and .sh) builds the operators, launches tests/td/test.toe, runs the suites inside TouchDesigner, waits for a results.json sentinel, terminates TouchDesigner and exits non-zero on failure. The suite moves from td/tests_scripts/basic_tests.py to tests/td/animation_test.py. It already took the operator as a parameter, so the changes are small: it no longer runs itself on import or looks the operator up by a hardcoded name, its suites are isolated from each other so one blowing up does not hide the rest, and assertions are recorded structurally as well as printed so the host script can report several hundred of them as a handful of lines plus the failures. Adds cooked-output checks, which is the part only an in-TouchDesigner run can do: configure the node's output parameters, let it cook, and verify the emitted samples match Channel.evaluate() across the range. td_test_runner schedules them a few frames after the API suites, since the node has to actually cook in between. CMake now copies the operators into tests/td/Plugins/ as well -- TouchDesigner only loads Custom Operators from a Plugins/ folder beside the .toe, and td/ is what gets packaged for release, so the test project needs its own copy rather than borrowing the example project's. tests/td/test.toe itself is not in the repo; TESTING.md documents the one-time wiring (module DATs synced to these files, plus a bootstrap Execute DAT).
Range mode sized its output as end_time * sample_rate, which ignored the range start and dropped the inclusive end. With Range = [10, 70] at 60fps that asked for 4200 samples to cover a 60-second span. The values were still right -- execute() fills via evaluate_range(start, end, numSamples), which distributes across whatever length it is given -- so this was a length bug, and one that put the CHOP at odds with Animation.num_samples for the same range. Both range and auto-range now use ceil(length * rate) + 1, matching evaluate_range() and anim's own convention. AnimationViewCHOP had the same off-by-one in the seconds branch of its samples view, one short of the range, while its samples branch already had the +1. Both now clamp to at least one sample so an inverted range cannot ask TouchDesigner for a negative count. Range mode also assigned start then end unconditionally. The setters clamp against the current opposite bound, so a node moving from [0,30] to [50,70] clamped the new start against the stale end and cooked [30,70] for a frame. Widening before narrowing settles it in one cook -- which is what the Python start_time/end_time setters already did, so the two paths now agree. Outputmode defaulted to "fullrange", which is not one of its menu entries (range, autorange, input, sequence); it landed on index 0 by fallback. Samplerate had its slider bounds swapped, min 120 and max 30. Tests: the in-TD cook checks now run over a non-zero range start, which is the case that was broken, and assert the cooked count agrees with Animation.num_samples. Added pytest coverage for the range setters pushing the opposite bound.
Adds the files a public repository needs: MIT LICENSE, NOTICE covering the third-party components, a root README, and CONTRIBUTING. NOTICE matters more than usual here because two dependencies are not MIT and one of them is redistributed. ext/td/ is Derivative's Custom Operator SDK under their Shared Use License, and the Windows build vendors CPython 3.11 headers and import libraries under the PSF License. anim is a submodule, built from source rather than vendored. CI and release no longer mint a GitHub App token to fetch anim -- it is public now, so the default checkout token reaches it. Drops copilot-setup-steps.yml, which existed only to give Copilot that token. CI now runs the tests it was already capable of running: it configures with ANIMATIONCHOP_BUILD_TESTS=ON and runs ctest, so the pytest suite gates every push. It also fixes the macOS artifact paths, which pointed at .dylib files the build has never produced -- the macOS targets are .plugin bundles. release.yml now publishes a bundle that works on unzip rather than two bare libraries: Plugins/ beside the example project, Keyframer.tox, the modules the project loads, and the licence files. It runs the tests before publishing. Also fixes the same swapped Sample Rate slider bounds in AnimationViewCHOP that the previous commit fixed in AnimationCHOP.
Implements TouchDesigner's saveData()/loadData(), so an operator's channels and keyframes are stored inside the project file and restored on load. This replaces saving state by hand from the component, and needs neither an external file nor a Python interpreter -- loadData() runs during node construction, before one is available. The format is a lean versioned binary blob rather than JSON or the Python state dict: it is written on every project save, a keyframe-heavy animation runs to tens of thousands of doubles, and there is no interpreter to hand. src/animation_codec.cpp is kept free of both TouchDesigner and Python so it can be tested on its own. Those bytes are persisted user data, so decode() validates rather than trusts. A blob that is truncated, foreign, from a newer format version, or carrying an out-of-range enum is rejected outright, and the operator keeps the animation it already had instead of coming back half-built. A failed restore is reported through getErrorString rather than swallowed -- a node that silently returns empty looks like data loss, and the keyframes are still in the file. Restoring replays the explicit handles of Free and Aligned keyframes after every neighbour exists. Insertion re-solves handles incrementally, which is right for the derived modes but otherwise loses what an explicit handle was holding. Adds tests/cpp (Catch2) over the codec: round-trip fidelity including that a decoded curve evaluates identically, and rejection of every truncation of a valid blob. One known gap is pinned rather than left to be discovered -- anim caches a channel's last keyframe's pre-inheritance function and handle mode, that cache is private, so appending a keyframe after a reload no longer reverts it. Also settles versioning: the project moves to 0.4.0, staying on 0.x until the API is stable, and gains a CHANGELOG. release.yml now triggers on tag and takes its release body from the matching CHANGELOG section, as anim does, failing the release if the section is missing. The heading match there is a plain string prefix test rather than the dynamic regex anim uses. Building one as "^## \[" v "\]" reads correctly but is not portable: gawk strips the backslashes when converting a string to a regex, leaving [0.4.0] as a character class matching a single character. It fails by producing empty notes, not an error. Documents the minimum TouchDesigner build (2025.33070) now that the API it requires is known.
Pins anim at fix/half-open-rate-sampling. Sampling by rate now covers a half-open span: 30 seconds at 60 fps is 1800 samples rather than 1801, the range end is not sampled, and the times come from the sample index so they sit exactly one period apart with no drift. Both operators now fill their output with evaluate_range_by_rate() instead of evaluate_range(). This is the part that matters beyond the count. evaluate_range spreads a sample count across a *closed* interval, dividing by n - 1, so it only produces 1/rate spacing for one particular count -- and not at all when the span is not a whole number of periods. A CHOP's samples are implicitly one period apart, since the format stores no per-sample times, so pairing a half-open count with the closed-range fill would have declared 60 Hz while emitting data spaced 1/59.5 apart, skewing the whole channel. evaluate_range keeps its closed meaning for callers who want the end included. rangeSampleCount now delegates to Animation::num_samples rather than recomputing the formula, so the declared length and the generated data share one rounding rule and cannot drift apart. It still guards a non-positive rate, which num_samples throws on, and clamps up because a CHOP cannot have zero samples while an animation with no channels has none. Tests follow the new convention, and add the cases that distinguish it: that the last cooked sample sits one period short of the range end rather than on it, that evaluate_range stays closed while evaluate_range_by_rate is half-open, and that a duration landing a few ulps off a whole number of periods does not gain a sample. Also aligns the changelog-extraction awk with the form anim settled on -- the same fix, arrived at independently.
Follows anim to d8737f5, which adds RangeEnd as a trailing argument on both range methods and both num_samples overloads. The default is Exclusive everywhere, so the operators' own calls are unchanged, and evaluate_range is now half-open by default rather than closed. Exposes RangeEnd to Python alongside the other enums, and threads the optional argument through Channel.evaluate_range, evaluate_range_by_rate and num_samples. Without it a user plotting a curve or building a lookup table has no way to reach the closing sample, which is the case the enum exists for. Splits the in-TouchDesigner suite into one module per operator with a shared assertion harness, and adds the AnimationViewCHOP suite that was missing entirely. That operator has no Python API of its own, so integration is the only place it can be tested at all: the new suite steps through all five view modes, checking each publishes its documented channel table and that the values track the source animation. The runner becomes a list of steps a few frames apart, since each view mode has to cook before its output can be read. Writing that suite surfaced a crash. AnimationViewCHOP computed its segment count as size() - 1 on an unsigned type, so a channel with no keyframes wrapped to SIZE_MAX and came back as -1, undersizing the segment table that the fill loop then wrote past the end of. The fill loop had no guard either, and would run to SIZE_MAX. Creating a channel before keying it is ordinary, so this was reachable from the first thing a user does. Both are guarded now, and the fill loop is bounded by the output length like its neighbours. The failing in-TD assertion from the half-open change is updated, and now also covers the inclusive form.
tests/td/test.toe is the integration harness's project, wired with the module DATs and the bootstrap Execute DAT that TESTING.md describes. Committing it means run_td_tests.ps1 works on a fresh clone rather than after a manual setup nobody would do before their first run. It is 22 KB. keyframer.py had an accidental `from cProfile import run` at the top, shadowing TouchDesigner's builtin run(). Every call site passes delayFrames, which cProfile.run does not take, so all four would have raised TypeError. Ignores Backup/ anywhere rather than only at the root. TouchDesigner writes incremental saves beside whichever project it opens, and the ones under tests/td were being ignored only incidentally, by the *.*.toe rule matching the names TD happens to give them. Documents the DLL lock: with TouchDesigner open, the post-build copy into Plugins/ cannot overwrite a loaded operator. Nothing to fix -- killing the process to win the race would risk unsaved work -- so the copy fails loudly rather than being skipped, since a silently stale plugin means testing the previous build without knowing it. Notes the test-only build targets, which do not touch Plugins/.
Issue #15 reports a NaN in the final sample, appearing only when the range end lands exactly on the last keyframe and going away when the range is nudged. It cannot come from the curve. evaluate() was swept over ~2000 channel geometries -- every function and handle mode, tiny final segments, extreme handles, degenerate ranges, plus a randomised sweep -- and produced a NaN only for keyframe values at 1e300, where the Bezier maths overflows. So the NaN is a sample the operator declared but never wrote: the SDK header says the sample buffer is "already allocated for you", not initialised, so what is left in it is whatever was there, and a CHOP's buffer appears to be pre-filled with NaN precisely so that shows up. That cannot be reproduced headlessly, since the buffer only exists inside TouchDesigner. This adds twelve configurations to the in-TD suite -- starting with the reported one exactly, then varying whether the range starts at zero, whether the span is a whole number of periods, and whether the rate divides it evenly -- and scans every cooked sample of every channel for NaN or a wild magnitude. It also instruments the likeliest remaining mechanism. getOutputInfo sizes the output from the Sample Rate parameter while execute fills it from output->sampleRate, and nothing in the API guarantees TouchDesigner passes that through untouched. If it ever substitutes one, the count and the data would be derived from different rates and the tail would go unwritten -- intermittently, and sensitive to the range, which is what was reported. The suite now records the cooked rate against the parameter, and the cooked count against Animation.num_samples, so a divergence is visible rather than inferred. Findings are collected and printed together at the end, since which configurations trigger it is the diagnosis.
TouchDesigner allocates a CHOP's sample buffer but does not initialise it -- the SDK header promises only that it is "already allocated for you" -- so any sample an operator does not write keeps whatever was in that memory. That surfaces as NaN, which looks deliberate on Derivative's part: an unwritten sample is glaring rather than plausibly stale. Several paths wrote nothing at all. AnimationCHOP in Input mode sets an error and returns when nothing is connected, or when the input's shape does not match, leaving every sample of every channel untouched. AnimationViewCHOP does the same when it has no source operator selected, which is the state a freshly created node is in. Both are one click from the default, and both produced a full buffer of NaN while reporting only that something was misconfigured. Each path that produces no data now writes zeros itself: those returns, the "not enough channels allocated" returns, and any output channel with no animation channel behind it. The paths that do produce data are untouched, so every sample is still written exactly once. Clearing the whole buffer up front and then filling it would be two writes per sample on every cook of a realtime node. Filling from evaluate_range(start, end, output->numSamples) was considered, since it returns exactly the count asked for and a shortfall would become structurally impossible. It cannot be used here: it spaces samples (end - start) / n apart, which equals 1 / rate only when the span is a whole number of sample periods. Measured over 400 random ranges the values diverge by up to 24 where it is not. A CHOP's samples are one period apart by definition, so the rate-based call is the correct one and the count has to be made to agree rather than derived from it. The range fill also warns if it ever produces fewer samples than the output declares. That should be impossible -- the length comes from Animation::num_samples and the data from evaluate_range_by_rate over the same span, both through the same rounding -- but absorbing a shortfall silently is what made this class of bug hard to place. Adds regression coverage for both states to the in-TD suite.
Moves the submodule from a branch commit to the v0.4.0 tag, which is origin/main -- reachable, unlike the branch tip it was pinned to. v0.4.0 carries more than the branch it was cut from: it removes Channel::num_samples. That method took a rate and counted over the channel's keyframe extent without naming that range, which is where a curve's data happens to lie rather than the span a host samples over. A channel bound to a timeline was counted over the wrong span with nothing to indicate it, and a channel holds no reference to the animation that owns it to answer otherwise. The Python binding had the same flaw, so it goes too. Counting belongs to the operator, whose num_samples reads the configured range, or to TouchDesigner's own numSamples on the cooked output. RangeEnd stays on the two evaluate_range calls, where the caller is asking for values over a range they named rather than reading a node's length. Tests and docs follow. The removal is pinned by a test rather than merely deleted, so the reasoning survives where someone would otherwise be tempted to add it back.
The Windows job failed to configure. windows-latest now runs the windows-2025-vs2026 image, and the pinned CMake 3.25.3 predates that Visual Studio by four years, so it has no generator for it. The failure is indirect: CMake does not report that it found no Visual Studio. It falls back to NMake Makefiles, which is only correct inside a developer command prompt, and the error surfaces one step later as CMAKE_CXX_COMPILER not set after a missing nmake. Not a regression -- the previous workflow pinned the same version, so this would have broken whenever the image rolled to VS 2026. It had simply never run on a pull request. Moves both workflows to the action and version anim builds with, whose CI is green on the same images. CMake 4.x drops compatibility with cmake_minimum_required below 3.5 and this project declares 3.14, and anim fetches an older Catch2 than this one does under the same version.
Squash-merging a pull request rewrites the commit, so a tag pushed to the pre-merge branch tip builds and tests green while pointing at a commit reachable only from the tag. Publishing that produces a release whose history is not on main, which is only noticed much later by whoever pins it -- as happened with anim v0.3.0, whose tag had to be replaced. Ports the check anim uses, verified against that case: anim's original v0.3.0 commit is correctly refused and v0.4.0 accepted. Also adds a README to td/ marking the example project as work in progress and likely to move out of this repository, so nobody builds against a component that has no stability commitment while the operators themselves do.
release.yml takes its release body from the CHANGELOG section matching the tag, and refuses to publish if there is none. That check only ever runs on a tag, which is the worst moment to find the section missing or renamed -- and the failure mode it guards against is a silent one, since a non-matching heading yields empty notes rather than an error. Runs the same extraction on every push. The version comes from CMakeLists.txt rather than a tag, so this also keeps the declared project version and the CHANGELOG from drifting apart. They were out of step before 0.4.0, with the build claiming 1.0.0 while the tags were 0.3.x. No friction in the normal flow: after a release the declared version still has its section, and new work accumulates under Unreleased. It fails only when the version is bumped without adding the matching section, which is when it should.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Everything needed to make this repository public, plus the operator fixes and test suites that came out of writing them.
Licensing and docs
LICENSE(MIT),NOTICE, a rootREADME.mdandCONTRIBUTING.md.NOTICEmatters more than usual here: two dependencies are not MIT and one is redistributed.ext/td/is Derivative's Custom Operator SDK under their Shared Use License, and the Windows build vendors CPython 3.11 headers and import libraries under the PSF License. anim is a submodule, built from source.Persistence
The operator implements TouchDesigner's
saveData()/loadData(), so channels and keyframes are stored in the.toeand restored on load — no external file, and no Python, sinceloadData()runs during node construction before an interpreter exists.The format is a versioned binary blob (
src/animation_codec.cpp), kept free of both TouchDesigner and Python so it can be tested standalone. Those bytes are persisted user data, sodecode()validates rather than trusts: anything truncated, foreign, from a newer version, or carrying an out-of-range enum is rejected outright and the operator keeps the animation it already had.Operator fixes
end_time * sample_rate, ignoring the range start —Range = [10, 70]at 60fps emitted 4200 samples for a 60-second span. Both range and auto-range now take their count fromAnimation::num_samples. Output lengths change for any node whose range does not start at zero.1/rateapart. Both operators fill fromevaluate_range_by_raterather thanevaluate_range, which spreads a count across a closed interval and so only lands on1/ratespacing for one particular count.size() - 1on an unsigned type, so a channel with no keyframes wrapped toSIZE_MAXand undersized the table its fill loop then wrote past. Reachable from the first thing a user does. Closes AnimationViewCHOP - error when a channel has no keyframes #16.Output Modedefaulted to a value not in its menu;Sample Ratehad its slider bounds swapped on both operators.Channelpast aremove_channel()unwound a C++ exception through the CPython boundary instead of raising.Tests
tests/cpp(Catch2)tests/python(pytest)tests/tdtests/pythoncompiles the real binding sources against a fakePY_Context, so it exercises the actual bindings rather than a copy.tests/tdcovers both operators inside a real project, including all five AnimationViewCHOP view modes — that operator has no Python API of its own, so integration is the only place it can be tested at all.CI now runs the two headless suites on every push; it previously built without testing, and its macOS artifact paths pointed at
.dylibfiles the build has never produced.API
channel[index] = keyframe, a synonym forupdate_keyframe. Keyframes come out of a channel as detached copies, so editing one is a read-modify-write; assignment makes that discoverable from the subscript people already reach for.RangeEndon the twoevaluate_rangecalls, for when samples are points on a curve rather than spans of time.Channel.num_samples(rate), following anim. A channel knows only the extent of its own keyframes, so a count taken from it silently answered about the wrong span.Release
ext/animis pinned to the v0.4.0 tag, which isorigin/main.release.ymltriggers onv*.*.*tags, gates on both platforms building and testing, and takes its release body from the matchingCHANGELOG.mdsection.Requires TouchDesigner 2025.33070 or newer — the build where the Custom Operator API gained node data persistence.
Verified locally: clean build both operators, 18/18 ctest, 103 pytest, and the full in-TD suite passing.