From d10c6dd1b3bc6e8eea06efccc781a7bcbf29c0db Mon Sep 17 00:00:00 2001 From: Andrew Zonenberg Date: Sat, 11 Jul 2015 15:31:01 -0700 Subject: [PATCH 1/8] Fixed bug where GAL module editor does not release mouse capture after exiting text tool --- pcbnew/tools/drawing_tool.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pcbnew/tools/drawing_tool.cpp b/pcbnew/tools/drawing_tool.cpp index 719e386984..b46e06eca7 100644 --- a/pcbnew/tools/drawing_tool.cpp +++ b/pcbnew/tools/drawing_tool.cpp @@ -1350,7 +1350,7 @@ int DRAWING_TOOL::placeTextModule() m_controls->ShowCursor( false ); m_controls->SetSnapping( false ); m_controls->SetAutoPan( false ); - m_controls->CaptureCursor( true ); + m_controls->CaptureCursor( false ); m_view->Remove( &preview ); m_frame->SetToolID( ID_NO_TOOL_SELECTED, wxCURSOR_DEFAULT, wxEmptyString ); From 186d0a4fd34cb7018fddd0582a4b6af2fd326cbf Mon Sep 17 00:00:00 2001 From: Andrew Zonenberg Date: Sat, 11 Jul 2015 16:03:07 -0700 Subject: [PATCH 2/8] Fixed bug where GAL text tool was overzealously capturing the cursor when not necessary --- pcbnew/tools/drawing_tool.cpp | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/pcbnew/tools/drawing_tool.cpp b/pcbnew/tools/drawing_tool.cpp index b46e06eca7..3681044686 100644 --- a/pcbnew/tools/drawing_tool.cpp +++ b/pcbnew/tools/drawing_tool.cpp @@ -1251,8 +1251,7 @@ int DRAWING_TOOL::placeTextModule() m_toolMgr->RunAction( COMMON_ACTIONS::selectionClear, true ); m_controls->ShowCursor( true ); m_controls->SetSnapping( true ); - m_controls->SetAutoPan( true ); - m_controls->CaptureCursor( true ); + // do not capture or auto-pan until we start placing some text Activate(); m_frame->SetToolID( ID_PCB_ADD_TEXT_BUTT, wxCURSOR_PENCIL, _( "Add text" ) ); @@ -1267,6 +1266,8 @@ int DRAWING_TOOL::placeTextModule() { preview.Clear(); preview.ViewUpdate(); + m_controls->SetAutoPan( false ); + m_controls->CaptureCursor( false ); m_controls->ShowCursor( true ); if( !placing || evt->IsActivate() ) @@ -1307,7 +1308,9 @@ int DRAWING_TOOL::placeTextModule() if( !placing ) continue; - + + m_controls->CaptureCursor( true ); + m_controls->SetAutoPan( true ); m_controls->ShowCursor( false ); text->SetParent( m_board->m_Modules ); // it has to set after the settings dialog // otherwise the dialog stores it in undo buffer @@ -1331,6 +1334,8 @@ int DRAWING_TOOL::placeTextModule() m_frame->OnModify(); preview.Remove( text ); + m_controls->CaptureCursor( false ); + m_controls->SetAutoPan( false ); m_controls->ShowCursor( true ); text = new TEXTE_MODULE( NULL ); @@ -1370,8 +1375,7 @@ int DRAWING_TOOL::placeTextPcb() m_toolMgr->RunAction( COMMON_ACTIONS::selectionClear, true ); m_controls->ShowCursor( true ); m_controls->SetSnapping( true ); - m_controls->SetAutoPan( true ); - m_controls->CaptureCursor( true ); + // do not capture or auto-pan until we start placing some text Activate(); m_frame->SetToolID( ID_PCB_ADD_TEXT_BUTT, wxCURSOR_PENCIL, _( "Add text" ) ); @@ -1391,6 +1395,8 @@ int DRAWING_TOOL::placeTextPcb() preview.Clear(); preview.ViewUpdate( KIGFX::VIEW_ITEM::GEOMETRY ); + m_controls->SetAutoPan( false ); + m_controls->CaptureCursor( false ); m_controls->ShowCursor( true ); } else @@ -1424,6 +1430,8 @@ int DRAWING_TOOL::placeTextPcb() if( text == NULL ) continue; + m_controls->CaptureCursor( true ); + m_controls->SetAutoPan( true ); m_controls->ShowCursor( false ); preview.Add( text ); } @@ -1441,6 +1449,8 @@ int DRAWING_TOOL::placeTextPcb() m_frame->SaveCopyInUndoList( text, UR_NEW ); preview.Remove( text ); + m_controls->CaptureCursor( false ); + m_controls->SetAutoPan( false ); m_controls->ShowCursor( true ); text = NULL; From 4c065e63c88df5cd25293fc3bac93dcf70322cf8 Mon Sep 17 00:00:00 2001 From: Andrew Zonenberg Date: Sat, 11 Jul 2015 16:50:31 -0700 Subject: [PATCH 3/8] Removed leading space --- pcbnew/tools/drawing_tool.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pcbnew/tools/drawing_tool.cpp b/pcbnew/tools/drawing_tool.cpp index 3681044686..769e1c689f 100644 --- a/pcbnew/tools/drawing_tool.cpp +++ b/pcbnew/tools/drawing_tool.cpp @@ -1308,7 +1308,7 @@ int DRAWING_TOOL::placeTextModule() if( !placing ) continue; - + m_controls->CaptureCursor( true ); m_controls->SetAutoPan( true ); m_controls->ShowCursor( false ); From 31c9856d229f9ce92da50c7d0b2f4a6c655e8885 Mon Sep 17 00:00:00 2001 From: Andrew Zonenberg Date: Sat, 11 Jul 2015 17:03:50 -0700 Subject: [PATCH 4/8] Fixed bug where "add text" tool in module editor did not update the toolbar radio buttons correctly --- pcbnew/tools/drawing_tool.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pcbnew/tools/drawing_tool.cpp b/pcbnew/tools/drawing_tool.cpp index 769e1c689f..4dbc4e8fda 100644 --- a/pcbnew/tools/drawing_tool.cpp +++ b/pcbnew/tools/drawing_tool.cpp @@ -1254,7 +1254,7 @@ int DRAWING_TOOL::placeTextModule() // do not capture or auto-pan until we start placing some text Activate(); - m_frame->SetToolID( ID_PCB_ADD_TEXT_BUTT, wxCURSOR_PENCIL, _( "Add text" ) ); + m_frame->SetToolID( ID_MODEDIT_TEXT_TOOL, wxCURSOR_PENCIL, _( "Add text" ) ); bool placing = false; // Main loop: keep receiving events From ed43694682e6421802d9479402f5ef49cebacc73 Mon Sep 17 00:00:00 2001 From: Andrew Zonenberg Date: Sun, 12 Jul 2015 12:10:59 -0700 Subject: [PATCH 5/8] GAL now hides cursor when mouse is outside the view. No change to legacy view. --- common/view/wx_view_controls.cpp | 33 +++++++++++++++++++++--- include/gal/graphics_abstraction_layer.h | 10 +++++++ include/view/wx_view_controls.h | 13 ++++++++++ 3 files changed, 53 insertions(+), 3 deletions(-) diff --git a/common/view/wx_view_controls.cpp b/common/view/wx_view_controls.cpp index 7bb333004a..0f1c734c8d 100644 --- a/common/view/wx_view_controls.cpp +++ b/common/view/wx_view_controls.cpp @@ -36,7 +36,8 @@ using namespace KIGFX; const wxEventType WX_VIEW_CONTROLS::EVT_REFRESH_MOUSE = wxNewEventType(); WX_VIEW_CONTROLS::WX_VIEW_CONTROLS( VIEW* aView, wxScrolledCanvas* aParentPanel ) : - VIEW_CONTROLS( aView ), m_state( IDLE ), m_parentPanel( aParentPanel ), m_scrollScale( 1.0, 1.0 ) + VIEW_CONTROLS( aView ), m_state( IDLE ), m_parentPanel( aParentPanel ), m_scrollScale( 1.0, 1.0 ), + m_mouseIsInView(false), m_cursorWasDisplayedOnLeave(false) { m_parentPanel->Connect( wxEVT_MOTION, wxMouseEventHandler( WX_VIEW_CONTROLS::onMotion ), NULL, this ); @@ -54,10 +55,8 @@ WX_VIEW_CONTROLS::WX_VIEW_CONTROLS( VIEW* aView, wxScrolledCanvas* aParentPanel wxMouseEventHandler( WX_VIEW_CONTROLS::onButton ), NULL, this ); m_parentPanel->Connect( wxEVT_LEFT_DOWN, wxMouseEventHandler( WX_VIEW_CONTROLS::onButton ), NULL, this ); -#if defined _WIN32 || defined _WIN64 m_parentPanel->Connect( wxEVT_ENTER_WINDOW, wxMouseEventHandler( WX_VIEW_CONTROLS::onEnter ), NULL, this ); -#endif m_parentPanel->Connect( wxEVT_LEAVE_WINDOW, wxMouseEventHandler( WX_VIEW_CONTROLS::onLeave ), NULL, this ); m_parentPanel->Connect( wxEVT_SCROLLWIN_THUMBTRACK, @@ -212,12 +211,28 @@ void WX_VIEW_CONTROLS::onButton( wxMouseEvent& aEvent ) void WX_VIEW_CONTROLS::onEnter( wxMouseEvent& aEvent ) { + //If the cursor was enabled when we left, turn it on now + m_mouseIsInView = true; + m_view->GetGAL()->SetCursorEnabled(m_cursorWasDisplayedOnLeave); + +#if defined _WIN32 || defined _WIN64 m_parentPanel->SetFocus(); +#endif } void WX_VIEW_CONTROLS::onLeave( wxMouseEvent& aEvent ) { + // Always hide the cursor when the mouse leaves the window to avoid confusing the user + // But we may need to re-enable it, so keep track of whether it was visible + m_mouseIsInView = false; + m_cursorWasDisplayedOnLeave = m_view->GetGAL()->GetCursorEnabled(); + m_view->GetGAL()->SetCursorEnabled(false); + + // If we hid the cursor, redraw the window to get rid of it + if(m_cursorWasDisplayedOnLeave) + m_view->Redraw(); + if( m_cursorCaptured ) { bool warp = false; @@ -434,3 +449,15 @@ void WX_VIEW_CONTROLS::UpdateScrollbars() ( viewport.Centre().x - boundary.GetLeft() ) / boundary.GetWidth() * m_scrollScale.x, ( viewport.Centre().y - boundary.GetTop() ) / boundary.GetHeight() * m_scrollScale.y ); } + +void WX_VIEW_CONTROLS::ShowCursor( bool aEnabled ) +{ + // If the mouse is outside the view (on a toolbar etc), don't show the cursor + // until the mouse enters the view again. Just remember we want to show it now + if(!m_mouseIsInView) + m_cursorWasDisplayedOnLeave = aEnabled; + + // but if it's in the view, we want to show it right now + else + m_view->GetGAL()->SetCursorEnabled( aEnabled ); +} diff --git a/include/gal/graphics_abstraction_layer.h b/include/gal/graphics_abstraction_layer.h index 6c478e8558..281b7d6fc9 100644 --- a/include/gal/graphics_abstraction_layer.h +++ b/include/gal/graphics_abstraction_layer.h @@ -762,6 +762,16 @@ public: { isCursorEnabled = aCursorEnabled; } + + /** + * @brief Returns the cursor enable state + * + * @return True if the cursor is being drawn, else false + */ + inline bool GetCursorEnabled() const + { + return isCursorEnabled; + } /** * @brief Set the cursor color. diff --git a/include/view/wx_view_controls.h b/include/view/wx_view_controls.h index 67ad9b4567..33710be6de 100644 --- a/include/view/wx_view_controls.h +++ b/include/view/wx_view_controls.h @@ -84,6 +84,13 @@ public: if( m_state == AUTO_PANNING ) m_state = IDLE; } + + /** + * Function ShowCursor() + * Enables or disables display of cursor. + * @param aEnabled decides if the cursor should be shown. + */ + virtual void ShowCursor( bool aEnabled ); /// @copydoc VIEW_CONTROLS::GetMousePosition() VECTOR2D GetMousePosition() const; @@ -140,6 +147,12 @@ private: /// Ratio used for scaling world coordinates to scrollbar position. VECTOR2D m_scrollScale; + + /// Indicates whether the mouse is inside the window + bool m_mouseIsInView; + + /// Indicates whether the cursor was being displayed at the time the mouse left the window + bool m_cursorWasDisplayedOnLeave; }; } // namespace KIGFX From b87ab79d6d1e785b44199eecd99f348abf2fce77 Mon Sep 17 00:00:00 2001 From: Andrew Zonenberg Date: Sun, 12 Jul 2015 12:54:11 -0700 Subject: [PATCH 6/8] Reverting last commit since it causes regressions --- common/view/wx_view_controls.cpp | 33 +++--------------------- include/gal/graphics_abstraction_layer.h | 10 ------- include/view/wx_view_controls.h | 13 ---------- 3 files changed, 3 insertions(+), 53 deletions(-) diff --git a/common/view/wx_view_controls.cpp b/common/view/wx_view_controls.cpp index 0f1c734c8d..7bb333004a 100644 --- a/common/view/wx_view_controls.cpp +++ b/common/view/wx_view_controls.cpp @@ -36,8 +36,7 @@ using namespace KIGFX; const wxEventType WX_VIEW_CONTROLS::EVT_REFRESH_MOUSE = wxNewEventType(); WX_VIEW_CONTROLS::WX_VIEW_CONTROLS( VIEW* aView, wxScrolledCanvas* aParentPanel ) : - VIEW_CONTROLS( aView ), m_state( IDLE ), m_parentPanel( aParentPanel ), m_scrollScale( 1.0, 1.0 ), - m_mouseIsInView(false), m_cursorWasDisplayedOnLeave(false) + VIEW_CONTROLS( aView ), m_state( IDLE ), m_parentPanel( aParentPanel ), m_scrollScale( 1.0, 1.0 ) { m_parentPanel->Connect( wxEVT_MOTION, wxMouseEventHandler( WX_VIEW_CONTROLS::onMotion ), NULL, this ); @@ -55,8 +54,10 @@ WX_VIEW_CONTROLS::WX_VIEW_CONTROLS( VIEW* aView, wxScrolledCanvas* aParentPanel wxMouseEventHandler( WX_VIEW_CONTROLS::onButton ), NULL, this ); m_parentPanel->Connect( wxEVT_LEFT_DOWN, wxMouseEventHandler( WX_VIEW_CONTROLS::onButton ), NULL, this ); +#if defined _WIN32 || defined _WIN64 m_parentPanel->Connect( wxEVT_ENTER_WINDOW, wxMouseEventHandler( WX_VIEW_CONTROLS::onEnter ), NULL, this ); +#endif m_parentPanel->Connect( wxEVT_LEAVE_WINDOW, wxMouseEventHandler( WX_VIEW_CONTROLS::onLeave ), NULL, this ); m_parentPanel->Connect( wxEVT_SCROLLWIN_THUMBTRACK, @@ -211,28 +212,12 @@ void WX_VIEW_CONTROLS::onButton( wxMouseEvent& aEvent ) void WX_VIEW_CONTROLS::onEnter( wxMouseEvent& aEvent ) { - //If the cursor was enabled when we left, turn it on now - m_mouseIsInView = true; - m_view->GetGAL()->SetCursorEnabled(m_cursorWasDisplayedOnLeave); - -#if defined _WIN32 || defined _WIN64 m_parentPanel->SetFocus(); -#endif } void WX_VIEW_CONTROLS::onLeave( wxMouseEvent& aEvent ) { - // Always hide the cursor when the mouse leaves the window to avoid confusing the user - // But we may need to re-enable it, so keep track of whether it was visible - m_mouseIsInView = false; - m_cursorWasDisplayedOnLeave = m_view->GetGAL()->GetCursorEnabled(); - m_view->GetGAL()->SetCursorEnabled(false); - - // If we hid the cursor, redraw the window to get rid of it - if(m_cursorWasDisplayedOnLeave) - m_view->Redraw(); - if( m_cursorCaptured ) { bool warp = false; @@ -449,15 +434,3 @@ void WX_VIEW_CONTROLS::UpdateScrollbars() ( viewport.Centre().x - boundary.GetLeft() ) / boundary.GetWidth() * m_scrollScale.x, ( viewport.Centre().y - boundary.GetTop() ) / boundary.GetHeight() * m_scrollScale.y ); } - -void WX_VIEW_CONTROLS::ShowCursor( bool aEnabled ) -{ - // If the mouse is outside the view (on a toolbar etc), don't show the cursor - // until the mouse enters the view again. Just remember we want to show it now - if(!m_mouseIsInView) - m_cursorWasDisplayedOnLeave = aEnabled; - - // but if it's in the view, we want to show it right now - else - m_view->GetGAL()->SetCursorEnabled( aEnabled ); -} diff --git a/include/gal/graphics_abstraction_layer.h b/include/gal/graphics_abstraction_layer.h index 281b7d6fc9..6c478e8558 100644 --- a/include/gal/graphics_abstraction_layer.h +++ b/include/gal/graphics_abstraction_layer.h @@ -762,16 +762,6 @@ public: { isCursorEnabled = aCursorEnabled; } - - /** - * @brief Returns the cursor enable state - * - * @return True if the cursor is being drawn, else false - */ - inline bool GetCursorEnabled() const - { - return isCursorEnabled; - } /** * @brief Set the cursor color. diff --git a/include/view/wx_view_controls.h b/include/view/wx_view_controls.h index 33710be6de..67ad9b4567 100644 --- a/include/view/wx_view_controls.h +++ b/include/view/wx_view_controls.h @@ -84,13 +84,6 @@ public: if( m_state == AUTO_PANNING ) m_state = IDLE; } - - /** - * Function ShowCursor() - * Enables or disables display of cursor. - * @param aEnabled decides if the cursor should be shown. - */ - virtual void ShowCursor( bool aEnabled ); /// @copydoc VIEW_CONTROLS::GetMousePosition() VECTOR2D GetMousePosition() const; @@ -147,12 +140,6 @@ private: /// Ratio used for scaling world coordinates to scrollbar position. VECTOR2D m_scrollScale; - - /// Indicates whether the mouse is inside the window - bool m_mouseIsInView; - - /// Indicates whether the cursor was being displayed at the time the mouse left the window - bool m_cursorWasDisplayedOnLeave; }; } // namespace KIGFX From 1ad3b056649d40e9d675db453269f97366eb18f7 Mon Sep 17 00:00:00 2001 From: Andrew Zonenberg Date: Sun, 12 Jul 2015 21:52:17 -0700 Subject: [PATCH 7/8] Fixed a bug causing layer-switch events to not be handled by GAL --- include/tool/tool_event.h | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/include/tool/tool_event.h b/include/tool/tool_event.h index f4ccf4f817..3bd62ec3cf 100644 --- a/include/tool/tool_event.h +++ b/include/tool/tool_event.h @@ -338,9 +338,6 @@ public: if( !( m_category & aEvent.m_category ) ) return false; - if( !( m_actions & aEvent.m_actions ) ) - return false; - if( m_category == TC_COMMAND || m_category == TC_MESSAGE ) { if( (bool) m_commandStr && (bool) aEvent.m_commandStr ) @@ -349,6 +346,15 @@ public: if( (bool) m_commandId && (bool) aEvent.m_commandId ) return *m_commandId == *aEvent.m_commandId; } + + // BUGFIX: TA_ANY should match EVERYTHING, even TA_NONE (for TC_MESSAGE) + if( m_actions == TA_ANY && aEvent.m_actions == TA_NONE && aEvent.m_category == TC_MESSAGE) + return true; + + // BUGFIX: This check must happen after the TC_COMMAND check because otherwise events of + // the form { TC_COMMAND, TA_NONE } will be incorrectly skipped + if( !( m_actions & aEvent.m_actions ) ) + return false; return true; } From 0a5847c71aef07b0e1cfe04d16bed35a2d7774a2 Mon Sep 17 00:00:00 2001 From: Andrew Zonenberg Date: Sun, 12 Jul 2015 21:57:07 -0700 Subject: [PATCH 8/8] Removed a couple of trailing spaces --- include/tool/tool_event.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/tool/tool_event.h b/include/tool/tool_event.h index 3bd62ec3cf..f1cd8687d7 100644 --- a/include/tool/tool_event.h +++ b/include/tool/tool_event.h @@ -346,11 +346,11 @@ public: if( (bool) m_commandId && (bool) aEvent.m_commandId ) return *m_commandId == *aEvent.m_commandId; } - + // BUGFIX: TA_ANY should match EVERYTHING, even TA_NONE (for TC_MESSAGE) if( m_actions == TA_ANY && aEvent.m_actions == TA_NONE && aEvent.m_category == TC_MESSAGE) return true; - + // BUGFIX: This check must happen after the TC_COMMAND check because otherwise events of // the form { TC_COMMAND, TA_NONE } will be incorrectly skipped if( !( m_actions & aEvent.m_actions ) )