Add some safety to layer swapping

Fixes https://gitlab.com/kicad/code/kicad/-/issues/17371
This commit is contained in:
Jon Evans 2024-03-12 19:32:40 -04:00
parent 4b6a1f4dbd
commit ece8521e68
4 changed files with 26 additions and 10 deletions

View File

@ -27,6 +27,7 @@
#include <kiplatform/ui.h> #include <kiplatform/ui.h>
#include <widgets/wx_grid.h> #include <widgets/wx_grid.h>
#include <board.h> #include <board.h>
#include <magic_enum.hpp>
#include "dialog_swap_layers.h" #include "dialog_swap_layers.h"
@ -67,7 +68,8 @@ public:
}; };
DIALOG_SWAP_LAYERS::DIALOG_SWAP_LAYERS( PCB_BASE_EDIT_FRAME* aParent, PCB_LAYER_ID* aArray ) : DIALOG_SWAP_LAYERS::DIALOG_SWAP_LAYERS( PCB_BASE_EDIT_FRAME* aParent,
std::map<PCB_LAYER_ID, PCB_LAYER_ID>& aArray ) :
DIALOG_SWAP_LAYERS_BASE( aParent ), DIALOG_SWAP_LAYERS_BASE( aParent ),
m_parent( aParent ), m_parent( aParent ),
m_layerDestinations( aArray ) m_layerDestinations( aArray )
@ -133,10 +135,20 @@ bool DIALOG_SWAP_LAYERS::TransferDataFromWindow()
for( size_t layer = 0; layer < PCB_LAYER_ID_COUNT; ++layer ) for( size_t layer = 0; layer < PCB_LAYER_ID_COUNT; ++layer )
{ {
std::optional<PCB_LAYER_ID> src = magic_enum::enum_cast<PCB_LAYER_ID>( layer );
wxCHECK2( src.has_value(), continue );
if( enabledCopperLayers.test( layer ) ) if( enabledCopperLayers.test( layer ) )
m_layerDestinations[ layer ] = (PCB_LAYER_ID) table->GetValueAsLong( row++, 1 ); {
std::optional<PCB_LAYER_ID> dest =
magic_enum::enum_cast<PCB_LAYER_ID>( table->GetValueAsLong( row++, 1 ) );
wxCHECK2( dest.has_value(), m_layerDestinations[ *src ] = *dest );
m_layerDestinations[ *src ] = *dest;
}
else else
m_layerDestinations[ layer ] = (PCB_LAYER_ID) layer; {
m_layerDestinations[ *src ] = *src;
}
} }
return true; return true;

View File

@ -33,7 +33,8 @@ class LAYER_GRID_TABLE;
class DIALOG_SWAP_LAYERS : public DIALOG_SWAP_LAYERS_BASE class DIALOG_SWAP_LAYERS : public DIALOG_SWAP_LAYERS_BASE
{ {
public: public:
DIALOG_SWAP_LAYERS( PCB_BASE_EDIT_FRAME* aParent, PCB_LAYER_ID* aArray ); DIALOG_SWAP_LAYERS( PCB_BASE_EDIT_FRAME* aParent,
std::map<PCB_LAYER_ID, PCB_LAYER_ID>& aArray );
~DIALOG_SWAP_LAYERS() override; ~DIALOG_SWAP_LAYERS() override;
private: private:
@ -45,7 +46,7 @@ private:
void adjustGridColumns(); void adjustGridColumns();
PCB_BASE_EDIT_FRAME* m_parent; PCB_BASE_EDIT_FRAME* m_parent;
PCB_LAYER_ID* m_layerDestinations; std::map<PCB_LAYER_ID, PCB_LAYER_ID>& m_layerDestinations;
LAYER_GRID_TABLE* m_gridTable; LAYER_GRID_TABLE* m_gridTable;
}; };

View File

@ -111,12 +111,15 @@ int GLOBAL_EDIT_TOOL::ExchangeFootprints( const TOOL_EVENT& aEvent )
} }
bool GLOBAL_EDIT_TOOL::swapBoardItem( BOARD_ITEM* aItem, PCB_LAYER_ID* aLayerMap ) bool GLOBAL_EDIT_TOOL::swapBoardItem( BOARD_ITEM* aItem,
std::map<PCB_LAYER_ID, PCB_LAYER_ID>& aLayerMap )
{ {
if( aLayerMap[ aItem->GetLayer() ] != aItem->GetLayer() ) PCB_LAYER_ID original = aItem->GetLayer();
if( aLayerMap.count( original ) && aLayerMap.at( original ) != original )
{ {
m_commit->Modify( aItem ); m_commit->Modify( aItem );
aItem->SetLayer( aLayerMap[ aItem->GetLayer() ] ); aItem->SetLayer( aLayerMap.at( original ) );
frame()->GetCanvas()->GetView()->Update( aItem, KIGFX::GEOMETRY ); frame()->GetCanvas()->GetView()->Update( aItem, KIGFX::GEOMETRY );
return true; return true;
} }
@ -127,7 +130,7 @@ bool GLOBAL_EDIT_TOOL::swapBoardItem( BOARD_ITEM* aItem, PCB_LAYER_ID* aLayerMap
int GLOBAL_EDIT_TOOL::SwapLayers( const TOOL_EVENT& aEvent ) int GLOBAL_EDIT_TOOL::SwapLayers( const TOOL_EVENT& aEvent )
{ {
PCB_LAYER_ID layerMap[PCB_LAYER_ID_COUNT]; std::map<PCB_LAYER_ID, PCB_LAYER_ID> layerMap;
DIALOG_SWAP_LAYERS dlg( frame(), layerMap ); DIALOG_SWAP_LAYERS dlg( frame(), layerMap );

View File

@ -66,7 +66,7 @@ public:
int ZonesManager( const TOOL_EVENT& aEvent ); int ZonesManager( const TOOL_EVENT& aEvent );
private: private:
bool swapBoardItem( BOARD_ITEM* aItem, PCB_LAYER_ID* aLayerMap ); bool swapBoardItem( BOARD_ITEM* aItem, std::map<PCB_LAYER_ID, PCB_LAYER_ID>& aLayerMap );
///< Set up handlers for various events. ///< Set up handlers for various events.
void setTransitions() override; void setTransitions() override;