Make SVG import an advanced config, not a compile option.

This demos the advanced config and allows no-recompile switching
of the SVG importer. It also allows the import manager to be
tested more completely.

common/advanced_config.cpp

@@ -47,6 +47,20 @@ static const wxChar AdvancedConfigMask[] = wxT( "KICAD_ADVANCED_CONFIG" );
  */
 namespace AC_KEYS
 {
+
+/**
+ * Currently (Version 5.1) SVG import is disabled by default, to avoid issues
+ * SVG needs some enhancements.
+ *
+ * Especially, all SVG shapes are imported as curves and converted to a lot of segments.
+ * A better approach is to convert to polylines (not yet existing in Pcbnew) and keep
+ * arcs and circles as primitives (not yet possible with tinysvg library.
+ * So, until these issues are solved, disable SVG import option.
+ *
+ * Warning: enable svg import is currently only for developers.
+ */
+static const wxChar EnableSvgImport[] = wxT( "EnableSvgImport" );
+
 } // namespace KEYS
 
 
@@ -121,6 +135,11 @@ static wxFileName getAdvancedCfgFilename()
 ADVANCED_CFG::ADVANCED_CFG()
 {
     wxLogTrace( AdvancedConfigMask, "Init advanced config" );
+
+    // Init defaults - this is done in case the config doesn't exist,
+    // then the values will remain as set here.
+    m_enableSvgImport = false;
+
     loadFromConfigFile();
 }
 
@@ -153,7 +172,8 @@ void ADVANCED_CFG::loadSettings( wxConfigBase& aCfg )
 {
     PARAM_CFG_ARRAY configParams;
 
-    // Add configs here
+    configParams.push_back(
+            new PARAM_CFG_BOOL( true, AC_KEYS::EnableSvgImport, &m_enableSvgImport, false ) );
 
     wxConfigLoadSetups( &aCfg, configParams );
 

include/advanced_config.h

@@ -68,6 +68,11 @@ public:
      */
     static const ADVANCED_CFG& GetCfg();
 
+    /**
+     * Enable SVG import.
+     */
+    bool m_enableSvgImport;
+
 private:
     ADVANCED_CFG();
 

pcbnew/CMakeLists.txt

@@ -852,23 +852,3 @@ if( APPLE )
             )
     endif()
 endif()
-
-# Experimental feature flags
-#
-
-# Currently (Version 5.1) SVG import is disabled by default, to avoid issues
-# SVG needs some enhancements.
-# Especially, all SVG shapes are imported as curves and converted to a lot of segments.
-# A better approach is to convert to polylines (not yet existing in Pcbnew) and keep
-# arcs and circles as primitives (not yet possible with tinysvg library.
-# So, until these issues are solved, disable SVG import option.
-# Warning: enable svg import is currently only for developers.
-option( KICAD_EXPERIMENTAL_ENABLE_SVG "Experimental: Enable SVG import" OFF)
-
-if( NOT KICAD_EXPERIMENTAL_ENABLE_SVG )
-    set_property(
-        SOURCE import_gfx/dialog_import_gfx.cpp
-        PROPERTY COMPILE_DEFINITIONS DISABLE_SVG_IMPORT
-        APPEND
-    )
-endif()

pcbnew/import_gfx/dialog_import_gfx.cpp

@@ -27,8 +27,11 @@
  * 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA
  */
 
-#include <kiface_i.h>
+#include "dialog_import_gfx.h"
+
+#include <advanced_config.h>
 #include <convert_to_biu.h>
+#include <kiface_i.h>
 #include <pcb_layer_box_selector.h>
 #include <wildcards_and_files_ext.h>
 
@@ -37,7 +40,6 @@
 #include <class_edge_mod.h>
 #include <class_text_mod.h>
 #include <class_pcb_text.h>
-#include "dialog_import_gfx.h"
 
 // Keys to store setup in config
 #define IMPORT_GFX_LAYER_OPTION_KEY             "GfxImportBrdLayer"
