From 0d86b88008d101101a1ac873fcc6ed711c5e6156 Mon Sep 17 00:00:00 2001
From: Seth Hillbrand <seth@kipro-pcb.com>
Date: Sun, 26 Jan 2025 16:51:47 -0800
Subject: [PATCH] Unify embedded files handling btw editors

Each editor needs the embedded files tab in the
schematic/pcb/symbol/footprint settings.  But the footprint may add an
embedded file from the 3d models tab and symbols/footprints may add
embedded files from the grid.  This should be immediately visible in the
embedded files tab.  Additionally, removing the reference to an embedded
file in the grid or 3d models now removes the embedded file as well.

Fixes https://gitlab.com/kicad/code/kicad/-/issues/18934
---
 common/dialogs/panel_embedded_files.cpp       | 131 ++++++++++++------
 common/dialogs/panel_embedded_files.h         |   7 +-
 common/embedded_files.cpp                     |  13 +-
 include/embedded_files.h                      |  14 ++
 .../dialogs/dialog_footprint_properties.cpp   |  48 +++++--
 .../dialog_footprint_properties_fp_editor.cpp |  90 ++++++++----
 .../dialog_footprint_properties_fp_editor.h   |   2 +
 .../dialogs/panel_fp_properties_3d_model.cpp  |  22 +--
 pcbnew/dialogs/panel_fp_properties_3d_model.h |   7 +-
 pcbnew/pcb_fields_grid_table.cpp              |  43 +-----
 pcbnew/pcb_fields_grid_table.h                |   2 +-
 11 files changed, 240 insertions(+), 139 deletions(-)

diff --git a/common/dialogs/panel_embedded_files.cpp b/common/dialogs/panel_embedded_files.cpp
index 9642ca4943..8fdf9e0871 100644
--- a/common/dialogs/panel_embedded_files.cpp
+++ b/common/dialogs/panel_embedded_files.cpp
@@ -103,6 +103,24 @@ PANEL_EMBEDDED_FILES::PANEL_EMBEDDED_FILES( wxWindow* parent, EMBEDDED_FILES* aF
     m_files_grid->EnableAlternateRowColors();
 
     m_files_grid->PushEventHandler( new EMBEDDED_FILES_GRID_TRICKS( m_files_grid ) );
+
+    m_localFiles->SetFileAddedCallback( [this](EMBEDDED_FILES::EMBEDDED_FILE* file) {
+
+        for( int ii = 0; ii < m_files_grid->GetNumberRows(); ii++ )
+        {
+            if( m_files_grid->GetCellValue( ii, 1 ) == file->GetLink() )
+            {
+                m_files_grid->DeleteRows( ii );
+                break;
+            }
+        }
+
+        m_files_grid->AppendRows( 1 );
+        int ii = m_files_grid->GetNumberRows() - 1;
+        m_files_grid->SetCellValue( ii, 0, file->name );
+        m_files_grid->SetCellValue( ii, 1, file->GetLink() );
+
+    });
 }
 
 
@@ -255,56 +273,85 @@ void PANEL_EMBEDDED_FILES::onFontEmbedClick( wxCommandEvent& event )
 }
 
 
