kicad-developers team mailing list archive

[Maciej Sumiński <maciej.suminski@xxxxxxx>]

Hi Wayne,

Would you have a look at the attached patches? I do not want to
introduce changes to the schematic plugins code without your approval.

The first one removes multiple chunks of code that regenerate properties
basing on the PART_LIB field values. Instead, a PROPERTIES object is
maintained, so it is always ready whenever needed.

Regarding the second patch: I am looking for a way to store temporary
changes to libraries. Libraries are linked to files, and now the binding
is even stronger, as files are reloaded upon a change [1].

It means that once I save only a part of modifications to a library, my
buffer will be reloaded and the remaining unsaved changes will be gone.
Do you think it is reasonable to disable cache updating when buffering
is enabled?

Regards,
Orson

1. https://git.launchpad.net/kicad/tree/eeschema/sch_legacy_plugin.cpp#n3359

From 5eeffc5ac5ea8abcb5534ae14f58d3c35bdd181c Mon Sep 17 00:00:00 2001
From: Maciej Suminski <maciej.suminski@xxxxxxx>
Date: Tue, 28 Feb 2017 10:17:11 +0100
Subject: [PATCH] Store information about buffering & caching in properties

PROPERTIES object has been recreated every time it was needed, using
two fields in PART_LIB class. Now the buffering & caching settings are
stored directly in a PROPERTIES object.
---
 eeschema/class_library.cpp     | 117 ++++++++++++++---------------------------
 eeschema/class_library.h       |  12 ++---
 eeschema/sch_legacy_plugin.cpp |   7 +--
 include/properties.h           |   9 ++++
 4 files changed, 56 insertions(+), 89 deletions(-)

diff --git a/eeschema/class_library.cpp b/eeschema/class_library.cpp
index 11a9db654..7d933d9dc 100644
--- a/eeschema/class_library.cpp
+++ b/eeschema/class_library.cpp
@@ -64,11 +64,9 @@ PART_LIB::PART_LIB( int aType, const wxString& aFileName, SCH_IO_MGR::SCH_FILE_T
     type = aType;
     isModified = false;
     timeStamp = 0;
-    isCache = false;
     timeStamp = wxDateTime::Now();
     versionMajor = 0;       // Will be updated after reading the lib file
     versionMinor = 0;       // Will be updated after reading the lib file
-    m_buffering = false;
 
     fileName = aFileName;
 
@@ -76,6 +74,7 @@ PART_LIB::PART_LIB( int aType, const wxString& aFileName, SCH_IO_MGR::SCH_FILE_T
         fileName = "unnamed.lib";
 
     m_plugin.reset( SCH_IO_MGR::FindPlugin( m_pluginType ) );
+    m_properties = std::make_unique<PROPERTIES>();
 }
 
 
@@ -89,35 +88,24 @@ void PART_LIB::Save( bool aSaveDocFile )
     wxCHECK_RET( m_plugin != NULL, wxString::Format( "no plugin defined for library `%s`.",
                                                      fileName.GetFullPath() ) );
 
-    std::unique_ptr< PROPERTIES > props;
+    PROPERTIES props;
 
     if( !aSaveDocFile )
-    {
-        props.reset( new PROPERTIES );
-        (*props.get())[ SCH_LEGACY_PLUGIN::PropNoDocFile ] = "";
-    }
+        props[ SCH_LEGACY_PLUGIN::PropNoDocFile ] = "";
 
-    m_plugin->SaveLibrary( fileName.GetFullPath(), props.get() );
+    m_plugin->SaveLibrary( fileName.GetFullPath(), &props );
     isModified = false;
 }
 
 
 void PART_LIB::Create( const wxString& aFileName )
 {
-    std::unique_ptr< PROPERTIES > props;
-
-    if( isCache )
-    {
-        props.reset( new PROPERTIES );
-        (*props.get())[ SCH_LEGACY_PLUGIN::PropNoDocFile ] = "";
-    }
-
     wxString tmpFileName = fileName.GetFullPath();
 
     if( !aFileName.IsEmpty() )
         tmpFileName = aFileName;
 
-    m_plugin->CreateSymbolLib( tmpFileName, props.get() );
+    m_plugin->CreateSymbolLib( tmpFileName, m_properties.get() );
 }
 
 
@@ -131,6 +119,33 @@ void PART_LIB::SetPluginType( SCH_IO_MGR::SCH_FILE_T aPluginType )
 }
 
 
