fix(ticks): label the tick at the origin 0, not from the atto end of the table
CI / debug (tests + all adapters) (push) Canceled after 0s
CI / coverage (floor + measured table) (push) Canceled after 0s
CI / asan (AddressSanitizer) (push) Canceled after 0s

A range straddling zero never lands *on* zero: the walk starts at
`ceil(min / stride) * stride`, which is not representable whenever the stride is
not a power of two -- 0.1 and 0.2 are the everyday ones -- so the tick that means
zero arrives a few ULP off and `format_si` labels it from six prefixes down.
`[-1, 1]` read `-1 -500m -139a 500m 1`. Deriving the tick by multiplication
rather than accumulation does not help, because the error is already in the
first term.

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 -- ten orders of magnitude apart. Anything inside a millionth of the
stride is therefore the origin and nothing else. It is stored as exactly `0.0`
rather than merely labelled so: `Axis::refresh()` re-formats a time axis from
`tick.value`, where the same residue reads `3.19e-13ms`. The walk variable is
left alone, since `minor_index_is_major` counts indices from `first_major` and
both operands have to stay on the walk's own grid.

The two narrow-y-title cases were calibrated on the defect -- the bogus `-55.5a`
was a six-character label, and its width is what spent the chrome allowance and
made the titled and untitled frames comparable. They are re-seated on a
milli-scale range, whose `-200m` is wide for a real reason, and the zero-width
strip the frame comparison depends on is now asserted rather than assumed.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Martino FerrariandClaude Opus 4.6 committed 2026-10-02 00:52:20 +02:00
1 parent 525759f9a9
commit e337c6a60c
4 files changed
+104 -11

No files matched your search

+9
View File
@@ -73,6 +73,15 @@ inline constexpr usize kMaxTicks = 256;
/// its labelled majors -- every major at or below `hi` is emitted, so the far /// its labelled majors -- every major at or below `hi` is emitted, so the far
/// end of the axis keeps its numbers. /// 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` /// Hostile input clears `out` and returns: a non-finite endpoint, `max <= min`
/// (a zero or negative span, which includes the collapsed range of a /// (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 /// single-sample series), a non-positive `px` or `min_px`, and a span whose step
+21 -2
View File
@@ -189,6 +189,23 @@ void generate_ticks(Range range, f32 px, f32 min_px, std::vector<Tick>& out, boo
const f64 first = include_minors ? std::ceil(lo / minor_step) * minor_step : first_major; const f64 first = include_minors ? std::ceil(lo / minor_step) * minor_step : first_major;
if (!std::isfinite(first)) return; 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]; char label[kLabelBuf];
usize minors = 0; usize minors = 0;
for (f64 v = first; v <= hi + stride * 1e-9;) { 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<Tick>& out, boo
} }
if (!major) ++minors; if (!major) ++minors;
Tick tick; 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; tick.major = major;
if (major) { if (major) {
// format_si owns both label guards: it never writes past the buffer, and it // 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. // 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; tick.label = label;
} }
out.push_back(std::move(tick)); out.push_back(std::move(tick));
+28 -9
View File
@@ -1572,9 +1572,17 @@ PLOT_TEST(narrow_viewport_draws_no_y_title_it_cannot_fit) {
plot::Plot plot; plot::Plot plot;
plot.set_viewport({0, 0, static_cast<plot::f32>(kNarrowW), static_cast<plot::f32>(kNarrowH)}); plot.set_viewport({0, 0, static_cast<plot::f32>(kNarrowW), static_cast<plot::f32>(kNarrowH)});
plot.add_x_axis({"", {0.0, 1.0}, false, plot::AxisPlace::Bottom, false}); 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}); // A milli-scale range, for the *width of its labels*: a symmetric span ticks at
plot.add_y_axis({title, {-1.0, 1.0}, false, plot::AxisPlace::Right, false}); // +/-step and 0, so the labels here are `-200m 0 200m` -- five characters, wide
plot.add_y_axis({title, {-1.0, 1.0}, false, plot::AxisPlace::Left, false}); // 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; return plot;
}; };
const auto render = [&](plot::Plot& 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. // 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(titled.titled_y_axis_count() == 3u);
PLOT_CHECK(layout.y_title_area.w < r.axis_font_px() + r.style().grid_label_pad); 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 // ... and the plot kept its floor in a viewport this narrow, so nothing here is
// degenerate for some other reason. // degenerate for some other reason.
PLOT_CHECK(layout.axes.w >= 0.5f * static_cast<plot::f32>(kNarrowW)); PLOT_CHECK(layout.axes.w >= 0.5f * static_cast<plot::f32>(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" // 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. // 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) { 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 // 170 px is where the clamp leaves room for some of the three columns and not all
// takes 51.2 px of the 85 px allowance, leaving 33.8 px against the 19.7 px a // of them: the label strip takes 44 px of the 85 px allowance and a column needs
// column needs); the assertions below re-derive that rather than assume it. // 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 kNarrowW = 170;
constexpr plot::u32 kNarrowH = 200; constexpr plot::u32 kNarrowH = 200;
plottest::HeadlessContext ctx(kNarrowW, kNarrowH); plottest::HeadlessContext ctx(kNarrowW, kNarrowH);
@@ -1648,9 +1664,12 @@ PLOT_TEST(narrow_viewport_keeps_only_the_y_title_columns_that_fit) {
plot::Plot p; plot::Plot p;
p.set_viewport({0, 0, static_cast<plot::f32>(kNarrowW), static_cast<plot::f32>(kNarrowH)}); p.set_viewport({0, 0, static_cast<plot::f32>(kNarrowW), static_cast<plot::f32>(kNarrowH)});
p.add_x_axis({"", {0.0, 1.0}, false, plot::AxisPlace::Bottom, false}); 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}); // Milli-scale for the label width, as in the zero-wide-strip case above: the strip
p.add_y_axis({"amps", {-1.0, 1.0}, false, plot::AxisPlace::Right, false}); // this test wants is the one that holds *some* columns and not all three, and what
p.add_y_axis({"watts", {-1.0, 1.0}, false, plot::AxisPlace::Left, false}); // 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, r.begin_frame(kNarrowW, kNarrowH,
{0, 0, static_cast<plot::f32>(kNarrowW), static_cast<plot::f32>(kNarrowH)}); {0, 0, static_cast<plot::f32>(kNarrowW), static_cast<plot::f32>(kNarrowH)});
r.draw(p); r.draw(p);
+46
View File
@@ -158,6 +158,52 @@ PLOT_TEST(epoch_window_yields_labelled_majors) {
PLOT_CHECK(deep_majors >= 9); 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<plot::Tick> 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) { PLOT_TEST(si_labels_are_utf8_and_never_overrun) {
char b[32]; char b[32];
plot::format_si(0.0000015, b, sizeof b); plot::format_si(0.0000015, b, sizeof b);