haskal
/
kicad
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Actions with a COMMIT must be run synchronously. 

Note that "immediate" doesn't mean quite the same thing: while it will
enter the tool immediately, it won't necessarily finish the tool during
the call if the tool has an event loop.  So for something like Rotate
"immediate" and "synchronous" have the same behaviour, but for something
like Move they do not.

Fixes https://gitlab.com/kicad/code/kicad/-/issues/15085
This commit is contained in:
Jeff Young 2023-06-30 18:57:01 +01:00
parent 8d46ed3c72
commit 773e1a1ab6
12 changed files with 172 additions and 108 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
common/tool/tool_manager.cpp
View File

@ -326,9 +326,13 @@ bool TOOL_MANAGER::doRunAction( const TOOL_ACTION& aAction, bool aNow, std::any
if( m_shuttingDown )
return true;
bool handled = false;
bool retVal = false;
TOOL_EVENT event = aAction.MakeEvent();
// We initialize the SYNCHRONOUS state to finished so that tools that don't have an event
// loop won't hang if someone forgets to set the state.
std::atomic<SYNCRONOUS_TOOL_STATE> synchronousControl = STS_FINISHED;
if( event.Category() == TC_COMMAND )
event.SetMousePosition( GetCursorPosition() );
@ -338,12 +342,33 @@ bool TOOL_MANAGER::doRunAction( const TOOL_ACTION& aAction, bool aNow, std::any
// Pass the commit (if any)
if( aCommit )
{
event.SetSynchronous( &synchronousControl );
event.SetCommit( aCommit );
}
if( aNow )
{
TOOL_STATE* current = m_activeState;
handled = processEvent( event );
if( aCommit )
{
// An event with a commit must be run synchronously
processEvent( event );
while( synchronousControl == STS_RUNNING )
{
wxMilliSleep( 50 );
wxYield();
}
retVal = synchronousControl != STS_CANCELLED;
}
else
{
retVal = processEvent( event );
}
setActiveState( current );
UpdateUI( event );
}
@ -352,7 +377,7 @@ bool TOOL_MANAGER::doRunAction( const TOOL_ACTION& aAction, bool aNow, std::any
PostEvent( event );
}
return handled;
return retVal;
}

37
eeschema/tools/sch_edit_tool.cpp
View File

@ -1246,6 +1246,7 @@ int SCH_EDIT_TOOL::RepeatDrawItem( const TOOL_EVENT& aEvent )
{
SCH_ITEM* newItem = item->Duplicate();
EESCHEMA_SETTINGS* cfg = Pgm().GetSettingsManager().GetAppSettings<EESCHEMA_SETTINGS>();
bool restore_state = false;
if( SCH_LABEL_BASE* label = dynamic_cast<SCH_LABEL_BASE*>( newItem ) )
{
@ -1288,33 +1289,27 @@ int SCH_EDIT_TOOL::RepeatDrawItem( const TOOL_EVENT& aEvent )
annotateStartNum, false, false, reporter );
}
m_toolMgr->RunAction( EE_ACTIONS::move, &commit );
restore_state = !m_toolMgr->RunSynchronousAction( EE_ACTIONS::move, &commit );
}
while( m_toolMgr->GetTool<SCH_MOVE_TOOL>()->IsToolActive() )
if( restore_state )
{
wxMilliSleep( 50 );
wxYield();
commit.Revert();
}
}
else
{
newItems.Add( newItem );
newItem->ClearFlags();
}
if( !newItems.Empty() )
{
SCH_LINE_WIRE_BUS_TOOL* lwbTool = m_toolMgr->GetTool<SCH_LINE_WIRE_BUS_TOOL>();
lwbTool->TrimOverLappingWires( &commit, &newItems );
lwbTool->AddJunctionsIfNeeded( &commit, &newItems );
m_frame->SchematicCleanUp( &commit );
m_frame->GetCanvas()->Refresh();
commit.Push( _( "Repeat Item" ) );
}
m_frame->GetCanvas()->Refresh();
if( !commit.Empty() )
commit.Push( _( "Repeat Item" ) );
}
if( !newItems.Empty() )
m_frame->SaveCopyForRepeatItem( static_cast<SCH_ITEM*>( newItems[0] ) );
@ -2410,16 +2405,10 @@ int SCH_EDIT_TOOL::BreakWire( const TOOL_EVENT& aEvent )
m_frame->TestDanglingEnds();
m_frame->GetCanvas()->Refresh();
m_toolMgr->RunAction( EE_ACTIONS::drag, &commit );
while( m_toolMgr->GetTool<SCH_MOVE_TOOL>()->IsToolActive() )
{
wxMilliSleep( 50 );
wxYield();
}
if( !commit.Empty() )
if( m_toolMgr->RunSynchronousAction( EE_ACTIONS::drag, &commit, isSlice ) )
commit.Push( isSlice ? _( "Slice Wire" ) : _( "Break Wire" ) );
else
commit.Revert();
}
return 0;

