kicad-developers team mailing list archive

[hauptmech <hauptmech@xxxxxxxxx>]

Updated patch attached. Coding standard stuff fixed.

I took a second look at the case where all items in a module are deleted.

I checked the parser and writer. In the case where all items are 
deleted, the modules are still OK as far as reading and writing to the file.

I don't think deleting the module is the right way to go. They are on 
layers which cannot be deleted in the current implementation.

Most modules will have pads on the undeleteable layers, so they will be 
fine.

For the case of someone with modules composed only of non-top/bottom 
elements and who are messing with the layer setup midway through the 
design, I think we should just leave it as it is. Either they are a 
ninja doing things that kicad should stay out of the way of  or they are 
a very confused beginner for whom a little extra cruft in the file is 
the least of their problems.

-hauptmech



On 15/03/18 05:17, Seth Hillbrand wrote:
> Hi hautmech-
>
> Looks good. Two comments:
>
> - Code style. There are a couple of small issues: one line is too 
> long, there should be spaces in parentheses for 
> pad->GetParent()->Remove(pad) and no space before parentheses in "if ( 
> pad->GetLayerSet()..."
>
> - We need to handle the case where all items in a module are deleted. 
> Currently, the code will remove all items except the value and 
> reference.  This means that we can no longer select the module in 
> pcbnew to delete it.  Either we need to fully delete the module when 
> the last selectable item is removed or we need to keep sufficient 
> information to allow the module to be selected when the layers are 
> re-enabled.
>
> Best-
> Seth
>
> 2018-03-14 2:54 GMT-07:00 hauptmech <hauptmech@xxxxxxxxx 
> <mailto:hauptmech@xxxxxxxxx>>:
>
>     https://bugs.launchpad.net/kicad/+bug/1754049
>     <https://bugs.launchpad.net/kicad/+bug/1754049>
>
>     This patch follows the edict (stated in dialog_layers_setup.cpp)
>     that items on a layer that is removed must be deleted.
>
>
>
>     _______________________________________________
>     Mailing list: https://launchpad.net/~kicad-developers
>     <https://launchpad.net/%7Ekicad-developers>
>     Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
>     <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx>
>     Unsubscribe : https://launchpad.net/~kicad-developers
>     <https://launchpad.net/%7Ekicad-developers>
>     More help   : https://help.launchpad.net/ListHelp
>     <https://help.launchpad.net/ListHelp>

>From a1dee36c3f0fde28646d5448d7aacd25996e159d Mon Sep 17 00:00:00 2001
From: hauptmech <hauptmech@xxxxxxxxx>
Date: Wed, 14 Mar 2018 22:31:59 +1300
Subject: [PATCH] Fix Layer setup can be changed leading to data loss Edit
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="------------2.16.2"

This is a multi-part message in MIME format.
--------------2.16.2
Content-Type: text/plain; charset=UTF-8; format=fixed
Content-Transfer-Encoding: 8bit


Check for items inside modules and delete them if they belong to the layer we are removing.
Pads get the offending layer removed from their layers list and deleted if they have no more layers.

Fixes: lp:1754049
https://bugs.launchpad.net/kicad/+bug/1754049
---
 pcbnew/collectors.cpp                  | 14 +++++++++++++-
 pcbnew/dialogs/dialog_layers_setup.cpp | 27 ++++++++++++++++++++++++++-
 2 files changed, 39 insertions(+), 2 deletions(-)


--------------2.16.2
Content-Type: text/x-patch; name="0001-Fix-Layer-setup-can-be-changed-leading-to-data-loss-.patch"
Content-Transfer-Encoding: 8bit
Content-Disposition: attachment; filename="0001-Fix-Layer-setup-can-be-changed-leading-to-data-loss-.patch"

diff --git a/pcbnew/collectors.cpp b/pcbnew/collectors.cpp
index 2219886c2..6e1477f52 100644
--- a/pcbnew/collectors.cpp
+++ b/pcbnew/collectors.cpp
@@ -65,6 +65,9 @@ const KICAD_T GENERAL_COLLECTOR::BoardLevelItems[] = {
     PCB_VIA_T,
     PCB_TRACE_T,
     PCB_MODULE_T,
+    PCB_MODULE_TEXT_T,
+    PCB_MODULE_EDGE_T,
+    PCB_PAD_T,
     PCB_ZONE_T,
     PCB_ZONE_AREA_T,
     EOT
@@ -499,7 +502,16 @@ SEARCH_RESULT PCB_LAYER_COLLECTOR::Inspect( EDA_ITEM* testItem, void* testData )
 {
     BOARD_ITEM* item = (BOARD_ITEM*) testItem;
 
-    if( item->GetLayer() == m_layer_id )
+    if( item->Type() == PCB_PAD_T )
+    {
+        D_PAD* pad = nullptr;
+
+        wxASSERT( !pad );
+        pad = static_cast<D_PAD*>( item );
+        if( pad->GetLayerSet()[m_layer_id] )
+            Append( testItem );
+    }
+    else if( item->GetLayer() == m_layer_id )
         Append( testItem );
 
     return SEARCH_CONTINUE;
diff --git a/pcbnew/dialogs/dialog_layers_setup.cpp b/pcbnew/dialogs/dialog_layers_setup.cpp
index 28b4d2d33..0c9817b2e 100644
--- a/pcbnew/dialogs/dialog_layers_setup.cpp
+++ b/pcbnew/dialogs/dialog_layers_setup.cpp
@@ -35,6 +35,7 @@
 #include <collectors.h>
 
 #include <dialog_layers_setup_base.h>
+#include <class_module.h>
 
 
 // some define to choose how copper layers widgets are shown
@@ -655,7 +656,31 @@ bool DIALOG_LAYERS_SETUP::TransferDataFromWindow()
             if( collector.GetCount() != 0 )
             {
                 for( int i = 0; i < collector.GetCount(); i++ )
-                    m_pcb->Remove( collector[i] );
+                    if( collector[i]->Type() == PCB_PAD_T )
+                    {
+                        //pads are multi-layer items that need to be handled specially
+                        D_PAD* pad = nullptr;
+
+                        wxASSERT( !pad );
+                        pad = static_cast<D_PAD*>( collector[i] );
+                        pad->SetLayerSet( pad->GetLayerSet().set( layer_id, false ) );
+                        if( pad->GetLayerSet().count() == 0 )
+                            pad->GetParent()->Remove( pad );
+                    }
+                    else if( collector[i]->Type() == PCB_MODULE_TEXT_T )
+                    {
+                        TEXTE_MODULE* text = static_cast<TEXTE_MODULE*>( collector[i] );
+                        if( text->GetType() == TEXTE_MODULE::TEXT_is_DIVERS )
+                            collector[i]->GetParent()->Remove( collector[i] );
+                    }
+                    else if( collector[i]->Type() == PCB_MODULE_EDGE_T )
+                    {
+                        collector[i]->GetParent()->Remove( collector[i] );
+                    }
+                    else
+                    {
+                        m_pcb->Remove( collector[i] );
+                    }
             }
         }
     }

--------------2.16.2--