7
mirror of https://gitlab.com/kicad/code/kicad.git synced 2025-04-21 00:21:25 +00:00

Increment tool: use parent commit when appropriate, avoid double preview

Cloning the item into the previewe means that if we later increment
it, the preview clone doesn't update. Use the non-owning preview
interface to use the item directly in the preview.

Fixes: https://gitlab.com/kicad/code/kicad/-/issues/19433
This commit is contained in:
John Beard 2025-01-14 22:02:24 +08:00
parent 799dadeeec
commit d55877ce27
7 changed files with 80 additions and 20 deletions

View File

@ -1817,10 +1817,10 @@ int SCH_DRAWING_TOOLS::TwoClickPlace( const TOOL_EVENT& aEvent )
[&]()
{
m_view->ClearPreview();
m_view->AddToPreview( item->Clone() );
m_view->AddToPreview( item, false );
item->RunOnChildren( [&]( SCH_ITEM* aChild )
{
m_view->AddToPreview( aChild->Clone() );
m_view->AddToPreview( aChild, false );
} );
m_frame->SetMsgPanel( item );
};
@ -1853,6 +1853,8 @@ int SCH_DRAWING_TOOLS::TwoClickPlace( const TOOL_EVENT& aEvent )
ignorePrimePosition = true;
}
SCH_COMMIT commit( m_toolMgr );
// Main loop: keep receiving events
while( TOOL_EVENT* evt = Wait() )
{
@ -2056,8 +2058,6 @@ int SCH_DRAWING_TOOLS::TwoClickPlace( const TOOL_EVENT& aEvent )
}
else // ... and second click places:
{
SCH_COMMIT commit( m_toolMgr );
item->ClearFlags( IS_MOVING );
if( item->IsConnectable() )
@ -2143,6 +2143,11 @@ int SCH_DRAWING_TOOLS::TwoClickPlace( const TOOL_EVENT& aEvent )
item = nullptr;
}
}
else if( evt->IsAction( &ACTIONS::increment ) )
{
m_toolMgr->RunSynchronousAction( ACTIONS::increment, &commit,
evt->Parameter<ACTIONS::INCREMENT>() );
}
else if( evt->IsAction( &ACTIONS::duplicate )
|| evt->IsAction( &EE_ACTIONS::repeatDrawItem ) )
{

View File

@ -3031,10 +3031,23 @@ int SCH_EDIT_TOOL::Increment( const TOOL_EVENT& aEvent )
STRING_INCREMENTER incrementer;
// In schematics, it's probably less common to be operating
// on pin numbers which are uusally IOSQXZ-skippy.
// on pin numbers which are usually IOSQXZ-skippy.
incrementer.SetSkipIOSQXZ( false );
SCH_COMMIT commit( m_frame );
// If we're coming via another action like 'Move', use that commit
SCH_COMMIT localCommit( m_toolMgr );
SCH_COMMIT* commit = dynamic_cast<SCH_COMMIT*>( aEvent.Commit() );
if( !commit )
commit = &localCommit;
const auto modifyItem = [&]( EDA_ITEM& aItem )
{
if( aItem.IsNew() )
m_toolMgr->PostAction( ACTIONS::refreshPreview );
else
commit->Modify( &aItem, m_frame->GetScreen() );
};
for( EDA_ITEM* item : selection )
{
@ -3051,7 +3064,7 @@ int SCH_EDIT_TOOL::Increment( const TOOL_EVENT& aEvent )
incrementer.Increment( label.GetText(), incParam.Delta, incParam.Index );
if( newLabel )
{
commit.Modify( &label, m_frame->GetScreen() );
modifyItem( label );
label.SetText( *newLabel );
}
break;
@ -3062,7 +3075,7 @@ int SCH_EDIT_TOOL::Increment( const TOOL_EVENT& aEvent )
}
}
commit.Push( _( "Increment" ) );
commit->Push( _( "Increment" ) );
return 0;
}

View File