12
eeschema/tools/sch_editor_control.cpp
View File

@ -1952,16 +1952,10 @@ int SCH_EDITOR_CONTROL::Paste( const TOOL_EVENT& aEvent )
selection.SetReferencePoint( item->GetPosition() );
}
m_toolMgr->RunAction( EE_ACTIONS::move, &commit );
while( m_toolMgr->GetTool<SCH_MOVE_TOOL>()->IsToolActive() )
{
wxMilliSleep( 50 );
wxYield();
}
if( !commit.Empty() )
if( m_toolMgr->RunSynchronousAction( EE_ACTIONS::move, &commit ) )
commit.Push( _( "Paste" ) );
else
commit.Revert();
}
return 0;

106
eeschema/tools/sch_move_tool.cpp
View File

@ -355,19 +355,6 @@ void SCH_MOVE_TOOL::orthoLineDrag( SCH_COMMIT* aCommit, SCH_LINE* line, const VE
int SCH_MOVE_TOOL::Main( const TOOL_EVENT& aEvent )
{
EESCHEMA_SETTINGS* cfg = Pgm().GetSettingsManager().GetAppSettings<EESCHEMA_SETTINGS>();
KIGFX::VIEW_CONTROLS* controls = getViewControls();
EE_GRID_HELPER grid( m_toolMgr );
bool wasDragging = m_moveInProgress && m_isDrag;
bool isSlice = false;
SCH_COMMIT localCommit( m_toolMgr );
SCH_COMMIT* commit = dynamic_cast<SCH_COMMIT*>( aEvent.Commit() );
if( !commit )
commit = &localCommit;
m_anchorPos.reset();
if( aEvent.IsAction( &EE_ACTIONS::move ) )
{
m_isDrag = false;
@ -375,17 +362,55 @@ int SCH_MOVE_TOOL::Main( const TOOL_EVENT& aEvent )
else if( aEvent.IsAction( &EE_ACTIONS::drag ) )
{
m_isDrag = true;
isSlice = commit != &localCommit;
}
else if( aEvent.IsAction( &EE_ACTIONS::moveActivate ) )
{
EESCHEMA_SETTINGS* cfg = Pgm().GetSettingsManager().GetAppSettings<EESCHEMA_SETTINGS>();
m_isDrag = !cfg->m_Input.drag_is_move;
}
else
{
return false;
}
if( SCH_COMMIT* commit = dynamic_cast<SCH_COMMIT*>( aEvent.Commit() ) )
{
bool isSlice = false;
if( m_isDrag )
isSlice = aEvent.Parameter<bool>();
wxCHECK( aEvent.SynchronousState(), 0 );
aEvent.SynchronousState()->store( STS_RUNNING );
if( doMoveSelection( aEvent, commit, isSlice ) )
aEvent.SynchronousState()->store( STS_FINISHED );
else
aEvent.SynchronousState()->store( STS_CANCELLED );
}
else
{
SCH_COMMIT localCommit( m_toolMgr );
if( doMoveSelection( aEvent, &localCommit, false ) )
localCommit.Push( m_isDrag ? _( "Drag" ) : _( "Move" ) );
else
localCommit.Revert();
}
return 0;
}
bool SCH_MOVE_TOOL::doMoveSelection( const TOOL_EVENT& aEvent, SCH_COMMIT* aCommit, bool aIsSlice )
{
EESCHEMA_SETTINGS* cfg = Pgm().GetSettingsManager().GetAppSettings<EESCHEMA_SETTINGS>();
KIGFX::VIEW_CONTROLS* controls = getViewControls();
EE_GRID_HELPER grid( m_toolMgr );
bool wasDragging = m_moveInProgress && m_isDrag;
m_anchorPos.reset();
if( m_moveInProgress )
{
if( m_isDrag != wasDragging )
@ -396,16 +421,7 @@ int SCH_MOVE_TOOL::Main( const TOOL_EVENT& aEvent )
{
// Reset the selected items so we can start again with the current m_isDrag
// state.
try
{
commit->Revert();
}
catch( const IO_ERROR& exc )
{
wxLogWarning( wxS( "Exception \"%s\" rolling back schematic undo ocurred." ),
exc.What() );
return 1;
}
aCommit->Revert();
m_selectionTool->RemoveItemsFromSel( &m_dragAdditions, QUIET_MODE );
m_anchorPos = m_cursor - m_moveOffset;
@ -423,11 +439,11 @@ int SCH_MOVE_TOOL::Main( const TOOL_EVENT& aEvent )
m_toolMgr->PostAction( ACTIONS::cursorClick );
}
return 0;
return false;
}
if( m_inMoveTool ) // Must come after m_moveInProgress checks above...
return 0;
return false;
REENTRANCY_GUARD guard( &m_inMoveTool );
@ -451,7 +467,7 @@ int SCH_MOVE_TOOL::Main( const TOOL_EVENT& aEvent )
// Note that it's important to go through push/pop even when the selection is empty.
// This keeps other tools from having to special-case an empty move.
m_frame->PopTool( aEvent );
return 0;
return false;
}
bool restore_state = false;
@ -502,7 +518,7 @@ int SCH_MOVE_TOOL::Main( const TOOL_EVENT& aEvent )
// Drag of split items start over top of their other segment so
// we want to skip grabbing the segments we split from
if( m_isDrag && !isSlice )
if( m_isDrag && !aIsSlice )
{
EDA_ITEMS connectedDragItems;
@ -533,7 +549,7 @@ int SCH_MOVE_TOOL::Main( const TOOL_EVENT& aEvent )
}
for( const VECTOR2I& point : connections )
getConnectedDragItems( commit, item, point, connectedDragItems );
getConnectedDragItems( aCommit, item, point, connectedDragItems );
}
// Go back and get all label connections now that we can test for drag-selected
@ -541,7 +557,7 @@ int SCH_MOVE_TOOL::Main( const TOOL_EVENT& aEvent )
for( SCH_ITEM* item : stageTwo )
{
for( const VECTOR2I& point : item->GetConnectionPoints() )
getConnectedDragItems( commit, item, point, connectedDragItems );
getConnectedDragItems( aCommit, item, point, connectedDragItems );
}
for( EDA_ITEM* item : connectedDragItems )
@ -608,7 +624,7 @@ int SCH_MOVE_TOOL::Main( const TOOL_EVENT& aEvent )
}
else
{
commit->Modify( (SCH_ITEM*) item, m_frame->GetScreen() );
aCommit->Modify( (SCH_ITEM*) item, m_frame->GetScreen() );
}
SCH_ITEM* schItem = (SCH_ITEM*) item;
@ -751,7 +767,7 @@ int SCH_MOVE_TOOL::Main( const TOOL_EVENT& aEvent )
&& line
&& line->HasFlag( STARTPOINT ) != line->HasFlag( ENDPOINT ) )
{
orthoLineDrag( commit, line, splitDelta, xBendCount, yBendCount, grid );
orthoLineDrag( aCommit, line, splitDelta, xBendCount, yBendCount, grid );
}
// Move all other items normally, including the selected end of partially
@ -842,11 +858,11 @@ int SCH_MOVE_TOOL::Main( const TOOL_EVENT& aEvent )
}
else if( evt->IsAction( &EE_ACTIONS::rotateCW ) )
{
m_toolMgr->RunAction( EE_ACTIONS::rotateCW, &commit );
m_toolMgr->RunSynchronousAction( EE_ACTIONS::rotateCW, aCommit );
}
else if( evt->IsAction( &EE_ACTIONS::rotateCCW ) )
{
m_toolMgr->RunAction( EE_ACTIONS::rotateCCW, &commit );
m_toolMgr->RunSynchronousAction( EE_ACTIONS::rotateCCW, aCommit );
}
else if( evt->Action() == TA_CHOICE_MENU_CHOICE )
{
@ -894,7 +910,7 @@ int SCH_MOVE_TOOL::Main( const TOOL_EVENT& aEvent )
} while( ( evt = Wait() ) ); //Should be assignment not equality test
// Create a selection of original selection, drag selected/changed items, and new
// bend lines for later before we clear them in the commit. We'll need these
// bend lines for later before we clear them in the aCommit. We'll need these
// to check for new junctions needed, etc.
EE_SELECTION selectionCopy( selection );
@ -908,7 +924,7 @@ int SCH_MOVE_TOOL::Main( const TOOL_EVENT& aEvent )
for( SCH_LINE* newLine : m_newDragLines )
{
newLine->ClearEditFlags();
commit->Added( newLine, m_frame->GetScreen() );
aCommit->Added( newLine, m_frame->GetScreen() );
}
// These lines have been changed, but aren't selected. We need
@ -931,7 +947,6 @@ int SCH_MOVE_TOOL::Main( const TOOL_EVENT& aEvent )
if( restore_state )
{
m_selectionTool->RemoveItemsFromSel( &m_dragAdditions, QUIET_MODE );
commit->Revert();
}
else
{
@ -951,26 +966,23 @@ int SCH_MOVE_TOOL::Main( const TOOL_EVENT& aEvent )
for( const DANGLING_END_ITEM& it : internalPoints )
{
if( m_frame->GetScreen()->IsExplicitJunctionNeeded( it.GetPosition()) )
m_frame->AddJunction( commit, m_frame->GetScreen(), it.GetPosition() );
m_frame->AddJunction( aCommit, m_frame->GetScreen(), it.GetPosition() );
}
SCH_LINE_WIRE_BUS_TOOL* lwbTool = m_toolMgr->GetTool<SCH_LINE_WIRE_BUS_TOOL>();
lwbTool->TrimOverLappingWires( commit, &selectionCopy );
lwbTool->AddJunctionsIfNeeded( commit, &selectionCopy );
lwbTool->TrimOverLappingWires( aCommit, &selectionCopy );
lwbTool->AddJunctionsIfNeeded( aCommit, &selectionCopy );
// This needs to run prior to `RecalculateConnections` because we need to identify
// the lines that are newly dangling
if( m_isDrag && !isSlice )
trimDanglingLines( commit );
if( m_isDrag && !aIsSlice )
trimDanglingLines( aCommit );
// Auto-rotate any moved labels
for( EDA_ITEM* item : selection )
m_frame->AutoRotateItem( m_frame->GetScreen(), static_cast<SCH_ITEM*>( item ) );
m_frame->SchematicCleanUp( commit );
if( commit == &localCommit )
commit->Push( m_isDrag ? _( "Drag" ) : _( "Move" ) );
m_frame->SchematicCleanUp( aCommit );
}
for( EDA_ITEM* item : m_frame->GetScreen()->Items() )
@ -991,7 +1003,7 @@ int SCH_MOVE_TOOL::Main( const TOOL_EVENT& aEvent )
m_moveInProgress = false;
m_frame->PopTool( aEvent );
return 0;
return !restore_state;
}