+bool PART_LIB::IsCache() const
+{
+    return m_properties->Exists( SCH_LEGACY_PLUGIN::PropNoDocFile );
+}
+
+
+void PART_LIB::SetCache()
+{
+    (*m_properties)[ SCH_LEGACY_PLUGIN::PropNoDocFile ] = "";
+}
+
+
+bool PART_LIB::IsBuffering() const
+{
+    return m_properties->Exists( SCH_LEGACY_PLUGIN::PropBuffering );
+}
+
+
+void PART_LIB::EnableBuffering( bool aEnable )
+{
+    if( aEnable )
+        (*m_properties)[ SCH_LEGACY_PLUGIN::PropBuffering ] = "";
+    else
+        m_properties->Clear( SCH_LEGACY_PLUGIN::PropBuffering );
+}
+
+
 void PART_LIB::GetAliasNames( wxArrayString& aNames )
 {
     m_plugin->EnumerateSymbolLib( aNames, fileName.GetFullPath() );
@@ -177,20 +192,7 @@ void PART_LIB::GetEntryTypePowerNames( wxArrayString& aNames )
 
 LIB_ALIAS* PART_LIB::FindAlias( const wxString& aName )
 {
-    std::unique_ptr< PROPERTIES > props;
-
-    if( isCache || m_buffering )
-    {
-        props.reset( new PROPERTIES );
-
-        if( isCache )
-            (*props.get())[ SCH_LEGACY_PLUGIN::PropNoDocFile ] = "";
-
-        if( m_buffering )
-            (*props.get())[ SCH_LEGACY_PLUGIN::PropBuffering ] = "";
-    }
-
-    return m_plugin->LoadSymbol( fileName.GetFullPath(), aName, props.get() );
+    return m_plugin->LoadSymbol( fileName.GetFullPath(), aName, m_properties.get() );
 }
 
 
@@ -232,25 +234,12 @@ bool PART_LIB::HasPowerParts()
 
 void PART_LIB::AddPart( LIB_PART* aPart )
 {
-    std::unique_ptr< PROPERTIES > props;
-
-    if( isCache || m_buffering )
-    {
-        props.reset( new PROPERTIES );
-
-        if( isCache )
-            (*props.get())[ SCH_LEGACY_PLUGIN::PropNoDocFile ] = "";
-
-        if( m_buffering )
-            (*props.get())[ SCH_LEGACY_PLUGIN::PropBuffering ] = "";
-    }
-
     // add a clone, not the caller's copy, the plugin take ownership of the new symbol.
-    m_plugin->SaveSymbol( fileName.GetFullPath(), new LIB_PART( *aPart, this ), props.get() );
+    m_plugin->SaveSymbol( fileName.GetFullPath(), new LIB_PART( *aPart, this ), m_properties.get() );
 
     // If we are not buffering, the library file is updated immediately when the plugin
     // SaveSymbol() function is called.
-    if( m_buffering )
+    if( IsBuffering() )
         isModified = true;
 
     ++m_mod_hash;
@@ -261,24 +250,11 @@ LIB_ALIAS* PART_LIB::RemoveAlias( LIB_ALIAS* aEntry )
 {
     wxCHECK_MSG( aEntry != NULL, NULL, "NULL pointer cannot be removed from library." );
 
-    std::unique_ptr< PROPERTIES > props;
-
-    if( isCache || m_buffering )
-    {
-        props.reset( new PROPERTIES );
-
-        if( isCache )
-            (*props.get())[ SCH_LEGACY_PLUGIN::PropNoDocFile ] = "";
-
-        if( m_buffering )
-            (*props.get())[ SCH_LEGACY_PLUGIN::PropBuffering ] = "";
-    }
-
-    m_plugin->DeleteAlias( fileName.GetFullPath(), aEntry->GetName(), props.get() );
+    m_plugin->DeleteAlias( fileName.GetFullPath(), aEntry->GetName(), m_properties.get() );
 
     // If we are not buffering, the library file is updated immediately when the plugin
     // SaveSymbol() function is called.
-    if( m_buffering )
+    if( IsBuffering() )
         isModified = true;
 
     ++m_mod_hash;
@@ -291,28 +267,15 @@ LIB_PART* PART_LIB::ReplacePart( LIB_PART* aOldPart, LIB_PART* aNewPart )
     wxASSERT( aOldPart != NULL );
     wxASSERT( aNewPart != NULL );
 
-    std::unique_ptr< PROPERTIES > props;
-
-    if( isCache || m_buffering )
-    {
-        props.reset( new PROPERTIES );
-
-        if( isCache )
-            (*props.get())[ SCH_LEGACY_PLUGIN::PropNoDocFile ] = "";
-
-        if( m_buffering )
-            (*props.get())[ SCH_LEGACY_PLUGIN::PropBuffering ] = "";
-    }
-
-    m_plugin->DeleteSymbol( fileName.GetFullPath(), aOldPart->GetName(), props.get() );
+    m_plugin->DeleteSymbol( fileName.GetFullPath(), aOldPart->GetName(), m_properties.get() );
 
     LIB_PART* my_part = new LIB_PART( *aNewPart, this );
 
-    m_plugin->SaveSymbol( fileName.GetFullPath(), my_part, props.get() );
+    m_plugin->SaveSymbol( fileName.GetFullPath(), my_part, m_properties.get() );
 
     // If we are not buffering, the library file is updated immediately when the plugin
     // SaveSymbol() function is called.
-    if( m_buffering )
+    if( IsBuffering() )
         isModified = true;
 
     ++m_mod_hash;
diff --git a/eeschema/class_library.h b/eeschema/class_library.h
index 548ccae05..59c7144ee 100644
--- a/eeschema/class_library.h
+++ b/eeschema/class_library.h
@@ -328,15 +328,13 @@ class PART_LIB
     wxDateTime      timeStamp;      ///< Library save time and date.
     int             versionMajor;   ///< Library major version number.
     int             versionMinor;   ///< Library minor version number.
-    bool            isCache;        /**< False for the "standard" libraries,
-                                         True for the library cache */
     wxString        header;         ///< first line of loaded library.
     bool            isModified;     ///< Library modification status.
     int             m_mod_hash;     ///< incremented each time library is changed.
-    bool            m_buffering;    ///< Set to true to prevent file write on every change.
 
     SCH_IO_MGR::SCH_FILE_T        m_pluginType;
     std::unique_ptr< SCH_PLUGIN > m_plugin;
+    std::unique_ptr< PROPERTIES > m_properties;   ///< Library properties
 
 public:
     PART_LIB( int aType, const wxString& aFileName,
@@ -379,11 +377,13 @@ public:
         return isModified;
     }
 
-    bool IsCache() const { return isCache; }
+    bool IsCache() const;
 
-    void SetCache( void ) { isCache = true; }
+    void SetCache();
 
-    void EnableBuffering( bool aEnable = true ) { m_buffering = aEnable; }
+    bool IsBuffering() const;
+
+    void EnableBuffering( bool aEnable = true );
 
     void Save( bool aSaveDocFile = true );
 
diff --git a/eeschema/sch_legacy_plugin.cpp b/eeschema/sch_legacy_plugin.cpp
index 9a29bad47..25cd58136 100644
--- a/eeschema/sch_legacy_plugin.cpp
+++ b/eeschema/sch_legacy_plugin.cpp
@@ -3383,12 +3383,7 @@ bool SCH_LEGACY_PLUGIN::writeDocFile( const PROPERTIES* aProperties )
 
 bool SCH_LEGACY_PLUGIN::isBuffering( const PROPERTIES* aProperties )
 {
-    std::string propName( SCH_LEGACY_PLUGIN::PropBuffering );
-
-    if( aProperties && aProperties->find( propName ) != aProperties->end() )
-        return true;
-
-    return false;
+    return ( aProperties && aProperties->Exists( SCH_LEGACY_PLUGIN::PropBuffering ) );
 }
 
 
diff --git a/include/properties.h b/include/properties.h
index 8202cd5dc..66043da3b 100644
--- a/include/properties.h
+++ b/include/properties.h
@@ -36,6 +36,15 @@ class PROPERTIES : public std::map< std::string, UTF8 >
     // alphabetical tuple of name and value hereby defined.
 
 public:
+    bool Clear( const std::string& aProperty )
+    {
+        return erase( aProperty ) > 0;
+    }
+
+    bool Exists( const std::string& aProperty ) const
+    {
+        return count( aProperty ) > 0;
+    }
 
     /**
      * Function Value
-- 
2.11.0

From cec7fad17e81c3f15275df5d73405c6cb297ebd0 Mon Sep 17 00:00:00 2001
From: Maciej Suminski <maciej.suminski@xxxxxxx>
Date: Tue, 28 Feb 2017 10:33:11 +0100
Subject: [PATCH 2/2] Do not update cache for buffered libraries in eeschema

---
 eeschema/sch_legacy_plugin.cpp | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/eeschema/sch_legacy_plugin.cpp b/eeschema/sch_legacy_plugin.cpp
index 25cd58136..76a286efc 100644
--- a/eeschema/sch_legacy_plugin.cpp
+++ b/eeschema/sch_legacy_plugin.cpp
@@ -3358,14 +3358,14 @@ void SCH_LEGACY_PLUGIN_CACHE::DeleteSymbol( const wxString& aAliasName )
 
 void SCH_LEGACY_PLUGIN::cacheLib( const wxString& aLibraryFileName )
 {
-    if( !m_cache || !m_cache->IsFile( aLibraryFileName ) || m_cache->IsFileChanged() )
+    // buffered libraries are not reloaded
+    if( !m_cache || !m_cache->IsFile( aLibraryFileName )
+            || ( !isBuffering( m_props ) && m_cache->IsFileChanged() ) )
     {
         // a spectacular episode in memory management:
         delete m_cache;
         m_cache = new SCH_LEGACY_PLUGIN_CACHE( aLibraryFileName );
-
-        if( !isBuffering( m_props ) )
-            m_cache->Load();
+        m_cache->Load();
     }
 }
 
-- 
2.11.0

Attachment: signature.asc
Description: OpenPGP digital signature