@ -866,6 +866,11 @@ bool SCH_MOVE_TOOL::doMoveSelection( const TOOL_EVENT& aEvent, SCH_COMMIT* aComm
{
m_toolMgr->RunSynchronousAction( EE_ACTIONS::rotateCCW, aCommit );
}
else if( evt->IsAction( &ACTIONS::increment ) )
{
m_toolMgr->RunSynchronousAction( ACTIONS::increment, aCommit,
evt->Parameter<ACTIONS::INCREMENT>() );
}
else if( evt->Action() == TA_CHOICE_MENU_CHOICE )
{
if( *evt->GetCommandId() >= ID_POPUP_SCH_SELECT_UNIT

View File

@ -141,6 +141,8 @@ int SYMBOL_EDITOR_DRAWING_TOOLS::TwoClickPlace( const TOOL_EVENT& aEvent )
ignorePrimePosition = true;
}
SCH_COMMIT commit( m_toolMgr );
// Main loop: keep receiving events
while( TOOL_EVENT* evt = Wait() )
{
@ -271,7 +273,7 @@ int SYMBOL_EDITOR_DRAWING_TOOLS::TwoClickPlace( const TOOL_EVENT& aEvent )
item->SetFlags( IS_NEW | IS_MOVING );
m_view->ClearPreview();
m_view->AddToPreview( item->Clone() );
m_view->AddToPreview( item, false );
m_selectionTool->AddItemToSel( item );
// update the cursor so it looks correct before another event
@ -283,7 +285,6 @@ int SYMBOL_EDITOR_DRAWING_TOOLS::TwoClickPlace( const TOOL_EVENT& aEvent )
// ... and second click places:
else
{
SCH_COMMIT commit( m_toolMgr );
commit.Modify( symbol, m_frame->GetScreen() );
switch( item->Type() )
@ -317,11 +318,16 @@ int SYMBOL_EDITOR_DRAWING_TOOLS::TwoClickPlace( const TOOL_EVENT& aEvent )
m_menu->ShowContextMenu( m_selectionTool->GetSelection() );
}
else if( evt->IsAction( &ACTIONS::increment ) )
{
m_toolMgr->RunSynchronousAction( ACTIONS::increment, &commit,
evt->Parameter<ACTIONS::INCREMENT>() );
}
else if( item && ( evt->IsAction( &ACTIONS::refreshPreview ) || evt->IsMotion() ) )
{
item->SetPosition( VECTOR2I( cursorPos.x, cursorPos.y ) );
m_view->ClearPreview();
m_view->AddToPreview( item->Clone() );
m_view->AddToPreview( item, false );
}
else
{

View File

@ -1135,7 +1135,20 @@ int SYMBOL_EDITOR_EDIT_TOOL::Increment( const TOOL_EVENT& aEvent )
STRING_INCREMENTER incrementer;
incrementer.SetSkipIOSQXZ( true );
SCH_COMMIT commit( m_frame );
// If we're coming via another action like 'Move', use that commit
SCH_COMMIT localCommit( m_toolMgr );
SCH_COMMIT* commit = dynamic_cast<SCH_COMMIT*>( aEvent.Commit() );
if( !commit )
commit = &localCommit;
const auto modifyItem = [&]( EDA_ITEM& aItem )
{
if( aItem.IsNew() )
m_toolMgr->PostAction( ACTIONS::refreshPreview );
else
commit->Modify( &aItem, m_frame->GetScreen() );
};
for( EDA_ITEM* item : selection )
{
@ -1155,7 +1168,7 @@ int SYMBOL_EDITOR_EDIT_TOOL::Increment( const TOOL_EVENT& aEvent )
incrementer.Increment( pin.GetNumber(), incParam.Delta, incParam.Index );
if( nextNumber )
{
commit.Modify( &pin );
modifyItem( pin );
pin.SetNumber( *nextNumber );
}
found = true;
@ -1171,7 +1184,7 @@ int SYMBOL_EDITOR_EDIT_TOOL::Increment( const TOOL_EVENT& aEvent )
incrementer.Increment( pin.GetName(), incParam.Delta, incParam.Index );
if( nextName )
{
commit.Modify( &pin );
modifyItem( pin );
pin.SetName( *nextName );
}
found = true;
@ -1187,7 +1200,7 @@ int SYMBOL_EDITOR_EDIT_TOOL::Increment( const TOOL_EVENT& aEvent )
incrementer.Increment( label.GetText(), incParam.Delta, incParam.Index );
if( newLabel )
{
commit.Modify( &label, m_frame->GetScreen() );
modifyItem( label );
label.SetText( *newLabel );
}
break;
@ -1198,7 +1211,7 @@ int SYMBOL_EDITOR_EDIT_TOOL::Increment( const TOOL_EVENT& aEvent )
}
}
commit.Push( _( "Increment" ) );
commit->Push( _( "Increment" ) );
return 0;
}

View File

@ -3015,7 +3015,20 @@ int EDIT_TOOL::Increment( const TOOL_EVENT& aEvent )
STRING_INCREMENTER incrementer;
incrementer.SetSkipIOSQXZ( true );
BOARD_COMMIT commit( this );
// If we're coming via another action like 'Move', use that commit
BOARD_COMMIT localCommit( m_toolMgr );
BOARD_COMMIT* commit = dynamic_cast<BOARD_COMMIT*>( aEvent.Commit() );
if( !commit )
commit = &localCommit;
const auto modifyItem = [&]( EDA_ITEM& aItem )
{
if( aItem.IsNew() )
m_toolMgr->PostAction( ACTIONS::refreshPreview );
else
commit->Modify( &aItem );
};
for( EDA_ITEM* item : selection )
{
@ -3038,7 +3051,7 @@ int EDIT_TOOL::Increment( const TOOL_EVENT& aEvent )
if( newNumber )
{
commit.Modify( &pad );
modifyItem( pad );
pad.SetNumber( *newNumber );
}
@ -3053,7 +3066,7 @@ int EDIT_TOOL::Increment( const TOOL_EVENT& aEvent )
if( newText )
{
commit.Modify( &text );
modifyItem( text );
text.SetText( *newText );
}
@ -3064,7 +3077,7 @@ int EDIT_TOOL::Increment( const TOOL_EVENT& aEvent )
}
}
commit.Push( _( "Increment" ) );
commit->Push( _( "Increment" ) );
return 0;
}

View File

@ -762,6 +762,11 @@ bool EDIT_TOOL::doMoveSelection( const TOOL_EVENT& aEvent, BOARD_COMMIT* aCommit
displayConstraintsMessage( hv45Mode );
evt->SetPassEvent( false );
}
else if( evt->IsAction( &ACTIONS::increment ) )
{
m_toolMgr->RunSynchronousAction( ACTIONS::increment, aCommit,
evt->Parameter<ACTIONS::INCREMENT>() );
}
else if( ZONE_FILLER_TOOL::IsZoneFillAction( evt )
|| evt->IsAction( &PCB_ACTIONS::moveExact )
|| evt->IsAction( &PCB_ACTIONS::moveWithReference )