From 10c51dbf28299c0d04db25e14d4d6f5e3d093f0c Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 15 Mar 2020 18:09:11 +0100 Subject: [PATCH] Avoid spurious asserts when holding arrows in GTK wxSpinButton The change in the value can be greater than 1 when the arrows are held pressed, contrary to what the logic of determining the wraparound added in 086793ceef65348815a8dd211ecea2df37745420 supposed. Replace this with a check of whether we switch from the min value directly to the max one or vice versa, which mostly works and avoids asserts, even if it can still fail and produces wrong events when the arrows are held pressed for long enough to increase the delta to the range of the control, in which case we just can't distinguish between wraparound and passing from min to max (or vice versa) in a single step, which means that we have no way to determine the right event to send. But producing a wrong event is better than asserting, so this still counts as an improvement. Closes https://github.com/wxWidgets/wxWidgets/pull/1764 See #17957. Closes #18695. --- src/gtk/spinbutt.cpp | 36 ++++++++++++++++++++++++++---------- 1 file changed, 26 insertions(+), 10 deletions(-) diff --git a/src/gtk/spinbutt.cpp b/src/gtk/spinbutt.cpp index dd0066b449..266d8ea585 100644 --- a/src/gtk/spinbutt.cpp +++ b/src/gtk/spinbutt.cpp @@ -38,19 +38,35 @@ gtk_value_changed(GtkSpinButton* spinbutton, wxSpinButton* win) return; } - int inc = pos - oldPos; - // Adjust for wrap arounds - // (Doesn't work for degenerated cases, like [0..1] range.) + // Normally we can determine which way we're going by just comparing the + // old and the new values. + bool up = pos > oldPos; + + // However we need to account for the possibility of wrapping around. if ( win->HasFlag(wxSP_WRAP) ) { - if ( inc > 1 ) - inc = -1; - else if ( inc < -1 ) - inc = 1; - } - wxASSERT( inc == 1 || inc == -1 ); + // We have no way of distinguishing between wraparound and normal + // change when the range is just 1, as pressing either arrow results in + // the same change, so don't even try doing it in this case. + const int spinMin = win->GetMin(); + const int spinMax = win->GetMax(); - wxSpinEvent event(inc > 0 ? wxEVT_SCROLL_LINEUP : wxEVT_SCROLL_LINEDOWN, win->GetId()); + if ( spinMax - spinMin > 1 ) + { + if ( up ) + { + if ( oldPos == spinMin && pos == spinMax ) + up = false; + } + else // down + { + if ( oldPos == spinMax && pos == spinMin ) + up = true; + } + } + } + + wxSpinEvent event(up ? wxEVT_SCROLL_LINEUP : wxEVT_SCROLL_LINEDOWN, win->GetId()); event.SetPosition(pos); event.SetEventObject(win);