diff --git a/include/plot/plot/ticks.hpp b/include/plot/plot/ticks.hpp index f7596d1..ce2bece 100644 --- a/include/plot/plot/ticks.hpp +++ b/include/plot/plot/ticks.hpp @@ -73,6 +73,15 @@ inline constexpr usize kMaxTicks = 256; /// its labelled majors -- every major at or below `hi` is emitted, so the far /// end of the axis keeps its numbers. /// +/// A range that straddles the origin gets a tick of **exactly** `0.0` there, labelled +/// `0`. The walk cannot land on the origin by itself -- it starts at +/// `ceil(min / stride) * stride`, which is not representable whenever the stride is +/// not a power of two -- so the tick that means zero arrives a few ULP off and would +/// otherwise be labelled from the far end of the SI table (`-139a` between `-500m` and +/// `500m`). Every tick is an integer multiple of the stride, so anything within a +/// millionth of one *is* the origin and nothing else; a range whose ticks are +/// legitimately that small keeps them. +/// /// Hostile input clears `out` and returns: a non-finite endpoint, `max <= min` /// (a zero or negative span, which includes the collapsed range of a /// single-sample series), a non-positive `px` or `min_px`, and a span whose step diff --git a/src/plot/ticks.cpp b/src/plot/ticks.cpp index 437b295..c38ceb6 100644 --- a/src/plot/ticks.cpp +++ b/src/plot/ticks.cpp @@ -189,6 +189,23 @@ void generate_ticks(Range range, f32 px, f32 min_px, std::vector& out, boo const f64 first = include_minors ? std::ceil(lo / minor_step) * minor_step : first_major; if (!std::isfinite(first)) return; + // The walk starts at `ceil(lo / stride) * stride`, which is not representable + // whenever the stride is not a power of two -- 0.1 and 0.2 are the common ones -- + // so a range that straddles the origin never lands *on* it: [-1, 1] stepped by 0.1 + // reaches -1.39e-16 where it means zero, and that renders as `-139a`, a label from + // the far end of the SI table sitting between `-500m` and `500m`. Deriving `v` by + // multiplication instead of accumulation does not help, because the error is + // already in `first`. + // + // Every tick is an integer multiple of the stride, so the smallest magnitude a + // *non-zero* tick can have is the stride itself, while the residue is a few ULP of + // the range -- a relative gap of some ten orders of magnitude. Anything inside a + // millionth of the stride is therefore the zero tick and nothing else, and it is + // stored as exactly `0.0` rather than merely labelled as such: `Axis::refresh()` + // re-formats a time axis from `tick.value`, where the same residue reads + // `3.19e-13ms`. + const f64 zero_tol = (include_minors ? minor_step : step) * 1e-6; + char label[kLabelBuf]; usize minors = 0; for (f64 v = first; v <= hi + stride * 1e-9;) { @@ -213,12 +230,14 @@ void generate_ticks(Range range, f32 px, f32 min_px, std::vector& out, boo } if (!major) ++minors; Tick tick; - tick.value = v; + // The walk variable itself is left alone: `minor_index_is_major` counts indices + // from `first_major` and both operands have to stay on the walk's own grid. + tick.value = std::fabs(v) < zero_tol ? 0.0 : v; tick.major = major; if (major) { // format_si owns both label guards: it never writes past the buffer, and it // falls back to %.3g for a non-finite value or a label over 12 characters. - format_si(v, label, sizeof label); + format_si(tick.value, label, sizeof label); tick.label = label; } out.push_back(std::move(tick)); diff --git a/tests/test_render_smoke.cpp b/tests/test_render_smoke.cpp index 74b6f7e..1d86217 100644 --- a/tests/test_render_smoke.cpp +++ b/tests/test_render_smoke.cpp @@ -1572,9 +1572,17 @@ PLOT_TEST(narrow_viewport_draws_no_y_title_it_cannot_fit) { plot::Plot plot; plot.set_viewport({0, 0, static_cast(kNarrowW), static_cast(kNarrowH)}); plot.add_x_axis({"", {0.0, 1.0}, false, plot::AxisPlace::Bottom, false}); - plot.add_y_axis({title, {-1.0, 1.0}, false, plot::AxisPlace::Left, false}); - plot.add_y_axis({title, {-1.0, 1.0}, false, plot::AxisPlace::Right, false}); - plot.add_y_axis({title, {-1.0, 1.0}, false, plot::AxisPlace::Left, false}); + // A milli-scale range, for the *width of its labels*: a symmetric span ticks at + // +/-step and 0, so the labels here are `-200m 0 200m` -- five characters, wide + // enough that the label strip alone spends the whole chrome allowance and the + // title strip is left zero wide, which is the regime asserted below. A `[-1, 1]` + // axis labels `-1 0 1` instead and does not fill the allowance: the titled scene + // then keeps a 9.6 px title column -- still too narrow to stage a glyph, so the + // property holds -- but the column also pushes the label strip 9.6 px right, and + // the frame comparison would be reading that shift rather than title ink. + plot.add_y_axis({title, {-0.2, 0.2}, false, plot::AxisPlace::Left, false}); + plot.add_y_axis({title, {-0.2, 0.2}, false, plot::AxisPlace::Right, false}); + plot.add_y_axis({title, {-0.2, 0.2}, false, plot::AxisPlace::Left, false}); return plot; }; const auto render = [&](plot::Plot& plot) { @@ -1595,6 +1603,11 @@ PLOT_TEST(narrow_viewport_draws_no_y_title_it_cannot_fit) { // than the one column its three titled axes ask for, so no column can be staged. PLOT_CHECK(titled.titled_y_axis_count() == 3u); PLOT_CHECK(layout.y_title_area.w < r.axis_font_px() + r.style().grid_label_pad); + // Zero wide, not merely under one column -- the labels spend the allowance whole. + // That is this comparison's own premise as much as the clamp's: a strip with any + // width left in it would move the label strip right in the titled scene alone, and + // the two frames would then differ over the labels rather than over title ink. + PLOT_CHECK(layout.y_title_area.w == 0.0f); // ... and the plot kept its floor in a viewport this narrow, so nothing here is // degenerate for some other reason. PLOT_CHECK(layout.axes.w >= 0.5f * static_cast(kNarrowW)); @@ -1633,9 +1646,12 @@ PLOT_TEST(narrow_viewport_draws_no_y_title_it_cannot_fit) { // that survive are the leading ones -- which is what "keep the leading columns" // means, since the renderer lays them out left to right from the strip's padding. PLOT_TEST(narrow_viewport_keeps_only_the_y_title_columns_that_fit) { - // 170 px is where the clamp leaves room for exactly one column (the label strip - // takes 51.2 px of the 85 px allowance, leaving 33.8 px against the 19.7 px a - // column needs); the assertions below re-derive that rather than assume it. + // 170 px is where the clamp leaves room for some of the three columns and not all + // of them: the label strip takes 44 px of the 85 px allowance and a column needs + // 19.7, so the 41 px left over hold two. The assertions below re-derive the count + // from the measured strip rather than assume it, which is what lets the regime be + // "at least one and fewer than three" instead of a pixel figure that drifts with + // the font, the device scale or the tick labels. constexpr plot::u32 kNarrowW = 170; constexpr plot::u32 kNarrowH = 200; plottest::HeadlessContext ctx(kNarrowW, kNarrowH); @@ -1648,9 +1664,12 @@ PLOT_TEST(narrow_viewport_keeps_only_the_y_title_columns_that_fit) { plot::Plot p; p.set_viewport({0, 0, static_cast(kNarrowW), static_cast(kNarrowH)}); p.add_x_axis({"", {0.0, 1.0}, false, plot::AxisPlace::Bottom, false}); - p.add_y_axis({"volts", {-1.0, 1.0}, false, plot::AxisPlace::Left, false}); - p.add_y_axis({"amps", {-1.0, 1.0}, false, plot::AxisPlace::Right, false}); - p.add_y_axis({"watts", {-1.0, 1.0}, false, plot::AxisPlace::Left, false}); + // Milli-scale for the label width, as in the zero-wide-strip case above: the strip + // this test wants is the one that holds *some* columns and not all three, and what + // it has left to hold them in is the allowance minus the label strip. + p.add_y_axis({"volts", {-0.2, 0.2}, false, plot::AxisPlace::Left, false}); + p.add_y_axis({"amps", {-0.2, 0.2}, false, plot::AxisPlace::Right, false}); + p.add_y_axis({"watts", {-0.2, 0.2}, false, plot::AxisPlace::Left, false}); r.begin_frame(kNarrowW, kNarrowH, {0, 0, static_cast(kNarrowW), static_cast(kNarrowH)}); r.draw(p); diff --git a/tests/test_ticks.cpp b/tests/test_ticks.cpp index 2bb0c5a..f003621 100644 --- a/tests/test_ticks.cpp +++ b/tests/test_ticks.cpp @@ -158,6 +158,52 @@ PLOT_TEST(epoch_window_yields_labelled_majors) { PLOT_CHECK(deep_majors >= 9); } +/// The tick that *means* zero reads `0`. The walk starts at `ceil(lo/stride)*stride`, +/// inexact for every stride that is not a power of two, so a range straddling the +/// origin used to reach a few e-16 there and label it from the far end of the SI +/// table: `-1 -500m -139a 500m 1`. The tolerance is relative to the stride, which is +/// also the smallest magnitude a non-zero tick can have, so a range whose ticks are +/// *legitimately* that small keeps them. +PLOT_TEST(the_tick_at_the_origin_reads_zero) { + std::vector t; + const float widths[] = {80.0f, 40.0f, 10.0f}; + for (const float min_px : widths) { + for (const bool minors : {false, true}) { + plot::generate_ticks({-1.0, 1.0}, 400.0f, min_px, t, minors); + plot::usize zeros = 0; + for (const plot::Tick& tick : t) { + if (!tick.major) continue; + PLOT_CHECK(!tick.label.empty()); + // Nothing between -500m and 500m but the origin itself, so no major may + // carry a prefix from below `m`. + PLOT_CHECK(tick.label.find('a') == std::string::npos); + PLOT_CHECK(tick.label.find('f') == std::string::npos); + if (tick.label == "0") { + ++zeros; + PLOT_CHECK(tick.value == 0.0); // exact, so a Time axis relabels it as 0 too + } + } + PLOT_CHECK(zeros == 1u); + } + } + + // A range whose ticks really are atto-scale is untouched: only the origin folds. + plot::generate_ticks({-2e-16, 2e-16}, 400.0f, 80.0f, t, /*include_minors=*/true); + plot::usize tiny = 0; + plot::usize tiny_zeros = 0; + for (const plot::Tick& tick : t) { + if (!tick.major) continue; + if (tick.value == 0.0) { + ++tiny_zeros; + } else { + ++tiny; + PLOT_CHECK(tick.label.find('a') != std::string::npos); + } + } + PLOT_CHECK(tiny_zeros == 1u); + PLOT_CHECK(tiny >= 2u); +} + PLOT_TEST(si_labels_are_utf8_and_never_overrun) { char b[32]; plot::format_si(0.0000015, b, sizeof b);