diff --git a/docs/superpowers/plans/2026-09-27-plot-library-scaffold.md b/docs/superpowers/plans/2026-09-27-plot-library-scaffold.md index 8ed8bb9..0485624 100644 --- a/docs/superpowers/plans/2026-09-27-plot-library-scaffold.md +++ b/docs/superpowers/plans/2026-09-27-plot-library-scaffold.md @@ -1019,17 +1019,23 @@ Expected: FAIL to compile — `plot/plot/ticks.hpp` not found. `src/plot/ticks.cpp` — the nice-number algorithm: ```cpp -// generate_ticks(range, px, min_px, out): -// if (!(range.span() > 0) || px <= 0) { out.clear(); return; } +// generate_ticks(range, px, min_px, out, bool include_minors = false): +// if (!(range.span() > 0) || px <= 0 || !isfinite(span) || min > max) { out.clear(); return; } // const f64 target_count = std::max(1.0, f64(px) / f64(min_px)); // const f64 raw_step = range.span() / target_count; // const f64 mag = std::pow(10.0, std::floor(std::log10(raw_step))); // const f64 norm = raw_step / mag; // in [1, 10) -// const f64 step = mag * (norm <= 1.5 ? 1.0 : norm <= 3.0 ? 2.0 : norm <= 7.0 ? 5.0 : 10.0); +// // ROUND UP, not to nearest. Rounding to nearest can exceed target_count and +// // break the "at most px/min_px + 2 ticks" bound that the tests assert. +// const f64 step = mag * (norm <= 1.0 ? 1.0 : norm <= 2.0 ? 2.0 : norm <= 5.0 ? 5.0 : 10.0); // f64 first = std::ceil(range.min / step) * step; // for (f64 v = first; v <= range.max + step * 1e-9; v += step) out.push_back({v, true, {}}); -// // minor ticks: step/5 (or step/4 when step starts with 2, step/5 when with 5), same loop. -// // hard guard: if (out.size() > 256) out.resize(256); +// // include_minors defaults to false because minors share this same vector: a +// // caller that wants only majors must not have to filter, and the "every value +// // is a multiple of the major step" assertions only hold for the majors-only +// // form. Grid rendering passes true. +// // minor ticks: step/5 (step/4 for a step beginning with 2), same loop. +// // hard guard: 256 entries maximum INCLUDING minors. ``` Labels are assigned by `format_si` unless the axis is a time axis, in which case `format_time` is used. Ticks whose label would be non-finite or absurdly long (> 12 chars) fall back to `%.3g`. diff --git a/include/plot/plot/plot.hpp b/include/plot/plot/plot.hpp index 20b0e40..7ffaa85 100644 --- a/include/plot/plot/plot.hpp +++ b/include/plot/plot/plot.hpp @@ -109,6 +109,13 @@ class Plot { /// Fits every x axis to the union of the series bounds of the traces bound to /// it, padded by 2% of the span, and re-arms auto scale on the axes it fitted. /// An axis with no visible trace is left exactly as it was. + /// + /// A log axis is fitted in log space and needs a positive value to show. When + /// its traces hold none (every sample negative or zero) the fit is refused: the + /// axis keeps its current range, auto scale is not re-armed, and the next + /// auto-fit tries again -- so a stream that starts at zero begins fitting on its + /// own once it goes positive. A log axis is therefore never left with `min >= + /// max` or a non-positive bound by an auto-fit. void auto_fit_x(); /// Same for one y axis, over that axis' traces' y bounds. void auto_fit_y(u32 axis); @@ -161,10 +168,15 @@ class Plot { mutable u64 last_size = 0; }; - /// Fit one axis to its traces' bounds, leaving auto_scale alone. False when no - /// visible trace is bound to the axis, in which case the range is untouched. + /// Fit one axis to its traces' bounds, leaving auto_scale alone. False when the + /// axis cannot be fitted (no visible trace bound to it, or a log axis whose + /// traces hold no positive value), in which case the range is untouched. bool fit_x_axis(u32 axis); bool fit_y_axis(u32 axis); + /// Union of the bounds of the visible traces bound to one axis (`x_side` + /// selects x or y). False when there is no such trace; `out` then retains + /// whatever it held. + bool trace_bounds_union(bool x_side, u32 axis, Range& out) const; /// Refit every axis that is still on auto scale. layout() runs this, so a /// growing stream stays framed until a caller gesture turns auto scale off. void autoscale_axes(); diff --git a/include/plot/plot/ticks.hpp b/include/plot/plot/ticks.hpp index b60c510..5728026 100644 --- a/include/plot/plot/ticks.hpp +++ b/include/plot/plot/ticks.hpp @@ -38,8 +38,18 @@ enum class TickFormat { void format_si(f64 v, char* buf, usize n, const char* unit = nullptr); /// Writes `seconds` as a compact clock string to `buf`, NUL-terminated and never -/// past `n` bytes. Shapes: `500ms`, `1.5s`, `1:30`, `1:00:00`, `2.5d`; negatives -/// keep their sign. A non-finite value falls back to `"%.3g"`. +/// past `n` bytes. Negatives keep their sign; a non-finite value falls back to +/// `"%.3g"`. +/// +/// The whole value is rounded to a single whole-second count first and every +/// field is derived from that one number, so no field can ever reach its radix: +/// 119.5 s is `2:00`, not `1:60`, and 3599.5 s is `1:00:00`. +/// +/// Shapes, by magnitude: below 1 s `500ms`; below 60 s `1.5s`; below 1 h `1:30`; +/// at or above 24 h, as a decimal day count `2.5d`. Between 1 h and 24 h the +/// shape is `1:01:01` -- and because a value that rounds up to 24 h takes the day +/// shape, the hour field of that shape is always below 24. The 60 s and 1 h +/// boundaries are inclusive of the rounded value: 59.5 s is `1:00`. void format_time(f64 seconds, char* buf, usize n); /// The hard cap on `generate_ticks` output, minor ticks included. A range dense diff --git a/src/plot/plot.cpp b/src/plot/plot.cpp index 9a0fc55..d73a64b 100644 --- a/src/plot/plot.cpp +++ b/src/plot/plot.cpp @@ -32,28 +32,48 @@ Axis axis_from(const AxisDesc& desc) { return axis; } -/// Adds the margin an auto-fitted range needs. A zero-span union (a single-sample -/// series) is widened instead of padded, because a degenerate range has no span to -/// take 2% of and would leave the transform's inverse ambiguous. -Range fit_range(Range r, bool log) { - if (!std::isfinite(r.min) || !std::isfinite(r.max)) return Range{0.0, 1.0}; - f64 lo = std::min(r.min, r.max); - f64 hi = std::max(r.min, r.max); +/// Fits `axis`' range to the union of its traces' bounds, returning false when the +/// union cannot be fitted and the range is then left untouched. +/// +/// A linear axis takes 2% of the span on both ends; a zero-span union (a +/// single-sample series) is widened instead of padded, because a degenerate range +/// has no span to take 2% of and would leave the transform's inverse ambiguous. +/// +/// A log axis needs strictly positive data, and this is where a log axis could +/// otherwise be left broken: an all-negative union has no positive value to show, +/// and padding it would produce `min > max` once the lower edge was clamped to a +/// positive bound. Instead the positive part of the union is used -- `[hi*1e-3, +/// hi]` when the union straddles or starts below zero, since zero has no place on +/// a log axis -- the pad is multiplicative (2% of the span in log space), and a +/// union with no positive value at all is refused outright. The caller keeps its +/// range and is free to retry, which is what lets a stream that starts at zero +/// begin fitting by itself once it goes positive. +bool fit_axis_range(Axis& axis, Range bounds) { + const f64 lo = std::min(bounds.min, bounds.max); + const f64 hi = std::max(bounds.min, bounds.max); + if (!std::isfinite(lo) || !std::isfinite(hi)) return false; - if (log && lo > 0.0) { - if (hi > lo) { - const f64 factor = std::pow(hi / lo, 0.02); // 2% of the span, in log space - return Range{lo / factor, hi * factor}; + if (axis.log) { + if (!(hi > 0.0)) return false; // no positive sample: not auto-fittable + const f64 lower = lo > 0.0 ? lo : hi * 1e-3; + if (!(lower > 0.0) || lower > hi) return false; + if (lower == hi) { + axis.range = Range{lower / 10.0, hi * 10.0}; // one decade either side + return true; } - return Range{lo / 10.0, hi * 10.0}; // one decade either side + const f64 factor = std::pow(hi / lower, 0.02); // 2% of the span, in log space + axis.range = Range{lower / factor, hi * factor}; + return true; } - if (hi - lo > 0.0) { + if (hi > lo) { const f64 pad = (hi - lo) * 0.02; - return Range{lo - pad, hi + pad}; + axis.range = Range{lo - pad, hi + pad}; + return true; } const f64 pad = std::fabs(lo) * 1e-3 + 0.5; - return Range{lo - pad, hi + pad}; + axis.range = Range{lo - pad, hi + pad}; + return true; } } // namespace @@ -170,52 +190,33 @@ Transform Plot::y_transform(u32 axis) const { static_cast(r.y), false, a.log); } -bool Plot::fit_x_axis(u32 axis) { - Range union_range{}; +bool Plot::trace_bounds_union(bool x_side, u32 axis, Range& out) const { bool any = false; for (const Trace& t : traces_) { - if (t.axis_x != axis || !t.visible()) continue; // null or empty series: skip - const Range bounds = t.series->x_bounds(); + if ((x_side ? t.axis_x : t.axis_y) != axis || !t.visible()) continue; // null series: skip + const Range bounds = x_side ? t.series->x_bounds() : t.series->y_bounds(); if (!std::isfinite(bounds.min) || !std::isfinite(bounds.max)) continue; if (!any) { - union_range = bounds; + out = bounds; any = true; } else { - union_range.min = std::min(union_range.min, bounds.min); - union_range.max = std::max(union_range.max, bounds.max); + out.min = std::min(out.min, bounds.min); + out.max = std::max(out.max, bounds.max); } } - if (!any) return false; // nothing bound to this axis: leave the caller's range + return any; +} - Axis& a = x_axes_[axis]; - Range fitted = fit_range(union_range, a.log); - if (a.log && fitted.min <= 0.0) fitted.min = fitted.max > 0.0 ? fitted.max * 1e-3 : 1.0; - a.range = fitted; - return true; +bool Plot::fit_x_axis(u32 axis) { + Range union_range{}; + if (!trace_bounds_union(/*x_side=*/true, axis, union_range)) return false; + return fit_axis_range(x_axes_[axis], union_range); } bool Plot::fit_y_axis(u32 axis) { Range union_range{}; - bool any = false; - for (const Trace& t : traces_) { - if (t.axis_y != axis || !t.visible()) continue; - const Range bounds = t.series->y_bounds(); - if (!std::isfinite(bounds.min) || !std::isfinite(bounds.max)) continue; - if (!any) { - union_range = bounds; - any = true; - } else { - union_range.min = std::min(union_range.min, bounds.min); - union_range.max = std::max(union_range.max, bounds.max); - } - } - if (!any) return false; - - Axis& a = y_axes_[axis]; - Range fitted = fit_range(union_range, a.log); - if (a.log && fitted.min <= 0.0) fitted.min = fitted.max > 0.0 ? fitted.max * 1e-3 : 1.0; - a.range = fitted; - return true; + if (!trace_bounds_union(/*x_side=*/false, axis, union_range)) return false; + return fit_axis_range(y_axes_[axis], union_range); } void Plot::auto_fit_x() { diff --git a/src/plot/ticks.cpp b/src/plot/ticks.cpp index 25fcca4..ae6791e 100644 --- a/src/plot/ticks.cpp +++ b/src/plot/ticks.cpp @@ -94,23 +94,37 @@ void format_time(f64 seconds, char* buf, usize n) { const char* const sign = seconds < 0.0 ? "-" : ""; const f64 t = std::fabs(seconds); - if (t >= 86400.0) { - std::snprintf(buf, n, "%s%.3gd", sign, t / 86400.0); - } else if (t >= 3600.0) { - const auto hours = static_cast(t / 3600.0); - const auto mins = static_cast((t - 3600.0 * static_cast(hours)) / 60.0); - const auto secs = - static_cast(std::llround(t - 3600.0 * static_cast(hours) - - 60.0 * static_cast(mins))); - std::snprintf(buf, n, "%s%llu:%02llu:%02llu", sign, hours, mins, secs); - } else if (t >= 60.0) { - const auto mins = static_cast(t / 60.0); - const auto secs = static_cast(std::llround(t - 60.0 * static_cast(mins))); - std::snprintf(buf, n, "%s%llu:%02llu", sign, mins, secs); - } else if (t >= 1.0) { - std::snprintf(buf, n, "%s%.3gs", sign, t); + + if (t < 1.0) { + // Sub-second: milliseconds. The 999.5 cut keeps %.3g out of scientific + // notation (it would print "1e+03ms") right below the one-second boundary. + const f64 ms = t * 1000.0; + if (ms < 999.5) { + std::snprintf(buf, n, "%s%.3gms", sign, ms); + } else { + std::snprintf(buf, n, "%s%.3gs", sign, t); + } + return; + } + + // ONE rounding decision for the whole value. Deriving the fields from a single + // rounded second count is what keeps a field from reaching its radix: rounding + // the seconds leftover independently would print 119.5 s as "1:60". + const f64 whole = std::round(t); + if (whole >= 86400.0) { + // At or above a day the day field appears, so the h:mm:ss shape never sees an + // hour of 24 or more. + std::snprintf(buf, n, "%s%.3gd", sign, whole / 86400.0); + return; + } + const auto total = static_cast(whole); + if (whole >= 3600.0) { + std::snprintf(buf, n, "%s%llu:%02llu:%02llu", sign, total / 3600ull, + (total / 60ull) % 60ull, total % 60ull); + } else if (whole >= 60.0) { + std::snprintf(buf, n, "%s%llu:%02llu", sign, total / 60ull, total % 60ull); } else { - std::snprintf(buf, n, "%s%.3gms", sign, t * 1000.0); + std::snprintf(buf, n, "%s%.3gs", sign, t); } } diff --git a/tests/test_plot.cpp b/tests/test_plot.cpp index f342f12..9b01f18 100644 --- a/tests/test_plot.cpp +++ b/tests/test_plot.cpp @@ -24,6 +24,13 @@ f32 widest_major_label(const plot::Axis& a, const plot::render::TextBackend& t) return w; } +/// A log axis must never be left inverted or non-positive by an auto-fit. +void check_log_axis_range(const plot::Range& r) { + PLOT_CHECK(std::isfinite(r.min) && std::isfinite(r.max)); + PLOT_CHECK(r.min > 0.0); + PLOT_CHECK(r.min < r.max); +} + bool has_vertical(const plot::Grid& g, f64 x, bool major) { for (const plot::GridLine& l : g.lines()) { if (l.x0 == l.x1 && l.x0 == x && l.major == major) return true; @@ -259,6 +266,84 @@ PLOT_TEST(inverted_and_out_of_range_measurements_are_invalid) { PLOT_CHECK(!p.measurement(99u).valid); // unknown measure id } +PLOT_TEST(auto_fit_log_axis_stays_positive_and_ordered) { + // All-negative trace bounds: nothing to show on a log axis, so the fit is + // refused and the caller's (positive) range survives untouched. + { + std::vector y = {-5.0f, -3.0f, -1.0f}; + plot::data::ArraySeries s(std::move(y), 0.0, 1.0); + plot::Plot p; + p.set_viewport({0.0f, 0.0f, 800.0f, 400.0f}); + const u32 ax = p.add_x_axis({"t", {1.0, 10.0}, false, plot::AxisPlace::Bottom, false}); + const u32 ay = p.add_y_axis({"v", {1.0, 100.0}, true, plot::AxisPlace::Left, true}); + p.add_trace({&s, plot::Color::hex(0x40C0FF), 1.5f, ax, ay}); + p.auto_fit_y(ay); + check_log_axis_range(p.y_range(ay)); + PLOT_CHECK_NEAR(p.y_range(ay).min, 1.0, 1e-12); + PLOT_CHECK_NEAR(p.y_range(ay).max, 100.0, 1e-12); + (void)p.layout(); // the per-layout refit refuses too + check_log_axis_range(p.y_range(ay)); + PLOT_CHECK_NEAR(p.y_range(ay).max, 100.0, 1e-12); + } + + // All-zero bounds: same refusal, no "0" or negative bound on the axis. One + // sample at x = 0 makes both bounds exactly {0, 0}. + { + std::vector y = {0.0f}; + plot::data::ArraySeries s(std::move(y), 0.0, 1.0); + plot::Plot p; + p.set_viewport({0.0f, 0.0f, 800.0f, 400.0f}); + const u32 ax = p.add_x_axis({"t", {1.0, 10.0}, true, plot::AxisPlace::Bottom, true}); + const u32 ay = p.add_y_axis({"v", {1.0, 10.0}, true, plot::AxisPlace::Left, true}); + p.add_trace({&s, plot::Color::hex(0x40C0FF), 1.5f, ax, ay}); + p.auto_fit_x(); + p.auto_fit_y(ay); + check_log_axis_range(p.x_range(ax)); + check_log_axis_range(p.y_range(ay)); + PLOT_CHECK_NEAR(p.x_range(ax).min, 1.0, 1e-12); // refused: range untouched + PLOT_CHECK_NEAR(p.x_range(ax).max, 10.0, 1e-12); + PLOT_CHECK_NEAR(p.y_range(ay).min, 1.0, 1e-12); + PLOT_CHECK_NEAR(p.y_range(ay).max, 10.0, 1e-12); + } + + // Spanning zero with some positive samples: the fit uses the positive part, and + // the positive data stays inside the range. + { + std::vector y = {-1.0f, 0.5f, 3.0f}; + plot::data::ArraySeries s(std::move(y), -1.0, 1.0); // x bounds {-1, 1} + plot::Plot p; + p.set_viewport({0.0f, 0.0f, 800.0f, 400.0f}); + const u32 ax = p.add_x_axis({"t", {1.0, 10.0}, true, plot::AxisPlace::Bottom, true}); + const u32 ay = p.add_y_axis({"v", {1.0, 100.0}, true, plot::AxisPlace::Left, true}); + p.add_trace({&s, plot::Color::hex(0x40C0FF), 1.5f, ax, ay}); + p.auto_fit_x(); + p.auto_fit_y(ay); + check_log_axis_range(p.x_range(ax)); + check_log_axis_range(p.y_range(ay)); + PLOT_CHECK(p.x_range(ax).min <= 1.0); // the positive x data + PLOT_CHECK(p.x_range(ax).max >= 1.0); + PLOT_CHECK(p.y_range(ay).min <= 3.0); // the positive y data + PLOT_CHECK(p.y_range(ay).max >= 3.0); + } + + // Purely positive stays a plain log fit, still ordered and positive. + { + std::vector y = {1.0f, 2.0f, 3.0f}; + plot::data::ArraySeries s(std::move(y), 1.0, 1.0); // x bounds {1, 3} + plot::Plot p; + p.set_viewport({0.0f, 0.0f, 800.0f, 400.0f}); + const u32 ax = p.add_x_axis({"t", {1.0, 10.0}, true, plot::AxisPlace::Bottom, true}); + const u32 ay = p.add_y_axis({"v", {1.0, 100.0}, true, plot::AxisPlace::Left, true}); + p.add_trace({&s, plot::Color::hex(0x40C0FF), 1.5f, ax, ay}); + p.auto_fit_x(); + p.auto_fit_y(ay); + check_log_axis_range(p.x_range(ax)); + check_log_axis_range(p.y_range(ay)); + PLOT_CHECK(p.x_range(ax).min < 1.0 && p.x_range(ax).max > 3.0); + PLOT_CHECK(p.y_range(ay).min < 1.0 && p.y_range(ay).max > 3.0); + } +} + PLOT_TEST(grid_lines_are_tick_aligned_data_space_segments) { plot::Plot p; p.set_viewport({0.0f, 0.0f, 800.0f, 400.0f}); diff --git a/tests/test_ticks.cpp b/tests/test_ticks.cpp index 895a940..dbab83c 100644 --- a/tests/test_ticks.cpp +++ b/tests/test_ticks.cpp @@ -1,11 +1,42 @@ #include "harness.hpp" #include #include +#include #include #include #include #include +namespace { + +/// Fails if any colon-separated field of a clock label reaches its radix: minutes +/// and seconds must stay below 60 and hours below 24 (a value that rounds up to +/// 24 h takes the day shape instead), so "1:60" or "24:00:00" cannot slip through. +void check_no_field_reaches_its_radix(const std::string& label) { + const std::size_t colons = static_cast(std::count(label.begin(), label.end(), ':')); + if (colons == 0) return; // decimal shapes have no fields + // Two colons mean h:mm:ss, one means m:ss: only the former starts with hours. + const int first_radix = colons >= 2 ? 24 : 60; + std::string clock = label; + if (!clock.empty() && (clock[0] == '-' || clock[0] == '+')) clock.erase(0, 1); + std::size_t start = 0; + std::size_t field = 0; + while (start <= clock.size()) { + const std::size_t colon = clock.find(':', start); + const std::string part = + clock.substr(start, colon == std::string::npos ? std::string::npos : colon - start); + if (!part.empty()) { + const int value = std::atoi(part.c_str()); + PLOT_CHECK(value >= 0 && value < (field == 0 ? first_radix : 60)); + } + if (colon == std::string::npos) break; + start = colon + 1; + ++field; + } +} + +} // namespace + PLOT_TEST(nice_ticks_are_powers_of_ten_times_125) { std::vector t; plot::generate_ticks({0.0, 1.0}, /*px=*/500.0f, /*min_px=*/60.0f, t); @@ -107,3 +138,29 @@ PLOT_TEST(format_time_shapes) { plot::format_time(86400.0, b, sizeof b); PLOT_CHECK(std::string(b) == "1d"); plot::format_time(std::nan(""), b, sizeof b); PLOT_CHECK(std::string(b) == "nan"); } + +PLOT_TEST(format_time_never_lets_a_field_reach_its_radix) { + char b[32]; + // Rounding the seconds leftover on its own used to print 119.5 s as "1:60"; + // every field now comes from one rounded whole-second count. + plot::format_time(119.5, b, sizeof b); PLOT_CHECK(std::string(b) == "2:00"); + plot::format_time(3599.5, b, sizeof b); PLOT_CHECK(std::string(b) == "1:00:00"); + plot::format_time(59.4999, b, sizeof b); PLOT_CHECK(std::string(b) == "59.5s"); + plot::format_time(59.5, b, sizeof b); PLOT_CHECK(std::string(b) == "1:00"); + plot::format_time(60.0, b, sizeof b); PLOT_CHECK(std::string(b) == "1:00"); + // At 24 h the day field appears, so the hour field never reaches 24. + plot::format_time(86399.4, b, sizeof b); PLOT_CHECK(std::string(b) == "23:59:59"); + plot::format_time(86399.5, b, sizeof b); PLOT_CHECK(std::string(b) == "1d"); + plot::format_time(86400.0, b, sizeof b); PLOT_CHECK(std::string(b) == "1d"); + + // Sweep the boundary neighbourhoods and check every emitted field. + const double values[] = {0.4, 0.9995, 1.0, 59.4, 59.4999, 59.5, 59.9, 60.0, 90.0, 119.5, 3599.4, + 3599.5, 3599.9, 86399.4, 86399.5, 86399.9, 86400.0, 172800.0, -119.5, + -3599.5, -86399.5}; + for (const double v : values) { + plot::format_time(v, b, sizeof b); + const std::string label(b); + PLOT_CHECK(!label.empty()); + check_no_field_reaches_its_radix(label); + } +}