diff --git a/CHANGELOG.md b/CHANGELOG.md index 9da062d..f1ee6a3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,114 @@ While the major version is `0`, breaking changes may land in a minor release. ## [Unreleased] +## [0.4.0] - 2026-07-26 + +Sampling by rate now covers a half-open range: a span of n sample periods gives +n samples, not n + 1, so a sample count is the product of duration and rate as +callers expect. That is the convention frame- and audio-rate hosts use, where a +sample covers the interval that follows it and the end of a range is an edge +rather than a sample. Where the closing sample is wanted -- plotting a curve, +building a lookup table, integrating numerically -- pass the new `RangeEnd`. + +Fixing the count also fixed the spacing: a range that was not a whole number of +sample periods was previously not sampled at the requested rate at all. + +### Changed + +- **Breaking:** sampling by rate now covers a half-open range. `end_time` is no + longer sampled, so a span of n sample periods yields n samples rather than + n + 1: `evaluate_range_by_rate(0, 4, 30)` returns 120 values, the last at + 3.9667, and `Animation::num_samples(30)` over a 4 second animation returns 120 + rather than 121. This is the convention frame- and audio-rate hosts expect, and + it makes the sample count the product of duration and rate as callers assume. + Callers who want the closing sample can pass `RangeEnd::Inclusive`, described + below. +- **Breaking:** `Animation::num_samples` returns `size_t` rather than `int`. +- **Breaking:** `Channel::evaluate_range` samples a half-open range too, so both + range methods now share one convention. `evaluate_range(0, 4, 5)` gives 0.0, + 0.8, 1.6, 2.4 and 3.2 rather than 0.0 through 4.0. +- **Breaking:** `Channel::evaluate_range` now returns exactly the requested + number of samples in every case. It previously collapsed to a single sample + when `start_time` and `end_time` were equal, which silently broke callers + sizing a buffer from the count they passed in. An empty range now gives that + many copies of the value at that time. +- The `curve_visualization` example plots from a single `evaluate_range` call + with `RangeEnd::Inclusive`, taking its x axis from `sample_times`, and no + longer accumulates `t += step` in a loop that then has to append the end point + so the plotted line reaches the last keyframe. The sampled points differ from + the accumulated version by at most 2e-12, and the last one now lands on the + end time exactly. + +### Added + +- `RangeEnd`, selecting whether a sampled range includes its end time, accepted + as a trailing argument by `Channel::evaluate_range`, + `Channel::evaluate_range_by_rate` and `Animation::num_samples`. It defaults to + `RangeEnd::Exclusive` everywhere, which treats a sample as covering the + interval that follows it, so the end of a range is an edge rather than a + sample. + + `RangeEnd::Inclusive` treats samples as points on the curve instead, which is + what plotting a curve, building an interpolation lookup table, or integrating + numerically all need: without it the last point falls short of the end. For a + rate, the two differ only when the span is a whole number of sample periods, + because that is the only case where a sample lands on the end at all. +- `SampleTimes`, and the `sample_times` / `sample_times_by_rate` methods and free + functions that build one, reporting the times a range of samples was taken at. + The `Animation` methods span the animation's time range, so every channel baked + over it shares a single time base regardless of where its own keyframes fall, + and their size matches what `Animation::num_samples` reports. The free + functions take an arbitrary range. + + Sampling returns values without times because the times are fully described by + a start, a step and a count: materialising them would double the memory for no + information. `SampleTimes` holds those three numbers, computes a time from an + index, and measures as free — indexing it compiles to the same multiply and + add as writing the arithmetic out. What it removes is the need to restate that + arithmetic, whose divisor depends on the `RangeEnd` and so is easy to pair + with the wrong one. + +### Fixed + +- `Channel::evaluate_range_by_rate` did not sample at the requested rate when + the range was not a whole number of sample periods. It rounded the count up + and then handed it to `evaluate_range`, which spreads a count across a closed + range, compressing the spacing to fit: 1.05 seconds at 30 Hz came back as 33 + values 0.0328 apart rather than 30 Hz. It now hands over the span the samples + actually cover rather than the requested end, so the spacing works out to + exactly `1 / sample_rate` for any range. +- `Channel::evaluate_range` validated its range only for sample counts large + enough to reach the sampling loop, so `evaluate_range(10, 0, 0)` returned a + value while `evaluate_range(10, 0, 2)` threw on the same reversed range. The + range is now checked first, for every count. A count of zero returns an empty + vector rather than one sample, and a negative count is rejected rather than + quietly treated as one sample. +- The sample count is no longer inflated by floating-point error. It was + computed with `ceil`, so a duration whose product with the rate landed a few + ulps above a whole number, as 4.0 seconds at 30 Hz can, produced an extra + sample spanning a fraction of a period. + +### Removed + +- **Breaking:** `Channel::num_samples`. It took a rate and silently counted over + the channel's keyframe extent, which is an editing concept: it is where a + curve's data happens to lie, not the range 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. Every other sampling entry point already names its range, so this + was the only one that guessed. + + Count over a timeline with `Animation::num_samples` or + `Animation::sample_times_by_rate`, and over any other span with the free + `sample_times` functions. The direct replacement for the old behaviour is + `sample_times_by_rate(ch.start_time(), ch.end_time(), rate).size()`. + `Channel::start_time`, `end_time` and `length` are unchanged: keyframe extent + is still what an editor wants. +- `CODE_OF_CONDUCT.md`. The Contributor Covenant sets out a moderation and + enforcement process that overstates how this project is run. `CONTRIBUTING.md` + covers how to take part, and security reports have their own private channel + in `SECURITY.md`. + ## [0.3.0] - 2026-07-26 Two breaking changes that tighten the public API: `Id` lookups return @@ -144,7 +252,8 @@ Bézier handle constraints, sampling helpers, and the Catch2 test suite. in 0.2.0. History was rewritten between the v0.1.1 and v0.1.2 tags, so the compare links below are more reliable than a commit-by-commit listing. --> -[Unreleased]: https://github.com/Actualize-Interactive/anim/compare/v0.3.0...HEAD +[Unreleased]: https://github.com/Actualize-Interactive/anim/compare/v0.4.0...HEAD +[0.4.0]: https://github.com/Actualize-Interactive/anim/compare/v0.3.0...v0.4.0 [0.3.0]: https://github.com/Actualize-Interactive/anim/compare/v0.2.0...v0.3.0 [0.2.0]: https://github.com/Actualize-Interactive/anim/compare/v0.1.2...v0.2.0 [0.1.2]: https://github.com/Actualize-Interactive/anim/releases/tag/v0.1.2 diff --git a/CMakeLists.txt b/CMakeLists.txt index 4d7640e..228c6a0 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,5 +1,5 @@ cmake_minimum_required(VERSION 3.25) -project(anim VERSION 0.3.0 LANGUAGES CXX) +project(anim VERSION 0.4.0 LANGUAGES CXX) set(CMAKE_CXX_STANDARD 20) set(CMAKE_CXX_STANDARD_REQUIRED ON) diff --git a/CODE_OF_CONDUCT.md b/CODE_OF_CONDUCT.md deleted file mode 100644 index a62559b..0000000 --- a/CODE_OF_CONDUCT.md +++ /dev/null @@ -1,135 +0,0 @@ -# Contributor Covenant Code of Conduct - -## Our Pledge - -We as members, contributors, and leaders pledge to make participation in our -community a harassment-free experience for everyone, regardless of age, body -size, visible or invisible disability, ethnicity, sex characteristics, gender -identity and expression, level of experience, education, socio-economic status, -nationality, personal appearance, race, caste, color, religion, or sexual -identity and orientation. - -We pledge to act and interact in ways that contribute to an open, welcoming, -diverse, inclusive, and healthy community. - -## Our Standards - -Examples of behavior that contributes to a positive environment for our -community include: - -* Demonstrating empathy and kindness toward other people -* Being respectful of differing opinions, viewpoints, and experiences -* Giving and gracefully accepting constructive feedback -* Accepting responsibility and apologizing to those affected by our mistakes, - and learning from the experience -* Focusing on what is best not just for us as individuals, but for the overall - community - -Examples of unacceptable behavior include: - -* The use of sexualized language or imagery, and sexual attention or advances of - any kind -* Trolling, insulting or derogatory comments, and personal or political attacks -* Public or private harassment -* Publishing others' private information, such as a physical or email address, - without their explicit permission -* Other conduct which could reasonably be considered inappropriate in a - professional setting - -## Enforcement Responsibilities - -Community leaders are responsible for clarifying and enforcing our standards of -acceptable behavior and will take appropriate and fair corrective action in -response to any behavior that they deem inappropriate, threatening, offensive, -or harmful. - -Community leaders have the right and responsibility to remove, edit, or reject -comments, commits, code, wiki edits, issues, and other contributions that are -not aligned to this Code of Conduct, and will communicate reasons for moderation -decisions when appropriate. - -## Scope - -This Code of Conduct applies within all community spaces, and also applies when -an individual is officially representing the community in public spaces. -Examples of representing our community include using an official email address, -posting via an official social media account, or acting as an appointed -representative at an online or offline event. - -## Enforcement - -Instances of abusive, harassing, or otherwise unacceptable behavior may be -reported by [opening an issue](https://github.com/Actualize-Interactive/anim/issues/new/choose) -in this repository. - -Note that repository issues are public. If a report would expose you by being -public — or if it concerns a maintainer — use GitHub's -[report abuse](https://github.com/contact/report-abuse) form instead, which -goes privately to GitHub Support rather than to this project. - -All complaints will be reviewed and investigated promptly and fairly. - -All community leaders are obligated to respect the privacy and security of the -reporter of any incident. - -## Enforcement Guidelines - -Community leaders will follow these Community Impact Guidelines in determining -the consequences for any action they deem in violation of this Code of Conduct: - -### 1. Correction - -**Community Impact**: Use of inappropriate language or other behavior deemed -unprofessional or unwelcome in the community. - -**Consequence**: A private, written warning from community leaders, providing -clarity around the nature of the violation and an explanation of why the -behavior was inappropriate. A public apology may be requested. - -### 2. Warning - -**Community Impact**: A violation through a single incident or series of -actions. - -**Consequence**: A warning with consequences for continued behavior. No -interaction with the people involved, including unsolicited interaction with -those enforcing the Code of Conduct, for a specified period of time. This -includes avoiding interactions in community spaces as well as external channels -like social media. Violating these terms may lead to a temporary or permanent -ban. - -### 3. Temporary Ban - -**Community Impact**: A serious violation of community standards, including -sustained inappropriate behavior. - -**Consequence**: A temporary ban from any sort of interaction or public -communication with the community for a specified period of time. No public or -private interaction with the people involved, including unsolicited interaction -with those enforcing the Code of Conduct, is allowed during this period. -Violating these terms may lead to a permanent ban. - -### 4. Permanent Ban - -**Community Impact**: Demonstrating a pattern of violation of community -standards, including sustained inappropriate behavior, harassment of an -individual, or aggression toward or disparagement of classes of individuals. - -**Consequence**: A permanent ban from any sort of public interaction within the -community. - -## Attribution - -This Code of Conduct is adapted from the [Contributor Covenant][homepage], -version 2.1, available at -https://www.contributor-covenant.org/version/2/1/code_of_conduct.html. - -Community Impact Guidelines were inspired by -[Mozilla's code of conduct enforcement ladder][mozilla]. - -For answers to common questions about this code of conduct, see the FAQ at -https://www.contributor-covenant.org/faq. Translations are available at -https://www.contributor-covenant.org/translations. - -[homepage]: https://www.contributor-covenant.org -[mozilla]: https://github.com/mozilla/inclusion diff --git a/examples/curve_visualization.cpp b/examples/curve_visualization.cpp index 4e86105..befc249 100644 --- a/examples/curve_visualization.cpp +++ b/examples/curve_visualization.cpp @@ -395,14 +395,28 @@ int main() { y_data.push_back(curve.evaluate(start_time)); } else if (start_time < end_time) { - for (double t = start_time; t <= end_time; t += eval_step) { - x_data.push_back(t); - y_data.push_back(curve.evaluate(t)); - } - // Ensure the last point is plotted if not caught by the loop condition - if (x_data.empty() || x_data.back() < end_time) { - x_data.push_back(end_time); - y_data.push_back(curve.evaluate(end_time)); + // A closed range, so the plotted line reaches the last + // keyframe rather than stopping a step short of it. The + // count-based overload is what guarantees that: sampling + // by rate would only land on end_time when the span + // happens to be a whole number of steps. + const double duration = end_time - start_time; + const int num_points = + static_cast(std::ceil(duration / eval_step)) + 1; + y_data = curve.evaluate_range(start_time, end_time, num_points, + anim::RangeEnd::Inclusive); + + // Sampling returns values only; the matching times come + // from sample_times, which computes them from the index + // rather than storing a second buffer alongside them. + // The range is stated rather than implied: this plots a + // curve over its own keyframe extent, which is not the + // animation's timeline. + const anim::SampleTimes times = anim::sample_times( + start_time, end_time, num_points, anim::RangeEnd::Inclusive); + x_data.reserve(times.size()); + for (size_t s = 0; s < times.size(); ++s) { + x_data.push_back(times[s]); } } } diff --git a/include/anim.hpp b/include/anim.hpp index 6dfa285..5cf8a41 100644 --- a/include/anim.hpp +++ b/include/anim.hpp @@ -16,5 +16,7 @@ #include "anim/channel.hpp" #include "anim/animation.hpp" #include "anim/extend.hpp" +#include "anim/range_end.hpp" +#include "anim/sample_times.hpp" #endif // ANIM_HPP diff --git a/include/anim/animation.hpp b/include/anim/animation.hpp index 495375e..4e16666 100644 --- a/include/anim/animation.hpp +++ b/include/anim/animation.hpp @@ -3,6 +3,7 @@ #include "anim/channel.hpp" #include "anim/id.hpp" +#include "anim/sample_times.hpp" #include #include #include @@ -159,11 +160,38 @@ class Animation { /** * @brief Number of samples spanning the animation at @p sample_rate. + * + * The span is half-open by default, matching Channel::evaluate_range_by_rate(): + * the result is length() * sample_rate rounded up, so 4 seconds at 30 Hz + * gives 120 rather than 121. Pass RangeEnd::Inclusive to count the closing + * sample. An animation with no channels gives 0; one with no length gives 1. + * Matches Channel::num_samples() in convention, return type and default. * @param sample_rate Samples per unit time; must be positive. + * @param range_end Whether the end time is counted; half-open by default. * @return Sample count (0 when there are no channels). * @throws std::invalid_argument if @p sample_rate is not positive. */ - int num_samples(double sample_rate) const; + size_t num_samples(double sample_rate, RangeEnd range_end = RangeEnd::Exclusive) const; + + /** + * @brief Times spanning the animation, for @p num_samples samples. + * + * The range runs from start_time() to end_time(), so every channel baked + * over that range with the same count shares these times: they describe the + * animation's time base rather than any one channel's. Nothing is allocated. + * @throws std::invalid_argument if @p num_samples is negative. + */ + SampleTimes sample_times(int num_samples, RangeEnd range_end = RangeEnd::Exclusive) const; + + /** + * @brief Times spanning the animation at @p sample_rate. + * + * Its size() is what num_samples() reports for the same arguments, so an + * animation with no channels gives no times. Nothing is allocated. + * @throws std::invalid_argument if @p sample_rate is not positive. + */ + SampleTimes sample_times_by_rate(double sample_rate, + RangeEnd range_end = RangeEnd::Exclusive) const; /// @brief Equality across name, time range and channels. bool operator==(const Animation& other) const; diff --git a/include/anim/channel.hpp b/include/anim/channel.hpp index 1409c94..6c8c28d 100644 --- a/include/anim/channel.hpp +++ b/include/anim/channel.hpp @@ -5,6 +5,7 @@ #include "anim/handle_utils.hpp" #include "anim/id.hpp" #include "anim/extend.hpp" +#include "anim/range_end.hpp" #include #include #include @@ -157,17 +158,54 @@ class Channel { double evaluate(double time, double* prev_t = nullptr) const; /** * @brief Evaluates @p num_samples evenly spaced values from @p start_time to @p end_time. - * @return A vector of sampled values. + * + * By default the range is half-open, as in evaluate_range_by_rate(): sample + * @c i is at start_time + i * (end_time - start_time) / num_samples, + * so the last sample is one step short of @p end_time and @p end_time itself + * is not sampled. Sampling 0 to 4 with 5 samples gives 0.0, 0.8, 1.6, 2.4 + * and 3.2. + * + * Pass RangeEnd::Inclusive to divide the span by one less and land the last + * sample on @p end_time, giving 0.0, 1.0, 2.0, 3.0 and 4.0 for the same + * call. That is what plotting a curve or building a lookup table wants, + * where a sample is a point rather than the interval that follows it. + * + * Exactly @p num_samples values are returned, including when @p start_time + * and @p end_time are equal, in which case they are all the value at that + * time. + * @param range_end Whether @p end_time is sampled; half-open by default. + * @return A vector of @p num_samples values; empty if @p num_samples is 0. + * @throws std::invalid_argument if @p num_samples is negative, or if + * @p start_time is after @p end_time. */ - std::vector evaluate_range(double start_time, double end_time, int num_samples) const; + std::vector evaluate_range(double start_time, double end_time, int num_samples, + RangeEnd range_end = RangeEnd::Exclusive) const; /** * @brief Evaluates values from @p start_time to @p end_time at a fixed sample rate. + * + * Sample @c i is at start_time + i / sample_rate, so the spacing is + * exactly one sample period no matter what the range is. + * + * By default the range is half-open and @p end_time is not sampled, so a + * span of n periods gives n samples: 4 seconds at 30 Hz yields 120 values, + * the last at 3.9667, not 121 ending on 4.0. Pass RangeEnd::Inclusive for + * the closing sample, giving 121. + * + * The two differ only when the span is a whole number of periods, since that + * is the only case where a sample lands on @p end_time. A span that is not + * is rounded up either way, so the whole range is covered. * @param start_time First time to sample. - * @param end_time Last time to sample. + * @param end_time Upper bound of the range, sampled only if @p range_end + * is RangeEnd::Inclusive. * @param sample_rate Samples per unit time; must be positive. - * @return A vector of sampled values. + * @param range_end Whether @p end_time is sampled; half-open by default. + * @return A vector of num_samples() values, or a single value if + * @p start_time and @p end_time are equal. + * @throws std::invalid_argument if @p sample_rate is not positive, or if + * @p start_time is after @p end_time. */ - std::vector evaluate_range_by_rate(double start_time, double end_time, double sample_rate) const; + std::vector evaluate_range_by_rate(double start_time, double end_time, double sample_rate, + RangeEnd range_end = RangeEnd::Exclusive) const; /// @brief Time of the first keyframe (0 if empty). double start_time() const; @@ -175,9 +213,6 @@ class Channel { double end_time() const; /// @brief Duration spanned by the keyframes (end_time() - start_time()). double length() const; - /// @brief Number of samples that length() would produce at @p sample_rate. - size_t num_samples(double sample_rate) const; - /// @brief Extend behavior for times before the first keyframe. Extend extend_start() const; /// @brief Extend behavior for times after the last keyframe. diff --git a/include/anim/range_end.hpp b/include/anim/range_end.hpp new file mode 100644 index 0000000..eba5628 --- /dev/null +++ b/include/anim/range_end.hpp @@ -0,0 +1,28 @@ +#ifndef ANIM_RANGE_END_HPP +#define ANIM_RANGE_END_HPP + +#include + +namespace anim { + +/** + * @brief Whether a sampled range includes its end time. + * + * Passed to Channel::evaluate_range, Channel::evaluate_range_by_rate and the + * matching num_samples() overloads. The default everywhere is + * RangeEnd::Exclusive, which treats a sample as covering the interval that + * follows it -- the convention frame- and audio-rate hosts use, where the end + * of a range is an edge rather than a sample. + * + * RangeEnd::Inclusive treats samples as points on the curve instead, which is + * what plotting a curve, building an interpolation lookup table, or integrating + * numerically all need: without it the last point falls short of the end. + */ +enum class RangeEnd : uint8_t { + Exclusive = 0, ///< The end time is not sampled; the range is [start, end). + Inclusive = 1 ///< The end time is sampled; the range is [start, end]. +}; + +} // namespace anim + +#endif // ANIM_RANGE_END_HPP diff --git a/include/anim/sample_times.hpp b/include/anim/sample_times.hpp new file mode 100644 index 0000000..9ad99b4 --- /dev/null +++ b/include/anim/sample_times.hpp @@ -0,0 +1,96 @@ +#ifndef ANIM_SAMPLE_TIMES_HPP +#define ANIM_SAMPLE_TIMES_HPP + +#include "anim/range_end.hpp" + +#include +#include + +namespace anim { + +/** + * @brief The times a range of samples was taken at, as a closed form. + * + * Sampling returns values only, because the times they were taken at are fully + * described by a start, a step and a count -- storing them would double the + * memory for no information. This holds those three numbers and computes a time + * on demand, so indexing it costs a multiply and an add and nothing is + * allocated. + * + * Obtain one from Channel::sample_times(), Channel::sample_times_by_rate(), or + * the free functions of the same names for an arbitrary range. Those apply the + * same step rule the matching evaluate_range() call does, so element @c i is + * exactly the time element @c i of the returned values was evaluated at. + */ +class SampleTimes { +public: + /** + * @brief Constructs a sequence of @p count times from @p start, @p step apart. + * + * Prefer the sample_times() functions, which derive @p step from a range and + * a RangeEnd rather than leaving it to the caller to restate that rule. + */ + SampleTimes(double start, double step, size_t count) + : m_start(start), m_step(step), m_count(count) {} + + /// @brief The time of sample @p index. Not bounds-checked; see at(). + double operator[](size_t index) const { + return m_start + static_cast(index) * m_step; + } + + /// @brief The time of sample @p index. @throws std::out_of_range if out of range. + double at(size_t index) const { + if (index >= m_count) { + throw std::out_of_range("Sample index out of range"); + } + return (*this)[index]; + } + + /// @brief Number of samples described. + size_t size() const { return m_count; } + /// @brief Whether there are no samples. + bool empty() const { return m_count == 0; } + /// @brief Time of the first sample. + double front() const { return m_start; } + /// @brief Time of the last sample. Undefined when empty(). + double back() const { return (*this)[m_count - 1]; } + /// @brief Spacing between consecutive samples. + double step() const { return m_step; } + + bool operator==(const SampleTimes& other) const { + return m_start == other.m_start && m_step == other.m_step && m_count == other.m_count; + } + bool operator!=(const SampleTimes& other) const { return !(*this == other); } + +private: + double m_start; + double m_step; + size_t m_count; +}; + +/** + * @brief Times that Channel::evaluate_range() would sample an arbitrary range at. + * + * The step is the span divided by @p num_samples for a half-open range, or by + * one less for a closed one, matching evaluate_range() exactly. + * @throws std::invalid_argument if @p num_samples is negative, or if + * @p start_time is after @p end_time. + */ +SampleTimes sample_times(double start_time, double end_time, int num_samples, + RangeEnd range_end = RangeEnd::Exclusive); + +/** + * @brief Times that Channel::evaluate_range_by_rate() would sample an arbitrary + * range at. + * + * The step is one sample period, and the count is the one + * Channel::num_samples() reports for the same range and @p range_end. + * @throws std::invalid_argument if @p sample_rate is not positive, or if + * @p start_time is after @p end_time. + */ +SampleTimes sample_times_by_rate(double start_time, double end_time, double sample_rate, + RangeEnd range_end = RangeEnd::Exclusive); + +} // namespace anim + +#endif // ANIM_SAMPLE_TIMES_HPP diff --git a/src/animation.cpp b/src/animation.cpp index 81796e7..2eb8992 100644 --- a/src/animation.cpp +++ b/src/animation.cpp @@ -1,4 +1,5 @@ #include "anim/animation.hpp" +#include "sampling.hpp" namespace anim { @@ -274,14 +275,34 @@ void Animation::set_length(double length) { m_end_time = m_start_time + length; } -int Animation::num_samples(double sample_rate) const { +size_t Animation::num_samples(double sample_rate, RangeEnd range_end) const { if (sample_rate <= 0.0) { throw std::invalid_argument("Sample rate must be positive"); } if (m_channels.empty()) { return 0; } - return static_cast(std::ceil(length() * sample_rate)) + 1; // +1 to include the start time + if (length() == 0.0) { + // No span to divide, so the single sample at the start time. + return 1; + } + return detail::sample_count(length(), sample_rate, range_end); +} + +SampleTimes Animation::sample_times(int num_samples, RangeEnd range_end) const { + return anim::sample_times(m_start_time, m_end_time, num_samples, range_end); +} + +SampleTimes Animation::sample_times_by_rate(double sample_rate, RangeEnd range_end) const { + if (sample_rate <= 0.0) { + throw std::invalid_argument("Sample rate must be positive"); + } + // Matches num_samples, which reports nothing to sample until there is a + // channel to sample, whatever the animation's time range says. + if (m_channels.empty()) { + return SampleTimes(m_start_time, 0.0, 0); + } + return anim::sample_times_by_rate(m_start_time, m_end_time, sample_rate, range_end); } bool Animation::operator==(const Animation& other) const { diff --git a/src/channel.cpp b/src/channel.cpp index 9aa88f8..548573a 100644 --- a/src/channel.cpp +++ b/src/channel.cpp @@ -1,5 +1,6 @@ #include "anim/channel.hpp" #include "anim/bezier_utils.hpp" +#include "sampling.hpp" namespace anim { @@ -407,47 +408,71 @@ double Channel::evaluate(double time, double* prev_t) const { -std::vector Channel::evaluate_range(double start_time, double end_time, int num_samples) const { - if (num_samples <= 1) { - return {evaluate(start_time)}; +std::vector Channel::evaluate_range(double start_time, double end_time, int num_samples, + RangeEnd range_end) const { + // Validate before any early return, so a reversed range is rejected for + // every sample count rather than only for the ones that reach the loop. + if (num_samples < 0) { + throw std::invalid_argument("Sample count cannot be negative"); } if (start_time > end_time) { throw std::invalid_argument("Start time must be less than or equal to end time"); } - - // For equal times, just return the value at that time - if (start_time == end_time) { + + if (num_samples == 0) { + return {}; + } + // A closed range with one sample has no spacing to derive, and the divisor + // below would be zero. The single sample is at start_time either way. + if (num_samples == 1 && range_end == RangeEnd::Inclusive) { return {evaluate(start_time)}; } - + std::vector result(num_samples); - double step = (end_time - start_time) / (num_samples - 1); - - double* prev_t = nullptr; + // A half-open range divides the span by the sample count, so the last + // sample sits one step short of end_time; a closed range divides by one + // less, so the last sample lands on it. The step is zero when the range is + // empty, which is the right answer -- the caller asked for num_samples + // values and every one of them is at start_time. + const int divisor = (range_end == RangeEnd::Inclusive) ? num_samples - 1 : num_samples; + double step = (end_time - start_time) / divisor; + + double* prev_t = nullptr; for (int i = 0; i < num_samples; i++) { double time = start_time + i * step; result[i] = evaluate(time, prev_t); prev_t = &result[i]; // Update prev_t to point to the current value } - + return result; } -std::vector Channel::evaluate_range_by_rate(double start_time, double end_time, double sample_rate) const { +std::vector Channel::evaluate_range_by_rate(double start_time, double end_time, + double sample_rate, RangeEnd range_end) const { if (sample_rate <= 0.0) { throw std::invalid_argument("Sample rate must be positive"); } if (start_time > end_time) { throw std::invalid_argument("Start time must be less than or equal to end time"); } - + // For equal times, just return the value at that time if (start_time == end_time) { return {evaluate(start_time)}; } - - int num_samples = static_cast(std::ceil((end_time - start_time) * sample_rate)) + 1; - return evaluate_range(start_time, end_time, num_samples); + + const size_t count = detail::sample_count(end_time - start_time, sample_rate, range_end); + + // Hand evaluate_range the span those samples actually cover rather than the + // requested one. The two differ when the range is not a whole number of + // sample periods, and passing the requested end_time would have + // evaluate_range divide it by the rounded count and so space the samples + // slightly closer than the rate asked for. The divisor evaluate_range will + // use is the count for a half-open range and one less for a closed one, so + // spanning exactly that many periods leaves it with a step of 1 / rate. + const size_t divisor = (range_end == RangeEnd::Inclusive) ? count - 1 : count; + const double covered_end = start_time + static_cast(divisor) / sample_rate; + return evaluate_range(start_time, covered_end, static_cast(count), range_end); } double Channel::start_time() const { @@ -471,21 +496,6 @@ double Channel::length() const { return end_time() - start_time(); } -size_t Channel::num_samples(double sample_rate) const -{ - if (sample_rate <= 0.0) { - throw std::invalid_argument("Sample rate must be positive"); - } - - if (m_keyframes.empty()) { - return 0; - } - - double duration = end_time() - start_time(); - return static_cast(std::ceil(duration * sample_rate)) + 1; // +1 to include the start time - -} - Extend Channel::extend_start() const { return m_extend_start; } diff --git a/src/sample_times.cpp b/src/sample_times.cpp new file mode 100644 index 0000000..fcb08a4 --- /dev/null +++ b/src/sample_times.cpp @@ -0,0 +1,50 @@ +#include "anim/sample_times.hpp" +#include "sampling.hpp" + +namespace anim { + +SampleTimes sample_times(double start_time, double end_time, int num_samples, RangeEnd range_end) { + if (num_samples < 0) { + throw std::invalid_argument("Sample count cannot be negative"); + } + if (start_time > end_time) { + throw std::invalid_argument("Start time must be less than or equal to end time"); + } + + // Mirrors Channel::evaluate_range: a half-open range divides the span by + // the count, a closed one by one less. With a single sample there is no + // spacing to derive and the step is never used, so it stays zero rather + // than dividing by a closed range's zero divisor. + double step = 0.0; + if (num_samples > 1) { + const int divisor = (range_end == RangeEnd::Inclusive) ? num_samples - 1 : num_samples; + step = (end_time - start_time) / divisor; + } + return SampleTimes(start_time, step, static_cast(num_samples)); +} + +SampleTimes sample_times_by_rate(double start_time, double end_time, double sample_rate, + RangeEnd range_end) { + if (sample_rate <= 0.0) { + throw std::invalid_argument("Sample rate must be positive"); + } + if (start_time > end_time) { + throw std::invalid_argument("Start time must be less than or equal to end time"); + } + + // evaluate_range_by_rate answers an empty range with the single value at + // that time, whatever the rate. + if (start_time == end_time) { + return SampleTimes(start_time, 0.0, 1); + } + + // Derived the way evaluate_range_by_rate derives it -- the span the samples + // cover, handed to the same rule as above -- so the two agree exactly + // rather than merely to within rounding. + const size_t count = detail::sample_count(end_time - start_time, sample_rate, range_end); + const size_t divisor = (range_end == RangeEnd::Inclusive) ? count - 1 : count; + const double covered_end = start_time + static_cast(divisor) / sample_rate; + return sample_times(start_time, covered_end, static_cast(count), range_end); +} + +} // namespace anim diff --git a/src/sampling.hpp b/src/sampling.hpp new file mode 100644 index 0000000..2881e8e --- /dev/null +++ b/src/sampling.hpp @@ -0,0 +1,51 @@ +#ifndef ANIM_SRC_SAMPLING_HPP +#define ANIM_SRC_SAMPLING_HPP + +#include "anim/range_end.hpp" + +#include +#include + +// Internal helper shared by Channel and Animation. Not installed, not part of +// the public API. +namespace anim { +namespace detail { + +/** + * @brief How many points spaced 1 / @p sample_rate apart, starting at 0, fall + * within a span of @p duration. + * + * With RangeEnd::Exclusive the span is [0, duration); with RangeEnd::Inclusive + * it is [0, duration]. The two differ only when the span is a whole number of + * sample periods, because that is the only case where a point lands exactly on + * the end: 4 seconds at 30 Hz gives 120 points half-open and 121 closed, while + * 1.05 seconds gives 32 either way, the next point falling past the end + * regardless of which end is asked for. + * + * A product that misses a whole number only by floating-point noise counts as + * that whole number, so 4.0 seconds at 30 Hz cannot come out as 121 points + * because the multiplication landed a few ulps high. + * + * @param duration Length of the span; must not be negative. + * @param sample_rate Samples per unit time; must be positive. + */ +inline std::size_t sample_count(double duration, double sample_rate, RangeEnd range_end) { + const bool include_end = (range_end == RangeEnd::Inclusive); + const double exact = duration * sample_rate; + if (exact <= 0.0) { + return include_end ? 1 : 0; + } + const double nearest = std::round(exact); + // Scale the tolerance with the magnitude, so it stays meaningful for long + // spans where the absolute representation error is correspondingly larger. + const double tolerance = 1e-9 * std::fmax(1.0, exact); + if (std::fabs(exact - nearest) <= tolerance) { + return static_cast(nearest) + (include_end ? 1 : 0); + } + return static_cast(std::ceil(exact)); +} + +} // namespace detail +} // namespace anim + +#endif // ANIM_SRC_SAMPLING_HPP diff --git a/tests/test_animation.cpp b/tests/test_animation.cpp index 8401906..2aed47f 100644 --- a/tests/test_animation.cpp +++ b/tests/test_animation.cpp @@ -328,26 +328,25 @@ TEST_CASE("Animation Sample Calculation", "[Animation]") { animation.create_channel("SampleChan"); SECTION("Valid sample rate") { - // length = 1.0, sample_rate = 10.0 - // samples = ceil(1.0 * 10.0) + 1 = 10 + 1 = 11 - REQUIRE(animation.num_samples(10.0) == 11); + // The range is half-open, so the count is length * sample_rate rounded + // up: length = 1.0 at 10.0 gives 10 samples spanning [0.0, 1.0). + REQUIRE(animation.num_samples(10.0) == 10); - // length = 1.0, sample_rate = 1.0 - // samples = ceil(1.0 * 1.0) + 1 = 1 + 1 = 2 - REQUIRE(animation.num_samples(1.0) == 2); + // length = 1.0, sample_rate = 1.0 -> 1 sample, at the start time + REQUIRE(animation.num_samples(1.0) == 1); animation.set_end_time(0.9); // Length = 0.9 - // samples = ceil(0.9 * 10.0) + 1 = ceil(9.0) + 1 = 9 + 1 = 10 - REQUIRE(animation.num_samples(10.0) == 10); - + // 0.9 * 10.0 = 9 exactly, so 9 samples and no rounding up + REQUIRE(animation.num_samples(10.0) == 9); + animation.set_end_time(0.95); // Length = 0.95 - // samples = ceil(0.95 * 10.0) + 1 = ceil(9.5) + 1 = 10 + 1 = 11 - REQUIRE(animation.num_samples(10.0) == 11); + // 0.95 * 10.0 = 9.5, rounded up so the whole span is covered + REQUIRE(animation.num_samples(10.0) == 10); } - + SECTION("Zero length animation") { animation.set_end_time(0.0); // Length = 0.0 - // samples = ceil(0.0 * 10.0) + 1 = 0 + 1 = 1 + // No span to divide, so the single sample at the start time REQUIRE(animation.num_samples(10.0) == 1); } @@ -357,6 +356,86 @@ TEST_CASE("Animation Sample Calculation", "[Animation]") { } } +TEST_CASE("Animation sample_times spans the animation's time base", "[Animation][sampling]") { + Animation animation("timed"); + animation.set_start_time(1.0); + animation.set_end_time(5.0); // length 4.0 + + SECTION("Count-based times span start_time to end_time") { + auto times = animation.sample_times(4); // half-open: step 1.0 + REQUIRE(times.size() == 4); + REQUIRE(times.front() == Catch::Approx(1.0)); + REQUIRE(times.step() == Catch::Approx(1.0)); + REQUIRE(times.back() == Catch::Approx(4.0)); // 5.0 is the edge, not a sample + + auto closed = animation.sample_times(5, RangeEnd::Inclusive); + REQUIRE(closed.size() == 5); + REQUIRE(closed.back() == Catch::Approx(5.0)); + } + + SECTION("By-rate times agree with num_samples") { + animation.create_channel("c"); + for (RangeEnd range_end : {RangeEnd::Exclusive, RangeEnd::Inclusive}) { + for (double rate : {1.0, 30.0}) { + auto times = animation.sample_times_by_rate(rate, range_end); + REQUIRE(times.size() == animation.num_samples(rate, range_end)); + REQUIRE(times.front() == Catch::Approx(animation.start_time())); + REQUIRE(times.step() == Catch::Approx(1.0 / rate).margin(1e-12)); + } + } + // 4 seconds at 30 Hz: 120 half-open, 121 closed, the extra one on the end. + REQUIRE(animation.sample_times_by_rate(30.0).size() == 120); + auto closed = animation.sample_times_by_rate(30.0, RangeEnd::Inclusive); + REQUIRE(closed.size() == 121); + REQUIRE(closed.back() == Catch::Approx(5.0)); + } + + SECTION("Every channel shares one time base") { + Channel& a = animation.create_channel("a"); + Channel& b = animation.create_channel("b"); + a.create_keyframe(1.0, 0.0, Point(), Point(), Function::Linear); + a.create_keyframe(5.0, 10.0, Point(), Point(), Function::Linear); + b.create_keyframe(2.0, 0.0, Point(), Point(), Function::Linear); + b.create_keyframe(4.0, 20.0, Point(), Point(), Function::Linear); + + // The channels cover different spans, but baking both over the + // animation's range gives values that line up against the same times. + const size_t n = animation.num_samples(10.0); + auto times = animation.sample_times_by_rate(10.0); + auto a_values = a.evaluate_range(animation.start_time(), animation.end_time(), + static_cast(n)); + auto b_values = b.evaluate_range(animation.start_time(), animation.end_time(), + static_cast(n)); + REQUIRE(times.size() == n); + REQUIRE(a_values.size() == n); + REQUIRE(b_values.size() == n); + for (size_t i = 0; i < n; ++i) { + REQUIRE(a.evaluate(times[i]) == Catch::Approx(a_values[i]).margin(1e-12)); + REQUIRE(b.evaluate(times[i]) == Catch::Approx(b_values[i]).margin(1e-12)); + } + } + + SECTION("No channels means no times, matching num_samples") { + REQUIRE(animation.num_samples(30.0) == 0); + REQUIRE(animation.sample_times_by_rate(30.0).empty()); + } + + SECTION("A zero-length animation gives the single sample at its start") { + animation.create_channel("c"); + animation.set_end_time(1.0); // length 0 + REQUIRE(animation.num_samples(30.0) == 1); + auto times = animation.sample_times_by_rate(30.0); + REQUIRE(times.size() == 1); + REQUIRE(times.front() == Catch::Approx(1.0)); + } + + SECTION("Invalid arguments") { + REQUIRE_THROWS_AS(animation.sample_times(-1), std::invalid_argument); + REQUIRE_THROWS_AS(animation.sample_times_by_rate(0.0), std::invalid_argument); + REQUIRE_THROWS_AS(animation.sample_times_by_rate(-1.0), std::invalid_argument); + } +} + TEST_CASE("Animation API Comprehensive Test", "[Animation]") { SECTION("Complete Animation API functionality test") { // Create a test animation @@ -507,12 +586,12 @@ TEST_CASE("Animation API Comprehensive Test", "[Animation]") { // Test zero length animation samples animation.create_channel("test_channel"); - REQUIRE(animation.num_samples(30.0) == 1); // Zero length + 1 = 1 sample - + REQUIRE(animation.num_samples(30.0) == 1); // Zero length, single sample + // Test normal timing animation.set_start_time(0.0); animation.set_end_time(1.0); - REQUIRE(animation.num_samples(30.0) == 31); // 1 second at 30fps + 1 = 31 samples + REQUIRE(animation.num_samples(30.0) == 30); // 1 second at 30fps = 30 samples } SECTION("Animation with no channels edge cases") { @@ -529,7 +608,7 @@ TEST_CASE("Animation API Comprehensive Test", "[Animation]") { // After adding a channel, samples should be calculated animation.create_channel("new_channel"); - REQUIRE(animation.num_samples(30.0) == 151); // 5 seconds at 30fps + 1 = 151 samples + REQUIRE(animation.num_samples(30.0) == 150); // 5 seconds at 30fps = 150 samples } SECTION("Animation const correctness") { diff --git a/tests/test_channel.cpp b/tests/test_channel.cpp index 72c4217..ed37741 100644 --- a/tests/test_channel.cpp +++ b/tests/test_channel.cpp @@ -4,6 +4,7 @@ #include #include // Required for Keyframe and Point #include // Required for GrabbedHandle enum +#include using namespace anim; @@ -542,7 +543,6 @@ TEST_CASE("Channel Time Properties", "[channel]") { REQUIRE(ch.start_time() == 0.0); REQUIRE(ch.end_time() == 0.0); REQUIRE(ch.length() == 0.0); - REQUIRE(ch.num_samples(30.0) == 0); } SECTION("Single keyframe") { @@ -552,7 +552,9 @@ TEST_CASE("Channel Time Properties", "[channel]") { REQUIRE(ch.start_time() == 2.5); REQUIRE(ch.end_time() == 2.5); REQUIRE(ch.length() == 0.0); - REQUIRE(ch.num_samples(30.0) == 1); // Single sample at the keyframe time + // A channel no longer counts its own samples: the range has to be + // stated, because a keyframe span and a timeline are not the same thing. + REQUIRE(sample_times_by_rate(ch.start_time(), ch.end_time(), 30.0).size() == 1); } SECTION("Multiple keyframes") { Animation anim; auto& ch = anim.create_channel("ch"); @@ -564,9 +566,10 @@ TEST_CASE("Channel Time Properties", "[channel]") { REQUIRE(ch.end_time() == 5.0); REQUIRE(ch.length() == 4.0); // 5.0 - 1.0 - // Duration is 4 seconds, at 30fps = 120 samples + 1 for endpoint - REQUIRE(ch.num_samples(30.0) == 121); - REQUIRE(ch.num_samples(1.0) == 5); // 4 seconds + 1 for endpoint + // Duration is 4 seconds; the range is half-open, so 30fps gives 120 + // samples spanning [1.0, 5.0), the last at 4.9667. + REQUIRE(sample_times_by_rate(ch.start_time(), ch.end_time(), 30.0).size() == 120); + REQUIRE(sample_times_by_rate(ch.start_time(), ch.end_time(), 1.0).size() == 4); } } @@ -629,10 +632,22 @@ TEST_CASE("Channel Evaluation Range", "[channel]") { ch.create_keyframe(0.0, 0.0, Point(), Point(), Function::Linear); ch.create_keyframe(2.0, 20.0, Point(), Point(), Function::Linear); - SECTION("evaluate_range") { + SECTION("evaluate_range defaults to a half-open range") { + // Step is 2.0 / 5, so the samples are at 0.0, 0.4, 0.8, 1.2 and 1.6. auto result = ch.evaluate_range(0.0, 2.0, 5); REQUIRE(result.size() == 5); REQUIRE(result[0] == Catch::Approx(0.0)); + REQUIRE(result[1] == Catch::Approx(4.0)); + REQUIRE(result[2] == Catch::Approx(8.0)); + REQUIRE(result[3] == Catch::Approx(12.0)); + REQUIRE(result[4] == Catch::Approx(16.0)); + } + + SECTION("evaluate_range closed lands the last sample on the end") { + // Step is 2.0 / 4, so the samples are at 0.0, 0.5, 1.0, 1.5 and 2.0. + auto result = ch.evaluate_range(0.0, 2.0, 5, RangeEnd::Inclusive); + REQUIRE(result.size() == 5); + REQUIRE(result[0] == Catch::Approx(0.0)); REQUIRE(result[1] == Catch::Approx(5.0)); REQUIRE(result[2] == Catch::Approx(10.0)); REQUIRE(result[3] == Catch::Approx(15.0)); @@ -646,18 +661,39 @@ TEST_CASE("Channel Evaluation Range", "[channel]") { } SECTION("evaluate_range with equal start and end times") { + // The count is what the caller asked for, so an empty range gives that + // many copies of the value at that time rather than a single one. auto result = ch.evaluate_range(1.0, 1.0, 5); - REQUIRE(result.size() == 1); - REQUIRE(result[0] == Catch::Approx(10.0)); + REQUIRE(result.size() == 5); + for (double v : result) { + REQUIRE(v == Catch::Approx(10.0)); + } + } + + SECTION("evaluate_range with no samples") { + REQUIRE(ch.evaluate_range(0.0, 2.0, 0).empty()); } SECTION("evaluate_range invalid arguments") { REQUIRE_THROWS_AS(ch.evaluate_range(2.0, 1.0, 5), std::invalid_argument); + REQUIRE_THROWS_AS(ch.evaluate_range(0.0, 2.0, -1), std::invalid_argument); + + // A reversed range is rejected for every count, not only for the ones + // large enough to reach the sampling loop. + REQUIRE_THROWS_AS(ch.evaluate_range(2.0, 1.0, 0), std::invalid_argument); + REQUIRE_THROWS_AS(ch.evaluate_range(2.0, 1.0, 1), std::invalid_argument); } SECTION("evaluate_range_by_rate") { auto result = ch.evaluate_range_by_rate(0.0, 2.0, 1.0); // 1 sample per second - REQUIRE(result.size() == 3); // 0, 1, 2 seconds + REQUIRE(result.size() == 2); // 0, 1 -- the range is half-open, 2 is excluded + REQUIRE(result[0] == Catch::Approx(0.0)); + REQUIRE(result[1] == Catch::Approx(10.0)); + } + + SECTION("evaluate_range_by_rate closed adds the closing sample") { + auto result = ch.evaluate_range_by_rate(0.0, 2.0, 1.0, RangeEnd::Inclusive); + REQUIRE(result.size() == 3); // 0, 1, 2 REQUIRE(result[0] == Catch::Approx(0.0)); REQUIRE(result[1] == Catch::Approx(10.0)); REQUIRE(result[2] == Catch::Approx(20.0)); @@ -676,6 +712,256 @@ TEST_CASE("Channel Evaluation Range", "[channel]") { } } +TEST_CASE("Channel evaluate_range_by_rate samples a half-open range", "[channel][sampling]") { + // A linear ramp where value == time, so a sampled value reports the exact + // time it was taken at and the spacing can be asserted directly. + auto ramp = [](Animation& anim, double duration) -> Channel& { + Channel& ch = anim.create_channel("ramp"); + ch.create_keyframe(0.0, 0.0, Point(), Point(), Function::Linear); + ch.create_keyframe(duration, duration, Point(), Point(), Function::Linear); + return ch; + }; + + SECTION("A whole number of periods gives exactly that many samples") { + Animation anim; + Channel& ch = ramp(anim, 4.0); + + auto result = ch.evaluate_range_by_rate(0.0, 4.0, 30.0); + REQUIRE(result.size() == 120); // not 121: end_time is not sampled + REQUIRE(sample_times_by_rate(0.0, 4.0, 30.0).size() == result.size()); + } + + SECTION("Samples land exactly one period apart") { + Animation anim; + Channel& ch = ramp(anim, 4.0); + + auto result = ch.evaluate_range_by_rate(0.0, 4.0, 30.0); + for (size_t i = 0; i < result.size(); ++i) { + REQUIRE(result[i] == Catch::Approx(static_cast(i) / 30.0).margin(1e-12)); + } + // The last sample is one period short of the end, never on it. + REQUIRE(result.back() == Catch::Approx(119.0 / 30.0).margin(1e-12)); + } + + SECTION("A partial period is rounded up so the span stays covered") { + Animation anim; + Channel& ch = ramp(anim, 2.0); + + // 1.05 * 30 = 31.5 -> 32 samples, the last at 31/30 = 1.0333, still + // inside the requested range. Spacing stays exactly 1/30 throughout. + auto result = ch.evaluate_range_by_rate(0.0, 1.05, 30.0); + REQUIRE(result.size() == 32); + REQUIRE(result.back() == Catch::Approx(31.0 / 30.0).margin(1e-12)); + REQUIRE(result.back() < 1.05); + for (size_t i = 1; i < result.size(); ++i) { + REQUIRE((result[i] - result[i - 1]) == Catch::Approx(1.0 / 30.0).margin(1e-12)); + } + } + + SECTION("A span offset from zero keeps the same count and spacing") { + Animation anim; + Channel& ch = ramp(anim, 10.0); + + auto result = ch.evaluate_range_by_rate(1.0, 5.0, 30.0); + REQUIRE(result.size() == 120); + REQUIRE(result.front() == Catch::Approx(1.0).margin(1e-12)); + REQUIRE(result.back() == Catch::Approx(1.0 + 119.0 / 30.0).margin(1e-12)); + } + + SECTION("A product that overshoots by rounding error does not add a sample") { + Animation anim; + // Just above 4.0, so duration * 30 lands a few ulps above 120. Rounding + // that up would produce a 121st sample covering a span of ~1e-15. + const double duration = std::nextafter(4.0, 5.0); + Channel& ch = ramp(anim, duration); + + REQUIRE(duration * 30.0 > 120.0); + REQUIRE(ch.evaluate_range_by_rate(0.0, duration, 30.0).size() == 120); + REQUIRE(sample_times_by_rate(0.0, duration, 30.0).size() == 120); + } + + SECTION("num_samples agrees with what evaluate_range_by_rate returns") { + for (double duration : {0.5, 1.0, 2.5, 4.0, 7.3}) { + for (double rate : {1.0, 24.0, 30.0, 60.0, 120.0}) { + Animation anim; + Channel& ch = ramp(anim, duration); + REQUIRE(sample_times_by_rate(0.0, duration, rate).size() + == ch.evaluate_range_by_rate(0.0, duration, rate).size()); + } + } + } + + SECTION("A closed range adds the closing sample and keeps the spacing") { + Animation anim; + Channel& ch = ramp(anim, 4.0); + + auto result = ch.evaluate_range_by_rate(0.0, 4.0, 30.0, RangeEnd::Inclusive); + REQUIRE(result.size() == 121); // the 120 half-open samples, plus 4.0 + REQUIRE(sample_times_by_rate(0.0, 4.0, 30.0, RangeEnd::Inclusive).size() + == result.size()); + REQUIRE(result.front() == Catch::Approx(0.0).margin(1e-9)); + REQUIRE(result.back() == Catch::Approx(4.0).margin(1e-9)); + for (size_t i = 1; i < result.size(); ++i) { + REQUIRE((result[i] - result[i - 1]) == Catch::Approx(1.0 / 30.0).margin(1e-9)); + } + } + + SECTION("The two ends agree when the span is not a whole number of periods") { + Animation anim; + Channel& ch = ramp(anim, 2.0); + + // 1.05 * 30 = 31.5, so no sample lands on the end and asking for it + // changes nothing: the next one would be past the end either way. + auto open = ch.evaluate_range_by_rate(0.0, 1.05, 30.0); + auto closed = ch.evaluate_range_by_rate(0.0, 1.05, 30.0, RangeEnd::Inclusive); + REQUIRE(open.size() == closed.size()); + REQUIRE(open.size() == 32); + } + + SECTION("A span shorter than one period gives the single opening sample") { + Animation anim; + Channel& ch = ramp(anim, 2.0); + + // 0.02 * 30 = 0.6, so only the sample at the start falls inside the + // range; the next period lands past the end for either range end. + REQUIRE(ch.evaluate_range_by_rate(0.0, 0.02, 30.0).size() == 1); + auto closed = ch.evaluate_range_by_rate(0.0, 0.02, 30.0, RangeEnd::Inclusive); + REQUIRE(closed.size() == 1); + REQUIRE(closed.front() == Catch::Approx(0.0).margin(1e-12)); + } + + SECTION("evaluate_range covers the closed range when asked") { + Animation anim; + Channel& ch = ramp(anim, 4.0); + + auto result = ch.evaluate_range(0.0, 4.0, 121, RangeEnd::Inclusive); + REQUIRE(result.size() == 121); + REQUIRE(result.front() == Catch::Approx(0.0).margin(1e-12)); + REQUIRE(result.back() == Catch::Approx(4.0).margin(1e-12)); + + // The default half-open call over the same range stops one step short. + auto open = ch.evaluate_range(0.0, 4.0, 121); + REQUIRE(open.size() == 121); + REQUIRE(open.back() < result.back()); + } +} + +TEST_CASE("sample_times reports where the samples were taken", "[channel][sampling]") { + // value == time, so the values a range returns are the times it sampled at. + // The comparison is close rather than exact: recovering the time through the + // linear interpolation rounds, so it pins where the samples were taken + // without pinning the last bit. The step rule itself is checked exactly, + // against the formula, further down. + auto ramp = [](Animation& anim, double from, double to) -> Channel& { + Channel& ch = anim.create_channel("ramp"); + ch.create_keyframe(from, from, Point(), Point(), Function::Linear); + ch.create_keyframe(to, to, Point(), Point(), Function::Linear); + return ch; + }; + + static_assert(sizeof(SampleTimes) <= 2 * sizeof(double) + sizeof(size_t), + "SampleTimes must stay a small value: a start, a step and a count."); + + SECTION("Times match evaluate_range over the same range") { + Animation anim; + Channel& ch = ramp(anim, 1.0, 5.0); + + for (RangeEnd range_end : {RangeEnd::Exclusive, RangeEnd::Inclusive}) { + for (int n : {1, 2, 7, 120}) { + auto values = ch.evaluate_range(ch.start_time(), ch.end_time(), n, range_end); + auto times = sample_times(ch.start_time(), ch.end_time(), n, range_end); + REQUIRE(times.size() == values.size()); + for (size_t i = 0; i < values.size(); ++i) { + REQUIRE(times[i] == Catch::Approx(values[i]).margin(1e-12)); + } + + // The step rule, pinned exactly: the span over the count for a + // half-open range, over one less for a closed one. + if (n > 1) { + const double span = ch.end_time() - ch.start_time(); + const double expected_step = + span / ((range_end == RangeEnd::Inclusive) ? (n - 1) : n); + REQUIRE(times.step() == expected_step); + REQUIRE(times.front() == ch.start_time()); + REQUIRE(times[n - 1] == ch.start_time() + (n - 1) * expected_step); + } + } + } + } + + SECTION("By-rate times match evaluate_range_by_rate") { + Animation anim; + Channel& ch = ramp(anim, 0.0, 4.0); + + for (RangeEnd range_end : {RangeEnd::Exclusive, RangeEnd::Inclusive}) { + for (double rate : {1.0, 24.0, 30.0}) { + auto values = ch.evaluate_range_by_rate(ch.start_time(), ch.end_time(), + rate, range_end); + auto times = sample_times_by_rate(ch.start_time(), ch.end_time(), + rate, range_end); + REQUIRE(times.size() == values.size()); + for (size_t i = 0; i < values.size(); ++i) { + REQUIRE(times[i] == Catch::Approx(values[i]).margin(1e-12)); + } + // Spacing is one sample period, exactly, for either range end. + REQUIRE(times.step() == Catch::Approx(1.0 / rate).margin(1e-12)); + } + } + } + + SECTION("Free functions match evaluate_range over an arbitrary range") { + Animation anim; + Channel& ch = ramp(anim, -10.0, 10.0); // wider than the range sampled below + + auto values = ch.evaluate_range(-2.5, 3.5, 9, RangeEnd::Inclusive); + auto times = sample_times(-2.5, 3.5, 9, RangeEnd::Inclusive); + REQUIRE(times.size() == values.size()); + REQUIRE(times.back() == 3.5); // a closed range ends exactly on its end + for (size_t i = 0; i < values.size(); ++i) { + REQUIRE(times[i] == Catch::Approx(values[i]).margin(1e-12)); + } + + auto by_rate_values = ch.evaluate_range_by_rate(-2.5, 3.5, 30.0); + auto by_rate_times = sample_times_by_rate(-2.5, 3.5, 30.0); + REQUIRE(by_rate_times.size() == by_rate_values.size()); + for (size_t i = 0; i < by_rate_values.size(); ++i) { + REQUIRE(by_rate_times[i] == Catch::Approx(by_rate_values[i]).margin(1e-12)); + } + } + + SECTION("Accessors") { + auto times = sample_times(0.0, 4.0, 5); // half-open: step 0.8 + REQUIRE(times.size() == 5); + REQUIRE_FALSE(times.empty()); + REQUIRE(times.step() == Catch::Approx(0.8)); + REQUIRE(times.front() == Catch::Approx(0.0)); + REQUIRE(times.back() == Catch::Approx(3.2)); + REQUIRE(times.at(2) == Catch::Approx(1.6)); + REQUIRE_THROWS_AS(times.at(5), std::out_of_range); + REQUIRE(times == sample_times(0.0, 4.0, 5)); + REQUIRE(times != sample_times(0.0, 4.0, 5, RangeEnd::Inclusive)); + } + + SECTION("An empty channel spans nothing, so a rate yields one sample") { + Animation anim; + Channel& ch = anim.create_channel("empty"); + // start_time() and end_time() are both 0 with no keyframes, so this is + // the degenerate empty range rather than a count of zero. Sampling a + // whole animation goes through Animation, which reports 0 channels. + REQUIRE(sample_times_by_rate(ch.start_time(), ch.end_time(), 30.0).size() == 1); + } + + SECTION("Invalid arguments") { + Animation anim; + Channel& ch = ramp(anim, 0.0, 4.0); + REQUIRE_THROWS_AS(sample_times(0.0, 4.0, -1), std::invalid_argument); + REQUIRE_THROWS_AS(sample_times_by_rate(0.0, 4.0, 0.0), std::invalid_argument); + REQUIRE_THROWS_AS(sample_times_by_rate(0.0, 4.0, -1.0), std::invalid_argument); + REQUIRE_THROWS_AS(sample_times(2.0, 1.0, 5), std::invalid_argument); + REQUIRE_THROWS_AS(sample_times_by_rate(2.0, 1.0, 30.0), std::invalid_argument); + } +} + TEST_CASE("Channel Handle Updates", "[channel]") { SECTION("Smooth handles update when keyframes change") { Animation anim; @@ -839,7 +1125,7 @@ TEST_CASE("Channel Complex Animation Scenario", "[channel]") { REQUIRE_NOTHROW(pos_x.evaluate_range_by_rate(0.0, 4.0, rate)); auto values = pos_x.evaluate_range_by_rate(0.0, 4.0, rate); - size_t expected_samples = static_cast(std::ceil(4.0 * rate)) + 1; + size_t expected_samples = static_cast(std::ceil(4.0 * rate)); REQUIRE(values.size() == expected_samples); // Verify all values are finite @@ -981,15 +1267,16 @@ TEST_CASE("Channel API Comprehensive Test", "[channel]") { } auto values_by_rate = channel.evaluate_range_by_rate(0.0, 2.0, 1.0); - REQUIRE(values_by_rate.size() == 3); // 0, 1, 2 seconds at 1Hz + REQUIRE(values_by_rate.size() == 2); // 0 and 1 seconds at 1Hz; 2 is excluded // Test channel timing properties REQUIRE(channel.start_time() == 0.0); REQUIRE(channel.end_time() == 4.0); REQUIRE(channel.length() == 4.0); - // Test num_samples calculation - size_t num_samples = channel.num_samples(30.0); + // Test sample count over the channel's own span + size_t num_samples = + sample_times_by_rate(channel.start_time(), channel.end_time(), 30.0).size(); REQUIRE(num_samples > 0); // Test keyframe removal @@ -1035,9 +1322,9 @@ TEST_CASE("Channel API Comprehensive Test", "[channel]") { REQUIRE_THROWS_AS(channel.evaluate_range_by_rate(0.0, 2.0, -1.0), std::invalid_argument); // negative rate REQUIRE_THROWS_AS(channel.evaluate_range_by_rate(2.0, 1.0, 1.0), std::invalid_argument); // start > end - // Test num_samples error - REQUIRE_THROWS_AS(channel.num_samples(0.0), std::invalid_argument); // zero rate - REQUIRE_THROWS_AS(channel.num_samples(-1.0), std::invalid_argument); // negative rate + // Test sample-count error paths + REQUIRE_THROWS_AS(sample_times_by_rate(0.0, 4.0, 0.0), std::invalid_argument); + REQUIRE_THROWS_AS(sample_times_by_rate(0.0, 4.0, -1.0), std::invalid_argument); } SECTION("Channel emplace keyframe functionality") { Animation anim;