fix(input): clamp a dragged ruler on an ordered y range and share the orientation test

include/plot/plot/cursor.hpp

@@ -66,6 +66,17 @@ struct Cursor {
   u32 axis_x = 0;  ///< the x axis `x` is in. Dead for `Horizontal`.
   u32 axis_y = 0;  ///< the y axis `y` is in. Dead for `Vertical`.
   bool visible = true;
+
+  /// True when this is a horizontal ruler (spans the axes' width, reads its `y`). The one
+  /// place the orientation becomes a branch, so every consumer takes the same decision and
+  /// the two orientations cannot drift apart as new ones are added.
+  bool is_horizontal() const noexcept { return orientation == CursorOrientation::Horizontal; }
+
+  /// The live data coordinate: `y` for a horizontal ruler, `x` for a vertical marker.
+  f64 coordinate() const noexcept { return is_horizontal() ? y : x; }
+
+  /// The axis `coordinate()` is in: `axis_y` for a horizontal ruler, `axis_x` for a vertical.
+  u32 coordinate_axis() const noexcept { return is_horizontal() ? axis_y : axis_x; }
 };
 
 }  // namespace plot

src/input/controller.cpp

@@ -505,7 +505,7 @@ void Controller::move_cursor(const PointerEvent& e) {
   // A grabbed cursor mutates the coordinate its own orientation owns and nothing else, and
   // each half names the one setter for it -- so a ruler dragged along y can never disturb a
   // marker's x, and the dead coordinate of either is untouched by construction.
-  if (cursor.orientation == CursorOrientation::Horizontal) {
+  if (cursor.is_horizontal()) {
     // Clamped to the ruler's own y axis: a pointer that has left the axes has no data under
     // it, and the coordinate it extrapolates to is past the end of the axis the ruler reads
     // values off -- a position where the ruler would either vanish (the renderer skips a row
@@ -514,7 +514,12 @@ void Controller::move_cursor(const PointerEvent& e) {
     // verbatim, so a *caller* that means to park a ruler off range still can.
     const Axis& axis = plot_->y_axis(cursor.axis_y);
     const f64 y = plot_->y_transform(cursor.axis_y).to_data_y(static_cast<f64>(e.y));
-    plot_->set_cursor_y(id, std::clamp(y, axis.range.min, axis.range.max));
+    // `std::clamp` needs an ordered pair and a y range may be inverted -- descending depth,
+    // row index or time is ordinary -- because `set_y_range()` stores it verbatim and only
+    // the transform widens it. Order the bounds rather than assuming `min <= max`, or an
+    // inverted axis would collapse the dragged ruler onto one end (and the call is UB).
+    plot_->set_cursor_y(id, std::clamp(y, std::min(axis.range.min, axis.range.max),
+                                       std::max(axis.range.min, axis.range.max)));
     return;
   }
   plot_->set_cursor_x(id, plot_->x_transform(cursor.axis_x).to_data_x(static_cast<f64>(e.x)));
@@ -611,7 +616,7 @@ u64 Controller::cursor_at(f32 px, f32 py) const {
     // either way, which is what keeps hover and grab agreeing about a ruler exactly as
     // they do about a marker. A point within reach of one of each resolves to the nearer,
     // so neither orientation widens the other's target.
-    const bool horizontal = cursor.orientation == CursorOrientation::Horizontal;
+    const bool horizontal = cursor.is_horizontal();
     const f32 d = std::fabs(plot_->cursor_screen_coordinate(i) - (horizontal ? py : px));
     if (d > radius) continue;
     if (best == kInvalid || d < best_d) {

src/plot/plot.cpp

@@ -239,13 +239,14 @@ const Cursor& Plot::cursor(u32 id) const {
 f32 Plot::cursor_screen_coordinate(u32 id) const {
   if (id >= cursors_.size()) return 0.0f;
   const Cursor& cursor = cursors_[id];
-  // Which coordinate is the marker's, decided once, here: the axis reached for is the one
-  // the cursor's own orientation names, and the other orientation's coordinate is not
+  // Which coordinate is the marker's, decided once, in `Cursor`: the axis reached for is the
+  // one the cursor's own orientation names, and the other orientation's coordinate is not
   // consulted at all -- a ruler is never placed by a dead x.
-  if (cursor.orientation == CursorOrientation::Horizontal) {
-    return static_cast<f32>(y_transform(cursor.axis_y).to_screen_y(cursor.y));
+  if (cursor.is_horizontal()) {
+    return static_cast<f32>(
+        y_transform(cursor.coordinate_axis()).to_screen_y(cursor.coordinate()));
   }
-  return static_cast<f32>(x_transform(cursor.axis_x).to_screen_x(cursor.x));
+  return static_cast<f32>(x_transform(cursor.coordinate_axis()).to_screen_x(cursor.coordinate()));
 }
 
 const Annotation& Plot::annotation(u32 id) const {
@@ -504,7 +505,7 @@ void Plot::set_cursor_x(u32 id, f64 x) {
   // `x` is the vertical cursor's coordinate and nobody else's: a ruler's `x` is dead, and
   // writing it would leave the struct holding two coordinates that disagree about where the
   // cursor is (see CursorOrientation).
-  if (cursor.orientation != CursorOrientation::Vertical) return;
+  if (cursor.is_horizontal()) return;
   cursor.x = x;
   cursor.x = snapped_x(cursor);
   mark_measures_dirty();
@@ -516,7 +517,7 @@ void Plot::set_cursor_snap(u32 id, SnapMode mode) {
   // The vertical snap, on a vertical cursor: a ruler's snap is `HRulerSnap`, and the two
   // must not drift into meaning each other's job -- `NearestSample` is a sample of an x
   // grid, which a ruler has no notion of.
-  if (cursor.orientation != CursorOrientation::Vertical) return;
+  if (cursor.is_horizontal()) return;
   cursor.snap = mode;
   cursor.x = snapped_x(cursor);
   mark_measures_dirty();
@@ -525,7 +526,7 @@ void Plot::set_cursor_snap(u32 id, SnapMode mode) {
 void Plot::set_cursor_y(u32 id, f64 y) {
   if (id >= cursors_.size()) return;
   Cursor& ruler = cursors_[id];
-  if (ruler.orientation != CursorOrientation::Horizontal) return;
+  if (!ruler.is_horizontal()) return;
   ruler.y = y;
   ruler.y = snapped_y(ruler);
   // No mark_measures_dirty(), for `add_h_ruler()`'s reason: a measurement is read from two
@@ -535,7 +536,7 @@ void Plot::set_cursor_y(u32 id, f64 y) {
 void Plot::set_cursor_h_snap(u32 id, HRulerSnap mode) {
   if (id >= cursors_.size()) return;
   Cursor& ruler = cursors_[id];
-  if (ruler.orientation != CursorOrientation::Horizontal) return;
+  if (!ruler.is_horizontal()) return;
   ruler.h_snap = mode;
   ruler.y = snapped_y(ruler);
 }

src/render/renderer.cpp

@@ -939,7 +939,7 @@ void Renderer::draw_cursors(const Plot& plot, const PlotLayout& layout) {
     // stroke (and which axis maps the span it is drawn along), and where the label is
     // written. Everything else -- the skip at the far edge of the axes, the highlight, the
     // text path -- is this one loop's, which is the point of one type rather than two.
-    const bool horizontal = cursor.orientation == CursorOrientation::Horizontal;
+    const bool horizontal = cursor.is_horizontal();
     const f32 at = plot.cursor_screen_coordinate(i);
     // A cursor has no position on the plot when it is strictly outside its range's mapped
     // span -- a marker scrolled past the right edge, a ruler past the bottom -- and the
@@ -985,7 +985,7 @@ void Renderer::draw_cursors(const Plot& plot, const PlotLayout& layout) {
 void Renderer::draw_cursor_label(const Plot& plot, const PlotLayout& layout, const Cursor& cursor,
                                  f32 at) {
   const f32 gap = style_.device(3.0f);
-  if (cursor.orientation == CursorOrientation::Horizontal) {
+  if (cursor.is_horizontal()) {
     // A ruler's text is the value it is *for*, written the way the y axis writes its own
     // tick labels, so a value read off a ruler reads exactly like the tick beside it. The
     // caller's caption, when there is one, is staged first on the same line, so neither the

tests/test_controller.cpp

@@ -874,6 +874,37 @@ PLOT_TEST(horizontal_ruler_is_grabbed_and_dragged_along_y) {
   PLOT_CHECK(f.plot.cursor(ruler).y > 40.0);
 }
 
+/// An inverted (descending) y range is ordinary -- depth, row index, descending time -- because
+/// `set_y_range()` stores the range verbatim and only the transform widens it. The drag's clamp
+/// must therefore order the bounds: `std::clamp` requires `min <= max`, so a raw
+/// `clamp(y, range.min, range.max)` collapses the ruler onto one end on such an axis (and is
+/// undefined behaviour besides). The ruler has to follow the pointer here, and stay inside the
+/// range.
+PLOT_TEST(horizontal_ruler_follows_the_pointer_on_an_inverted_y_range) {
+  Fixture f;
+  // Descending: the layout maps `range.min` (100) to the axes' bottom and `range.max` (0) to its
+  // top, so a pointer further down is a *larger* y.
+  f.ay = f.plot.add_y_axis({"v", {100.0, 0.0}, false, plot::AxisPlace::Left, false});
+  const u32 ruler = f.plot.add_h_ruler(50.0, plot::Color::hex(0x35D0A8));
+  f.plot.layout();
+  const f32 column = screen_x_of(f.plot, f.ax, 50.0);
+  const f32 row = row_of(f.plot, f.ay, 50.0);
+
+  hover(f.controller, column, row);
+  PLOT_CHECK(f.controller.hovered_cursor() == ruler);
+  press(f.controller, column, row);
+  PLOT_CHECK(f.controller.grabbed_cursor() == ruler);
+
+  const f32 moved = row + 60.0f;
+  move_to(f.controller, column, moved);
+  // Exactly the data under the pointer, not a collapsed bound, and inside the range.
+  PLOT_CHECK_NEAR(f.plot.cursor(ruler).y, data_at_row(f.plot, f.ay, moved), 1e-9);
+  PLOT_CHECK(f.plot.cursor(ruler).y > 50.0);
+  PLOT_CHECK(f.plot.cursor(ruler).y < 100.0);
+  release(f.controller, column, moved);
+  PLOT_CHECK(f.plot.cursor(ruler).y > 50.0);
+}
+
 /// Nearest-first resolution *across* orientations: a pointer within reach of both a marker and a
 /// ruler takes the nearer of the two, so neither widens the other's target -- which is what lets
 /// one hit test serve two instruments. Each press below is 1 px from its winner and 4 px from the