fix(input): refuse a non-finite wheel amount instead of zeroing it
Zeroing an unusable wheel amount turned it into a zero-notch zoom: the factor was exactly one, the range did not change, but the zoom body still ran through set_x_range/set_y_range, which clear auto_scale unconditionally. One garbage wheel therefore disarmed auto-fit on every axis. on_wheel now rejects an unusable amount before the event can mutate anything, keeping the position fallback. The tests that should have caught it are strengthened: the wheel case now asserts auto scale is still armed (it was passing either way), and the invalid-position case checks a pan measured from the last real position with the drag alive, rather than a range nothing could have moved.
This commit is contained in:
1 parent
b5b9b42be1
commit
dc0a977c86
4 files changed
+41
-6
No files matched your search
@@ -66,6 +66,11 @@ class Controller {
|
||||
/// user can neither pan nor zoom back. The buttons in the same event are still
|
||||
/// acted on, so a drag whose release arrives with a bad position still ends.
|
||||
void on_pointer(const PointerEvent& e);
|
||||
/// Same, and a wheel amount that is not a number is *refused* rather than
|
||||
/// zeroed: a zero-notch zoom changes no range but still goes through the range
|
||||
/// setters, which clear auto scale -- so a sanitised amount would quietly disarm
|
||||
/// auto-fit. An unusable amount leaves the plot exactly as it was; an unusable
|
||||
/// *position* only loses its anchor, falling back to the last known one.
|
||||
void on_wheel(const WheelEvent& e);
|
||||
void on_key(const KeyEvent& e);
|
||||
|
||||
|
||||
@@ -66,14 +66,16 @@ PointerEvent sanitized(const PointerEvent& e, bool have_pointer, f32 last_x,
|
||||
return in;
|
||||
}
|
||||
|
||||
/// `e`, with an unusable position replaced by the last known one. The wheel's
|
||||
/// amount is deliberately left alone: `on_wheel` rejects an unusable one *before*
|
||||
/// the event is acted on, because a zeroed amount is a zoom of exactly one, and
|
||||
/// running that over an unchanged range would still clear every axis' auto scale.
|
||||
WheelEvent sanitized(const WheelEvent& e, bool have_pointer, f32 last_x, f32 last_y) noexcept {
|
||||
WheelEvent in = e;
|
||||
if (!finite(in.x) || !finite(in.y)) {
|
||||
in.x = have_pointer ? last_x : 0.0f;
|
||||
in.y = have_pointer ? last_y : 0.0f;
|
||||
}
|
||||
if (!finite(in.dx)) in.dx = 0.0f;
|
||||
if (!finite(in.dy)) in.dy = 0.0f;
|
||||
return in;
|
||||
}
|
||||
|
||||
@@ -106,6 +108,13 @@ void Controller::on_pointer(const PointerEvent& raw) {
|
||||
}
|
||||
|
||||
void Controller::on_wheel(const WheelEvent& raw) {
|
||||
// An amount that is not a number is not a wheel: sanitising it to zero would look
|
||||
// like a zero-notch zoom -- a factor of exactly one -- and the zoom body would
|
||||
// still run, calling the range setters with the unchanged range. Those clear
|
||||
// `auto_scale` unconditionally, so a single garbage wheel would silently disarm
|
||||
// auto-fit on every axis. Return before the event can mutate anything.
|
||||
if (!std::isfinite(raw.dy)) return;
|
||||
|
||||
const WheelEvent e = sanitized(raw, has_pointer_, pointer_x_, pointer_y_);
|
||||
plot_->layout();
|
||||
const f64 factor = zoom_factor(static_cast<f64>(e.dy));
|
||||
|
||||
@@ -322,7 +322,12 @@ PLOT_TEST(non_finite_wheel_does_not_break_a_range) {
|
||||
PLOT_CHECK(std::fabs(p.x_range(ax).min) < 1e6 && std::fabs(p.x_range(ax).max) < 1e6);
|
||||
PLOT_CHECK(p.x_range(ax).span() < span_before);
|
||||
|
||||
// A non-finite amount is no zoom at all.
|
||||
// A non-finite amount is no zoom at all -- and, just as important, it must not
|
||||
// touch the axes. A *zeroed* amount would be a zero-notch zoom (a factor of
|
||||
// exactly one): the range would survive, but the range setters it runs through
|
||||
// clear auto scale, so one garbage wheel would quietly disarm auto-fit.
|
||||
p.set_x_auto_scale(ax, true);
|
||||
p.set_y_auto_scale(ay, true);
|
||||
const plot::Range xr = p.x_range(ax);
|
||||
const plot::Range yr = p.y_range(ay);
|
||||
c.on_wheel({400.0f, 200.0f, 0.0f, nan, 0});
|
||||
@@ -330,6 +335,8 @@ PLOT_TEST(non_finite_wheel_does_not_break_a_range) {
|
||||
PLOT_CHECK_NEAR(p.x_range(ax).min, xr.min, 1e-12);
|
||||
PLOT_CHECK_NEAR(p.x_range(ax).max, xr.max, 1e-12);
|
||||
PLOT_CHECK_NEAR(p.y_range(ay).min, yr.min, 1e-12);
|
||||
PLOT_CHECK(p.x_axis(ax).auto_scale);
|
||||
PLOT_CHECK(p.y_axis(ay).auto_scale);
|
||||
}
|
||||
|
||||
PLOT_TEST(key_f_auto_fits_traces) {
|
||||
|
||||
@@ -344,10 +344,24 @@ PLOT_TEST(imgui_widget_invalid_position_does_not_move_the_range) {
|
||||
f.frame(ImVec2(-kMaxFloat, -kMaxFloat), false);
|
||||
PLOT_CHECK(!f.panning());
|
||||
|
||||
// And a real position after the sentinel is not differenced against the sentinel.
|
||||
// A drag held across a sentinel, then a real position again. The movement must be
|
||||
// measured from the last *real* position -- 100 px from (200, 200) to (300, 300) --
|
||||
// and not from the sentinel. This is asserted with the drag alive on purpose:
|
||||
// nothing can move a range once the button is up, so the same check after the
|
||||
// release could not fail and would prove nothing.
|
||||
f.frame(ImVec2(200.0f, 200.0f), false);
|
||||
f.frame(ImVec2(200.0f, 200.0f), true);
|
||||
const plot::Range second_press = f.x_range();
|
||||
f.frame(ImVec2(-kMaxFloat, -kMaxFloat), true);
|
||||
PLOT_CHECK(f.panning());
|
||||
const double axes_w = static_cast<double>(std::as_const(f.plot).layout().axes.w);
|
||||
const double expected_shift = 100.0 / axes_w * second_press.span();
|
||||
f.frame(ImVec2(300.0f, 300.0f), true);
|
||||
PLOT_CHECK(f.panning());
|
||||
PLOT_CHECK(std::isfinite(f.x_range().min));
|
||||
PLOT_CHECK_NEAR(f.x_range().min, second_press.min - expected_shift, 0.05 * expected_shift);
|
||||
f.frame(ImVec2(300.0f, 300.0f), false);
|
||||
PLOT_CHECK_NEAR(f.x_range().min, pressed.min, 1e-9);
|
||||
PLOT_CHECK_NEAR(f.x_range().max, pressed.max, 1e-9);
|
||||
PLOT_CHECK(!f.panning());
|
||||
}
|
||||
|
||||
PLOT_TEST(imgui_widget_paints_the_whole_child_when_nothing_clips_it) {
|
||||
|
||||
Reference in new issue
Block a user