From 9d7db3d13588fba4a6a81b1825fdf8daba5b7e0b Mon Sep 17 00:00:00 2001 From: Jeff Young Date: Sun, 31 Aug 2025 14:10:25 +0100 Subject: [PATCH] Better interactive feedback. Don't wait for mouse to move before refreshing after a command. Also fixes a crash bug deleting an item after an Unstage() (the Unstage() has already deleted the item). Also fixes a crash when changing label type during creation of a label produces a mess. Also Keep cursor aligned to grid when not warping back to original point. Fixes https://gitlab.com/kicad/code/kicad/-/issues/16828 --- eeschema/tools/sch_actions.cpp | 6 +- eeschema/tools/sch_actions.h | 2 +- eeschema/tools/sch_drawing_tools.cpp | 65 ++++++++++++++++--- eeschema/tools/sch_edit_tool.cpp | 9 +-- eeschema/tools/sch_move_tool.cpp | 14 +++- .../tools/symbol_editor_drawing_tools.cpp | 7 +- 6 files changed, 81 insertions(+), 22 deletions(-) diff --git a/eeschema/tools/sch_actions.cpp b/eeschema/tools/sch_actions.cpp index 2031b3ded4..cd7f91aeed 100644 --- a/eeschema/tools/sch_actions.cpp +++ b/eeschema/tools/sch_actions.cpp @@ -856,8 +856,10 @@ TOOL_ACTION SCH_ACTIONS::toLabel( TOOL_ACTION_ARGS() .Flags( AF_NONE ) .Parameter( SCH_LABEL_T ) ); -TOOL_ACTION SCH_ACTIONS::toCLabel( TOOL_ACTION_ARGS() - .Name( "eeschema.InteractiveEdit.toCLabel" ) +TOOL_ACTION SCH_ACTIONS::toDLabel(TOOL_ACTION_ARGS() + .Name( "eeschema.InteractiveEdit.toCLabel" ) // Old name based on netClass label. + // There's no sense losing hotkey assignments, so we + // leave it as-is) .Scope( AS_GLOBAL ) .FriendlyName( _( "Change to Directive Label" ) ) .Tooltip( _( "Change existing item to a directive label" ) ) diff --git a/eeschema/tools/sch_actions.h b/eeschema/tools/sch_actions.h index b016931c37..6839f53605 100644 --- a/eeschema/tools/sch_actions.h +++ b/eeschema/tools/sch_actions.h @@ -130,7 +130,7 @@ public: static TOOL_ACTION cycleBodyStyle; static TOOL_ACTION editSymbolUnit; static TOOL_ACTION toLabel; - static TOOL_ACTION toCLabel; + static TOOL_ACTION toDLabel; static TOOL_ACTION toHLabel; static TOOL_ACTION toGLabel; static TOOL_ACTION toText; diff --git a/eeschema/tools/sch_drawing_tools.cpp b/eeschema/tools/sch_drawing_tools.cpp index 4cd0378779..3801ca6f71 100644 --- a/eeschema/tools/sch_drawing_tools.cpp +++ b/eeschema/tools/sch_drawing_tools.cpp @@ -567,10 +567,30 @@ int SCH_DRAWING_TOOLS::PlaceSymbol( const TOOL_EVENT& aEvent ) { cleanup(); } - else if( symbol && evt->IsAction( &ACTIONS::redo ) ) + else if( symbol && ( evt->IsAction( &ACTIONS::redo ) + || evt->IsAction( &SCH_ACTIONS::editWithLibEdit ) + || evt->IsAction( &SCH_ACTIONS::changeSymbol ) ) ) { wxBell(); } + else if( symbol && ( evt->IsAction( &SCH_ACTIONS::properties ) + || evt->IsAction( &SCH_ACTIONS::editReference ) + || evt->IsAction( &SCH_ACTIONS::editValue ) + || evt->IsAction( &SCH_ACTIONS::editFootprint ) + || evt->IsAction( &SCH_ACTIONS::autoplaceFields ) + || evt->IsAction( &SCH_ACTIONS::cycleBodyStyle ) + || evt->IsAction( &SCH_ACTIONS::setExcludeFromBOM ) + || evt->IsAction( &SCH_ACTIONS::setExcludeFromBoard ) + || evt->IsAction( &SCH_ACTIONS::setExcludeFromSimulation ) + || evt->IsAction( &SCH_ACTIONS::setDNP ) + || evt->IsAction( &SCH_ACTIONS::rotateCW ) + || evt->IsAction( &SCH_ACTIONS::rotateCCW ) + || evt->IsAction( &SCH_ACTIONS::mirrorV ) + || evt->IsAction( &SCH_ACTIONS::mirrorH ) ) ) + { + m_toolMgr->PostAction( ACTIONS::refreshPreview ); + evt->SetPassEvent(); + } else { evt->SetPassEvent(); @@ -1946,7 +1966,6 @@ int SCH_DRAWING_TOOLS::TwoClickPlace( const TOOL_EVENT& aEvent ) setCursor(); }; - Activate(); // Must be done after Activate() so that it gets set into the correct context @@ -2156,7 +2175,10 @@ int SCH_DRAWING_TOOLS::TwoClickPlace( const TOOL_EVENT& aEvent ) if( item ) prepItemForPlacement( item, cursorPos ); - controls->SetCursorPosition( cursorPos, false ); + if( m_frame->GetMoveWarpsCursor() ) + controls->SetCursorPosition( cursorPos, false ); + + m_toolMgr->PostAction( ACTIONS::refreshPreview ); } else // ... and second click places: { @@ -2289,6 +2311,25 @@ int SCH_DRAWING_TOOLS::TwoClickPlace( const TOOL_EVENT& aEvent ) { wxBell(); } + else if( item && ( evt->IsAction( &SCH_ACTIONS::toDLabel ) + || evt->IsAction( &SCH_ACTIONS::toGLabel ) + || evt->IsAction( &SCH_ACTIONS::toHLabel ) + || evt->IsAction( &SCH_ACTIONS::toLabel ) + || evt->IsAction( &SCH_ACTIONS::toText ) + || evt->IsAction( &SCH_ACTIONS::toTextBox ) ) ) + { + wxBell(); + } + else if( item && ( evt->IsAction( &SCH_ACTIONS::properties ) + || evt->IsAction( &SCH_ACTIONS::autoplaceFields ) + || evt->IsAction( &SCH_ACTIONS::rotateCW ) + || evt->IsAction( &SCH_ACTIONS::rotateCCW ) + || evt->IsAction( &SCH_ACTIONS::mirrorV ) + || evt->IsAction( &SCH_ACTIONS::mirrorH ) ) ) + { + m_toolMgr->PostAction( ACTIONS::refreshPreview ); + evt->SetPassEvent(); + } else { evt->SetPassEvent(); @@ -2464,9 +2505,18 @@ int SCH_DRAWING_TOOLS::DrawShape( const TOOL_EVENT& aEvent ) || isSyntheticClick || evt->IsAction( &ACTIONS::finishInteractive ) ) ) { - if( evt->IsDblClick( BUT_LEFT ) - || evt->IsAction( &ACTIONS::finishInteractive ) - || !item->ContinueEdit( cursorPos ) ) + bool finished = false; + + if( evt->IsDblClick( BUT_LEFT ) || evt->IsAction( &ACTIONS::finishInteractive ) ) + { + finished = true; + } + else + { + finished = !item->ContinueEdit( cursorPos ); + } + + if( finished ) { item->EndEdit(); item->ClearEditFlags(); @@ -2514,8 +2564,7 @@ int SCH_DRAWING_TOOLS::DrawShape( const TOOL_EVENT& aEvent ) m_toolMgr->PostAction( ACTIONS::activatePointEditor ); } } - else if( evt->IsAction( &ACTIONS::duplicate ) - || evt->IsAction( &SCH_ACTIONS::repeatDrawItem ) ) + else if( evt->IsAction( &ACTIONS::duplicate ) || evt->IsAction( &SCH_ACTIONS::repeatDrawItem ) ) { if( item ) { diff --git a/eeschema/tools/sch_edit_tool.cpp b/eeschema/tools/sch_edit_tool.cpp index 84157447ed..93c9328f8d 100644 --- a/eeschema/tools/sch_edit_tool.cpp +++ b/eeschema/tools/sch_edit_tool.cpp @@ -640,7 +640,7 @@ bool SCH_EDIT_TOOL::Init() menu->SetIcon( BITMAPS::right ); menu->AddItem( SCH_ACTIONS::toLabel, toLabelCondition ); - menu->AddItem( SCH_ACTIONS::toCLabel, toCLabelCondition ); + menu->AddItem(SCH_ACTIONS::toDLabel, toCLabelCondition ); menu->AddItem( SCH_ACTIONS::toHLabel, toHLabelCondition ); menu->AddItem( SCH_ACTIONS::toGLabel, toGLabelCondition ); menu->AddItem( SCH_ACTIONS::toText, toTextCondition ); @@ -2718,14 +2718,9 @@ int SCH_EDIT_TOOL::ChangeTextType( const TOOL_EVENT& aEvent ) m_frame->RemoveFromScreen( item, m_frame->GetScreen() ); if( commit->GetStatus( item, m_frame->GetScreen() ) == CHT_ADD ) - { commit->Unstage( item, m_frame->GetScreen() ); - delete item; - } else - { commit->Removed( item, m_frame->GetScreen() ); - } m_frame->AddToScreen( newtext, m_frame->GetScreen() ); commit->Added( newtext, m_frame->GetScreen() ); @@ -3184,7 +3179,7 @@ void SCH_EDIT_TOOL::setTransitions() Go( &SCH_EDIT_TOOL::ChangeTextType, SCH_ACTIONS::toLabel.MakeEvent() ); Go( &SCH_EDIT_TOOL::ChangeTextType, SCH_ACTIONS::toHLabel.MakeEvent() ); Go( &SCH_EDIT_TOOL::ChangeTextType, SCH_ACTIONS::toGLabel.MakeEvent() ); - Go( &SCH_EDIT_TOOL::ChangeTextType, SCH_ACTIONS::toCLabel.MakeEvent() ); + Go( &SCH_EDIT_TOOL::ChangeTextType, SCH_ACTIONS::toDLabel.MakeEvent() ); Go( &SCH_EDIT_TOOL::ChangeTextType, SCH_ACTIONS::toText.MakeEvent() ); Go( &SCH_EDIT_TOOL::ChangeTextType, SCH_ACTIONS::toTextBox.MakeEvent() ); Go( &SCH_EDIT_TOOL::JustifyText, ACTIONS::leftJustify.MakeEvent() ); diff --git a/eeschema/tools/sch_move_tool.cpp b/eeschema/tools/sch_move_tool.cpp index d414ba154d..989b292f3a 100644 --- a/eeschema/tools/sch_move_tool.cpp +++ b/eeschema/tools/sch_move_tool.cpp @@ -929,10 +929,12 @@ bool SCH_MOVE_TOOL::doMoveSelection( const TOOL_EVENT& aEvent, SCH_COMMIT* aComm else if( evt->IsAction( &SCH_ACTIONS::rotateCW ) ) { m_toolMgr->RunSynchronousAction( SCH_ACTIONS::rotateCW, aCommit ); + m_toolMgr->PostAction( ACTIONS::refreshPreview ); } else if( evt->IsAction( &SCH_ACTIONS::rotateCCW ) ) { m_toolMgr->RunSynchronousAction( SCH_ACTIONS::rotateCCW, aCommit ); + m_toolMgr->PostAction( ACTIONS::refreshPreview ); } else if( evt->IsAction( &ACTIONS::increment ) ) { @@ -940,30 +942,38 @@ bool SCH_MOVE_TOOL::doMoveSelection( const TOOL_EVENT& aEvent, SCH_COMMIT* aComm m_toolMgr->RunSynchronousAction( ACTIONS::increment, aCommit, evt->Parameter() ); else m_toolMgr->RunSynchronousAction( ACTIONS::increment, aCommit, ACTIONS::INCREMENT { 1, 0 } ); + + m_toolMgr->PostAction( ACTIONS::refreshPreview ); } - else if( evt->IsAction( &SCH_ACTIONS::toCLabel ) ) + else if( evt->IsAction( &SCH_ACTIONS::toDLabel ) ) { - m_toolMgr->RunSynchronousAction( SCH_ACTIONS::toCLabel, aCommit ); + m_toolMgr->RunSynchronousAction(SCH_ACTIONS::toDLabel, aCommit ); + m_toolMgr->PostAction( ACTIONS::refreshPreview ); } else if( evt->IsAction( &SCH_ACTIONS::toGLabel ) ) { m_toolMgr->RunSynchronousAction( SCH_ACTIONS::toGLabel, aCommit ); + m_toolMgr->PostAction( ACTIONS::refreshPreview ); } else if( evt->IsAction( &SCH_ACTIONS::toHLabel ) ) { m_toolMgr->RunSynchronousAction( SCH_ACTIONS::toHLabel, aCommit ); + m_toolMgr->PostAction( ACTIONS::refreshPreview ); } else if( evt->IsAction( &SCH_ACTIONS::toLabel ) ) { m_toolMgr->RunSynchronousAction( SCH_ACTIONS::toLabel, aCommit ); + m_toolMgr->PostAction( ACTIONS::refreshPreview ); } else if( evt->IsAction( &SCH_ACTIONS::toText ) ) { m_toolMgr->RunSynchronousAction( SCH_ACTIONS::toText, aCommit ); + m_toolMgr->PostAction( ACTIONS::refreshPreview ); } else if( evt->IsAction( &SCH_ACTIONS::toTextBox ) ) { m_toolMgr->RunSynchronousAction( SCH_ACTIONS::toTextBox, aCommit ); + m_toolMgr->PostAction( ACTIONS::refreshPreview ); } else if( evt->Action() == TA_CHOICE_MENU_CHOICE ) { diff --git a/eeschema/tools/symbol_editor_drawing_tools.cpp b/eeschema/tools/symbol_editor_drawing_tools.cpp index 9f75c12c74..aedf877ee0 100644 --- a/eeschema/tools/symbol_editor_drawing_tools.cpp +++ b/eeschema/tools/symbol_editor_drawing_tools.cpp @@ -267,7 +267,7 @@ int SYMBOL_EDITOR_DRAWING_TOOLS::TwoClickPlace( const TOOL_EVENT& aEvent ) else { getViewControls()->PinCursorInsideNonAutoscrollArea( true ); - cursorPos = getViewControls()->GetMousePosition(); + cursorPos = grid.Align( getViewControls()->GetMousePosition(), grid.GetItemGrid( item ) ); } if( item ) @@ -283,7 +283,10 @@ int SYMBOL_EDITOR_DRAWING_TOOLS::TwoClickPlace( const TOOL_EVENT& aEvent ) setCursor(); } - controls->SetCursorPosition( cursorPos, false ); + if( m_frame->GetMoveWarpsCursor() ) + controls->SetCursorPosition( cursorPos, false ); + + m_toolMgr->PostAction( ACTIONS::refreshPreview ); } // ... and second click places: else