+EMBEDDED_FILES::EMBEDDED_FILE* PANEL_EMBEDDED_FILES::AddEmbeddedFile( const wxString& aFile )
+{
+    wxFileName fileName( aFile );
+    wxString   name = fileName.GetFullName();
+
+    if( m_localFiles->HasFile( name ) )
+    {
+        wxString msg = wxString::Format( _( "File '%s' already exists." ), name );
+
+        KIDIALOG errorDlg( m_parent, msg, _( "Confirmation" ), wxOK | wxCANCEL | wxICON_WARNING );
+        errorDlg.SetOKLabel( _( "Overwrite" ) );
+
+        if( errorDlg.ShowModal() != wxID_OK )
+            return nullptr;
+
+        for( int ii = 0; ii < m_files_grid->GetNumberRows(); ii++ )
+        {
+            if( m_files_grid->GetCellValue( ii, 0 ) == name )
+            {
+                m_files_grid->DeleteRows( ii );
+                break;
+            }
+        }
+    }
+
+    EMBEDDED_FILES::EMBEDDED_FILE* result = m_localFiles->AddFile( fileName, true );
+
+    if( !result )
+    {
+        wxString msg = wxString::Format( _( "Failed to add file '%s'." ), name );
+
+        KIDIALOG errorDlg( m_parent, msg, _( "Error" ), wxOK | wxICON_ERROR );
+        errorDlg.ShowModal();
+        return nullptr;
+    }
+
+    return result;
+}
+
+
 void PANEL_EMBEDDED_FILES::onAddEmbeddedFile( wxCommandEvent& event )
 {
     wxFileDialog fileDialog( this, _( "Select a file to embed" ), wxEmptyString, wxEmptyString,
                              _( "All files|*.*" ), wxFD_OPEN | wxFD_FILE_MUST_EXIST );
 
     if( fileDialog.ShowModal() == wxID_OK )
-    {
-        wxFileName fileName( fileDialog.GetPath() );
-        wxString name = fileName.GetFullName();
-
-        if( m_localFiles->HasFile( name ) )
-        {
-            wxString msg = wxString::Format( _( "File '%s' already exists." ), name );
-
-            KIDIALOG errorDlg( m_parent, msg, _( "Confirmation" ),
-                                wxOK | wxCANCEL | wxICON_WARNING );
-            errorDlg.SetOKLabel( _( "Overwrite" ) );
-
-            if( errorDlg.ShowModal() != wxID_OK )
-                return;
-
-            for( int ii = 0; ii < m_files_grid->GetNumberRows(); ii++ )
-            {
-                if( m_files_grid->GetCellValue( ii, 0 ) == name )
-                {
-                    m_files_grid->DeleteRows( ii );
-                    break;
-                }
-            }
-        }
-
-        EMBEDDED_FILES::EMBEDDED_FILE* result = m_localFiles->AddFile( fileName, true );
-
-        if( !result )
-        {
-            wxString msg = wxString::Format( _( "Failed to add file '%s'." ),
-                        name );
-
-            KIDIALOG errorDlg( m_parent, msg, _( "Error" ), wxOK | wxICON_ERROR );
-            errorDlg.ShowModal();
-            return;
-        }
-
-        m_files_grid->AppendRows( 1 );
-        int ii = m_files_grid->GetNumberRows() - 1;
-        m_files_grid->SetCellValue( ii, 0, name );
-        m_files_grid->SetCellValue( ii, 1, result->GetLink() );
-    }
+        AddEmbeddedFile( fileDialog.GetPath() );
 }
 
