From e0cdd95a3b08344538e2928e1a8ad4b9dea036b0 Mon Sep 17 00:00:00 2001 From: Martino Ferrari Date: Sun, 27 Sep 2026 22:40:40 +0200 Subject: [PATCH] fix(input): pan and box zoom through layout().axes, not the viewport Every pixel-to-data conversion in the controller now maps through the same rectangle Plot::x_transform/y_transform are built from: - pan() derives the scale from layout().axes.w/.h, so a drag moves the grabbed data point by exactly the drag distance (the viewport model stranded it behind the pointer by the label-strip fraction). - apply_box_zoom() clamps the selection to layout().axes and sets each range from the transforms (x_transform().to_data_x(l/r), y_transform() .to_data_y(bottom/top)); the selection's bottom edge is the y minimum. - end_drag() takes the release event and refreshes the selection from its coordinates before applying, so a host that reports only press and release still frames the box instead of degrading it to a click. - axis_zoom() uses a per-pixel sensitivity (200 px == one decade) rather than reusing the wheel's per-notch constant, so a pixel drag is no longer treated as a wheel notch. Tests now assert the grab-and-drag invariant through the transform (the old pixel-fraction assertions certified the viewport defect), add a press+release-only box zoom, and pin the axis-zoom magnitude. --- .../plans/2026-09-27-plot-library-scaffold.md | 24 ++++-- include/plot/input/controller.hpp | 10 ++- src/input/controller.cpp | 81 ++++++++++--------- tests/test_controller.cpp | 46 +++++++++-- 4 files changed, 106 insertions(+), 55 deletions(-) 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 0485624..0982616 100644 --- a/docs/superpowers/plans/2026-09-27-plot-library-scaffold.md +++ b/docs/superpowers/plans/2026-09-27-plot-library-scaffold.md @@ -1119,14 +1119,22 @@ PLOT_TEST(left_drag_pans_without_changing_span) { const u32 ay = p.add_y_axis({"v", {-1.0, 1.0}, true, plot::AxisPlace::Left, false}); plot::input::Controller c(p); c.set_viewport({0, 0, 800, 400}); const double span_before = p.x_range(ax).span(); + const f64 grabbed = p.x_transform(ax).to_data_x(100.0f); // data under the press point c.on_pointer({100, 100, 0, 0, plot::input::Button::Left, 0, true, true, false}); PLOT_CHECK(c.drag_mode() == plot::input::DragMode::Pan); c.on_pointer({180, 130, 80, 30, plot::input::Button::Left, 0, true, false, false}); c.on_pointer({180, 130, 0, 0, plot::input::Button::Left, 0, false, false, true}); PLOT_CHECK(c.drag_mode() == plot::input::DragMode::None); PLOT_CHECK_NEAR(p.x_range(ax).span(), span_before, 1e-9); // span unchanged - PLOT_CHECK_NEAR(p.x_range(ax).min, 10.0, 1e-6); // 80/800 * 100 data units - PLOT_CHECK(p.y_range(ay).max > 1.0); // moving down raises y range + // Assert the INVARIANT, not a pixel fraction. Grab-and-drag means the data + // grabbed at the press point is under the pointer afterwards. Pixels map to data + // through layout().axes (744 px wide for an 800 px viewport given the default + // y_axis_width of 56), NOT through the whole viewport: the viewport model makes + // dragged data lag the cursor by the label-strip fraction (56/800 = 7%), which + // is a defect no pixel-fraction assertion can see because it certifies its own + // arithmetic. + PLOT_CHECK_NEAR(p.x_transform(ax).to_data_x(180.0f), grabbed, 1e-9); + PLOT_CHECK(p.y_range(ay).max > 1.0); // moving down reveals higher y } PLOT_TEST(shift_drag_box_zooms_to_selection) { @@ -1134,14 +1142,18 @@ PLOT_TEST(shift_drag_box_zooms_to_selection) { const u32 ax = p.add_x_axis({"t", {0.0, 800.0}, true, plot::AxisPlace::Bottom, false}); plot::input::Controller c(p); c.set_viewport({0, 0, 800, 400}); const u32 e_shift = 1u << 0; // Mod::Shift + const f64 want_lo = p.x_transform(ax).to_data_x(100.0f); + const f64 want_hi = p.x_transform(ax).to_data_x(300.0f); c.on_pointer({100, 50, 0, 0, plot::input::Button::Left, e_shift, true, true, false}); PLOT_CHECK(c.drag_mode() == plot::input::DragMode::BoxZoom); c.on_pointer({300, 150, 200, 100, plot::input::Button::Left, e_shift, true, false, false}); PLOT_CHECK_NEAR(c.selection_rect().x, 100.0, 1e-6); PLOT_CHECK_NEAR(c.selection_rect().w, 200.0, 1e-6); c.on_pointer({300, 150, 0, 0, plot::input::Button::Left, e_shift, false, false, true}); - PLOT_CHECK_NEAR(p.x_range(ax).min, 100.0, 1e-6); - PLOT_CHECK_NEAR(p.x_range(ax).max, 300.0, 1e-6); + // The box frames exactly the data it encloses: the ranges come from the axes + // transform, and the selection is clamped to layout().axes, not the viewport. + PLOT_CHECK_NEAR(p.x_range(ax).min, want_lo, 1e-6); + PLOT_CHECK_NEAR(p.x_range(ax).max, want_hi, 1e-6); } PLOT_TEST(auto_scale_disabled_by_user_interaction) { @@ -1166,8 +1178,8 @@ Expected: FAIL to compile — `plot/input/controller.hpp` not found. | gesture | mode | effect | |---|---|---| -| left drag | `Pan` | translate both ranges by `-dx_px / plot_width * x_span` (and y likewise, inverted) | -| shift + left drag | `BoxZoom` | record `selection_rect()`; on release set both ranges from the rect (ignore sub-4px rects — treat as a click) | +| left drag | `Pan` | translate both ranges so the grabbed data point stays under the pointer: the pixel-to-data scale is `span / layout().axes.w` for x and `span / layout().axes.h` for y (inverted), i.e. translate by `-dx_px / axes.w * x_span`. Using the whole-viewport width here makes dragged data lag the cursor by the label-strip fraction. | +| shift + left drag | `BoxZoom` | record `selection_rect()`; on release set both ranges from the rect. Clamp the rect to `layout().axes`, map it with `x_transform(i).to_data_x(l/r)` and `y_transform(i).to_data_y(bottom/top)`, and ignore sub-4px rects (treat as a click). The **release event's coordinates are authoritative** for the gesture end: refresh the selection from them before applying, or a host that reports only press and release leaves the box empty and the zoom silently degrades to a click. | | ctrl + left drag near a cursor | `MoveCursor` | move that cursor, re-snapping and re-evaluating its measures | | wheel at (x,y) | — | zoom both axes about the pointer's data coordinate by `exp(-dy * k)`; hold shift to zoom x only, ctrl to zoom y only | | middle drag on the x-axis strip | `AxisZoomX` | x-only zoom | diff --git a/include/plot/input/controller.hpp b/include/plot/input/controller.hpp index adb887a..904ddf8 100644 --- a/include/plot/input/controller.hpp +++ b/include/plot/input/controller.hpp @@ -42,9 +42,11 @@ enum class DragMode { /// | `+` / `-` | -- | zoom about the plot centre | /// | `f` | -- | auto-fit every axis | /// -/// Both axis-zoom drags read the vertical `dy` with the wheel's sign convention -/// (dragging down zooms in), anchored at the pointer's data coordinate on the -/// axis they zoom. +/// Both axis-zoom drags read the vertical `dy` in pointer pixels with the +/// wheel's sign convention (dragging down zooms in), anchored at the pointer's +/// data coordinate on the axis they zoom. The drag has its own per-pixel +/// sensitivity -- a pixel is not a wheel notch -- of one decade per 200 px, so +/// a 200 px drag scales the span by 10. /// /// A drag that changes no range is harmless: a box-zoom selection under 4 px on /// either side is a click, and leaves the ranges alone. @@ -70,7 +72,7 @@ class Controller { private: void begin_drag(const PointerEvent& e); void update_drag(const PointerEvent& e); - void end_drag(); + void end_drag(const PointerEvent& e); void pan(const PointerEvent& e); void box_zoom_update(const PointerEvent& e); diff --git a/src/input/controller.cpp b/src/input/controller.cpp index 7166088..d89ab75 100644 --- a/src/input/controller.cpp +++ b/src/input/controller.cpp @@ -5,6 +5,7 @@ #include #include +#include #include namespace plot::input { @@ -15,6 +16,10 @@ namespace { constexpr f64 kZoomPerNotch = 0.15; /// A keyboard zoom press is one wheel notch. constexpr f64 kKeyZoomNotches = 1.0; +/// Zoom per pixel of a middle-drag along an axis strip. A pointer drag reports +/// pixels, a different unit from a wheel notch, so it must not reuse +/// kZoomPerNotch: 200 px of drag is one decade, exp(-200 * ln10/200) == 0.1. +constexpr f64 kZoomPerPixel = std::numbers::ln10 / 200.0; /// How close (in pixels) the pointer must be to a cursor to grab it. constexpr f32 kCursorHitRadiusPx = 5.0f; /// A box-zoom selection narrower than this on either side is a click. @@ -55,7 +60,7 @@ void Controller::on_pointer(const PointerEvent& e) { return; } if (e.released) { - end_drag(); + end_drag(e); return; } if (e.down && mode_ != DragMode::None) update_drag(e); @@ -163,8 +168,15 @@ void Controller::update_drag(const PointerEvent& e) { } } -void Controller::end_drag() { - if (mode_ == DragMode::BoxZoom) apply_box_zoom(); +void Controller::end_drag(const PointerEvent& e) { + // The release carries the authoritative end of the gesture: refresh the + // selection from it first, so a host that reports only a press and a release + // (no motion event in between) still frames the box the release describes + // instead of leaving selection_ at 0x0 and degrading the zoom to a click. + if (mode_ == DragMode::BoxZoom) { + box_zoom_update(e); + apply_box_zoom(); + } mode_ = DragMode::None; selection_ = Rect{}; drag_cursor_ = kInvalid; @@ -173,10 +185,14 @@ void Controller::end_drag() { // --- gestures ------------------------------------------------------------- void Controller::pan(const PointerEvent& e) { - const Rect vp = plot_->viewport(); - if (!(vp.w > 0.0f) || !(vp.h > 0.0f)) return; - const f64 fx = static_cast(e.dx) / static_cast(vp.w); - const f64 fy = static_cast(e.dy) / static_cast(vp.h); + // The transforms map a range onto layout().axes, so the pixel-to-data scale is + // the range span over the axes rect -- not over the whole viewport. Using the + // viewport (56 px of label strip on an 800 px plot) makes dragged data lag the + // pointer by the strip's fraction, breaking grab-and-drag. + const Rect axes = std::as_const(*plot_).layout().axes; + if (!(axes.w > 0.0f) || !(axes.h > 0.0f)) return; + const f64 fx = static_cast(e.dx) / static_cast(axes.w); + const f64 fy = static_cast(e.dy) / static_cast(axes.h); for (u32 i = 0; i < plot_->x_axis_count(); ++i) { const Axis& a = plot_->x_axis(i); @@ -205,41 +221,28 @@ void Controller::apply_box_zoom() { // A selection under a few pixels on either side is a click: no zoom. if (sel.w < kMinBoxPixels || sel.h < kMinBoxPixels) return; - const Rect vp = plot_->viewport(); - if (!(vp.w > 0.0f) || !(vp.h > 0.0f)) return; - const f64 vx = static_cast(vp.x); - const f64 vy = static_cast(vp.y); - const f64 vw = static_cast(vp.w); - const f64 vh = static_cast(vp.h); - const f64 vright = static_cast(vp.right()); - const f64 vbottom = static_cast(vp.bottom()); - - // Clamped to the viewport so a drag that left it cannot ask for a range - // outside anything the caller can see. - const f64 l = std::clamp(static_cast(sel.x), vx, vright); - const f64 r = std::clamp(static_cast(sel.right()), vx, vright); - const f64 t = std::clamp(static_cast(sel.y), vy, vbottom); - const f64 b = std::clamp(static_cast(sel.bottom()), vy, vbottom); + // Clamped to the axes rect the transforms are built from, so a drag that left + // the plot area cannot ask for data outside what the box covers on screen. + const Rect axes = std::as_const(*plot_).layout().axes; + const f64 ax = static_cast(axes.x); + const f64 ar = static_cast(axes.right()); + const f64 ay = static_cast(axes.y); + const f64 ab = static_cast(axes.bottom()); + const f64 l = std::clamp(static_cast(sel.x), ax, ar); + const f64 r = std::clamp(static_cast(sel.right()), ax, ar); + const f64 t = std::clamp(static_cast(sel.y), ay, ab); + const f64 b = std::clamp(static_cast(sel.bottom()), ay, ab); if (!(r > l) || !(b > t)) return; - const f64 fx0 = (l - vx) / vw; - const f64 fx1 = (r - vx) / vw; - const f64 fy0 = (vbottom - b) / vh; // the selection's bottom edge is range min - const f64 fy1 = (vbottom - t) / vh; - + // The ranges come straight from the transforms, so the box frames exactly the + // data under it; the selection's bottom edge is the range minimum on y. for (u32 i = 0; i < plot_->x_axis_count(); ++i) { - const Axis& a = plot_->x_axis(i); - const f64 lo = to_space(a.range.min, a.log); - const f64 hi = to_space(a.range.max, a.log); - plot_->set_x_range(i, Range{from_space(lo + fx0 * (hi - lo), a.log), - from_space(lo + fx1 * (hi - lo), a.log)}); + const Transform tx = plot_->x_transform(i); + plot_->set_x_range(i, Range{tx.to_data_x(l), tx.to_data_x(r)}); } for (u32 i = 0; i < plot_->y_axis_count(); ++i) { - const Axis& a = plot_->y_axis(i); - const f64 lo = to_space(a.range.min, a.log); - const f64 hi = to_space(a.range.max, a.log); - plot_->set_y_range(i, Range{from_space(lo + fy0 * (hi - lo), a.log), - from_space(lo + fy1 * (hi - lo), a.log)}); + const Transform ty = plot_->y_transform(i); + plot_->set_y_range(i, Range{ty.to_data_y(b), ty.to_data_y(t)}); } } @@ -251,7 +254,9 @@ void Controller::move_cursor(const PointerEvent& e) { } void Controller::axis_zoom(const PointerEvent& e) { - const f64 factor = zoom_factor(static_cast(e.dy)); + // A pointer drag reports pixels, not wheel notches, so it uses its own + // sensitivity: 200 px is one decade. + const f64 factor = std::exp(-static_cast(e.dy) * kZoomPerPixel); if (!(factor > 0.0)) return; if (mode_ == DragMode::AxisZoomX) { for (u32 i = 0; i < plot_->x_axis_count(); ++i) { diff --git a/tests/test_controller.cpp b/tests/test_controller.cpp index 3e87473..f9a5974 100644 --- a/tests/test_controller.cpp +++ b/tests/test_controller.cpp @@ -61,16 +61,21 @@ PLOT_TEST(left_drag_pans_without_changing_span) { plot::input::Controller c(p); c.set_viewport({0, 0, 800, 400}); const double span_before = p.x_range(ax).span(); + const f64 grabbed = p.x_transform(ax).to_data_x(100.0f); // data under the press point c.on_pointer({100, 100, 0, 0, plot::input::Button::Left, 0, true, true, false}); PLOT_CHECK(c.drag_mode() == plot::input::DragMode::Pan); c.on_pointer({180, 130, 80, 30, plot::input::Button::Left, 0, true, false, false}); c.on_pointer({180, 130, 0, 0, plot::input::Button::Left, 0, false, false, true}); PLOT_CHECK(c.drag_mode() == plot::input::DragMode::None); PLOT_CHECK_NEAR(p.x_range(ax).span(), span_before, 1e-9); // span unchanged - // 80 px right-drag on an 800 px plot showing [0, 100] reveals 10 units of - // earlier data: the data follows the cursor. (The brief printed +10.0 here.) - PLOT_CHECK_NEAR(p.x_range(ax).min, -10.0, 1e-6); + // Assert the INVARIANT, not a pixel fraction. Grab-and-drag means the data + // grabbed at the press point is under the pointer afterwards. Pixels map to + // data through layout().axes, NOT the whole viewport: the viewport model makes + // dragged data lag the cursor by the label-strip fraction, a defect no + // pixel-fraction assertion can see because it certifies its own arithmetic. + PLOT_CHECK_NEAR(p.x_transform(ax).to_data_x(180.0f), grabbed, 1e-9); PLOT_CHECK(p.y_range(ay).max > 1.0); // moving down raises y range + PLOT_CHECK(!p.x_axis(ax).auto_scale); // a pan is a user choice, not a fit } PLOT_TEST(shift_drag_box_zooms_to_selection) { @@ -80,14 +85,39 @@ PLOT_TEST(shift_drag_box_zooms_to_selection) { plot::input::Controller c(p); c.set_viewport({0, 0, 800, 400}); const u32 e_shift = 1u << 0; // Mod::Shift + const f64 want_lo = p.x_transform(ax).to_data_x(100.0f); + const f64 want_hi = p.x_transform(ax).to_data_x(300.0f); c.on_pointer({100, 50, 0, 0, plot::input::Button::Left, e_shift, true, true, false}); PLOT_CHECK(c.drag_mode() == plot::input::DragMode::BoxZoom); c.on_pointer({300, 150, 200, 100, plot::input::Button::Left, e_shift, true, false, false}); PLOT_CHECK_NEAR(c.selection_rect().x, 100.0, 1e-6); PLOT_CHECK_NEAR(c.selection_rect().w, 200.0, 1e-6); c.on_pointer({300, 150, 0, 0, plot::input::Button::Left, e_shift, false, false, true}); - PLOT_CHECK_NEAR(p.x_range(ax).min, 100.0, 1e-6); - PLOT_CHECK_NEAR(p.x_range(ax).max, 300.0, 1e-6); + // The box frames exactly the data it encloses: the ranges come from the axes + // transform, and the selection is clamped to layout().axes, not the viewport. + PLOT_CHECK_NEAR(p.x_range(ax).min, want_lo, 1e-6); + PLOT_CHECK_NEAR(p.x_range(ax).max, want_hi, 1e-6); +} + +PLOT_TEST(box_zoom_uses_release_position_without_a_move) { + // A host that reports only a press and a release -- no motion event between + // them -- MUST still frame the box the release describes: the release position + // is authoritative for the end of the gesture. + plot::Plot p; + p.set_viewport({0, 0, 800, 400}); + const u32 ax = p.add_x_axis({"t", {0.0, 800.0}, true, plot::AxisPlace::Bottom, false}); + plot::input::Controller c(p); + c.set_viewport({0, 0, 800, 400}); + const f64 want_lo = p.x_transform(ax).to_data_x(100.0f); + const f64 want_hi = p.x_transform(ax).to_data_x(300.0f); + c.on_pointer({100, 50, 0, 0, plot::input::Button::Left, kShift, true, true, false}); + PLOT_CHECK(c.drag_mode() == plot::input::DragMode::BoxZoom); + // No move: selection_ is still 0x0 until the release supplies the far corner. + c.on_pointer({300, 150, 0, 0, plot::input::Button::Left, kShift, false, false, true}); + PLOT_CHECK(c.drag_mode() == plot::input::DragMode::None); + PLOT_CHECK_NEAR(p.x_range(ax).min, want_lo, 1e-6); + PLOT_CHECK_NEAR(p.x_range(ax).max, want_hi, 1e-6); + PLOT_CHECK(!p.x_axis(ax).auto_scale); // a box zoom is a user choice, not a fit } PLOT_TEST(auto_scale_disabled_by_user_interaction) { @@ -179,7 +209,9 @@ PLOT_TEST(middle_drag_on_x_strip_zooms_x_only) { // Same sign convention as the wheel: a positive dy zooms in. f.controller.on_pointer({strip_x, strip_y + 20, 0, 20, plot::input::Button::Middle, 0, true, false, false}); f.controller.on_pointer({strip_x, strip_y + 20, 0, 0, plot::input::Button::Middle, 0, false, false, true}); - PLOT_CHECK(f.plot.x_range(f.ax).span() < 100.0); + // 20 px of drag is a tenth of a decade: the span is 10^(-20/200) of its + // former self. A pixel is not a wheel notch, so this pins the magnitude. + PLOT_CHECK_NEAR(f.plot.x_range(f.ax).span(), 100.0 * std::pow(10.0, -20.0 / 200.0), 1e-6); PLOT_CHECK_NEAR(f.plot.y_range(f.ay).span(), 2.0, 1e-9); PLOT_CHECK(!f.plot.x_axis(f.ax).auto_scale); PLOT_CHECK(f.plot.y_axis(f.ay).auto_scale); // the untouched y axis keeps auto scale @@ -196,7 +228,7 @@ PLOT_TEST(middle_drag_on_y_strip_zooms_y_only) { f.controller.on_pointer({strip_x, strip_y + 20, 0, 20, plot::input::Button::Middle, 0, true, false, false}); f.controller.on_pointer({strip_x, strip_y + 20, 0, 0, plot::input::Button::Middle, 0, false, false, true}); PLOT_CHECK_NEAR(f.plot.x_range(f.ax).span(), 100.0, 1e-9); - PLOT_CHECK(f.plot.y_range(f.ay).span() < 2.0); + PLOT_CHECK_NEAR(f.plot.y_range(f.ay).span(), 2.0 * std::pow(10.0, -20.0 / 200.0), 1e-6); } PLOT_TEST(key_plus_zooms_in_about_the_centre) {