@@ -66,6 +68,16 @@ DIALOG_IMPORT_GFX::DIALOG_IMPORT_GFX( PCB_BASE_FRAME* aParent, bool aImportAsFoo
     else
         m_importer.reset( new GRAPHICS_IMPORTER_BOARD( m_parent->GetBoard() ) );
 
+    // construct an import manager with options from config
+    {
+        GRAPHICS_IMPORT_MGR::TYPE_LIST blacklist;
+
+        if( !ADVANCED_CFG::GetCfg().m_enableSvgImport )
+            blacklist.push_back( GRAPHICS_IMPORT_MGR::SVG );
+
+        m_gfxImportMgr = std::make_unique<GRAPHICS_IMPORT_MGR>( blacklist );
+    }
+
     m_config = Kiface().KifaceSettings();
     m_originImportUnits = 0;
     m_importOrigin.x = 0.0;         // always in mm
@@ -232,15 +244,11 @@ void DIALOG_IMPORT_GFX::onBrowseFiles( wxCommandEvent& event )
 
     // Generate the list of handled file formats
     wxString wildcardsDesc;
-
-#ifdef DISABLE_SVG_IMPORT
-    wildcardsDesc = DxfFileWildcard();
-#else
     wxString allWildcards;
 
-    for( auto pluginType : GRAPHICS_IMPORT_MGR::GFX_FILE_TYPES )
+    for( auto pluginType : m_gfxImportMgr->GetImportableFileTypes() )
     {
-        auto plugin = GRAPHICS_IMPORT_MGR::GetPlugin( pluginType );
+        auto       plugin = m_gfxImportMgr->GetPlugin( pluginType );
         const auto wildcards = plugin->GetWildcards();
 
         wildcardsDesc += "|" + plugin->GetName() + " (" + wildcards + ")|" + wildcards;
@@ -248,7 +256,6 @@ void DIALOG_IMPORT_GFX::onBrowseFiles( wxCommandEvent& event )
     }
 
     wildcardsDesc = "All supported formats|" + allWildcards + wildcardsDesc;
-#endif
 
     wxFileDialog dlg( m_parent, _( "Open File" ), path, filename,
                        wildcardsDesc, wxFD_OPEN|wxFD_FILE_MUST_EXIST );
@@ -289,11 +296,8 @@ void DIALOG_IMPORT_GFX::onOKClick( wxCommandEvent& event )
     m_default_lineWidth = getPCBdefaultLineWidthMM();
 
     m_importer->SetLayer( PCB_LAYER_ID( m_layer ) );
-#ifdef DISABLE_SVG_IMPORT
+
-    auto plugin = GRAPHICS_IMPORT_MGR::GetPluginByExt( "dxf" );
+    auto plugin = m_gfxImportMgr->GetPluginByExt( wxFileName( m_filename ).GetExt() );
-#else
-    auto plugin = GRAPHICS_IMPORT_MGR::GetPluginByExt( wxFileName( m_filename ).GetExt() );
-#endif
 
     if( plugin )
     {

pcbnew/import_gfx/dialog_import_gfx.h

@@ -26,6 +26,8 @@
 #include "dialog_import_gfx_base.h"
 #include <import_gfx/graphics_importer_pcbnew.h>
 
+class GRAPHICS_IMPORT_MGR;
+
 class DIALOG_IMPORT_GFX : public DIALOG_IMPORT_GFX_BASE
 {
 public:
@@ -55,6 +57,7 @@ private:
     PCB_BASE_FRAME*      m_parent;
     wxConfigBase*        m_config;              // Current config
     std::unique_ptr<GRAPHICS_IMPORTER_PCBNEW> m_importer;
+    std::unique_ptr<GRAPHICS_IMPORT_MGR>      m_gfxImportMgr;
     int                  m_originImportUnits;
     VECTOR2D             m_importOrigin;        // This is the offset to add to imported coordinates
                                                 // Always in mm

pcbnew/import_gfx/graphics_import_mgr.cpp

@@ -27,34 +27,44 @@
 #include "dxf_import_plugin.h"
 #include "svg_import_plugin.h"
 
-using namespace std;
 
-unique_ptr<GRAPHICS_IMPORT_PLUGIN> GRAPHICS_IMPORT_MGR::GetPlugin( GFX_FILE_T aType )
+GRAPHICS_IMPORT_MGR::GRAPHICS_IMPORT_MGR( const TYPE_LIST& aBlacklist )
 {
-    unique_ptr<GRAPHICS_IMPORT_PLUGIN> ret;
+    // This is the full list of types, from which we'll subtract our blacklist
+    static const TYPE_LIST all_types = {
+        DXF,
+        SVG,
+    };
+
+    std::copy_if( all_types.begin(), all_types.end(), std::back_inserter( m_importableTypes ),
+            [&aBlacklist]( const GFX_FILE_T& arg ) {
+                return ( std::find( aBlacklist.begin(), aBlacklist.end(), arg )
+                         == aBlacklist.end() );
+            } );
+}
+
+
+std::unique_ptr<GRAPHICS_IMPORT_PLUGIN> GRAPHICS_IMPORT_MGR::GetPlugin( GFX_FILE_T aType ) const
+{
+    std::unique_ptr<GRAPHICS_IMPORT_PLUGIN> ret;
 
     switch( aType )
     {
-        case DXF:
+    case DXF: ret = std::make_unique<DXF_IMPORT_PLUGIN>(); break;
-            ret.reset( new DXF_IMPORT_PLUGIN() );
-            break;
 
-        case SVG:
+    case SVG: ret = std::make_unique<SVG_IMPORT_PLUGIN>(); break;
-            ret.reset( new SVG_IMPORT_PLUGIN() );
-            break;
 
-        default:
+    default: throw std::runtime_error( "Unhandled graphics format" ); break;
-            throw std::runtime_error( "Unhandled graphics format" );
-            break;
     }
 
     return ret;
 }
 
 
-unique_ptr<GRAPHICS_IMPORT_PLUGIN> GRAPHICS_IMPORT_MGR::GetPluginByExt( const wxString& aExtension )
+std::unique_ptr<GRAPHICS_IMPORT_PLUGIN> GRAPHICS_IMPORT_MGR::GetPluginByExt(
+        const wxString& aExtension ) const
 {
-    for( auto fileType : GFX_FILE_TYPES )
+    for( auto fileType : GetImportableFileTypes() )
     {
         auto plugin = GetPlugin( fileType );
         auto fileExtensions = plugin->GetFileExtensions();
@@ -66,8 +76,5 @@ unique_ptr<GRAPHICS_IMPORT_PLUGIN> GRAPHICS_IMPORT_MGR::GetPluginByExt( const wx
         }
     }
 
-    return unique_ptr<GRAPHICS_IMPORT_PLUGIN>();
+    return {};
-}
+}
-
-
-const vector<GRAPHICS_IMPORT_MGR::GFX_FILE_T> GRAPHICS_IMPORT_MGR::GFX_FILE_TYPES = { DXF, SVG };

pcbnew/import_gfx/graphics_import_mgr.h

@@ -44,19 +44,29 @@ public:
         SVG
     };
 
-    ///> Vector containing all GFX_FILE_T values.
+    using TYPE_LIST = std::vector<GFX_FILE_T>;
-    static const std::vector<GFX_FILE_T> GFX_FILE_TYPES;
+
+    /**
+     * Construct an import plugin manager, with a specified list of filetypes
+     * that are not permitted (can be used for when some file type support
+     * is not available due to config or other reasons)
+     */
+    GRAPHICS_IMPORT_MGR( const TYPE_LIST& aBlacklist );
+
+    ///> Vector containing all GFX_FILE_T values that can be imported.
+    TYPE_LIST GetImportableFileTypes() const
+    {
+        return m_importableTypes;
+    }
 
     ///> Returns a plugin that handles a specific file extension.
-    static std::unique_ptr<GRAPHICS_IMPORT_PLUGIN> GetPluginByExt( const wxString& aExtension );
+    std::unique_ptr<GRAPHICS_IMPORT_PLUGIN> GetPluginByExt( const wxString& aExtension ) const;
 
     ///> Returns a plugin instance for a specific file type.
-    static std::unique_ptr<GRAPHICS_IMPORT_PLUGIN> GetPlugin( GFX_FILE_T aType );
+    std::unique_ptr<GRAPHICS_IMPORT_PLUGIN> GetPlugin( GFX_FILE_T aType ) const;
 
 private:
-    GRAPHICS_IMPORT_MGR()
+    TYPE_LIST m_importableTypes;
-    {
-    }
 };
 
 #endif /* GRAPHICS_IMPORT_MGR_H */