From 902048a172cccccc07d2174a5cb6928679a091c2 Mon Sep 17 00:00:00 2001 From: Seth Hillbrand Date: Mon, 16 Mar 2026 11:41:24 -0700 Subject: [PATCH] Move history autosave Prettify, file I/O, and git ops to background thread Savers now serialize to in-memory HISTORY_FILE_DATA on the UI thread, then Prettify + file writes + git stage/diff/commit run on a background thread via GetKiCadThreadPool(). An atomic skip-if-busy flag prevents overlapping saves; WaitForPendingSave() ensures clean shutdown. Extracted FormatBoardToFormatter and FormatSchematicToFormatter so SaveToHistory can serialize without touching disk. PROJECT savers use sourcePath for file-copy semantics since project files are small. --- common/local_history.cpp | 156 ++++++++++++------ common/project.cpp | 18 +- eeschema/sch_edit_frame.cpp | 4 +- .../sch_io/kicad_sexpr/sch_io_kicad_sexpr.cpp | 23 ++- .../sch_io/kicad_sexpr/sch_io_kicad_sexpr.h | 7 + eeschema/schematic.cpp | 32 +++- eeschema/schematic.h | 8 +- include/local_history.h | 41 ++++- include/project.h | 8 +- kicad/kicad_manager_frame.cpp | 4 +- pcbnew/board.cpp | 31 +++- pcbnew/board.h | 8 +- pcbnew/pcb_edit_frame.cpp | 8 +- .../pcb_io/kicad_sexpr/pcb_io_kicad_sexpr.cpp | 14 +- .../pcb_io/kicad_sexpr/pcb_io_kicad_sexpr.h | 7 + .../eeschema/test_history_autosave_legacy.cpp | 12 +- 16 files changed, 272 insertions(+), 109 deletions(-) diff --git a/common/local_history.cpp b/common/local_history.cpp index 9d8d44b651..64797ea0f3 100644 --- a/common/local_history.cpp +++ b/common/local_history.cpp @@ -23,9 +23,11 @@ #include #include +#include #include #include #include +#include #include #include #include @@ -62,6 +64,7 @@ LOCAL_HISTORY::LOCAL_HISTORY() LOCAL_HISTORY::~LOCAL_HISTORY() { + WaitForPendingSave(); } void LOCAL_HISTORY::NoteFileChange( const wxString& aFile ) @@ -75,8 +78,9 @@ void LOCAL_HISTORY::NoteFileChange( const wxString& aFile ) } -void LOCAL_HISTORY::RegisterSaver( const void* aSaverObject, - const std::function& )>& aSaver ) +void LOCAL_HISTORY::RegisterSaver( + const void* aSaverObject, + const std::function& )>& aSaver ) { if( m_savers.find( aSaverObject ) != m_savers.end() ) { @@ -91,6 +95,8 @@ void LOCAL_HISTORY::RegisterSaver( const void* aSaverObject, void LOCAL_HISTORY::UnregisterSaver( const void* aSaverObject ) { + WaitForPendingSave(); + auto it = m_savers.find( aSaverObject ); if( it != m_savers.end() ) @@ -103,6 +109,7 @@ void LOCAL_HISTORY::UnregisterSaver( const void* aSaverObject ) void LOCAL_HISTORY::ClearAllSavers() { + WaitForPendingSave(); m_savers.clear(); wxLogTrace( traceAutoSave, wxS("[history] Cleared all savers") ); } @@ -125,88 +132,129 @@ bool LOCAL_HISTORY::RunRegisteredSaversAndCommit( const wxString& aProjectPath, return false; } - std::vector files; + // Skip if previous background save is still running + if( m_saveInProgress.load( std::memory_order_acquire ) ) + { + wxLogTrace( traceAutoSave, wxS("[history] previous save still in progress; skipping cycle") ); + return false; + } + + // Phase 1 (UI thread): call savers to collect serialized data + std::vector fileData; for( const auto& [saverObject, saver] : m_savers ) { - size_t before = files.size(); - saver( aProjectPath, files ); - wxLogTrace( traceAutoSave, wxS("[history] saver %p added %zu files (total=%zu)"), - saverObject, files.size() - before, files.size() ); + size_t before = fileData.size(); + saver( aProjectPath, fileData ); + wxLogTrace( traceAutoSave, wxS("[history] saver %p produced %zu entries (total=%zu)"), + saverObject, fileData.size() - before, fileData.size() ); } - // Filter out any files not within the project directory + // Filter out any entries not within the project directory wxString projectDir = aProjectPath; if( !projectDir.EndsWith( wxFileName::GetPathSeparator() ) ) projectDir += wxFileName::GetPathSeparator(); - auto it = std::remove_if( files.begin(), files.end(), - [&projectDir]( const wxString& file ) + auto it = std::remove_if( fileData.begin(), fileData.end(), + [&projectDir]( const HISTORY_FILE_DATA& entry ) { - if( !file.StartsWith( projectDir ) ) + if( !entry.path.StartsWith( projectDir ) ) { - wxLogTrace( traceAutoSave, wxS("[history] filtered out file outside project: %s"), file ); + wxLogTrace( traceAutoSave, wxS("[history] filtered out entry outside project: %s"), entry.path ); return true; } return false; } ); - files.erase( it, files.end() ); + fileData.erase( it, fileData.end() ); - if( files.empty() ) + if( fileData.empty() ) { - wxLogTrace( traceAutoSave, wxS("[history] saver set produced no files; skipping") ); + wxLogTrace( traceAutoSave, wxS("[history] saver set produced no entries; skipping") ); return false; } + // Phase 2: submit Prettify + file I/O + git to background thread + m_saveInProgress.store( true, std::memory_order_release ); + + m_pendingFuture = GetKiCadThreadPool().submit_task( + [this, projectPath = aProjectPath, title = aTitle, + data = std::move( fileData )]() mutable -> bool + { + bool result = commitInBackground( projectPath, title, data ); + m_saveInProgress.store( false, std::memory_order_release ); + return result; + } ); + + return true; +} + + +bool LOCAL_HISTORY::commitInBackground( const wxString& aProjectPath, const wxString& aTitle, + const std::vector& aFileData ) +{ + wxLogTrace( traceAutoSave, wxS("[history] background: writing %zu entries for '%s'"), + aFileData.size(), aProjectPath ); + + wxString hist = historyPath( aProjectPath ); + + // Write files to the .history mirror + for( const HISTORY_FILE_DATA& entry : aFileData ) + { + if( !entry.content.empty() ) + { + std::string buf = entry.content; + + if( entry.prettify ) + KICAD_FORMAT::Prettify( buf, entry.formatMode ); + + wxFFile fp( entry.path, wxS( "wb" ) ); + + if( fp.IsOpened() ) + { + fp.Write( buf.data(), buf.size() ); + fp.Close(); + wxLogTrace( traceAutoSave, wxS("[history] background: wrote %zu bytes to '%s'"), + buf.size(), entry.path ); + } + else + { + wxLogTrace( traceAutoSave, wxS("[history] background: failed to open '%s' for writing"), + entry.path ); + } + } + else if( !entry.sourcePath.IsEmpty() ) + { + wxCopyFile( entry.sourcePath, entry.path, true ); + wxLogTrace( traceAutoSave, wxS("[history] background: copied '%s' -> '%s'"), + entry.sourcePath, entry.path ); + } + } + // Acquire locks using hybrid locking strategy HISTORY_LOCK_MANAGER lock( aProjectPath ); if( !lock.IsLocked() ) { - wxLogTrace( traceAutoSave, wxS("[history] failed to acquire lock: %s"), lock.GetLockError() ); + wxLogTrace( traceAutoSave, wxS("[history] background: failed to acquire lock: %s"), lock.GetLockError() ); return false; } git_repository* repo = lock.GetRepository(); git_index* index = lock.GetIndex(); - wxString hist = historyPath( aProjectPath ); - // Stage selected files (mirroring logic from CommitSnapshot but limited to given files) - for( const wxString& file : files ) + // Stage all written files + for( const HISTORY_FILE_DATA& entry : aFileData ) { - wxFileName src( file ); + wxFileName src( entry.path ); if( !src.FileExists() ) - { - wxLogTrace( traceAutoSave, wxS("[history] skip missing '%s'"), file ); continue; - } - // If saver already produced a path inside history mirror, just stage it. if( src.GetFullPath().StartsWith( hist + wxFILE_SEP_PATH ) ) { std::string relHist = src.GetFullPath().ToStdString().substr( hist.length() + 1 ); git_index_add_bypath( index, relHist.c_str() ); - wxLogTrace( traceAutoSave, wxS("[history] staged pre-mirrored '%s'"), file ); - continue; } - - wxString relStr; - wxString proj = wxFileName( aProjectPath ).GetFullPath(); - - if( src.GetFullPath().StartsWith( proj + wxFILE_SEP_PATH ) ) - relStr = src.GetFullPath().Mid( proj.length() + 1 ); - else - relStr = src.GetFullName(); - - wxFileName dst( hist + wxFILE_SEP_PATH + relStr ); - wxFileName dstDir( dst ); - dstDir.SetFullName( wxEmptyString ); - wxFileName::Mkdir( dstDir.GetPath(), 0777, wxPATH_MKDIR_FULL ); - wxCopyFile( src.GetFullPath(), dst.GetFullPath(), true ); - std::string rel = dst.GetFullPath().ToStdString().substr( hist.length() + 1 ); - git_index_add_bypath( index, rel.c_str() ); - wxLogTrace( traceAutoSave, wxS("[history] staged '%s' as '%s'"), file, wxString::FromUTF8( rel ) ); } // Compare index to HEAD; if no diff -> abort to avoid empty commit. @@ -225,7 +273,7 @@ bool LOCAL_HISTORY::RunRegisteredSaversAndCommit( const wxString& aProjectPath, { if( head_tree ) git_tree_free( head_tree ); if( head_commit ) git_commit_free( head_commit ); - wxLogTrace( traceAutoSave, wxS("[history] failed to write index tree" ) ); + wxLogTrace( traceAutoSave, wxS("[history] background: failed to write index tree" ) ); return false; } @@ -241,7 +289,7 @@ bool LOCAL_HISTORY::RunRegisteredSaversAndCommit( const wxString& aProjectPath, if( git_diff_tree_to_tree( &diff, repo, head_tree, indexTree.get(), nullptr ) == 0 ) { hasChanges = git_diff_num_deltas( diff ) > 0; - wxLogTrace( traceAutoSave, wxS("[history] diff deltas=%u"), (unsigned) git_diff_num_deltas( diff ) ); + wxLogTrace( traceAutoSave, wxS("[history] background: diff deltas=%u"), (unsigned) git_diff_num_deltas( diff ) ); git_diff_free( diff ); } } @@ -251,7 +299,7 @@ bool LOCAL_HISTORY::RunRegisteredSaversAndCommit( const wxString& aProjectPath, if( !hasChanges ) { - wxLogTrace( traceAutoSave, wxS("[history] no changes detected; no commit") ); + wxLogTrace( traceAutoSave, wxS("[history] background: no changes detected; no commit") ); return false; // Nothing new; skip commit. } @@ -278,10 +326,10 @@ bool LOCAL_HISTORY::RunRegisteredSaversAndCommit( const wxString& aProjectPath, parents ? &constParent : nullptr ); if( rc == 0 ) - wxLogTrace( traceAutoSave, wxS("[history] commit created %s (%s files=%zu)"), - wxString::FromUTF8( git_oid_tostr_s( &commit_id ) ), msg, files.size() ); + wxLogTrace( traceAutoSave, wxS("[history] background: commit created %s (%s entries=%zu)"), + wxString::FromUTF8( git_oid_tostr_s( &commit_id ) ), msg, aFileData.size() ); else - wxLogTrace( traceAutoSave, wxS("[history] commit failed rc=%d"), rc ); + wxLogTrace( traceAutoSave, wxS("[history] background: commit failed rc=%d"), rc ); if( parent ) git_commit_free( parent ); @@ -290,6 +338,16 @@ bool LOCAL_HISTORY::RunRegisteredSaversAndCommit( const wxString& aProjectPath, } +void LOCAL_HISTORY::WaitForPendingSave() +{ + if( m_pendingFuture.valid() ) + { + wxLogTrace( traceAutoSave, wxS("[history] waiting for pending background save") ); + m_pendingFuture.get(); + } +} + + bool LOCAL_HISTORY::CommitPending() { std::vector files( m_pendingFiles.begin(), m_pendingFiles.end() ); diff --git a/common/project.cpp b/common/project.cpp index a5dec7f87a..b0ee6011c6 100644 --- a/common/project.cpp +++ b/common/project.cpp @@ -458,7 +458,7 @@ void PROJECT::SetProjectLock( LOCKFILE* aLockFile ) } -void PROJECT::SaveToHistory( const wxString& aProjectPath, std::vector& aFiles ) +void PROJECT::SaveToHistory( const wxString& aProjectPath, std::vector& aFileData ) { wxString projectFile = GetProjectFullName(); @@ -486,11 +486,14 @@ void PROJECT::SaveToHistory( const wxString& aProjectPath, std::vector return; } - // Save project file (.kicad_pro) + // Save project file (.kicad_pro) via file-copy wxFileName historyProFile( historyDir.GetFullPath(), projectFn.GetName(), projectFn.GetExt() ); - wxCopyFile( projectFile, historyProFile.GetFullPath(), true ); - aFiles.push_back( historyProFile.GetFullPath() ); + + HISTORY_FILE_DATA proEntry; + proEntry.path = historyProFile.GetFullPath(); + proEntry.sourcePath = projectFile; + aFileData.push_back( std::move( proEntry ) ); // Save project local settings (.kicad_prl) if it exists wxFileName prlFile( projectFn.GetPath(), projectFn.GetName(), FILEEXT::ProjectLocalSettingsFileExtension ); @@ -499,7 +502,10 @@ void PROJECT::SaveToHistory( const wxString& aProjectPath, std::vector { wxFileName historyPrlFile( historyDir.GetFullPath(), prlFile.GetName(), prlFile.GetExt() ); - wxCopyFile( prlFile.GetFullPath(), historyPrlFile.GetFullPath(), true ); - aFiles.push_back( historyPrlFile.GetFullPath() ); + + HISTORY_FILE_DATA prlEntry; + prlEntry.path = historyPrlFile.GetFullPath(); + prlEntry.sourcePath = prlFile.GetFullPath(); + aFileData.push_back( std::move( prlEntry ) ); } } diff --git a/eeschema/sch_edit_frame.cpp b/eeschema/sch_edit_frame.cpp index a545f41826..a773f6f899 100644 --- a/eeschema/sch_edit_frame.cpp +++ b/eeschema/sch_edit_frame.cpp @@ -1471,9 +1471,9 @@ void SCH_EDIT_FRAME::ProjectChanged() // Register schematic saver for autosave history Kiway().LocalHistory().RegisterSaver( m_schematic, - [this]( const wxString& aProjectPath, std::vector& aFiles ) + [this]( const wxString& aProjectPath, std::vector& aFileData ) { - m_schematic->SaveToHistory( aProjectPath, aFiles ); + m_schematic->SaveToHistory( aProjectPath, aFileData ); } ); m_designBlocksPane->ProjectChanged(); diff --git a/eeschema/sch_io/kicad_sexpr/sch_io_kicad_sexpr.cpp b/eeschema/sch_io/kicad_sexpr/sch_io_kicad_sexpr.cpp index 4e09792d16..dac032734e 100644 --- a/eeschema/sch_io/kicad_sexpr/sch_io_kicad_sexpr.cpp +++ b/eeschema/sch_io/kicad_sexpr/sch_io_kicad_sexpr.cpp @@ -367,8 +367,6 @@ void SCH_IO_KICAD_SEXPR::SaveSchematicFile( const wxString& aFileName, SCH_SHEET } } - init( aSchematic, aProperties ); - wxFileName fn = aFileName; // File names should be absolute. Don't assume everything relative to the project path @@ -376,16 +374,29 @@ void SCH_IO_KICAD_SEXPR::SaveSchematicFile( const wxString& aFileName, SCH_SHEET wxASSERT( fn.IsAbsolute() ); PRETTIFIED_FILE_OUTPUTFORMATTER formatter( fn.GetFullPath() ); - - m_out = &formatter; // no ownership - - Format( aSheet ); + FormatSchematicToFormatter( &formatter, aSheet, aSchematic, aProperties ); if( aSheet->GetScreen() ) aSheet->GetScreen()->SetFileExists( true ); } +void SCH_IO_KICAD_SEXPR::FormatSchematicToFormatter( OUTPUTFORMATTER* aOut, SCH_SHEET* aSheet, + SCHEMATIC* aSchematic, + const std::map* aProperties ) +{ + wxCHECK_RET( aSheet != nullptr, "NULL SCH_SHEET object." ); + + init( aSchematic, aProperties ); + + m_out = aOut; + + Format( aSheet ); + + m_out = nullptr; +} + + void SCH_IO_KICAD_SEXPR::Format( SCH_SHEET* aSheet ) { wxCHECK_RET( aSheet != nullptr, "NULL SCH_SHEET* object." ); diff --git a/eeschema/sch_io/kicad_sexpr/sch_io_kicad_sexpr.h b/eeschema/sch_io/kicad_sexpr/sch_io_kicad_sexpr.h index 725a5bb421..b2decaba5d 100644 --- a/eeschema/sch_io/kicad_sexpr/sch_io_kicad_sexpr.h +++ b/eeschema/sch_io/kicad_sexpr/sch_io_kicad_sexpr.h @@ -102,6 +102,13 @@ public: void SaveSchematicFile( const wxString& aFileName, SCH_SHEET* aSheet, SCHEMATIC* aSchematic, const std::map* aProperties = nullptr ) override; + /** Serialize a schematic sheet to an OUTPUTFORMATTER without file I/O or Prettify. + * Handles init() and Format(). + * Skips GroupsSanityCheck and SetFileExists side effects. */ + void FormatSchematicToFormatter( OUTPUTFORMATTER* aOut, SCH_SHEET* aSheet, + SCHEMATIC* aSchematic, + const std::map* aProperties = nullptr ); + void Format( SCH_SHEET* aSheet ); void Format( SCH_SELECTION* aSelection, SCH_SHEET_PATH* aSelectionPath, diff --git a/eeschema/schematic.cpp b/eeschema/schematic.cpp index 0b742441e8..9a9e066311 100644 --- a/eeschema/schematic.cpp +++ b/eeschema/schematic.cpp @@ -57,7 +57,9 @@ #include #include #include +#include #include +#include #include #include @@ -2355,7 +2357,7 @@ void SCHEMATIC::LoadVariants() } -void SCHEMATIC::SaveToHistory( const wxString& aProjectPath, std::vector& aFiles ) +void SCHEMATIC::SaveToHistory( const wxString& aProjectPath, std::vector& aFileData ) { if( !IsValid() ) return; @@ -2384,8 +2386,12 @@ void SCHEMATIC::SaveToHistory( const wxString& aProjectPath, std::vector pi( SCH_IO_MGR::FindPlugin( SCH_IO_MGR::SCH_KICAD ) ); + SCH_IO_KICAD_SEXPR pi; + + KICAD_FORMAT::FORMAT_MODE mode = KICAD_FORMAT::FORMAT_MODE::NORMAL; + + if( ADVANCED_CFG::GetCfg().m_CompactSave ) + mode = KICAD_FORMAT::FORMAT_MODE::COMPACT_TEXT_PROPERTIES; for( const SCH_SHEET_PATH& path : sheetList ) { @@ -2415,7 +2421,7 @@ void SCHEMATIC::SaveToHistory( const wxString& aProjectPath, std::vectorSaveSchematicFile( dst.GetFullPath(), sheet, this ); - aFiles.push_back( dst.GetFullPath() ); - wxLogTrace( traceAutoSave, wxS( "[history] sch saver exported sheet '%s' -> '%s'" ), absPath, - dst.GetFullPath() ); + STRING_FORMATTER formatter; + pi.FormatSchematicToFormatter( &formatter, sheet, this ); + + HISTORY_FILE_DATA entry; + entry.path = dst.GetFullPath(); + entry.content = std::move( formatter.MutableString() ); + entry.prettify = true; + entry.formatMode = mode; + aFileData.push_back( std::move( entry ) ); + + wxLogTrace( traceAutoSave, wxS( "[history] sch saver serialized %zu bytes for '%s' -> '%s'" ), + aFileData.back().content.size(), absPath, dst.GetFullPath() ); } catch( const IO_ERROR& ioe ) { - wxLogTrace( traceAutoSave, wxS( "[history] sch saver export failed for '%s': %s" ), absPath, + wxLogTrace( traceAutoSave, wxS( "[history] sch saver serialize failed for '%s': %s" ), absPath, wxString::FromUTF8( ioe.What() ) ); } } diff --git a/eeschema/schematic.h b/eeschema/schematic.h index 1ccccbe192..77afc91822 100644 --- a/eeschema/schematic.h +++ b/eeschema/schematic.h @@ -28,6 +28,7 @@ #include +struct HISTORY_FILE_DATA; class BUS_ALIAS; class CONNECTION_GRAPH; class EDA_BASE_FRAME; @@ -533,14 +534,15 @@ public: PROJECT::ELEM ProjectElementType() override { return PROJECT::ELEM::SCHEMATIC; } /** - * Save schematic files to the .history directory. + * Serialize schematic sheets into HISTORY_FILE_DATA for non-blocking history commit. * * This method is used as a saver callback for LOCAL_HISTORY during autosave operations. + * Serialization runs on the UI thread; Prettify and file I/O happen in the background. * * @param aProjectPath The path to check against this schematic's project path - * @param aFiles Output vector to append absolute file paths for history inclusion + * @param aFileData Output vector to append serialized data for history inclusion */ - void SaveToHistory( const wxString& aProjectPath, std::vector& aFiles ); + void SaveToHistory( const wxString& aProjectPath, std::vector& aFileData ); private: friend class SCH_EDIT_FRAME; diff --git a/include/local_history.h b/include/local_history.h index f27b40a125..1263a775e2 100644 --- a/include/local_history.h +++ b/include/local_history.h @@ -24,13 +24,32 @@ #pragma once #include +#include + +#include +#include #include #include #include #include +#include #include #include + +/** + * Data produced by a registered saver on the UI thread, consumed by the background commit thread. + * Each entry represents one file to write into the .history mirror. + */ +struct KICOMMON_API HISTORY_FILE_DATA +{ + wxString path; ///< Destination inside .history/ + std::string content; ///< Serialized content (mutually exclusive with sourcePath) + wxString sourcePath; ///< For file-copy savers (small files like .kicad_pro) + bool prettify = false; + KICAD_FORMAT::FORMAT_MODE formatMode = KICAD_FORMAT::FORMAT_MODE::NORMAL; +}; + /** * Simple local history manager built on libgit2. Stores history for project files in * a hidden .history git repository within the project directory. @@ -55,12 +74,13 @@ public: bool CommitFullProjectSnapshot( const wxString& aProjectPath, const wxString& aTitle ); /** Register a saver callback invoked during autosave history commits. - * The callback receives the project path and should append absolute file paths - * (within that project) to aFiles for inclusion. + * The callback receives the project path and should populate aFileData with + * serialized content or source paths for inclusion. * @param aSaverObject Unique object pointer identifier for this saver (to prevent duplicate registration) * @param aSaver The saver callback function */ - void RegisterSaver( const void* aSaverObject, - const std::function& )>& aSaver ); + void RegisterSaver( + const void* aSaverObject, + const std::function& )>& aSaver ); /** Unregister a previously registered saver callback. * @param aSaverObject The object pointer that was used to register the saver */ @@ -106,7 +126,18 @@ public: void ShowRestoreDialog( const wxString& aProjectPath, wxWindow* aParent ); private: + /** Execute file writes and git commit on a background thread. */ + bool commitInBackground( const wxString& aProjectPath, const wxString& aTitle, + const std::vector& aFileData ); + + /** Block until any pending background save completes. */ + void WaitForPendingSave(); + std::set m_pendingFiles; - std::map&)>> m_savers; + std::map& )>> m_savers; + + std::atomic m_saveInProgress{ false }; + std::future m_pendingFuture; }; diff --git a/include/project.h b/include/project.h index d7440eff16..9452f3cfdf 100644 --- a/include/project.h +++ b/include/project.h @@ -43,6 +43,7 @@ /// default name for nameless projects #define NAMELESS_PROJECT _( "untitled" ) +struct HISTORY_FILE_DATA; class DESIGN_BLOCK_LIBRARY_ADAPTER; class LEGACY_SYMBOL_LIBS; class SEARCH_STACK; @@ -302,14 +303,15 @@ public: LOCKFILE* GetProjectLock() const; /** - * Save project files (.kicad_pro and .kicad_prl) to the .history directory. + * Produce HISTORY_FILE_DATA entries for project files (.kicad_pro and .kicad_prl). * + * Uses sourcePath for file-copy semantics since project files are small. * This method is used as a saver callback for LOCAL_HISTORY during autosave operations. * * @param aProjectPath The path to check against this project's path - * @param aFiles Output vector to append absolute file paths for history inclusion + * @param aFileData Output vector to append file-copy data for history inclusion */ - void SaveToHistory( const wxString& aProjectPath, std::vector& aFiles ); + void SaveToHistory( const wxString& aProjectPath, std::vector& aFileData ); private: friend class SETTINGS_MANAGER; // so that SM can set project path diff --git a/kicad/kicad_manager_frame.cpp b/kicad/kicad_manager_frame.cpp index 216b3c95e6..eedfd54b9a 100644 --- a/kicad/kicad_manager_frame.cpp +++ b/kicad/kicad_manager_frame.cpp @@ -1279,9 +1279,9 @@ void KICAD_MANAGER_FRAME::ProjectChanged() // Register project file saver. Ensures project file participates in // autosave history commits without affecting dirty state. Kiway().LocalHistory().RegisterSaver( &Prj(), - [this]( const wxString& aProjectPath, std::vector& aFiles ) + [this]( const wxString& aProjectPath, std::vector& aFileData ) { - Prj().SaveToHistory( aProjectPath, aFiles ); + Prj().SaveToHistory( aProjectPath, aFileData ); } ); } diff --git a/pcbnew/board.cpp b/pcbnew/board.cpp index 1fbc94de13..5f224cb255 100644 --- a/pcbnew/board.cpp +++ b/pcbnew/board.cpp @@ -78,6 +78,9 @@ #include #include #include +#include +#include +#include #include // This is an odd place for this, but CvPcb won't link if it's in board_item.cpp like I first @@ -3690,7 +3693,7 @@ int BOARD::GetPadWithCastellatedAttrCount() } -void BOARD::SaveToHistory( const wxString& aProjectPath, std::vector& aFiles ) +void BOARD::SaveToHistory( const wxString& aProjectPath, std::vector& aFileData ) { wxString projPath = GetProject()->GetProjectPath(); @@ -3724,7 +3727,7 @@ void BOARD::SaveToHistory( const wxString& aProjectPath, std::vector& historyRoot.AppendDir( wxS( ".history" ) ); wxFileName dst( historyRoot.GetPath(), rel ); - // Ensure destination directories exist. + // Ensure destination directories exist on the UI thread so the background task can write. wxFileName dstDir( dst ); dstDir.SetFullName( wxEmptyString ); @@ -3733,14 +3736,26 @@ void BOARD::SaveToHistory( const wxString& aProjectPath, std::vector& try { - IO_RELEASER pi( PCB_IO_MGR::FindPlugin( PCB_IO_MGR::KICAD_SEXP ) ); - // Save directly to history mirror path. - pi->SaveBoard( dst.GetFullPath(), this, nullptr ); - aFiles.push_back( dst.GetFullPath() ); - wxLogTrace( traceAutoSave, wxS( "[history] pcb saver exported '%s'" ), dst.GetFullPath() ); + PCB_IO_KICAD_SEXPR pi; + STRING_FORMATTER formatter; + + pi.FormatBoardToFormatter( &formatter, this, nullptr ); + + HISTORY_FILE_DATA entry; + entry.path = dst.GetFullPath(); + entry.content = std::move( formatter.MutableString() ); + entry.prettify = true; + + if( ADVANCED_CFG::GetCfg().m_CompactSave ) + entry.formatMode = KICAD_FORMAT::FORMAT_MODE::COMPACT_TEXT_PROPERTIES; + + aFileData.push_back( std::move( entry ) ); + + wxLogTrace( traceAutoSave, wxS( "[history] pcb saver serialized %zu bytes for '%s'" ), + aFileData.back().content.size(), dst.GetFullPath() ); } catch( const IO_ERROR& ioe ) { - wxLogTrace( traceAutoSave, wxS( "[history] pcb saver export failed: %s" ), wxString::FromUTF8( ioe.What() ) ); + wxLogTrace( traceAutoSave, wxS( "[history] pcb saver serialize failed: %s" ), wxString::FromUTF8( ioe.What() ) ); } } diff --git a/pcbnew/board.h b/pcbnew/board.h index 219adc79d3..db92920934 100644 --- a/pcbnew/board.h +++ b/pcbnew/board.h @@ -43,6 +43,7 @@ #include #include +struct HISTORY_FILE_DATA; class BOARD_DESIGN_SETTINGS; class BOARD_CONNECTED_ITEM; class BOARD_COMMIT; @@ -1409,14 +1410,15 @@ public: PROJECT::ELEM ProjectElementType() override { return PROJECT::ELEM::BOARD; } /** - * Save board file to the .history directory. + * Serialize board into HISTORY_FILE_DATA for non-blocking history commit. * * This method is used as a saver callback for LOCAL_HISTORY during autosave operations. + * Serialization runs on the UI thread; Prettify and file I/O happen in the background. * * @param aProjectPath The path to check against this board's project path - * @param aFiles Output vector to append absolute file paths for history inclusion + * @param aFileData Output vector to append serialized data for history inclusion */ - void SaveToHistory( const wxString& aProjectPath, std::vector& aFiles ); + void SaveToHistory( const wxString& aProjectPath, std::vector& aFileData ); const std::unordered_map& GetItemByIdCache() const { diff --git a/pcbnew/pcb_edit_frame.cpp b/pcbnew/pcb_edit_frame.cpp index e1c5425d75..9922e5d5de 100644 --- a/pcbnew/pcb_edit_frame.cpp +++ b/pcbnew/pcb_edit_frame.cpp @@ -3065,14 +3065,14 @@ void PCB_EDIT_FRAME::ProjectChanged() PythonSyncProjectName(); // Register autosave history saver for the board. - // Saver exports the in-memory BOARD into the history mirror preserving the original - // relative path and file name (reparented under .history) without touching dirty flags. + // Saver serializes the in-memory BOARD into HISTORY_FILE_DATA. Prettify and + // file I/O happen on a background thread to avoid blocking the UI. if( GetBoard() ) { Kiway().LocalHistory().RegisterSaver( GetBoard(), - [this]( const wxString& aProjectPath, std::vector& aFiles ) + [this]( const wxString& aProjectPath, std::vector& aFileData ) { - GetBoard()->SaveToHistory( aProjectPath, aFiles ); + GetBoard()->SaveToHistory( aProjectPath, aFileData ); } ); } } diff --git a/pcbnew/pcb_io/kicad_sexpr/pcb_io_kicad_sexpr.cpp b/pcbnew/pcb_io/kicad_sexpr/pcb_io_kicad_sexpr.cpp index 47546dfe0b..a73dcdca1c 100644 --- a/pcbnew/pcb_io/kicad_sexpr/pcb_io_kicad_sexpr.cpp +++ b/pcbnew/pcb_io/kicad_sexpr/pcb_io_kicad_sexpr.cpp @@ -312,6 +312,15 @@ void PCB_IO_KICAD_SEXPR::SaveBoard( const wxString& aFileName, BOARD* aBoard, } } + PRETTIFIED_FILE_OUTPUTFORMATTER formatter( aFileName ); + FormatBoardToFormatter( &formatter, aBoard, aProperties ); + formatter.Finish(); +} + + +void PCB_IO_KICAD_SEXPR::FormatBoardToFormatter( OUTPUTFORMATTER* aOut, BOARD* aBoard, + const std::map* aProperties ) +{ init( aProperties ); m_board = aBoard; // after init() @@ -323,9 +332,7 @@ void PCB_IO_KICAD_SEXPR::SaveBoard( const wxString& aFileName, BOARD* aBoard, else m_board->GetEmbeddedFiles()->ClearEmbeddedFonts(); - PRETTIFIED_FILE_OUTPUTFORMATTER formatter( aFileName ); - - m_out = &formatter; // no ownership + m_out = aOut; m_out->Print( "(kicad_pcb (version %d) (generator \"pcbnew\") (generator_version %s)", SEXPR_BOARD_FILE_VERSION, @@ -334,7 +341,6 @@ void PCB_IO_KICAD_SEXPR::SaveBoard( const wxString& aFileName, BOARD* aBoard, Format( aBoard ); m_out->Print( ")" ); - m_out->Finish(); m_out = nullptr; } diff --git a/pcbnew/pcb_io/kicad_sexpr/pcb_io_kicad_sexpr.h b/pcbnew/pcb_io/kicad_sexpr/pcb_io_kicad_sexpr.h index efea2453a3..cdfafee992 100644 --- a/pcbnew/pcb_io/kicad_sexpr/pcb_io_kicad_sexpr.h +++ b/pcbnew/pcb_io/kicad_sexpr/pcb_io_kicad_sexpr.h @@ -349,6 +349,13 @@ public: void SaveBoard( const wxString& aFileName, BOARD* aBoard, const std::map* aProperties = nullptr ) override; + /** Serialize a BOARD to an OUTPUTFORMATTER without file I/O or Prettify. + * Handles init(), EmbedFonts/ClearEmbeddedFonts, header, Format(), and footer. + * The caller owns the formatter and is responsible for flushing/closing it. + * Skips GroupsSanityCheck (no UI interaction allowed from timer callbacks). */ + void FormatBoardToFormatter( OUTPUTFORMATTER* aOut, BOARD* aBoard, + const std::map* aProperties = nullptr ); + BOARD* LoadBoard( const wxString& aFileName, BOARD* aAppendToMe, const std::map* aProperties = nullptr, PROJECT* aProject = nullptr ) override; diff --git a/qa/tests/eeschema/test_history_autosave_legacy.cpp b/qa/tests/eeschema/test_history_autosave_legacy.cpp index 89fef62f56..a6b4edfb1e 100644 --- a/qa/tests/eeschema/test_history_autosave_legacy.cpp +++ b/qa/tests/eeschema/test_history_autosave_legacy.cpp @@ -19,6 +19,7 @@ */ #include +#include #include #include #include @@ -67,15 +68,16 @@ BOOST_FIXTURE_TEST_CASE( SavesLegacySheetIntoHistoryPath, HISTORY_AUTOSAVE_FIXTU sheet->SetFileName( wxS( "legacy/legacy.sch" ) ); sheet->GetScreen()->SetFileName( wxS( "legacy/legacy.sch" ) ); - std::vector savedFiles; - m_schematic->SaveToHistory( m_settingsManager.Prj().GetProjectPath(), savedFiles ); + std::vector fileData; + m_schematic->SaveToHistory( m_settingsManager.Prj().GetProjectPath(), fileData ); - BOOST_REQUIRE_EQUAL( savedFiles.size(), 1 ); + BOOST_REQUIRE_EQUAL( fileData.size(), 1 ); - wxFileName saved( savedFiles[0] ); - BOOST_CHECK( saved.FileExists() ); + wxFileName saved( fileData[0].path ); BOOST_CHECK_EQUAL( saved.GetExt(), FILEEXT::LegacySchematicFileExtension ); BOOST_CHECK( saved.GetPath().Contains( wxS( "legacy" ) ) ); + BOOST_CHECK( !fileData[0].content.empty() ); + BOOST_CHECK( fileData[0].prettify ); } BOOST_AUTO_TEST_SUITE_END()