+
+bool PANEL_EMBEDDED_FILES::RemoveEmbeddedFile( const wxString& aFileName )
+{
+    wxString name = aFileName;
+
+    if( name.StartsWith( FILEEXT::KiCadUriPrefix ) )
+        name = name.Mid( FILEEXT::KiCadUriPrefix.size() + 3 );
+
+    int row = std::max( 0, m_files_grid->GetGridCursorRow() );
+
+    for( int ii = 0; ii < m_files_grid->GetNumberRows(); ii++ )
+    {
+        if( m_files_grid->GetCellValue( ii, 0 ) == name )
+        {
+            m_files_grid->DeleteRows( ii );
+            m_localFiles->RemoveFile( name );
+
+            if( row < m_files_grid->GetNumberRows() )
+                m_files_grid->SetGridCursor( row, 0 );
+            else if( m_files_grid->GetNumberRows() > 0 )
+                m_files_grid->SetGridCursor( m_files_grid->GetNumberRows() - 1, 0 );
+
+            return true;
+        }
+    }
+
+    return false;
+}
+
+
 void PANEL_EMBEDDED_FILES::onDeleteEmbeddedFile( wxCommandEvent& event )
 {
     int row = m_files_grid->GetGridCursorRow();
diff --git a/common/dialogs/panel_embedded_files.h b/common/dialogs/panel_embedded_files.h
index 3816658c19..b94d499473 100644
--- a/common/dialogs/panel_embedded_files.h
+++ b/common/dialogs/panel_embedded_files.h
@@ -24,12 +24,11 @@
 #ifndef PANEL_EMBEDDED_FILES_H
 #define PANEL_EMBEDDED_FILES_H
 
+#include <embedded_files.h>
 #include "panel_embedded_files_base.h"
 
 #include "grid_tricks.h"
 
-class EMBEDDED_FILES;
-
 class EMBEDDED_FILES_GRID_TRICKS : public GRID_TRICKS
 {
     enum
@@ -63,6 +62,10 @@ public:
     bool TransferDataToWindow() override;
     bool GetEmbedFonts() const { return m_cbEmbedFonts->GetValue(); }
 
+    EMBEDDED_FILES* GetLocalFiles() { return m_localFiles; }
+    EMBEDDED_FILES::EMBEDDED_FILE* AddEmbeddedFile( const wxString& aFileName );
+    bool RemoveEmbeddedFile( const wxString& aFileName );
+
 protected:
     void onFontEmbedClick( wxCommandEvent& event ) override;
     void onAddEmbeddedFile( wxCommandEvent& event ) override;
diff --git a/common/embedded_files.cpp b/common/embedded_files.cpp
index 6a7cf3f011..5e0a4c42fd 100644
--- a/common/embedded_files.cpp
+++ b/common/embedded_files.cpp
@@ -10,8 +10,8 @@
  *
  * 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.
+ * 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, see <http://www.gnu.org/licenses/>.
@@ -101,7 +101,11 @@ EMBEDDED_FILES::EMBEDDED_FILE* EMBEDDED_FILES::AddFile( const wxFileName& aName,
 
     efile->is_valid = true;
 
-    m_files[aName.GetFullName()] = efile.release();
+    EMBEDDED_FILE* result = efile.release();
+    m_files[aName.GetFullName()] = result;
+
+    if( m_fileAddedCallback )
+        m_fileAddedCallback( result );
 
     return m_files[aName.GetFullName()];
 }
@@ -110,6 +114,9 @@ EMBEDDED_FILES::EMBEDDED_FILE* EMBEDDED_FILES::AddFile( const wxFileName& aName,
 void EMBEDDED_FILES::AddFile( EMBEDDED_FILE* aFile )
 {
     m_files.insert( { aFile->name, aFile } );
+
+    if( m_fileAddedCallback )
+        m_fileAddedCallback( aFile );
 }
 
 
diff --git a/include/embedded_files.h b/include/embedded_files.h
index 8f06f69dab..b6e5a12cda 100644
--- a/include/embedded_files.h
+++ b/include/embedded_files.h
@@ -28,6 +28,7 @@
 #include <mmh3_hash.h>
 #include <picosha2.h>
 #include <wildcards_and_files_ext.h>
+#include <functional>
 
 class OUTPUTFORMATTER;
 
@@ -106,6 +107,18 @@ public:
             delete file.second;
     }
 
+    using FileAddedCallback = std::function<void(EMBEDDED_FILE*)>;
+
+    void SetFileAddedCallback(FileAddedCallback callback)
+    {
+        m_fileAddedCallback = callback;
+    }
+
+    FileAddedCallback GetFileAddedCallback() const
+    {
+        return m_fileAddedCallback;
+    }
+
     /**
      * Load a file from disk and adds it to the collection.
      *
@@ -251,6 +264,7 @@ public:
 private:
     std::map<wxString, EMBEDDED_FILE*> m_files;
     std::vector<wxString>              m_fontFiles;
+    FileAddedCallback                  m_fileAddedCallback;
 
 protected:
     bool m_embedFonts = false; ///< If set, fonts will be embedded in the element on save.
diff --git a/pcbnew/dialogs/dialog_footprint_properties.cpp b/pcbnew/dialogs/dialog_footprint_properties.cpp
index b1e35c239e..ff1095e656 100644
--- a/pcbnew/dialogs/dialog_footprint_properties.cpp
+++ b/pcbnew/dialogs/dialog_footprint_properties.cpp
@@ -71,18 +71,18 @@ DIALOG_FOOTPRINT_PROPERTIES::DIALOG_FOOTPRINT_PROPERTIES( PCB_EDIT_FRAME* aParen
         m_gridSize( 0, 0 ),
         m_lastRequestedSize( 0, 0 )
 {
-    // Create the 3D models page
-    m_3dPanel = new PANEL_FP_PROPERTIES_3D_MODEL( m_frame, m_footprint, this, m_NoteBook );
-    m_NoteBook->AddPage( m_3dPanel, _("3D Models"), false );
-
+    // Create the extra panels.  Embedded files is referenced by the 3D model panel.
     m_embeddedFiles = new PANEL_EMBEDDED_FILES( m_NoteBook, m_footprint );
+    m_3dPanel = new PANEL_FP_PROPERTIES_3D_MODEL( m_frame, m_footprint, this, m_embeddedFiles, m_NoteBook );
+
+    m_NoteBook->AddPage( m_3dPanel, _("3D Models"), false );
     m_NoteBook->AddPage( m_embeddedFiles, _( "Embedded Files" ) );
 
     // Configure display origin transforms
     m_posX.SetCoordType( ORIGIN_TRANSFORMS::ABS_X_COORD );
     m_posY.SetCoordType( ORIGIN_TRANSFORMS::ABS_Y_COORD );
 
-    m_fields = new PCB_FIELDS_GRID_TABLE( m_frame, this );
+    m_fields = new PCB_FIELDS_GRID_TABLE( m_frame, this, m_embeddedFiles->GetLocalFiles() );
 
     m_delayedErrorMessage = wxEmptyString;
     m_delayedFocusGrid = nullptr;
@@ -501,17 +501,45 @@ bool DIALOG_FOOTPRINT_PROPERTIES::TransferDataFromWindow()
     if( !m_itemsGrid->CommitPendingChanges() )
         return false;
 
-    // This only commits the editor, model updating is done below so it is inside
-    // the commit
+    auto view = m_frame->GetCanvas()->GetView();
+    BOARD_COMMIT commit( m_frame );
+    commit.Modify( m_footprint );
+
+    // Make sure this happens inside a commit to capture any changed files
     if( !m_3dPanel->TransferDataFromWindow() )
         return false;
 
     if( !m_embeddedFiles->TransferDataFromWindow() )
         return false;
 
-    auto view = m_frame->GetCanvas()->GetView();
-    BOARD_COMMIT commit( m_frame );
-    commit.Modify( m_footprint );
+    // Clear out embedded files that are no longer in use
+    std::set<wxString> files;
+    std::set<wxString> files_to_delete;
+
+    // Get the new files from the footprint fields
+    for( size_t ii = 0; ii < m_fields->size(); ++ii )
+    {
+        const wxString& name = m_fields->at( ii ).GetText();
+
+        if( name.StartsWith( FILEEXT::KiCadUriPrefix ) )
+            files.insert( name );
+    }
+
+    // Find any files referenced in the old fields that are not in the new fields
+    for( PCB_FIELD* field : m_footprint->GetFields() )
+    {
+        if( field->GetText().StartsWith( FILEEXT::KiCadUriPrefix ) )
+        {
+            if( files.find( field->GetText() ) == files.end() )
+                files_to_delete.insert( field->GetText() );
+        }
+    }
+
+    for( const wxString& file : files_to_delete )
+    {
+        wxString name = file.Mid( FILEEXT::KiCadUriPrefix.size() + 3 ); // Skip "kicad-embed://"
+        m_footprint->RemoveFile( name );
+    }
 
     // Update fields
     for( size_t ii = 0; ii < m_fields->size(); ++ii )
diff --git a/pcbnew/dialogs/dialog_footprint_properties_fp_editor.cpp b/pcbnew/dialogs/dialog_footprint_properties_fp_editor.cpp
index ea8aa03a2b..af6d0c00e3 100644
--- a/pcbnew/dialogs/dialog_footprint_properties_fp_editor.cpp
+++ b/pcbnew/dialogs/dialog_footprint_properties_fp_editor.cpp
@@ -24,31 +24,33 @@
  * 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA
  */
 
-#include <confirm.h>
-#include <dialogs/dialog_text_entry.h>
+#include <3d_rendering/opengl/3d_model.h>
 #include <3d_viewer/eda_3d_viewer_frame.h>
-#include <validators.h>
-#include <board_design_settings.h>
-#include <board_commit.h>
 #include <bitmaps.h>
-#include <kiplatform/ui.h>
-#include <widgets/grid_text_button_helpers.h>
-#include <widgets/wx_grid.h>
-#include <widgets/std_bitmap_button.h>
-#include <widgets/text_ctrl_eval.h>
+#include <board_commit.h>
+#include <board_design_settings.h>
+#include <confirm.h>
+#include <dialog_footprint_properties_fp_editor.h>
+#include <dialogs/dialog_text_entry.h>
+#include <dialogs/panel_preview_3d_model.h>
+#include <embedded_files.h>
+#include <filename_resolver.h>
 #include <footprint.h>
 #include <footprint_edit_frame.h>
 #include <footprint_editor_settings.h>
-#include <dialog_footprint_properties_fp_editor.h>
+#include <grid_layer_box_helpers.h>
+#include <kiplatform/ui.h>
+#include <panel_embedded_files.h>
 #include <panel_fp_properties_3d_model.h>
-#include "3d_rendering/opengl/3d_model.h"
-#include "filename_resolver.h"
 #include <pgm_base.h>
-#include "dialogs/panel_preview_3d_model.h"
 #include <settings/settings_manager.h>
 #include <tool/tool_manager.h>
 #include <tools/pcb_selection_tool.h>
-#include <grid_layer_box_helpers.h>
+#include <validators.h>
+#include <widgets/grid_text_button_helpers.h>
+#include <widgets/std_bitmap_button.h>
+#include <widgets/text_ctrl_eval.h>
+#include <widgets/wx_grid.h>
 
 #include <fp_lib_table.h>
 
@@ -138,11 +140,15 @@ DIALOG_FOOTPRINT_PROPERTIES_FP_EDITOR::DIALOG_FOOTPRINT_PROPERTIES_FP_EDITOR(
         m_lastRequestedSize( 0, 0 )
 {
     SetEvtHandlerEnabled( false );
-    // Create the 3D models page
-    m_3dPanel = new PANEL_FP_PROPERTIES_3D_MODEL( m_frame, m_footprint, this, m_NoteBook );
-    m_NoteBook->AddPage( m_3dPanel, _("3D Models"), false );
 
-    m_fields = new PCB_FIELDS_GRID_TABLE( m_frame, this );
+    // Create the extra panels.
+    m_embeddedFiles = new PANEL_EMBEDDED_FILES( m_NoteBook, m_footprint );
+    m_3dPanel = new PANEL_FP_PROPERTIES_3D_MODEL( m_frame, m_footprint, this, m_embeddedFiles, m_NoteBook );
+
+    m_NoteBook->AddPage( m_3dPanel, _("3D Models"), false );
+    m_NoteBook->AddPage( m_embeddedFiles, _( "Embedded Files" ) );
+
+    m_fields = new PCB_FIELDS_GRID_TABLE( m_frame, this, m_embeddedFiles->GetLocalFiles() );
     m_privateLayers = new PRIVATE_LAYERS_GRID_TABLE( m_frame );
 
     m_delayedErrorMessage = wxEmptyString;
@@ -266,6 +272,9 @@ bool DIALOG_FOOTPRINT_PROPERTIES_FP_EDITOR::TransferDataToWindow()
     if( !m_3dPanel->TransferDataToWindow() )
         return false;
 
+    if( !m_embeddedFiles->TransferDataToWindow() )
+        return false;
+
     // Footprint Fields
     for( PCB_FIELD* field : m_footprint->GetFields() )
         m_fields->push_back( *field );
@@ -485,9 +494,6 @@ bool DIALOG_FOOTPRINT_PROPERTIES_FP_EDITOR::TransferDataFromWindow()
     if( !Validate() )
         return false;
 
-    if( !DIALOG_SHIM::TransferDataFromWindow() )
-        return false;
-
     if( !m_itemsGrid->CommitPendingChanges()
             || !m_privateLayersGrid->CommitPendingChanges()
             || !m_padGroupsGrid->CommitPendingChanges() )
@@ -495,15 +501,45 @@ bool DIALOG_FOOTPRINT_PROPERTIES_FP_EDITOR::TransferDataFromWindow()
         return false;
     }
 
-    // This only commits the editor, model updating is done below so it is inside
-    // the commit
-    if( !m_3dPanel->TransferDataFromWindow() )
-        return false;
-
     KIGFX::PCB_VIEW* view = m_frame->GetCanvas()->GetView();
     BOARD_COMMIT     commit( m_frame );
     commit.Modify( m_footprint );
 
+    // Must be done inside the commit to capture the undo state
+    // This will call TransferDataToWindow() on the 3D panel and
+    // the embedded files panel.
+    if( !DIALOG_SHIM::TransferDataFromWindow() )
+        return false;
+
+    // Clear out embedded files that are no longer in use
+    std::set<wxString> files;
+    std::set<wxString> files_to_delete;
+
+    // Get the new files from the footprint fields
+    for( size_t ii = 0; ii < m_fields->size(); ++ii )
+    {
+        const wxString& name = m_fields->at( ii ).GetText();
+
+        if( name.StartsWith( FILEEXT::KiCadUriPrefix ) )
+            files.insert( name );
+    }
+
+    // Find any files referenced in the old fields that are not in the new fields
+    for( PCB_FIELD* field : m_footprint->GetFields() )
+    {
+        if( field->GetText().StartsWith( FILEEXT::KiCadUriPrefix ) )
+        {
+            if( files.find( field->GetText() ) == files.end() )
+                files_to_delete.insert( field->GetText() );
+        }
+    }
+
+    for( const wxString& file : files_to_delete )
+    {
+        wxString name = file.Mid( FILEEXT::KiCadUriPrefix.size() + 3 ); // Skip "kicad-embed://"
+        m_footprint->RemoveFile( name );
+    }
+
     LIB_ID fpID = m_footprint->GetFPID();
     fpID.SetLibItemName( m_FootprintNameCtrl->GetValue() );
     m_footprint->SetFPID( fpID );
diff --git a/pcbnew/dialogs/dialog_footprint_properties_fp_editor.h b/pcbnew/dialogs/dialog_footprint_properties_fp_editor.h
index 7bf3d05c5a..52892941c2 100644
--- a/pcbnew/dialogs/dialog_footprint_properties_fp_editor.h
+++ b/pcbnew/dialogs/dialog_footprint_properties_fp_editor.h
@@ -34,6 +34,7 @@
 
 class FOOTPRINT_EDIT_FRAME;
 class PANEL_FP_PROPERTIES_3D_MODEL;
+class PANEL_EMBEDDED_FILES;
 
 
 class PRIVATE_LAYERS_GRID_TABLE : public WX_GRID_TABLE_BASE, public std::vector<PCB_LAYER_ID>
@@ -126,6 +127,7 @@ private:
 
     wxSize                     m_gridSize;
     wxSize                     m_lastRequestedSize;
+    PANEL_EMBEDDED_FILES*      m_embeddedFiles;
 };
 
 
diff --git a/pcbnew/dialogs/panel_fp_properties_3d_model.cpp b/pcbnew/dialogs/panel_fp_properties_3d_model.cpp
index 35a982e258..bb202df02e 100644
--- a/pcbnew/dialogs/panel_fp_properties_3d_model.cpp
+++ b/pcbnew/dialogs/panel_fp_properties_3d_model.cpp
@@ -45,6 +45,7 @@
 #include <kiplatform/ui.h>
 #include <dialogs/panel_preview_3d_model.h>
 #include <dialogs/dialog_select_3d_model.h>
+#include <dialogs/panel_embedded_files.h>
 #include <settings/settings_manager.h>
 #include <project_pcb.h>
 
@@ -63,6 +64,7 @@ wxDEFINE_EVENT( wxCUSTOM_PANEL_SHOWN_EVENT, wxCommandEvent );
 PANEL_FP_PROPERTIES_3D_MODEL::PANEL_FP_PROPERTIES_3D_MODEL( PCB_BASE_EDIT_FRAME* aFrame,
                                                             FOOTPRINT* aFootprint,
                                                             DIALOG_SHIM* aDialogParent,
+                                                            PANEL_EMBEDDED_FILES* aFilesPanel,
                                                             wxWindow* aParent, wxWindowID aId,
                                                             const wxPoint& aPos,
                                                             const wxSize& aSize, long aStyle,
@@ -71,6 +73,7 @@ PANEL_FP_PROPERTIES_3D_MODEL::PANEL_FP_PROPERTIES_3D_MODEL( PCB_BASE_EDIT_FRAME*
         m_parentDialog( aDialogParent ),
         m_frame( aFrame ),
         m_footprint( aFootprint ),
+        m_filesPanel( aFilesPanel ),
         m_inSelect( false )
 {
     m_modelsGrid->SetDefaultRowSize( m_modelsGrid->GetDefaultRowSize() + 4 );
@@ -156,20 +159,7 @@ bool PANEL_FP_PROPERTIES_3D_MODEL::TransferDataToWindow()
 
 bool PANEL_FP_PROPERTIES_3D_MODEL::TransferDataFromWindow()
 {
-    // Only commit changes in the editor, not the models
-    // The container dialog is responsible for moving the new models into
-    // the footprint inside a commit.
-    if( !m_modelsGrid->CommitPendingChanges() )
-        return false;
-
-    FOOTPRINT* fp = m_previewPane->GetDummyFootprint();
-
-    for( const auto& [name, file] : fp->EmbeddedFileMap() )
-    {
-        if( !m_footprint->HasFile( name ) )
-            m_footprint->AddFile( new EMBEDDED_FILES::EMBEDDED_FILE( *file ) );
-    }
-    return true;
+    return m_modelsGrid->CommitPendingChanges();
 }
 
 
@@ -301,6 +291,8 @@ void PANEL_FP_PROPERTIES_3D_MODEL::OnRemove3DModel( wxCommandEvent&  )
         // has a tendency to get its knickers in a knot....
         m_inSelect = true;
 
+        // Not all files are embedded but this will ignore the ones that are not
+        m_filesPanel->RemoveEmbeddedFile( m_shapes3D_list[ idx ].m_Filename );
         m_shapes3D_list.erase( m_shapes3D_list.begin() + idx );
         m_modelsGrid->DeleteRows( idx );
 
@@ -388,7 +380,7 @@ void PANEL_FP_PROPERTIES_3D_MODEL::OnAdd3DModel( wxCommandEvent&  )
         wxString fullPath = res->ResolvePath( model.m_Filename, footprintBasePath, nullptr );
         wxFileName fname( fullPath );
 
-        EMBEDDED_FILES::EMBEDDED_FILE* result = m_previewPane->GetDummyFootprint()->AddFile( fname, false );
+        EMBEDDED_FILES::EMBEDDED_FILE* result = m_filesPanel->AddEmbeddedFile( fname.GetFullPath() );                                                                               ;
 
         if( !result )
         {
diff --git a/pcbnew/dialogs/panel_fp_properties_3d_model.h b/pcbnew/dialogs/panel_fp_properties_3d_model.h
index cea62f04da..606dadc296 100644
--- a/pcbnew/dialogs/panel_fp_properties_3d_model.h
+++ b/pcbnew/dialogs/panel_fp_properties_3d_model.h
@@ -30,6 +30,7 @@
 #include <vector>
 
 class DIALOG_SHIM;
+class PANEL_EMBEDDED_FILES;
 class PANEL_PREVIEW_3D_MODEL;
 class PCB_BASE_EDIT_FRAME;
 
@@ -47,8 +48,9 @@ class PANEL_FP_PROPERTIES_3D_MODEL : public PANEL_FP_PROPERTIES_3D_MODEL_BASE
 
 public:
     PANEL_FP_PROPERTIES_3D_MODEL( PCB_BASE_EDIT_FRAME* aFrame, FOOTPRINT* aFootprint,
-                                  DIALOG_SHIM* aDialogParent, wxWindow* aParent,
-                                  wxWindowID aId = wxID_ANY,
+                                  DIALOG_SHIM* aDialogParent,
+                                  PANEL_EMBEDDED_FILES* aFilesPanel,
+                                  wxWindow* aParent, wxWindowID aId = wxID_ANY,
                                   const wxPoint& aPos = wxDefaultPosition,
                                   const wxSize& aSize = wxDefaultSize,
                                   long aStyle = wxTAB_TRAVERSAL,
@@ -101,6 +103,7 @@ private:
 
     std::vector<FP_3DMODEL> m_shapes3D_list;
     PANEL_PREVIEW_3D_MODEL* m_previewPane;
+    PANEL_EMBEDDED_FILES*   m_filesPanel;
 
     bool                    m_inSelect;
 };
diff --git a/pcbnew/pcb_fields_grid_table.cpp b/pcbnew/pcb_fields_grid_table.cpp
index 5bdd756399..1915783609 100644
--- a/pcbnew/pcb_fields_grid_table.cpp
+++ b/pcbnew/pcb_fields_grid_table.cpp
@@ -47,14 +47,11 @@ enum
 wxArrayString g_menuOrientations;
 
 
-PCB_FIELDS_GRID_TABLE::PCB_FIELDS_GRID_TABLE( PCB_BASE_FRAME* aFrame, DIALOG_SHIM* aDialog ) :
-        m_frame( aFrame ),
-        m_dialog( aDialog ),
-        m_fieldNameValidator( FIELD_NAME ),
-        m_referenceValidator( REFERENCE_FIELD ),
-        m_valueValidator( VALUE_FIELD ),
-        m_urlValidator( FIELD_VALUE ),
-        m_nonUrlValidator( FIELD_VALUE )
+PCB_FIELDS_GRID_TABLE::PCB_FIELDS_GRID_TABLE( PCB_BASE_FRAME* aFrame, DIALOG_SHIM* aDialog,
+                                              EMBEDDED_FILES* aFiles ) :
+        m_frame( aFrame ), m_dialog( aDialog ), m_fieldNameValidator( FIELD_NAME ),
+        m_referenceValidator( REFERENCE_FIELD ), m_valueValidator( VALUE_FIELD ),
+        m_urlValidator( FIELD_VALUE ), m_nonUrlValidator( FIELD_VALUE )
 {
     // Build the column attributes.
 
@@ -91,36 +88,8 @@ PCB_FIELDS_GRID_TABLE::PCB_FIELDS_GRID_TABLE( PCB_BASE_FRAME* aFrame, DIALOG_SHI
     valueEditor->SetValidator( m_valueValidator );
     m_valueAttr->SetEditor( valueEditor );
 
-    EMBEDDED_FILES* files = nullptr;
-
-    // In the case of the footprint editor, we need to distinguish between the footprint
-    // in the library where the embedded files are stored with the footprint and the footprint
-    // from the board where the embedded files are stored with the board.
-    if( m_frame->GetFrameType() == FRAME_FOOTPRINT_EDITOR )
-    {
-        FOOTPRINT_EDIT_FRAME* fpFrame = static_cast<FOOTPRINT_EDIT_FRAME*>( m_frame );
-
-        if( fpFrame->IsCurrentFPFromBoard() )
-        {
-            PCB_EDIT_FRAME* pcbframe = (PCB_EDIT_FRAME*) m_frame->Kiway().Player( FRAME_PCB_EDITOR, false );
-
-            if( pcbframe != nullptr )       // happens when the board editor is not active (or closed)
-            {
-                files = pcbframe->GetBoard();
-            }
-        }
-        else
-        {
-            files = fpFrame->GetBoard()->GetFirstFootprint();
-        }
-    }
-    else if( m_frame->GetFrameType() == FRAME_PCB_EDITOR )
-    {
-        files = static_cast<PCB_EDIT_FRAME*>( m_frame )->GetBoard();
-    }
-
     m_urlAttr = new wxGridCellAttr;
-    GRID_CELL_URL_EDITOR* urlEditor = new GRID_CELL_URL_EDITOR( m_dialog, nullptr, files );
+    GRID_CELL_URL_EDITOR* urlEditor = new GRID_CELL_URL_EDITOR( m_dialog, nullptr, aFiles );
     urlEditor->SetValidator( m_urlValidator );
     m_urlAttr->SetEditor( urlEditor );
 
diff --git a/pcbnew/pcb_fields_grid_table.h b/pcbnew/pcb_fields_grid_table.h
index 97d9df77a3..fa9857f01b 100644
--- a/pcbnew/pcb_fields_grid_table.h
+++ b/pcbnew/pcb_fields_grid_table.h
@@ -57,7 +57,7 @@ enum PCB_FIELDS_COL_ORDER
 class PCB_FIELDS_GRID_TABLE : public WX_GRID_TABLE_BASE, public std::vector<PCB_FIELD>
 {
 public:
-    PCB_FIELDS_GRID_TABLE( PCB_BASE_FRAME* aFrame, DIALOG_SHIM* aDialog );
+    PCB_FIELDS_GRID_TABLE( PCB_BASE_FRAME* aFrame, DIALOG_SHIM* aDialog, EMBEDDED_FILES* aFiles );
     ~PCB_FIELDS_GRID_TABLE();
 
     int GetNumberRows() override { return (int) size(); }