From 919931f0f08303e415f760a317acd0648c8c6759 Mon Sep 17 00:00:00 2001
From: Seth Hillbrand <seth@kipro-pcb.com>
Date: Mon, 27 Jan 2025 15:13:35 -0800
Subject: [PATCH] Get embedded files panel working for lib_symbol
The symbol library embedded files were implicit, which made them hard to
work with. Adding the panel makes it clearer and setting the panel
correctly to the callback ensures that the structure is updated on
close.
Adds some copy/move semantics for EMBEDDED_FILES to take advantage of
how the multiple copies are handled in the library editor
Fixes https://gitlab.com/kicad/code/kicad/-/issues/18935
---
common/embedded_files.cpp | 53 +++++++++++++++++++
.../dialogs/dialog_lib_symbol_properties.cpp | 5 +-
eeschema/dialogs/dialog_symbol_properties.cpp | 2 +-
eeschema/fields_grid_table.cpp | 23 +++-----
eeschema/fields_grid_table.h | 5 +-
eeschema/lib_symbol.cpp | 3 ++
include/embedded_files.h | 6 +++
pcbnew/footprint.cpp | 7 ++-
8 files changed, 81 insertions(+), 23 deletions(-)
diff --git a/common/embedded_files.cpp b/common/embedded_files.cpp
index 5e0a4c42fd..2ece333880 100644
--- a/common/embedded_files.cpp
+++ b/common/embedded_files.cpp
@@ -523,3 +523,56 @@ const std::vector<wxString>* EMBEDDED_FILES::UpdateFontFiles()
return &m_fontFiles;
}
+
+// Move constructor
+EMBEDDED_FILES::EMBEDDED_FILES( EMBEDDED_FILES&& other ) noexcept :
+ m_files( std::move( other.m_files ) ),
+ m_fontFiles( std::move( other.m_fontFiles ) ),
+ m_fileAddedCallback( std::move( other.m_fileAddedCallback ) ),
+ m_embedFonts( other.m_embedFonts )
+{
+ other.m_embedFonts = false;
+}
+
+// Move assignment operator
+EMBEDDED_FILES& EMBEDDED_FILES::operator=(EMBEDDED_FILES&& other) noexcept
+{
+ if (this != &other)
+ {
+ ClearEmbeddedFiles();
+ m_files = std::move( other.m_files );
+ m_fontFiles = std::move( other.m_fontFiles );
+ m_fileAddedCallback = std::move( other.m_fileAddedCallback );
+ m_embedFonts = other.m_embedFonts;
+ other.m_embedFonts = false;
+ }
+ return *this;
+}
+
+// Copy constructor
+EMBEDDED_FILES::EMBEDDED_FILES( const EMBEDDED_FILES& other ) : m_embedFonts( other.m_embedFonts )
+{
+ for( const auto& [name, file] : other.m_files )
+ {
+ m_files[name] = new EMBEDDED_FILE( *file );
+ }
+ m_fontFiles = other.m_fontFiles;
+ m_fileAddedCallback = other.m_fileAddedCallback;
+}
+
+// Copy assignment operator
+EMBEDDED_FILES& EMBEDDED_FILES::operator=( const EMBEDDED_FILES& other )
+{
+ if( this != &other )
+ {
+ ClearEmbeddedFiles();
+ for( const auto& [name, file] : other.m_files )
+ {
+ m_files[name] = new EMBEDDED_FILE( *file );
+ }
+ m_fontFiles = other.m_fontFiles;
+ m_fileAddedCallback = other.m_fileAddedCallback;
+ m_embedFonts = other.m_embedFonts;
+ }
+ return *this;
+}
diff --git a/eeschema/dialogs/dialog_lib_symbol_properties.cpp b/eeschema/dialogs/dialog_lib_symbol_properties.cpp
index e5a7116e22..c4775ae73f 100644
--- a/eeschema/dialogs/dialog_lib_symbol_properties.cpp
+++ b/eeschema/dialogs/dialog_lib_symbol_properties.cpp
@@ -74,7 +74,7 @@ DIALOG_LIB_SYMBOL_PROPERTIES::DIALOG_LIB_SYMBOL_PROPERTIES( SYMBOL_EDIT_FRAME* a
// Give a bit more room for combobox editors
m_grid->SetDefaultRowSize( m_grid->GetDefaultRowSize() + 4 );
- m_fields = new FIELDS_GRID_TABLE( this, aParent, m_grid, m_libEntry );
+ m_fields = new FIELDS_GRID_TABLE( this, aParent, m_grid, m_libEntry, m_embeddedFiles->GetLocalFiles() );
m_grid->SetTable( m_fields );
m_grid->PushEventHandler( new FIELDS_GRID_TRICKS( m_grid, this, aLibEntry,
[&]( wxCommandEvent& aEvent )
@@ -89,7 +89,7 @@ DIALOG_LIB_SYMBOL_PROPERTIES::DIALOG_LIB_SYMBOL_PROPERTIES( SYMBOL_EDIT_FRAME* a
wxGridCellAttr* attr = new wxGridCellAttr;
attr->SetEditor( new GRID_CELL_URL_EDITOR( this, PROJECT_SCH::SchSearchS( &Prj() ),
- aLibEntry ) );
+ m_embeddedFiles->GetLocalFiles() ) );
m_grid->SetAttr( DATASHEET_FIELD, FDC_VALUE, attr );
m_SymbolNameCtrl->SetValidator( FIELD_VALIDATOR( VALUE_FIELD ) );
@@ -515,7 +515,6 @@ bool DIALOG_LIB_SYMBOL_PROPERTIES::TransferDataFromWindow()
// occurs.
m_Parent->SetShowDeMorgan( m_hasAlternateBodyStyles->GetValue() );
- m_embeddedFiles->TransferDataFromWindow();
return true;
}
diff --git a/eeschema/dialogs/dialog_symbol_properties.cpp b/eeschema/dialogs/dialog_symbol_properties.cpp
index 0c637aa449..3da78b4978 100644
--- a/eeschema/dialogs/dialog_symbol_properties.cpp
+++ b/eeschema/dialogs/dialog_symbol_properties.cpp
@@ -327,7 +327,7 @@ DIALOG_SYMBOL_PROPERTIES::DIALOG_SYMBOL_PROPERTIES( SCH_EDIT_FRAME* aParent,
// so we need to handle m_part == nullptr
// wxASSERT( m_part );
- m_fields = new FIELDS_GRID_TABLE( this, aParent, m_fieldsGrid, m_symbol );
+ m_fields = new FIELDS_GRID_TABLE( this, aParent, m_fieldsGrid, m_symbol, &aParent->Schematic() );
// Give a bit more room for combobox editors
m_fieldsGrid->SetDefaultRowSize( m_fieldsGrid->GetDefaultRowSize() + 4 );
diff --git a/eeschema/fields_grid_table.cpp b/eeschema/fields_grid_table.cpp
index f4890f5b4f..d23403bca8 100644
--- a/eeschema/fields_grid_table.cpp
+++ b/eeschema/fields_grid_table.cpp
@@ -131,11 +131,12 @@ static wxString netList( LIB_SYMBOL* aSymbol )
FIELDS_GRID_TABLE::FIELDS_GRID_TABLE( DIALOG_SHIM* aDialog, SCH_BASE_FRAME* aFrame, WX_GRID* aGrid,
- LIB_SYMBOL* aSymbol ) :
+ LIB_SYMBOL* aSymbol, EMBEDDED_FILES* aFiles ) :
m_frame( aFrame ),
m_dialog( aDialog ),
m_parentType( SCH_SYMBOL_T ),
m_part( aSymbol ),
+ m_files( aFiles ),
m_symbolNetlist( netList( aSymbol ) ),
m_fieldNameValidator( FIELD_NAME ),
m_referenceValidator( REFERENCE_FIELD ),
@@ -149,11 +150,12 @@ FIELDS_GRID_TABLE::FIELDS_GRID_TABLE( DIALOG_SHIM* aDialog, SCH_BASE_FRAME* aFra
FIELDS_GRID_TABLE::FIELDS_GRID_TABLE( DIALOG_SHIM* aDialog, SCH_EDIT_FRAME* aFrame, WX_GRID* aGrid,
- SCH_SYMBOL* aSymbol ) :
+ SCH_SYMBOL* aSymbol, EMBEDDED_FILES* aFiles ) :
m_frame( aFrame ),
m_dialog( aDialog ),
m_parentType( SCH_SYMBOL_T ),
m_part( aSymbol->GetLibSymbolRef().get() ),
+ m_files( aFiles ),
m_symbolNetlist( netList( aSymbol, aFrame->GetCurrentSheet() ) ),
m_fieldNameValidator( FIELD_NAME ),
m_referenceValidator( REFERENCE_FIELD ),
@@ -167,11 +169,12 @@ FIELDS_GRID_TABLE::FIELDS_GRID_TABLE( DIALOG_SHIM* aDialog, SCH_EDIT_FRAME* aFra
FIELDS_GRID_TABLE::FIELDS_GRID_TABLE( DIALOG_SHIM* aDialog, SCH_EDIT_FRAME* aFrame, WX_GRID* aGrid,
-SCH_SHEET* aSheet ) :
+ SCH_SHEET* aSheet ) :
m_frame( aFrame ),
m_dialog( aDialog ),
m_parentType( SCH_SHEET_T ),
m_part( nullptr ),
+ m_files( nullptr ),
m_fieldNameValidator( FIELD_NAME ),
m_referenceValidator( SHEETNAME_V ),
m_valueValidator( VALUE_FIELD ),
@@ -257,21 +260,9 @@ void FIELDS_GRID_TABLE::initGrid( WX_GRID* aGrid )
fpIdEditor->SetValidator( m_nonUrlValidator );
m_footprintAttr->SetEditor( fpIdEditor );
- EMBEDDED_FILES* files = nullptr;
-
- if( m_frame->GetFrameType() == FRAME_SCH )
- {
- files = m_frame->GetScreen()->Schematic();
- }
- else if( m_frame->GetFrameType() == FRAME_SCH_SYMBOL_EDITOR
- || m_frame->GetFrameType() == FRAME_SCH_VIEWER )
- {
- files = m_part;
- }
-
m_urlAttr = new wxGridCellAttr;
SEARCH_STACK* prjSearchStack = PROJECT_SCH::SchSearchS( &m_frame->Prj() );
- GRID_CELL_URL_EDITOR* urlEditor = new GRID_CELL_URL_EDITOR( m_dialog, prjSearchStack, files );
+ GRID_CELL_URL_EDITOR* urlEditor = new GRID_CELL_URL_EDITOR( m_dialog, prjSearchStack, m_files );
urlEditor->SetValidator( m_urlValidator );
m_urlAttr->SetEditor( urlEditor );
diff --git a/eeschema/fields_grid_table.h b/eeschema/fields_grid_table.h
index c9427f8f2c..38dc1134a1 100644
--- a/eeschema/fields_grid_table.h
+++ b/eeschema/fields_grid_table.h
@@ -85,9 +85,9 @@ class FIELDS_GRID_TABLE : public WX_GRID_TABLE_BASE, public std::vector<SCH_FIEL
{
public:
FIELDS_GRID_TABLE( DIALOG_SHIM* aDialog, SCH_BASE_FRAME* aFrame, WX_GRID* aGrid,
- LIB_SYMBOL* aSymbol );
+ LIB_SYMBOL* aSymbol, EMBEDDED_FILES* aFiles = nullptr );
FIELDS_GRID_TABLE( DIALOG_SHIM* aDialog, SCH_EDIT_FRAME* aFrame, WX_GRID* aGrid,
- SCH_SYMBOL* aSymbol );
+ SCH_SYMBOL* aSymbol, EMBEDDED_FILES* aFiles = nullptr );
FIELDS_GRID_TABLE( DIALOG_SHIM* aDialog, SCH_EDIT_FRAME* aFrame, WX_GRID* aGrid,
SCH_SHEET* aSheet );
FIELDS_GRID_TABLE( DIALOG_SHIM* aDialog, SCH_EDIT_FRAME* aFrame, WX_GRID* aGrid,
@@ -136,6 +136,7 @@ private:
DIALOG_SHIM* m_dialog;
KICAD_T m_parentType;
LIB_SYMBOL* m_part;
+ EMBEDDED_FILES* m_files;
wxString m_symbolNetlist;
wxString m_curdir;
diff --git a/eeschema/lib_symbol.cpp b/eeschema/lib_symbol.cpp
index 504ac090e2..36b17f73be 100644
--- a/eeschema/lib_symbol.cpp
+++ b/eeschema/lib_symbol.cpp
@@ -134,6 +134,7 @@ LIB_SYMBOL::LIB_SYMBOL( const wxString& aName, LIB_SYMBOL* aParent, SYMBOL_LIB*
LIB_SYMBOL::LIB_SYMBOL( const LIB_SYMBOL& aSymbol, SYMBOL_LIB* aLibrary ) :
SYMBOL( aSymbol ),
+ EMBEDDED_FILES( aSymbol ),
m_me( this, null_deleter() )
{
m_library = aLibrary;
@@ -215,6 +216,8 @@ const LIB_SYMBOL& LIB_SYMBOL::operator=( const LIB_SYMBOL& aSymbol )
if( parent )
SetParent( parent.get() );
+ EMBEDDED_FILES::operator=( aSymbol );
+
return *this;
}
diff --git a/include/embedded_files.h b/include/embedded_files.h
index b6e5a12cda..1f03c19ecf 100644
--- a/include/embedded_files.h
+++ b/include/embedded_files.h
@@ -101,6 +101,9 @@ public:
EMBEDDED_FILES() = default;
+ EMBEDDED_FILES( EMBEDDED_FILES&& other ) noexcept;
+ EMBEDDED_FILES( const EMBEDDED_FILES& other );
+
~EMBEDDED_FILES()
{
for( auto& file : m_files )
@@ -261,6 +264,9 @@ public:
return 0xABBA2345;
}
+ EMBEDDED_FILES& operator=(EMBEDDED_FILES&& other) noexcept;
+ EMBEDDED_FILES& operator=( const EMBEDDED_FILES& other );
+
private:
std::map<wxString, EMBEDDED_FILE*> m_files;
std::vector<wxString> m_fontFiles;
diff --git a/pcbnew/footprint.cpp b/pcbnew/footprint.cpp
index b94bf34a77..f23819ec99 100644
--- a/pcbnew/footprint.cpp
+++ b/pcbnew/footprint.cpp
@@ -106,7 +106,8 @@ FOOTPRINT::FOOTPRINT( BOARD* parent ) :
FOOTPRINT::FOOTPRINT( const FOOTPRINT& aFootprint ) :
- BOARD_ITEM_CONTAINER( aFootprint )
+ BOARD_ITEM_CONTAINER( aFootprint ),
+ EMBEDDED_FILES( aFootprint )
{
m_pos = aFootprint.m_pos;
m_fpid = aFootprint.m_fpid;
@@ -830,6 +831,8 @@ FOOTPRINT& FOOTPRINT::operator=( FOOTPRINT&& aOther )
for( PCB_GROUP* group : aOther.Groups() )
Add( group );
+ EMBEDDED_FILES::operator=( std::move( aOther ) );
+
aOther.Groups().clear();
// Copy auxiliary data
@@ -958,6 +961,8 @@ FOOTPRINT& FOOTPRINT::operator=( const FOOTPRINT& aOther )
m_initial_comments = aOther.m_initial_comments ?
new wxArrayString( *aOther.m_initial_comments ) : nullptr;
+ EMBEDDED_FILES::operator=( aOther );
+
return *this;
}