Move commit handling outside of ZoneFiller.

This allows us to rever the commit on a cancel.

Fixes https://gitlab.com/kicad/code/kicad/issues/5107
This commit is contained in:
Jeff Young 2020-08-12 17:39:57 +01:00
parent fad18dccd5
commit a6d44676b3
7 changed files with 113 additions and 111 deletions

View File

@ -51,7 +51,7 @@
#include <convert_basic_shapes_to_polygon.h>
#include <geometry/geometry_utils.h>
#include <board_commit.h>
#include <zone_filler.h>
// minimum width (mm) of a VRML line
@ -315,10 +315,10 @@ static bool GetLayer( MODEL_VRML& aModel, LAYER_NUM layer, VRML_LAYER** vlayer )
}
static void create_vrml_shell( IFSG_TRANSFORM& PcbOutput, VRML_COLOR_INDEX colorID,
VRML_LAYER* layer, double top_z, double bottom_z );
VRML_LAYER* layer, double top_z, double bottom_z );
static void create_vrml_plane( IFSG_TRANSFORM& PcbOutput, VRML_COLOR_INDEX colorID,
VRML_LAYER* layer, double aHeight, bool aTopPlane );
VRML_LAYER* layer, double aHeight, bool aTopPlane );
static void write_triangle_bag( std::ostream& aOut_file, VRML_COLOR& aColor,
VRML_LAYER* aLayer, bool aPlane, bool aTop,
@ -420,8 +420,8 @@ static void write_triangle_bag( std::ostream& aOut_file, VRML_COLOR& aColor,
}
static void write_layers( MODEL_VRML& aModel, BOARD* aPcb,
const char* aFileName, OSTREAM* aOutputFile )
static void write_layers( MODEL_VRML& aModel, BOARD* aPcb, const char* aFileName,
OSTREAM* aOutputFile )
{
// VRML_LAYER board;
aModel.m_board.Tesselate( &aModel.m_holes );
@ -452,8 +452,8 @@ static void write_layers( MODEL_VRML& aModel, BOARD* aPcb,
if( USE_INLINES )
{
write_triangle_bag( *aOutputFile, aModel.GetColor( VRML_COLOR_TRACK ),
&aModel.m_top_copper, true, true,
aModel.GetLayerZ( F_Cu ), 0 );
&aModel.m_top_copper, true, true,
aModel.GetLayerZ( F_Cu ), 0 );
}
else
{
@ -923,7 +923,7 @@ static void export_round_padstack( MODEL_VRML& aModel, BOARD* pcb,
if( aModel.m_plainPCB )
return;
while( 1 )
while( true )
{
if( layer == B_Cu )
{
@ -1016,9 +1016,8 @@ static void export_vrml_tracks( MODEL_VRML& aModel, BOARD* pcb )
}
static void export_vrml_zones( MODEL_VRML& aModel, BOARD* aPcb )
static void export_vrml_zones( MODEL_VRML& aModel, BOARD* aPcb, COMMIT* aCommit )
{
for( int ii = 0; ii < aPcb->GetAreaCount(); ii++ )
{
ZONE_CONTAINER* zone = aPcb->GetArea( ii );
@ -1030,11 +1029,9 @@ static void export_vrml_zones( MODEL_VRML& aModel, BOARD* aPcb )
if( !GetLayer( aModel, layer, &vl ) )
continue;
// fixme: this modifies the board where it shouldn't, but I don't have the time
// to clean this up - TW
if( !zone->IsFilled() )
{
ZONE_FILLER filler( aPcb );
ZONE_FILLER filler( aPcb, aCommit );
zone->SetFillMode( ZONE_FILL_MODE::POLYGONS ); // use filled polygons
filler.Fill( { zone } );
}
@ -1586,6 +1583,8 @@ bool PCB_EDIT_FRAME::ExportVRML_File( const wxString& aFullFileName, double aMMt
{
BOARD* pcb = GetBoard();
bool ok = true;
BOARD_COMMIT commit( this ); // We may need to modify the board (for instance to
// fill zones), so make sure we can revert.
USE_INLINES = aExport3DFiles;
USE_DEFS = true;
@ -1630,7 +1629,7 @@ bool PCB_EDIT_FRAME::ExportVRML_File( const wxString& aFullFileName, double aMMt
// Export zone fills
if( !aUsePlainPCB )
export_vrml_zones( model3d, pcb);
export_vrml_zones( model3d, pcb, &commit );
if( USE_INLINES )
{
@ -1699,6 +1698,7 @@ bool PCB_EDIT_FRAME::ExportVRML_File( const wxString& aFullFileName, double aMMt
ok = false;
}
commit.Revert();
return ok;
}

View File

@ -477,9 +477,11 @@ int EDIT_TOOL::doMoveSelection( const TOOL_EVENT& aEvent, bool aPickReference )
// If moving a group, record position of all the descendants for undo
if( item->Type() == PCB_GROUP_T )
{
static_cast<PCB_GROUP*>( item )->RunOnDescendants( [&]( BOARD_ITEM* bItem ) {
m_commit->Modify( bItem );
});
static_cast<PCB_GROUP*>( item )->RunOnDescendants(
[&]( BOARD_ITEM* bItem )
{
m_commit->Modify( bItem );
});
}
}
}
@ -634,7 +636,8 @@ int EDIT_TOOL::doMoveSelection( const TOOL_EVENT& aEvent, bool aPickReference )
int EDIT_TOOL::ChangeTrackWidth( const TOOL_EVENT& aEvent )
{
const auto& selection = m_selectionTool->RequestSelection(
[]( const VECTOR2I& aPt, GENERAL_COLLECTOR& aCollector, SELECTION_TOOL* sTool ) {
[]( const VECTOR2I& aPt, GENERAL_COLLECTOR& aCollector, SELECTION_TOOL* sTool )
{
EditToolSelectionFilter( aCollector, EXCLUDE_TRANSIENTS, sTool );
} );
@ -649,7 +652,7 @@ int EDIT_TOOL::ChangeTrackWidth( const TOOL_EVENT& aEvent )
if( via->GetViaType() == VIATYPE::MICROVIA )
{
auto net = via->GetNet();
NETINFO_ITEM* net = via->GetNet();
new_width = net->GetMicroViaSize();
new_drill = net->GetMicroViaDrillSize();
@ -663,7 +666,7 @@ int EDIT_TOOL::ChangeTrackWidth( const TOOL_EVENT& aEvent )
via->SetDrill( new_drill );
via->SetWidth( new_width );
}
else if ( auto track = dyn_cast<TRACK*>( item ) )
else if ( TRACK* track = dyn_cast<TRACK*>( item ) )
{
m_commit->Modify( item );
@ -770,9 +773,10 @@ int EDIT_TOOL::Rotate( const TOOL_EVENT& aEvent )
// If rotating a group, record position of all the descendants for undo
if( item->Type() == PCB_GROUP_T )
{
static_cast<PCB_GROUP*>( item )->RunOnDescendants( [&]( BOARD_ITEM* bItem ) {
m_commit->Modify( bItem );
});
static_cast<PCB_GROUP*>( item )->RunOnDescendants( [&]( BOARD_ITEM* bItem )
{
m_commit->Modify( bItem );
});
}
}
@ -883,29 +887,29 @@ int EDIT_TOOL::Mirror( const TOOL_EVENT& aEvent )
{
case PCB_MODULE_EDGE_T:
{
auto& edge = static_cast<EDGE_MODULE&>( *item );
edge.Mirror( mirrorPoint, false );
EDGE_MODULE* edge = static_cast<EDGE_MODULE*>( item );
edge->Mirror( mirrorPoint, false );
break;
}
case PCB_MODULE_ZONE_AREA_T:
{
auto& zone = static_cast<MODULE_ZONE_CONTAINER&>( *item );
zone.Mirror( mirrorPoint, false );
MODULE_ZONE_CONTAINER* zone = static_cast<MODULE_ZONE_CONTAINER*>( item );
zone->Mirror( mirrorPoint, false );
break;
}
case PCB_MODULE_TEXT_T:
{
auto& modText = static_cast<TEXTE_MODULE&>( *item );
modText.Mirror( mirrorPoint, false );
TEXTE_MODULE* modText = static_cast<TEXTE_MODULE*>( item );
modText->Mirror( mirrorPoint, false );
break;
}
case PCB_PAD_T:
{
auto& pad = static_cast<D_PAD&>( *item );
mirrorPadX( pad, mirrorPoint );
D_PAD* pad = static_cast<D_PAD*>( item );
mirrorPadX( *pad, mirrorPoint );
break;
}
@ -973,9 +977,10 @@ int EDIT_TOOL::Flip( const TOOL_EVENT& aEvent )
if( item->Type() == PCB_GROUP_T )
{
static_cast<PCB_GROUP*>( item )->RunOnDescendants( [&]( BOARD_ITEM* bItem ) {
m_commit->Modify( bItem );
});
static_cast<PCB_GROUP*>( item )->RunOnDescendants( [&]( BOARD_ITEM* bItem )
{
m_commit->Modify( bItem );
});
}
static_cast<BOARD_ITEM*>( item )->Flip( modPoint, leftRight );
@ -1004,12 +1009,6 @@ int EDIT_TOOL::Remove( const TOOL_EVENT& aEvent )
return 0;
}
ROUTER_TOOL* routerTool = m_toolMgr->GetTool<ROUTER_TOOL>();
// Do not delete items while actively routing.
if( routerTool && routerTool->Router() && routerTool->Router()->RoutingInProgress() )
return 1;
std::vector<BOARD_ITEM*> lockedItems;
Activate();
@ -1078,8 +1077,8 @@ int EDIT_TOOL::Remove( const TOOL_EVENT& aEvent )
{
case PCB_MODULE_TEXT_T:
{
auto text = static_cast<TEXTE_MODULE*>( item );
auto parent = static_cast<MODULE*>( item->GetParent() );
TEXTE_MODULE* text = static_cast<TEXTE_MODULE*>( item );
MODULE* parent = static_cast<MODULE*>( item->GetParent() );
if( text->GetType() == TEXTE_MODULE::TEXT_is_DIVERS )
{
@ -1092,8 +1091,8 @@ int EDIT_TOOL::Remove( const TOOL_EVENT& aEvent )
case PCB_PAD_T:
{
auto pad = static_cast<D_PAD*>( item );
auto parent = static_cast<MODULE*>( item->GetParent() );
D_PAD* pad = static_cast<D_PAD*>( item );
MODULE* parent = static_cast<MODULE*>( item->GetParent() );
m_commit->Modify( parent );
getView()->Remove( pad );
@ -1103,8 +1102,8 @@ int EDIT_TOOL::Remove( const TOOL_EVENT& aEvent )
case PCB_MODULE_ZONE_AREA_T:
{
auto zone = static_cast<MODULE_ZONE_CONTAINER*>( item );
auto parent = static_cast<MODULE*>( item->GetParent() );
MODULE_ZONE_CONTAINER* zone = static_cast<MODULE_ZONE_CONTAINER*>( item );
MODULE* parent = static_cast<MODULE*>( item->GetParent() );
m_commit->Modify( parent );
getView()->Remove( zone );
@ -1119,8 +1118,8 @@ int EDIT_TOOL::Remove( const TOOL_EVENT& aEvent )
// Only interact with cutouts when deleting and a single item is selected
if( !isCut && selectionCopy.GetSize() == 1 )
{
VECTOR2I curPos = getViewControls()->GetCursorPosition();
auto zone = static_cast<ZONE_CONTAINER*>( item );
VECTOR2I curPos = getViewControls()->GetCursorPosition();
ZONE_CONTAINER* zone = static_cast<ZONE_CONTAINER*>( item );
int outlineIdx, holeIdx;
@ -1134,9 +1133,14 @@ int EDIT_TOOL::Remove( const TOOL_EVENT& aEvent )
toFill.emplace_back( zone );
// Fill the modified zone
ZONE_FILLER filler( board() );
ZONE_FILLER filler( board(), m_commit.get() );
filler.InstallNewProgressReporter( frame(), _( "Fill Zone" ), 4 );
filler.Fill( toFill );
if( !filler.Fill( toFill ) )
{
m_commit->Revert();
return 1;
}
// Update the display
zone->HatchBorder();
@ -1160,9 +1164,10 @@ int EDIT_TOOL::Remove( const TOOL_EVENT& aEvent )
m_commit->Remove( item );
removed.Add( item );
static_cast<PCB_GROUP*>( item )->RunOnDescendants( [&]( BOARD_ITEM* bItem ) {
m_commit->Remove( bItem );
});
static_cast<PCB_GROUP*>( item )->RunOnDescendants( [&]( BOARD_ITEM* bItem )
{
m_commit->Remove( bItem );
});
}
break;
@ -1280,9 +1285,11 @@ int EDIT_TOOL::MoveExact( const TOOL_EVENT& aEvent )
if( item->Type() == PCB_GROUP_T )
{
static_cast<PCB_GROUP*>( item )->RunOnDescendants( [&]( BOARD_ITEM* bItem ) {
m_commit->Modify( bItem );
});
static_cast<PCB_GROUP*>( item )->RunOnDescendants(
[&]( BOARD_ITEM* bItem )
{
m_commit->Modify( bItem );
});
}
}
@ -1411,9 +1418,10 @@ int EDIT_TOOL::Duplicate( const TOOL_EVENT& aEvent )
{
if( dupe_item->Type() == PCB_GROUP_T )
{
static_cast<PCB_GROUP*>( dupe_item )->RunOnDescendants( [&]( BOARD_ITEM* bItem ) {
m_commit->Add( bItem );
});
static_cast<PCB_GROUP*>( dupe_item )->RunOnDescendants( [&]( BOARD_ITEM* bItem )
{
m_commit->Add( bItem );
});
}
// Clear the selection flag here, otherwise the SELECTION_TOOL

View File

@ -166,10 +166,15 @@ void ZONE_CREATE_HELPER::performZoneCutout( ZONE_CONTAINER& aZone, ZONE_CONTAINE
}
commit.Remove( &aZone );
commit.Push( _( "Add a zone cutout" ) );
ZONE_FILLER filler( board );
filler.Fill( newZones );
ZONE_FILLER filler( board, &commit );
if( !filler.Fill( newZones ) )
{
commit.Revert();
return;
}
commit.Push( _( "Add a zone cutout" ) );
// Select the new zone and set it as the source for the next cutout
toolMgr->RunAction( PCB_ACTIONS::selectItem, true, newZones[0] );
@ -193,14 +198,19 @@ void ZONE_CREATE_HELPER::commitZone( std::unique_ptr<ZONE_CONTAINER> aZone )
BOARD_COMMIT bCommit( &m_tool );
aZone->HatchBorder();
bCommit.Add( aZone.get() );
if( !m_params.m_keepout )
{
ZONE_FILLER filler( m_tool.getModel<BOARD>() );
filler.Fill( { aZone.get() } );
ZONE_FILLER filler( m_tool.getModel<BOARD>(), &bCommit );
if( !filler.Fill( { aZone.get() } ) )
{
bCommit.Revert();
break;
}
}
bCommit.Add( aZone.get() );
bCommit.Push( _( "Add a zone" ) );
m_tool.GetManager()->RunAction( PCB_ACTIONS::selectItem, true, aZone.release() );
break;

View File

@ -70,9 +70,15 @@ void ZONE_FILLER_TOOL::CheckAllZones( wxWindow* aCaller )
if( filler.Fill( toFill, true ) )
{
commit.Push( _( "Fill Zone(s)" ), false );
getEditFrame<PCB_EDIT_FRAME>()->m_ZoneFillsDirty = false;
canvas()->Refresh();
}
else
{
commit.Revert();
}
canvas()->Refresh();
}
@ -96,7 +102,14 @@ void ZONE_FILLER_TOOL::FillAllZones( wxWindow* aCaller )
filler.InstallNewProgressReporter( aCaller, _( "Fill All Zones" ), 4 );
if( filler.Fill( toFill ) )
{
commit.Push( _( "Fill Zone(s)" ), false );
getEditFrame<PCB_EDIT_FRAME>()->m_ZoneFillsDirty = false;
}
else
{
commit.Revert();
}
canvas()->Refresh();
@ -128,7 +141,11 @@ int ZONE_FILLER_TOOL::ZoneFill( const TOOL_EVENT& aEvent )
ZONE_FILLER filler( board(), &commit );
filler.InstallNewProgressReporter( frame(), _( "Fill Zone" ), 4 );
filler.Fill( toFill );
if( filler.Fill( toFill ) )
commit.Push( _( "Fill Zone(s)" ), false );
else
commit.Revert();
canvas()->Refresh();
return 0;

View File

@ -141,8 +141,7 @@ bool ZONE_FILLER::Fill( const std::vector<ZONE_CONTAINER*>& aZones, bool aCheck
if( zone->GetIsKeepout() )
continue;
if( m_commit )
m_commit->Modify( zone );
m_commit->Modify( zone );
// calculate the hash value for filled areas. it will be used later
// to know if the current filled areas are up to date
@ -161,16 +160,6 @@ bool ZONE_FILLER::Fill( const std::vector<ZONE_CONTAINER*>& aZones, bool aCheck
zone->UnFill();
}
auto cleanupAfterCancel =
[&]()
{
if( m_commit )
m_commit->Revert();
for( ZONE_CONTAINER* zone : aZones )
zone->UnFill();
};
std::atomic<size_t> nextItem( 0 );
size_t parallelThreadCount = std::min<size_t>( std::thread::hardware_concurrency(),
aZones.size() );
@ -236,10 +225,7 @@ bool ZONE_FILLER::Fill( const std::vector<ZONE_CONTAINER*>& aZones, bool aCheck
if( m_progressReporter )
{
if( m_progressReporter->IsCancelled() )
{
cleanupAfterCancel();
return false;
}
m_progressReporter->AdvancePhase();
m_progressReporter->Report( _( "Removing insulated copper islands..." ) );
@ -251,10 +237,7 @@ bool ZONE_FILLER::Fill( const std::vector<ZONE_CONTAINER*>& aZones, bool aCheck
connectivity->SetProgressReporter( nullptr );
if( m_progressReporter && m_progressReporter->IsCancelled() )
{
cleanupAfterCancel();
return false;
}
// Now remove insulated copper islands and islands outside the board edge
bool outOfDate = false;
@ -319,10 +302,7 @@ bool ZONE_FILLER::Fill( const std::vector<ZONE_CONTAINER*>& aZones, bool aCheck
outOfDate = true;
if( m_progressReporter && m_progressReporter->IsCancelled() )
{
cleanupAfterCancel();
return false;
}
}
}
@ -336,10 +316,7 @@ bool ZONE_FILLER::Fill( const std::vector<ZONE_CONTAINER*>& aZones, bool aCheck
dlg.DoNotShowCheckbox( __FILE__, __LINE__ );
if( dlg.ShowModal() == wxID_CANCEL )
{
cleanupAfterCancel();
return false;
}
}
if( m_progressReporter )
@ -402,10 +379,7 @@ bool ZONE_FILLER::Fill( const std::vector<ZONE_CONTAINER*>& aZones, bool aCheck
if( m_progressReporter )
{
if( m_progressReporter->IsCancelled() )
{
cleanupAfterCancel();
return false;
}
m_progressReporter->AdvancePhase();
m_progressReporter->Report( _( "Committing changes..." ) );
@ -413,19 +387,6 @@ bool ZONE_FILLER::Fill( const std::vector<ZONE_CONTAINER*>& aZones, bool aCheck
}
connectivity->SetProgressReporter( nullptr );
if( m_commit )
{
m_commit->Push( _( "Fill Zone(s)" ), false );
}
else
{
for( auto& i : toFill )
connectivity->Update( i.first );
connectivity->RecalculateRatsnest();
}
return true;
}

View File

@ -39,7 +39,7 @@ class SHAPE_LINE_CHAIN;
class ZONE_FILLER
{
public:
ZONE_FILLER( BOARD* aBoard, COMMIT* aCommit = nullptr );
ZONE_FILLER( BOARD* aBoard, COMMIT* aCommit );
~ZONE_FILLER();
void InstallNewProgressReporter( wxWindow* aParent, const wxString& aTitle, int aNumPhases );

View File

@ -134,15 +134,21 @@ void PCB_EDIT_FRAME::Edit_Zone_Params( ZONE_CONTAINER* aZone )
zones_to_refill.push_back( zone );
}
commit.Stage( s_PickedList );
if( zones_to_refill.size() )
{
ZONE_FILLER filler( GetBoard() );
ZONE_FILLER filler( GetBoard(), &commit );
wxString title = wxString::Format( _( "Refill %d Zones" ), (int) zones_to_refill.size() );
filler.InstallNewProgressReporter( this, title, 4 );
filler.Fill( zones_to_refill );
if( !filler.Fill( zones_to_refill ) )
{
commit.Revert();
return;
}
}
commit.Stage( s_PickedList );
commit.Push( _( "Modify zone properties" ) );
GetBoard()->GetConnectivity()->RecalculateRatsnest();