2
eeschema/tools/sch_move_tool.h
View File

@ -63,6 +63,8 @@ public:
int AlignElements( const TOOL_EVENT& aEvent );
private:
bool doMoveSelection( const TOOL_EVENT& aEvent, SCH_COMMIT* aCommit, bool aIsSlice );
void moveItem( EDA_ITEM* aItem, const VECTOR2I& aDelta );
///< Find additional items for a drag operation.

10
eeschema/tools/symbol_editor_edit_tool.cpp
View File

@ -817,8 +817,11 @@ int SYMBOL_EDITOR_EDIT_TOOL::Paste( const TOOL_EVENT& aEvent )
if( !selection.Empty() )
{
selection.SetReferencePoint( getViewControls()->GetCursorPosition( true ) );
m_toolMgr->RunAction( EE_ACTIONS::move, &commit );
if( m_toolMgr->RunSynchronousAction( EE_ACTIONS::move, &commit ) )
commit.Push( _( "Paste" ) );
else
commit.Revert();
}
return 0;
@ -874,8 +877,11 @@ int SYMBOL_EDITOR_EDIT_TOOL::Duplicate( const TOOL_EVENT& aEvent )
m_toolMgr->RunAction<EDA_ITEMS*>( EE_ACTIONS::addItemsToSel, &newItems );
selection.SetReferencePoint( mapCoords( getViewControls()->GetCursorPosition( true ) ) );
m_toolMgr->RunAction( EE_ACTIONS::move, &commit );
if( m_toolMgr->RunSynchronousAction( EE_ACTIONS::move, &commit ) )
commit.Push( _( "Duplicate" ) );
else
commit.Revert();
return 0;
}

