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 <hauptm...@gmail.com
<mailto:hauptm...@gmail.com>>:
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@lists.launchpad.net
<mailto:kicad-developers@lists.launchpad.net>
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 <hauptm...@gmail.com>
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--
_______________________________________________
Mailing list: https://launchpad.net/~kicad-developers
Post to : kicad-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~kicad-developers
More help : https://help.launchpad.net/ListHelp
