From a8448b084ca6cc3566ae7434e65a3d1bf81afe9b Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Fri, 17 Aug 2018 00:26:42 +0200 Subject: [PATCH 1/3] Remove duplicate wxGrid::m_winCapture initialization No real changes, just remove one of 2 duplicate assignments. --- src/generic/grid.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/generic/grid.cpp b/src/generic/grid.cpp index 5a8831f792..f5bd9466a5 100644 --- a/src/generic/grid.cpp +++ b/src/generic/grid.cpp @@ -2418,7 +2418,6 @@ void wxGrid::Init() m_selection = NULL; m_defaultCellAttr = NULL; m_typeRegistry = NULL; - m_winCapture = NULL; m_rowLabelWidth = WXGRID_DEFAULT_ROW_LABEL_WIDTH; m_colLabelHeight = WXGRID_DEFAULT_COL_LABEL_HEIGHT; From 53972b17ee702f8dbd0c2fee998bdecbb4f0bdd3 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Fri, 17 Aug 2018 00:28:21 +0200 Subject: [PATCH 2/3] Add helper wxGrid::DoAfterDraggingEnd() function No real changes, just refactor the code to extract the part of CancelMouseCapture() which can be useful not only when the mouse capture is lost unexpectedly, but also when we release it of our own volition, into a separate function. --- include/wx/generic/grid.h | 4 ++++ src/generic/grid.cpp | 17 +++++++++++------ 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/include/wx/generic/grid.h b/include/wx/generic/grid.h index c1adbb0b24..b01c6b4e54 100644 --- a/include/wx/generic/grid.h +++ b/include/wx/generic/grid.h @@ -2218,6 +2218,10 @@ private: // SetColPos() and ResetColPos()) void RefreshAfterColPosChange(); + // reset the variables used during dragging operations after it ended + // because we lost mouse capture + void DoAfterDraggingEnd(); + // return the position (not index) of the column at the given logical pixel // position diff --git a/src/generic/grid.cpp b/src/generic/grid.cpp index f5bd9466a5..c3ab7f0594 100644 --- a/src/generic/grid.cpp +++ b/src/generic/grid.cpp @@ -3846,18 +3846,23 @@ void wxGrid::CancelMouseCapture() // cancel operation currently in progress, whatever it is if ( m_winCapture ) { - m_isDragging = false; - m_startDragPos = wxDefaultPosition; - - m_cursorMode = WXGRID_CURSOR_SELECT_CELL; - m_winCapture->SetCursor( *wxSTANDARD_CURSOR ); - m_winCapture = NULL; + DoAfterDraggingEnd(); // remove traces of whatever we drew on screen Refresh(); } } +void wxGrid::DoAfterDraggingEnd() +{ + m_isDragging = false; + m_startDragPos = wxDefaultPosition; + + m_cursorMode = WXGRID_CURSOR_SELECT_CELL; + m_winCapture->SetCursor( *wxSTANDARD_CURSOR ); + m_winCapture = NULL; +} + void wxGrid::ChangeCursorMode(CursorMode mode, wxWindow *win, bool captureMouse) From bb00501e77f52acc93eeee39a9c3ce3ae35e9fd4 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Fri, 17 Aug 2018 00:57:04 +0200 Subject: [PATCH 3/3] Fix mouse event handling while dragging in wxGrid Ignore all the mouse events other than "left up" while dragging to avoid releasing the mouse and ending the dragging operation too soon. This required non-trivial refactoring of the code which hopefully should also make it slightly more clear by centralizing high level logic in ProcessGridCellMouseEvent() itself and calling various helper functions from it instead of spreading this logic over the entire call tree. The code still remains pretty confusing and rewriting it to use wxMouseEventsManager (which would need to be generalized first to become a template class using arbitrary item type instead of just "int", as now) would undoubtedly do it a lot of good. Closes #18186. --- include/wx/generic/grid.h | 16 +++-- src/generic/grid.cpp | 133 ++++++++++++++++++++------------------ 2 files changed, 82 insertions(+), 67 deletions(-) diff --git a/include/wx/generic/grid.h b/include/wx/generic/grid.h index b01c6b4e54..a06c654855 100644 --- a/include/wx/generic/grid.h +++ b/include/wx/generic/grid.h @@ -2218,10 +2218,14 @@ private: // SetColPos() and ResetColPos()) void RefreshAfterColPosChange(); - // reset the variables used during dragging operations after it ended - // because we lost mouse capture + // reset the variables used during dragging operations after it ended, + // either because we called EndDraggingIfNecessary() ourselves or because + // we lost mouse capture void DoAfterDraggingEnd(); + // release the mouse capture if it's currently captured + void EndDraggingIfNecessary(); + // return the position (not index) of the column at the given logical pixel // position @@ -2242,8 +2246,12 @@ private: // process row/column resizing drag event void DoGridLineDrag(wxMouseEvent& event, const wxGridOperations& oper); - // process mouse drag event in the grid window - void DoGridDragEvent(wxMouseEvent& event, const wxGridCellCoords& coords); + // process mouse drag event in the grid window, return false if starting + // dragging was vetoed by the user-defined wxEVT_GRID_CELL_BEGIN_DRAG + // handler + bool DoGridDragEvent(wxMouseEvent& event, + const wxGridCellCoords& coords, + bool isFirstDrag); // process different clicks on grid cells void DoGridCellLeftDown(wxMouseEvent& event, diff --git a/src/generic/grid.cpp b/src/generic/grid.cpp index c3ab7f0594..2f01b2f667 100644 --- a/src/generic/grid.cpp +++ b/src/generic/grid.cpp @@ -3863,6 +3863,16 @@ void wxGrid::DoAfterDraggingEnd() m_winCapture = NULL; } +void wxGrid::EndDraggingIfNecessary() +{ + if ( m_winCapture ) + { + m_winCapture->ReleaseMouse(); + + DoAfterDraggingEnd(); + } +} + void wxGrid::ChangeCursorMode(CursorMode mode, wxWindow *win, bool captureMouse) @@ -3897,11 +3907,7 @@ void wxGrid::ChangeCursorMode(CursorMode mode, win = m_gridWin; } - if ( m_winCapture ) - { - m_winCapture->ReleaseMouse(); - m_winCapture = NULL; - } + EndDraggingIfNecessary(); m_cursorMode = mode; @@ -4019,34 +4025,14 @@ void wxGrid::DoGridLineDrag(wxMouseEvent& event, const wxGridOperations& oper) oper.DrawParallelLineInRect(dc, rectWin, m_dragLastPos); } -void wxGrid::DoGridDragEvent(wxMouseEvent& event, const wxGridCellCoords& coords) +bool wxGrid::DoGridDragEvent(wxMouseEvent& event, + const wxGridCellCoords& coords, + bool isFirstDrag) { - if ( !m_isDragging ) - { - // Don't start doing anything until the mouse has been dragged far - // enough - const wxPoint& pt = event.GetPosition(); - if ( m_startDragPos == wxDefaultPosition ) - { - m_startDragPos = pt; - return; - } - - if ( abs(m_startDragPos.x - pt.x) <= DRAG_SENSITIVITY && - abs(m_startDragPos.y - pt.y) <= DRAG_SENSITIVITY ) - return; - } - - const bool isFirstDrag = !m_isDragging; - m_isDragging = true; - switch ( m_cursorMode ) { case WXGRID_CURSOR_SELECT_CELL: - // no further handling if handled by user - if ( DoGridCellDrag(event, coords, isFirstDrag) == false ) - return; - break; + return DoGridCellDrag(event, coords, isFirstDrag); case WXGRID_CURSOR_RESIZE_ROW: DoGridLineDrag(event, wxGridRowOperations()); @@ -4060,13 +4046,7 @@ void wxGrid::DoGridDragEvent(wxMouseEvent& event, const wxGridCellCoords& coords event.Skip(); } - if ( isFirstDrag ) - { - wxASSERT_MSG( !m_winCapture, "shouldn't capture the mouse twice" ); - - m_winCapture = m_gridWin; - m_winCapture->CaptureMouse(); - } + return true; } void @@ -4160,12 +4140,6 @@ wxGrid::DoGridCellLeftUp(wxMouseEvent& event, const wxGridCellCoords& coords) { if ( m_cursorMode == WXGRID_CURSOR_SELECT_CELL ) { - if (m_winCapture) - { - m_winCapture->ReleaseMouse(); - m_winCapture = NULL; - } - if ( coords == m_currentCellCoords && m_waitForSlowClick && CanEnableCellControl() ) { ClearSelection(); @@ -4266,14 +4240,6 @@ wxGrid::DoGridMouseMoveEvent(wxMouseEvent& WXUNUSED(event), void wxGrid::ProcessGridCellMouseEvent(wxMouseEvent& event) { - if ( event.Entering() || event.Leaving() ) - { - // we don't care about these events but we must not reset m_isDragging - // if they happen so return before anything else is done - event.Skip(); - return; - } - const wxPoint pos = CalcUnscrolledPosition(event.GetPosition()); // coordinates of the cell under mouse @@ -4287,17 +4253,64 @@ void wxGrid::ProcessGridCellMouseEvent(wxMouseEvent& event) coords.SetCol(coords.GetCol() + cell_cols); } - if ( event.Dragging() ) + // Releasing the left mouse button must be processed in any case, so deal + // with it first. + if ( event.LeftUp() ) { - if ( event.LeftIsDown() ) - DoGridDragEvent(event, coords); - else - event.Skip(); + // Note that we must call this one first, before resetting the + // drag-related data, as it relies on m_cursorMode being still set and + // EndDraggingIfNecessary() resets it. + DoGridCellLeftUp(event, coords); + + EndDraggingIfNecessary(); return; } - m_isDragging = false; - m_startDragPos = wxDefaultPosition; + const bool isDraggingWithLeft = event.Dragging() && event.LeftIsDown(); + + // While dragging the mouse, only releasing the left mouse button, which + // cancels the drag operation, is processed (above) and any other events + // are just ignored while it's in progress. + if ( m_isDragging ) + { + if ( isDraggingWithLeft ) + DoGridDragEvent(event, coords, false /* not first drag */); + return; + } + + // Now check if we're starting a drag operation (if it had been already + // started, m_isDragging would be true above). + if ( isDraggingWithLeft ) + { + // To avoid accidental drags, don't start doing anything until the + // mouse has been dragged far enough. + const wxPoint& pt = event.GetPosition(); + if ( m_startDragPos == wxDefaultPosition ) + { + m_startDragPos = pt; + return; + } + + if ( abs(m_startDragPos.x - pt.x) <= DRAG_SENSITIVITY && + abs(m_startDragPos.y - pt.y) <= DRAG_SENSITIVITY ) + return; + + if ( DoGridDragEvent(event, coords, true /* first drag */) ) + { + wxASSERT_MSG( !m_winCapture, "shouldn't capture the mouse twice" ); + + m_winCapture = m_gridWin; + m_winCapture->CaptureMouse(); + + m_isDragging = true; + } + + return; + } + + // If we're not dragging, cancel any dragging operation which could have + // been in progress. + EndDraggingIfNecessary(); // deal with various button presses if ( event.IsButton() ) @@ -4315,12 +4328,6 @@ void wxGrid::ProcessGridCellMouseEvent(wxMouseEvent& event) else if ( event.RightDClick() ) SendEvent(wxEVT_GRID_CELL_RIGHT_DCLICK, coords, event); } - - // this one should be called even if we're not over any cell - if ( event.LeftUp() ) - { - DoGridCellLeftUp(event, coords); - } } else if ( event.Moving() ) {