Fix crash bug trying to delete zone from list twice.

Fixes https://gitlab.com/kicad/code/kicad/issues/6815

pcbnew/CMakeLists.txt

@@ -317,9 +317,8 @@ set( PCBNEW_CLASS_SRCS
     tracks_cleaner.cpp
     undo_redo.cpp
     zone_filler.cpp
-    zones_by_polygon.cpp
     zones_functions_for_undo_redo.cpp
-    zones_test_and_combine_areas.cpp
+    edit_zone_helpers.cpp
 
     ratsnest/ratsnest.cpp
 

pcbnew/board.cpp

@@ -1793,24 +1793,6 @@ ZONE* BOARD::AddArea( PICKED_ITEMS_LIST* aNewZonesList, int aNetcode, PCB_LAYER_
 }
 
 
-void BOARD::RemoveZone( PICKED_ITEMS_LIST* aDeletedList, ZONE* aZone )
-{
-    if( aZone == NULL )
-        return;
-
-    if( aDeletedList )
-    {
-        ITEM_PICKER picker( nullptr, aZone, UNDO_REDO::DELETED );
-        aDeletedList->PushItem( picker );
-        Remove( aZone );   // remove from zone list, but does not delete it
-    }
-    else
-    {
-        Delete( aZone );
-    }
-}
-
-
 bool BOARD::NormalizeAreaPolygon( PICKED_ITEMS_LIST * aNewZonesList, ZONE* aCurrArea )
 {
     // mark all areas as unmodified except this one, if modified

pcbnew/board.h

@@ -952,22 +952,9 @@ public:
      * @param aDeletedList = a PICKED_ITEMS_LIST * where to store deleted areas (useful
      *                       in undo commands can be NULL
      * @param aNetCode = net to consider
-     * @param aUseLocalFlags : if true, don't check areas if both local flags are 0
-     * Sets local flag = 1 for any areas modified
      * @return true if some areas modified
      */
-    bool CombineAllZonesInNet( PICKED_ITEMS_LIST* aDeletedList,
+    bool CombineAllZonesInNet( PICKED_ITEMS_LIST* aDeletedList, int aNetCode );
-                               int                aNetCode,
-                               bool               aUseLocalFlags );
-
-    /**
-     * Remove copper area from net, and put it in a deleted list (if exists).
-     *
-     * @param aDeletedList = a PICKED_ITEMS_LIST * where to store deleted areas (useful
-     * in undo commands can be NULL
-     * @param  aZone = area to delete or put in deleted list
-     */
-    void RemoveZone( PICKED_ITEMS_LIST* aDeletedList, ZONE* aZone );
 
     /**
      * Check for intersection of a given copper area with other areas in same net

pcbnew/edit_zone_helpers.cpp

@@ -24,8 +24,125 @@
  * 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA
  */
 
-#include <board.h>
+#include <kiface_i.h>
+#include <confirm.h>
+#include <pcb_edit_frame.h>
+#include <board_commit.h>
 #include <zone.h>
+#include <zones.h>
+#include <zones_functions_for_undo_redo.h>
+#include <connectivity/connectivity_data.h>
+#include <widgets/progress_reporter.h>
+#include <zone_filler.h>
+
+
+void PCB_EDIT_FRAME::Edit_Zone_Params( ZONE* aZone )
+{
+    int               dialogResult;
+    ZONE_SETTINGS     zoneInfo = GetZoneSettings();
+    PICKED_ITEMS_LIST pickedList;    // zones for undo/redo command
+    PICKED_ITEMS_LIST deletedList;   // zones that have been deleted when combined
+    BOARD_COMMIT      commit( this );
+
+    // Save initial zones configuration, for undo/redo, before adding new zone
+    // note the net name and the layer can be changed, so we must save all zones
+    deletedList.ClearListAndDeleteItems();
+    pickedList.ClearListAndDeleteItems();
+    SaveCopyOfZones( pickedList, GetBoard(), -1, UNDEFINED_LAYER );
+
+    if( aZone->GetIsRuleArea() )
+    {
+        // edit a rule area on a copper layer
+        zoneInfo << *aZone;
+        dialogResult = InvokeRuleAreaEditor( this, &zoneInfo );
+    }
+    else if( IsCopperLayer( aZone->GetLayer() ) )
+    {
+        // edit a zone on a copper layer
+        zoneInfo << *aZone;
+        dialogResult = InvokeCopperZonesEditor( this, &zoneInfo );
+    }
+    else
+    {
+        zoneInfo << *aZone;
+        dialogResult = InvokeNonCopperZonesEditor( this, &zoneInfo );
+    }
+
+    if( dialogResult == wxID_CANCEL )
+    {
+        deletedList.ClearListAndDeleteItems();
+        pickedList.ClearListAndDeleteItems();
+        return;
+    }
+
+    SetZoneSettings( zoneInfo );
+    OnModify();
+
+    if( dialogResult == ZONE_EXPORT_VALUES )
+    {
+        UpdateCopyOfZonesList( pickedList, deletedList, GetBoard() );
+        commit.Stage( pickedList );
+        commit.Push( _( "Modify zone properties" ) );
+        pickedList.ClearItemsList(); // s_ItemsListPicker is no more owner of picked items
+        return;
+    }
+
+    wxBusyCursor dummy;
+
+    // Undraw old zone outlines
+    for( ZONE* zone : GetBoard()->Zones() )
+        GetCanvas()->GetView()->Update( zone );
+
+    zoneInfo.ExportSetting( *aZone );
+
+    NETINFO_ITEM* net = GetBoard()->FindNet( zoneInfo.m_NetcodeSelection );
+
+    if( net )   // net == NULL should not occur
+        aZone->SetNetCode( net->GetNetCode() );
+
+    // Combine zones if possible
+    GetBoard()->OnAreaPolygonModified( &deletedList, aZone );
+
+    UpdateCopyOfZonesList( pickedList, deletedList, GetBoard() );
+
+    // refill zones with the new properties applied
+    std::vector<ZONE*> zones_to_refill;
+
+    for( unsigned i = 0; i < pickedList.GetCount(); ++i )
+    {
+        ZONE* zone = dyn_cast<ZONE*>( pickedList.GetPickedItem( i ) );
+
+        if( zone == nullptr )
+        {
+            wxASSERT_MSG( false, "Expected a zone after zone properties edit" );
+            continue;
+        }
+
+        // aZone won't be filled if the layer set was modified, but it needs to be updated
+        if( zone->IsFilled() || zone == aZone )
+            zones_to_refill.push_back( zone );
+    }
+
+    commit.Stage( pickedList );
+
+    if( zones_to_refill.size() )
+    {
+        ZONE_FILLER filler( GetBoard(), &commit );
+        wxString title = wxString::Format( _( "Refill %d Zones" ), (int) zones_to_refill.size() );
+        filler.InstallNewProgressReporter( this, title, 4 );
+
+        if( !filler.Fill( zones_to_refill ) )
+        {
+            commit.Revert();
+            return;
+        }
+    }
+
+    commit.Push( _( "Modify zone properties" ) );
+    GetBoard()->GetConnectivity()->RecalculateRatsnest();
+
+    pickedList.ClearItemsList();  // s_ItemsListPicker is no longer owner of picked items
+}
 
 
 bool BOARD::OnAreaPolygonModified( PICKED_ITEMS_LIST* aModifiedZonesList, ZONE* modified_area )
@@ -37,7 +154,7 @@ bool BOARD::OnAreaPolygonModified( PICKED_ITEMS_LIST* aModifiedZonesList, ZONE*
     if( TestZoneIntersections( modified_area ) )
     {
         modified = true;
-        CombineAllZonesInNet( aModifiedZonesList, modified_area->GetNetCode(), true );
+        CombineAllZonesInNet( aModifiedZonesList, modified_area->GetNetCode() );
     }
 
     // Test for bad areas: all zones must have more than 2 corners:
@@ -45,21 +162,27 @@ bool BOARD::OnAreaPolygonModified( PICKED_ITEMS_LIST* aModifiedZonesList, ZONE*
     for( ZONE* zone : m_zones )
     {
         if( zone->GetNumCorners() < 3 )
-            RemoveZone( aModifiedZonesList, zone );
+        {
+            ITEM_PICKER picker( nullptr, zone, UNDO_REDO::DELETED );
+            aModifiedZonesList->PushItem( picker );
+            zone->SetFlags( STRUCT_DELETED );
+        }
     }
 
     return modified;
 }
 
 
-bool BOARD::CombineAllZonesInNet( PICKED_ITEMS_LIST* aDeletedList, int aNetCode,
+bool BOARD::CombineAllZonesInNet( PICKED_ITEMS_LIST* aDeletedList, int aNetCode )
-                                  bool aUseLocalFlags )
 {
     if( m_zones.size() <= 1 )
         return false;
 
     bool modified = false;
 
+    for( ZONE* zone : m_zones )
+        zone->ClearFlags( STRUCT_DELETED );
+
     // Loop through all combinations
     for( unsigned ia1 = 0; ia1 < m_zones.size() - 1; ia1++ )
     {
@@ -76,6 +199,9 @@ bool BOARD::CombineAllZonesInNet( PICKED_ITEMS_LIST* aDeletedList, int aNetCode,
         {
             ZONE* otherZone = m_zones[ia2];
 
+            if( otherZone->HasFlag( STRUCT_DELETED ) )
+                continue;
+
             if( otherZone->GetNetCode() != aNetCode )
                 continue;
 
@@ -93,7 +219,7 @@ bool BOARD::CombineAllZonesInNet( PICKED_ITEMS_LIST* aDeletedList, int aNetCode,
             if( b1.Intersects( b2 ) )
             {
                 // check otherZone against refZone
-                if( refZone->GetLocalFlags() || otherZone->GetLocalFlags() || !aUseLocalFlags )
+                if( refZone->GetLocalFlags() || otherZone->GetLocalFlags() )
                 {
                     bool ret = TestZoneIntersection( refZone, otherZone );
 
@@ -275,7 +401,9 @@ bool BOARD::CombineZones( PICKED_ITEMS_LIST* aDeletedList, ZONE* aRefZone, ZONE*
     delete aRefZone->Outline();
     aRefZone->SetOutline( new SHAPE_POLY_SET( mergedOutlines ) );
 
-    RemoveZone( aDeletedList, aZoneToCombine );
+    ITEM_PICKER picker( nullptr, aZoneToCombine, UNDO_REDO::DELETED );
+    aDeletedList->PushItem( picker );
+    aZoneToCombine->SetFlags( STRUCT_DELETED );
 
     aRefZone->SetLocalFlags( 1 );
     aRefZone->HatchBorder();

pcbnew/zones_by_polygon.cpp

@@ -1,151 +0,0 @@
-/*
- * This program source code file is part of KiCad, a free EDA CAD application.
- *
- * Copyright (C) 2015 Jean-Pierre Charras, jp.charras at wanadoo.fr
- * Copyright (C) 2012 SoftPLC Corporation, Dick Hollenbeck <dick@softplc.com>
- * Copyright (C) 2012 Wayne Stambaugh <stambaughw@verizon.net>
- * Copyright (C) 1992-2017 KiCad Developers, see AUTHORS.txt for contributors.
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License
- * as published by the Free Software Foundation; either version 2
- * of the License, or (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, you may find one here:
- * http://www.gnu.org/licenses/old-licenses/gpl-2.0.html
- * or you may search the http://www.gnu.org website for the version 2 license,
- * or you may write to the Free Software Foundation, Inc.,
- * 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA
- */
-
-#include <kiface_i.h>
-#include <confirm.h>
-#include <pcb_edit_frame.h>
-#include <board_commit.h>
-#include <view/view.h>
-#include <board.h>
-#include <zone.h>
-#include <pcbnew.h>
-#include <zones.h>
-#include <zones_functions_for_undo_redo.h>
-#include <connectivity/connectivity_data.h>
-#include <widgets/progress_reporter.h>
-#include <zone_filler.h>
-
-// TODO: Remove these to the commit object below
-// Local variables
-static PICKED_ITEMS_LIST s_PickedList;    // a picked list to save zones for undo/redo command
-static PICKED_ITEMS_LIST s_AuxiliaryList; // a picked list to store zones that are deleted or added when combined
-
-void PCB_EDIT_FRAME::Edit_Zone_Params( ZONE* aZone )
-{
-    int           dialogResult;
-    ZONE_SETTINGS zoneInfo = GetZoneSettings();
-
-    BOARD_COMMIT commit( this );
-
-    // Save initial zones configuration, for undo/redo, before adding new zone
-    // note the net name and the layer can be changed, so we must save all zones
-    s_AuxiliaryList.ClearListAndDeleteItems();
-    s_PickedList.ClearListAndDeleteItems();
-    SaveCopyOfZones( s_PickedList, GetBoard(), -1, UNDEFINED_LAYER );
-
-    if( aZone->GetIsRuleArea() )
-    {
-        // edit a rule area on a copper layer
-        zoneInfo << *aZone;
-        dialogResult = InvokeRuleAreaEditor( this, &zoneInfo );
-    }
-    else if( IsCopperLayer( aZone->GetLayer() ) )
-    {
-        // edit a zone on a copper layer
-        zoneInfo << *aZone;
-        dialogResult = InvokeCopperZonesEditor( this, &zoneInfo );
-    }
-    else
-    {
-        zoneInfo << *aZone;
-        dialogResult = InvokeNonCopperZonesEditor( this, &zoneInfo );
-    }
-
-    if( dialogResult == wxID_CANCEL )
-    {
-        s_AuxiliaryList.ClearListAndDeleteItems();
-        s_PickedList.ClearListAndDeleteItems();
-        return;
-    }
-
-    SetZoneSettings( zoneInfo );
-    OnModify();
-
-    if( dialogResult == ZONE_EXPORT_VALUES )
-    {
-        UpdateCopyOfZonesList( s_PickedList, s_AuxiliaryList, GetBoard() );
-        commit.Stage( s_PickedList );
-        commit.Push( _( "Modify zone properties" ) );
-        s_PickedList.ClearItemsList(); // s_ItemsListPicker is no more owner of picked items
-        return;
-    }
-
-    wxBusyCursor dummy;
-
-    // Undraw old zone outlines
-    for( ZONE* zone : GetBoard()->Zones() )
-        GetCanvas()->GetView()->Update( zone );
-
-    zoneInfo.ExportSetting( *aZone );
-
-    NETINFO_ITEM* net = GetBoard()->FindNet( zoneInfo.m_NetcodeSelection );
-
-    if( net )   // net == NULL should not occur
-        aZone->SetNetCode( net->GetNetCode() );
-
-    // Combine zones if possible
-    GetBoard()->OnAreaPolygonModified( &s_AuxiliaryList, aZone );
-
-    UpdateCopyOfZonesList( s_PickedList, s_AuxiliaryList, GetBoard() );
-
-    // refill zones with the new properties applied
-    std::vector<ZONE*> zones_to_refill;
-
-    for( unsigned i = 0; i < s_PickedList.GetCount(); ++i )
-    {
-        ZONE* zone = dyn_cast<ZONE*>( s_PickedList.GetPickedItem( i ) );
-
-        if( zone == nullptr )
-        {
-            wxASSERT_MSG( false, "Expected a zone after zone properties edit" );
-            continue;
-        }
-
-        // aZone won't be filled if the layer set was modified, but it needs to be updated
-        if( zone->IsFilled() || zone == aZone )
-            zones_to_refill.push_back( zone );
-    }
-
-    commit.Stage( s_PickedList );
-
-    if( zones_to_refill.size() )
-    {
-        ZONE_FILLER filler( GetBoard(), &commit );
-        wxString title = wxString::Format( _( "Refill %d Zones" ), (int) zones_to_refill.size() );
-        filler.InstallNewProgressReporter( this, title, 4 );
-
-        if( !filler.Fill( zones_to_refill ) )
-        {
-            commit.Revert();
-            return;
-        }
-    }
-
-    commit.Push( _( "Modify zone properties" ) );
-    GetBoard()->GetConnectivity()->RecalculateRatsnest();
-
-    s_PickedList.ClearItemsList();  // s_ItemsListPicker is no longer owner of picked items
-}