15
include/tool/tool_event.h
View File

@ -152,6 +152,13 @@ enum CONTEXT_MENU_TRIGGER
CMENU_OFF // Never
};
enum SYNCRONOUS_TOOL_STATE
{
STS_RUNNING,
STS_FINISHED,
STS_CANCELLED
};
/**
* Generic, UI-independent tool event.
*/
@ -173,6 +180,7 @@ public:
m_mouseButtons( 0 ),
m_keyCode( 0 ),
m_modifiers( 0 ),
m_synchronousState( nullptr ),
m_commit( nullptr ),
m_firstResponder( nullptr )
{
@ -187,6 +195,7 @@ public:
m_mouseButtons( 0 ),
m_keyCode( 0 ),
m_modifiers( 0 ),
m_synchronousState( nullptr ),
m_commit( nullptr ),
m_firstResponder( nullptr )
{
@ -219,6 +228,7 @@ public:
m_mouseButtons( 0 ),
m_keyCode( 0 ),
m_modifiers( 0 ),
m_synchronousState( nullptr ),
m_commit( nullptr ),
m_firstResponder( nullptr )
{
@ -257,6 +267,9 @@ public:
bool IsReactivate() const { return m_reactivate; }
void SetReactivate( bool aReactivate = true ) { m_reactivate = aReactivate; }
void SetSynchronous( std::atomic<SYNCRONOUS_TOOL_STATE>* aState ) { m_synchronousState = aState; }
std::atomic<SYNCRONOUS_TOOL_STATE>* SynchronousState() const { return m_synchronousState; }
void SetCommit( COMMIT* aCommit ) { m_commit = aCommit; }
COMMIT* Commit() const { return m_commit; }
@ -583,6 +596,8 @@ private:
///< State of key modifiers (Ctrl/Alt/etc.)
int m_modifiers;
std::atomic<SYNCRONOUS_TOOL_STATE>* m_synchronousState;
/// Commit the tool handling the event should add to
COMMIT* m_commit;

11
include/tool/tool_manager.h
View File

@ -190,7 +190,16 @@ public:
* @param aCommit is the commit object the tool handling the action should add the new edits to
* @return True if the action was handled immediately
*/
bool RunAction( const TOOL_ACTION& aAction, COMMIT* aCommit )
template<typename T>
bool RunSynchronousAction( const TOOL_ACTION& aAction, COMMIT* aCommit, T aParam )
{
// Use a cast to ensure the proper type is stored inside the parameter
std::any a( static_cast<T>( aParam ) );
return doRunAction( aAction, true, a, aCommit );
}
bool RunSynchronousAction( const TOOL_ACTION& aAction, COMMIT* aCommit )
{
// Default initialize the parameter argument to an empty std::any
std::any a;

2
pcbnew/tools/edit_tool.cpp
View File

@ -2209,7 +2209,7 @@ int EDIT_TOOL::Duplicate( const TOOL_EVENT& aEvent )
(int) new_items.size() ) );
// If items were duplicated, pick them up
if( DoMoveSelection( aEvent, &commit ) )
if( doMoveSelection( aEvent, &commit ) )
commit.Push( _( "Duplicate" ) );
else
commit.Revert();

4
pcbnew/tools/edit_tool.h
View File

@ -154,8 +154,6 @@ public:
*/
int CreateArray( const TOOL_EVENT& aEvent );
bool DoMoveSelection( const TOOL_EVENT& aEvent, BOARD_COMMIT* aCommit );
/**
* A selection filter which prunes the selection to contain only items of type #PCB_MODULE_T.
*/
@ -197,6 +195,8 @@ private:
bool pickReferencePoint( const wxString& aTooltip, const wxString& aSuccessMessage,
const wxString& aCanceledMessage, VECTOR2I& aReferencePoint );
bool doMoveSelection( const TOOL_EVENT& aEvent, BOARD_COMMIT* aCommit );
///< Rebuilds the ratsnest for operations that require it outside the commit rebuild
void rebuildConnectivity();

25
pcbnew/tools/edit_tool_move_fct.cpp
View File

@ -195,7 +195,7 @@ int EDIT_TOOL::PackAndMoveFootprints( const TOOL_EVENT& aEvent )
SpreadFootprints( &footprintsToPack, footprintsBbox.Normalize().GetOrigin(), false );
if( DoMoveSelection( aEvent, &commit ) )
if( doMoveSelection( aEvent, &commit ) )
commit.Push( _( "Pack footprints" ) );
else
commit.Revert();
@ -212,12 +212,25 @@ int EDIT_TOOL::Move( const TOOL_EVENT& aEvent )
return 0;
}
BOARD_COMMIT commit( this );
if( BOARD_COMMIT* commit = dynamic_cast<BOARD_COMMIT*>( aEvent.Commit() ) )
{
wxCHECK( aEvent.SynchronousState(), 0 );
aEvent.SynchronousState()->store( STS_RUNNING );
if( DoMoveSelection( aEvent, &commit ) )
commit.Push( _( "Move" ) );
if( doMoveSelection( aEvent, commit ) )
aEvent.SynchronousState()->store( STS_FINISHED );
else
commit.Revert();
aEvent.SynchronousState()->store( STS_CANCELLED );
}
else
{
BOARD_COMMIT localCommit( this );
if( doMoveSelection( aEvent, &localCommit ) )
localCommit.Push( _( "Move" ) );
else
localCommit.Revert();
}
return 0;
}
@ -257,7 +270,7 @@ VECTOR2I EDIT_TOOL::getSafeMovement( const VECTOR2I& aMovement, const BOX2I& aSo
}
bool EDIT_TOOL::DoMoveSelection( const TOOL_EVENT& aEvent, BOARD_COMMIT* aCommit )
bool EDIT_TOOL::doMoveSelection( const TOOL_EVENT& aEvent, BOARD_COMMIT* aCommit )
{
bool moveWithReference = aEvent.IsAction( &PCB_ACTIONS::moveWithReference );
bool moveIndividually = aEvent.IsAction( &PCB_ACTIONS::moveIndividually );

3
pcbnew/tools/pcb_control.cpp
View File

@ -1115,7 +1115,6 @@ bool PCB_CONTROL::placeBoardItems( BOARD_COMMIT* aCommit, std::vector<BOARD_ITEM
m_toolMgr->RunAction( PCB_ACTIONS::selectionClear );
PCB_SELECTION_TOOL* selectionTool = m_toolMgr->GetTool<PCB_SELECTION_TOOL>();
EDIT_TOOL* editTool = m_toolMgr->GetTool<EDIT_TOOL>();
std::vector<BOARD_ITEM*> itemsToSel;
itemsToSel.reserve( aItems.size() );
@ -1194,7 +1193,7 @@ bool PCB_CONTROL::placeBoardItems( BOARD_COMMIT* aCommit, std::vector<BOARD_ITEM
m_toolMgr->ProcessEvent( EVENTS::SelectedEvent );
return editTool->DoMoveSelection( PCB_ACTIONS::move.MakeEvent(), aCommit );
return m_toolMgr->RunSynchronousAction( PCB_ACTIONS::move, aCommit );
}
return true;