fix(input): refuse non-finite region bounds, keep narrow regions movable, republish hot handles

src/input/controller.cpp

@@ -33,6 +33,15 @@ constexpr f32 kCursorHitRadiusLogical = 5.0f;
 /// A box-zoom selection narrower than this on either side is a click. Logical for
 /// the same reason: the gesture is the same hand movement on any display.
 constexpr f32 kMinBoxLogical = 4.0f;
+/// The narrowest *interior* a region's body keeps, in **logical** pixels: the edge bands are
+/// cut back until this much of the middle is left, so a region too narrow for the full grab
+/// radius on each side is still something a pointer can take hold of and *move*. Without it
+/// every interior point of a region narrower than twice the radius (10 logical px) lies
+/// within a bound's reach, and the bound branch would always win -- such a region could be
+/// resized but never moved. Half of it either side of the midpoint, which is what makes the
+/// middle of even the narrowest region with grabbable bounds (3 logical px) reachable.
+constexpr f32 kMinRegionBodyLogical = 2.0f;
+
 /// The narrowest region whose bounds are grabbable, in **logical** pixels. Closer
 /// together than this, the two bounds are one target the pointer cannot separate:
 /// an edge would win the hit test every time and the region could never be moved
@@ -159,12 +168,23 @@ void Controller::on_wheel(const WheelEvent& raw) {
       zoom_y_axis(i, plot_->y_transform(i).to_data_y(static_cast<f64>(e.y)), factor);
     }
   }
+  // The ranges moved under a pointer that did not, so the geometry under it changed: refresh
+  // the layout -- the region rects the hit test reads were measured for the range this wheel
+  // has just replaced -- and republish what is hot. A frame drawn right after a scroll then
+  // emphasizes the handle the pointer is over *now*, rather than the one it was over before,
+  // without waiting for the mouse to move again.
+  plot_->layout();
+  publish_hot();
 }
 
 void Controller::on_key(const KeyEvent& e) {
   if (!e.down) return;  // every action below is press-only
 
-  if (e.key == kKeyZoomIn || e.key == kKeyZoomOut) {
+  const bool zooms = e.key == kKeyZoomIn || e.key == kKeyZoomOut;
+  const bool fits = e.key == kKeyFit || e.key == kKeyFitUpper;
+  if (!zooms && !fits) return;  // a key this controller has no gesture for
+
+  if (zooms) {
     const f64 notches = e.key == kKeyZoomIn ? kKeyZoomNotches : -kKeyZoomNotches;
     const f64 factor = zoom_factor(notches);
     for (u32 i = 0; i < plot_->x_axis_count(); ++i) {
@@ -179,13 +199,16 @@ void Controller::on_key(const KeyEvent& e) {
       const f64 hi = to_space(a.range.max, a.log);
       zoom_y_axis(i, from_space(0.5 * (lo + hi), a.log), factor);
     }
-    return;
-  }
-
-  if (e.key == kKeyFit || e.key == kKeyFitUpper) {
+  } else {
     plot_->auto_fit_x();
     for (u32 i = 0; i < plot_->y_axis_count(); ++i) plot_->auto_fit_y(i);
   }
+  // A key stroke that moved a range moved the geometry under a stationary pointer, exactly
+  // as a wheel notch does: refresh the layout and republish what is hot, so the highlight a
+  // frame draws is the handle the pointer is over now (see on_wheel). A key this controller
+  // has no gesture for changes nothing and returns above.
+  plot_->layout();
+  publish_hot();
 }
 
 u64 Controller::hovered_cursor() const {
@@ -217,14 +240,23 @@ Controller::ZoneHit Controller::zone_hit(const RegionLayout& region, f32 px, f32
   const f32 left_x = handle_column(region.left_handle);
   const f32 right_x = handle_column(region.right_handle);
   const f32 min_w = plot_->style().device(kMinRegionEdgeGrabLogical);
-  // Grabbable bounds: the nearer of the two, when it is within the radius every
-  // other handle in this controller is hit-tested with -- one tolerance, so what
-  // `hovered_cursor()` calls a hit and what a region bound calls a hit agree.
+  // Grabbable bounds: the bounds are told apart only while the region is wide enough to
+  // hold a band at each end and an interior between them -- below that the body is the
+  // only handle it has, which is the collapsed-region rule above -- and each band is cut
+  // back so that interior always exists. The radius every other handle in this controller
+  // is hit-tested with is kept wherever a region can afford it: the band is that radius
+  // while twice it plus the minimum interior still fits, and half the remainder after
+  // that, so the middle of a 6 px region is a body rather than inside both bounds. One
+  // tolerance, so what `hovered_cursor()` calls a hit and what a region bound calls a hit
+  // agree wherever there is room for both.
   if (right_x - left_x >= min_w) {
+    const f32 radius = plot_->style().device(kCursorHitRadiusLogical);
+    const f32 band = std::min(radius, 0.5f * (right_x - left_x -
+                                              plot_->style().device(kMinRegionBodyLogical)));
     const f32 dl = std::fabs(px - left_x);
     const f32 dr = std::fabs(px - right_x);
     const f32 d = std::min(dl, dr);
-    if (d <= plot_->style().device(kCursorHitRadiusLogical)) {
+    if (d <= band) {
       hit.zone = dl <= dr ? RegionGrab::Left : RegionGrab::Right;
       hit.distance = d;
       return hit;

src/plot/plot.cpp

@@ -65,6 +65,14 @@ bool covered_samples(const data::Series& series, f64 lo, f64 hi, u64& begin, u64
   return true;
 }
 
+/// True when a pair of bounds can be a region's bounds: both of them finite. The one place
+/// this is decided, so `add_region()` and `set_region()` cannot disagree about what they
+/// will hold -- and so no NaN ever reaches the `std::min`/`std::max` below, which propagate
+/// it into *both* stored bounds and would leave a region no drag could recover from.
+bool finite_bounds(f64 x0, f64 x1) noexcept {
+  return std::isfinite(x0) && std::isfinite(x1);
+}
+
 const Measurement& invalid_measurement() noexcept {
   static const Measurement measurement{};
   return measurement;
@@ -175,6 +183,12 @@ u32 Plot::add_annotation(const Annotation& annotation) {
 }
 
 u32 Plot::add_region(const Region& region) {
+  // A region whose bounds are not numbers is not an interval this plot can hold, and a
+  // `Free` one stored that way could never be dragged back (every motion re-derives both
+  // bounds from the stored ones), so it is refused rather than stored: nothing is added,
+  // and the id handed back names no region -- `region()` reads a default for it and
+  // `region_count()` does not count it.
+  if (!finite_bounds(region.x0, region.x1)) return static_cast<u32>(kInvalid);
   // Ordered on the way in, so no caller -- and no drag -- can leave `x0 > x1`
   // behind: the store is the one place this invariant is enforced.
   Region stored = region;
@@ -505,6 +519,12 @@ void Plot::invalidate_measurements() noexcept {
 
 void Plot::set_region(u32 id, f64 x0, f64 x1) {
   if (id >= regions_.size()) return;
+  // A bound that is not a number is not a bound, and `std::min`/`std::max` would carry it
+  // through the whole pair: accepting one would replace the finite partner as well, so the
+  // region could never be dragged back. Refused outright -- as the other bound setters in
+  // this file refuse one -- which leaves the interval it already held, and with it the
+  // statistics cached for that interval.
+  if (!finite_bounds(x0, x1)) return;
   // min/max, so an inverted pair is stored ordered rather than clamped: a drag that
   // crosses one bound over the other *swaps* the interval, which is what keeps the
   // bound the pointer holds movable instead of pinned at the other one.