← Back to team overview

kicad-developers team mailing list archive

[PATCH] Run time advanced options

 

Hi,

This is a patch for run time options, which are a more flexible alternative
to compiler flags. Advantages include:

* you can change the config without rebuilding
* it's sensitive to XDG_CONFIG_DIR, so you can quickly flip back and forth
* there's better documention for the config (it's in doxygen all in one
place) and theres a trace to show it
* better type safety if the config has a value
* better compiler coverage, so less opportunity to break a different build
which uses a different preprocessor block and better static analysis
coverage

These configs are not intended for general use by users so they are in
their own file. You don't need this file, if you don't have it, you get
defaults.

The second patch uses the framework to add a config for the SVG import. The
disablement is recast so it happens in a single place, and the same system
could be used in future for other experimental importers. The reason it's
done as a blacklist is so that a unit test could be written that *doesn't*
disable the SVG plugin. However, nothing in Pcbnew can be unit tested yet,
as I haven't worked out how to link Pcbnew code as a unit test.

The third patch does another one for disabling legacy canvas on GTK3.
Again, this is done at run-time to avoid conditionally compiling code.

Cheers,

John
From 149eaafc2b9dc8fc453b73d8ffb76cc4206311fe Mon Sep 17 00:00:00 2001
From: John Beard <john.j.beard@gmail.com>
Date: Mon, 10 Dec 2018 10:56:28 +0000
Subject: [PATCH] Run-time config for advanced options

This can be used for "advanced" options which are for developers
to use for feature-flags and other configuration.

No advanced config should be required by a general users. If a general
user does use one of these configs, it's probably because:

* There is a bug and one of these configs is a workaround
* A config in here is generally useful and should be moved into the
  relevant application config and given UI.

For now, the config is read-only, and is read from the
kicad_advanced config file in the normal config dir.

This means that by changing XDG_CONFIG_DIR, you can load "pre-made"
configurations, which allows easier A/B testing.

The first advanced options is SVG import, which was previously
an ifdef, which needed a recompilation to enable. Now it can be
enabled by setting the config.
---
 common/CMakeLists.txt                     |   1 +
 common/advanced_config.cpp                | 238 ++++++++++++++++++++++
 include/advanced_config.h                 |  69 +++++++
 pcbnew/CMakeLists.txt                     |  20 --
 pcbnew/import_gfx/dialog_import_gfx.cpp   |  20 +-
 pcbnew/import_gfx/graphics_import_mgr.cpp |  20 +-
 pcbnew/import_gfx/graphics_import_mgr.h   |   6 +-
 7 files changed, 336 insertions(+), 38 deletions(-)
 create mode 100644 common/advanced_config.cpp
 create mode 100644 include/advanced_config.h

diff --git a/common/CMakeLists.txt b/common/CMakeLists.txt
index 569543a87..1ef0850b6 100644
--- a/common/CMakeLists.txt
+++ b/common/CMakeLists.txt
@@ -269,6 +269,7 @@ set( COMMON_SRCS
     ${COMMON_PAGE_LAYOUT_SRCS}
     ${COMMON_PREVIEW_ITEMS_SRCS}
     ${PLOTTERS_CONTROL_SRCS}
+    advanced_config.cpp
     base_struct.cpp
     bezier_curves.cpp
     bin_mod.cpp
diff --git a/common/advanced_config.cpp b/common/advanced_config.cpp
new file mode 100644
index 000000000..849f91f7e
--- /dev/null
+++ b/common/advanced_config.cpp
@@ -0,0 +1,238 @@
+/*
+ * This program source code file is part of KiCad, a free EDA CAD application.
+ *
+ * Copyright (C) 2018 KiCad Developers, see AUTHORS.txt for contributors.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, you may find one here:
+ * http://www.gnu.org/licenses/old-licenses/gpl-2.0.html
+ * or you may search the http://www.gnu.org website for the version 2 license,
+ * or you may write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA
+ */
+
+#include <advanced_config.h>
+
+#include <common.h>
+#include <config_params.h>
+
+#include <wx/config.h>
+#include <wx/filename.h>
+#include <wx/log.h>
+
+/*
+ * Flag to enable advanced config debugging
+ *
+ * Use "KICAD_ADVANCED_CONFIG" to enable.
+ *
+ * @ingroup trace_env_vars
+ */
+static const wxChar AdvancedConfigMask[] = wxT( "KICAD_ADVANCED_CONFIG" );
+
+
+namespace ADVANCED_CFG
+{
+
+
+/**
+ * List of known keys for config files.
+ */
+namespace 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
+
+
+/*
+ * Get a simple string for common parameters.
+ *
+ * This isn't exhaustive, but it covers most common types that might be
+ * used in the advance config
+ */
+wxString dumpParamCfg( const PARAM_CFG_BASE& aParam )
+{
+    wxString s = aParam.m_Ident + ": ";
+
+    /*
+     * This implementation is rather simplistic, but it is
+     * effective enough for simple uses. A better implementation would be
+     * some kind of visitor, but that's somewhat more work.
+     */
+    switch( aParam.m_Type )
+    {
+    case paramcfg_id::PARAM_INT:
+    case paramcfg_id::PARAM_INT_WITH_SCALE:
+        s << *static_cast<const PARAM_CFG_INT&>( aParam ).m_Pt_param;
+        break;
+    case paramcfg_id::PARAM_DOUBLE:
+        s << *static_cast<const PARAM_CFG_DOUBLE&>( aParam ).m_Pt_param;
+        break;
+    case paramcfg_id::PARAM_WXSTRING:
+        s << *static_cast<const PARAM_CFG_WXSTRING&>( aParam ).m_Pt_param;
+        break;
+    case paramcfg_id::PARAM_FILENAME:
+        s << *static_cast<const PARAM_CFG_FILENAME&>( aParam ).m_Pt_param;
+        break;
+    case paramcfg_id::PARAM_BOOL:
+        s << ( *static_cast<const PARAM_CFG_BOOL&>( aParam ).m_Pt_param ? "true" : "false" );
+        break;
+    default: s << "Unsupported PARAM_CFG variant: " << aParam.m_Type;
+    }
+
+    return s;
+}
+
+
+/**
+ * Dump the config to trace.
+ */
+static void dumpCfg( const PARAM_CFG_ARRAY& aArray )
+{
+    // only dump if we need to
+    if( !wxLog::IsAllowedTraceMask( AdvancedConfigMask ) )
+        return;
+
+    for( const auto& param : aArray )
+    {
+        wxLogTrace( AdvancedConfigMask, dumpParamCfg( param ) );
+    }
+}
+
+
+/**
+ * Singleton provider CFG object from the config file.
+ *
+ * This configuration is read-only - to set options, users should
+ * add the parameters to their config files at ~/.config/kicad/advanced, or the
+ * platform equivalent.
+ */
+class PROVIDER
+{
+public:
+    PROVIDER();
+
+    /**
+     * Get the singleton instance's config, which is shared by all
+     * consumers of advanced config.
+     */
+    static CFG& GetInstanceCfg();
+
+private:
+    /**
+     * Load the config from the normal config file
+     */
+    void loadFromConfigFile();
+
+    /*
+     * Load config from the given config base
+     */
+    void loadSettings( wxConfigBase& aCfg );
+
+    /**
+     * Storage of the actual config options
+     */
+    CFG m_cfg;
+
+    /**
+     * Become true when config is read (used for lazy-loading)
+     */
+    bool m_inited;
+};
+
+
+/**
+ * Get the filename for the advanced config file
+ *
+ * The user must check the file exists if they care.
+ */
+static wxFileName getAdvancedCfgFilename()
+{
+    const static wxString cfg_filename{ "kicad_advanced" };
+    return wxFileName( GetKicadConfigPath(), cfg_filename );
+}
+
+
+PROVIDER::PROVIDER() : m_inited( false )
+{
+}
+
+
+CFG& PROVIDER::GetInstanceCfg()
+{
+    static PROVIDER instance;
+
+    if( !instance.m_inited )
+    {
+        wxLogTrace( AdvancedConfigMask, "Init advanced config" );
+        instance.loadFromConfigFile();
+        instance.m_inited = true;
+    }
+
+    return instance.m_cfg;
+}
+
+
+void PROVIDER::loadFromConfigFile()
+{
+    const auto k_advanced = getAdvancedCfgFilename();
+
+    if( !k_advanced.FileExists() )
+    {
+        wxLogTrace( AdvancedConfigMask, "File does not exist %s", k_advanced.GetFullPath() );
+        return;
+    }
+
+    wxLogTrace( AdvancedConfigMask, "Loading advanced config from: %s", k_advanced.GetFullPath() );
+
+    wxFileConfig file_cfg( "", "", k_advanced.GetFullPath() );
+    loadSettings( file_cfg );
+}
+
+
+void PROVIDER::loadSettings( wxConfigBase& aCfg )
+{
+    PARAM_CFG_ARRAY configParams;
+
+    configParams.push_back(
+            new PARAM_CFG_BOOL( true, KEYS::EnableSvgImport, &m_cfg.m_enableSvgImport, false ) );
+
+    wxConfigLoadSetups( &aCfg, configParams );
+
+    dumpCfg( configParams );
+}
+
+
+CFG::CFG() : m_enableSvgImport( false )
+{
+}
+
+
+const CFG& GetCfg()
+{
+    const auto& instance = PROVIDER::GetInstanceCfg();
+    return instance;
+}
+
+} // namespace ADVANCED_CFG
\ No newline at end of file
diff --git a/include/advanced_config.h b/include/advanced_config.h
new file mode 100644
index 000000000..1221e0b8e
--- /dev/null
+++ b/include/advanced_config.h
@@ -0,0 +1,69 @@
+/*
+ * This program source code file is part of KiCad, a free EDA CAD application.
+ *
+ * Copyright (C) 2018 KiCad Developers, see AUTHORS.txt for contributors.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, you may find one here:
+ * http://www.gnu.org/licenses/old-licenses/gpl-2.0.html
+ * or you may search the http://www.gnu.org website for the version 2 license,
+ * or you may write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA
+ */
+
+#ifndef ADVANCED_CFG__H
+#define ADVANCED_CFG__H
+
+namespace ADVANCED_CFG
+{
+
+/**
+ * Class containing "advanced" configuration options.
+ *
+ * Options set here are for developer or advanced users only. If a general user
+ * needs to set one of these for normal KiCad use, either:
+ * * They are working around some bug that should be fixed, or
+ * * The parameter they are setting is of general interest and should be in the
+ *   main application config, with UI provided.
+ *
+ * Options in this class are, in general, preferable to #defines, as they
+ * allow more flexible configuration by developers, and don't hide code from
+ * the compiler on other configurations, which can result in broken builds.
+ *
+ * Sometimes you can just use values directly, and sometimes helper functions
+ * might be provided to allow extra logic (for example when a advanced config
+ * applies only on certain platforms).
+ *
+ * For more information on what config keys set these parameters, and why
+ * you might want to set them, see #ADVANCED_CFG::KEYS
+ */
+class CFG
+{
+public:
+    CFG();
+
+    /**
+     * Enable SVG import.
+     */
+    bool m_enableSvgImport;
+};
+
+/**
+ * Gets the "main" advanced config options, which are static and stared within
+ * this process.
+ */
+const CFG& GetCfg();
+
+} // namespace ADVANCED_CFG
+
+#endif // ADVANCED_CFG__H
diff --git a/pcbnew/CMakeLists.txt b/pcbnew/CMakeLists.txt
index a42519472..1b3d6721f 100644
--- a/pcbnew/CMakeLists.txt
+++ b/pcbnew/CMakeLists.txt
@@ -853,23 +853,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()
\ No newline at end of file
diff --git a/pcbnew/import_gfx/dialog_import_gfx.cpp b/pcbnew/import_gfx/dialog_import_gfx.cpp
index 48f098876..4b973e99c 100644
--- a/pcbnew/import_gfx/dialog_import_gfx.cpp
+++ b/pcbnew/import_gfx/dialog_import_gfx.cpp
@@ -32,12 +32,12 @@
 #include <pcb_layer_box_selector.h>
 #include <wildcards_and_files_ext.h>
 
+#include "dialog_import_gfx.h"
 #include <class_board.h>
-#include <class_module.h>
 #include <class_edge_mod.h>
-#include <class_text_mod.h>
+#include <class_module.h>
 #include <class_pcb_text.h>
-#include "dialog_import_gfx.h"
+#include <class_text_mod.h>
 
 // Keys to store setup in config
 #define IMPORT_GFX_LAYER_OPTION_KEY             "GfxImportBrdLayer"
@@ -232,15 +232,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 : GRAPHICS_IMPORT_MGR::GetImportableFileTypes() )
     {
-        auto plugin = GRAPHICS_IMPORT_MGR::GetPlugin( pluginType );
+        auto       plugin = GRAPHICS_IMPORT_MGR::GetPlugin( pluginType );
         const auto wildcards = plugin->GetWildcards();
 
         wildcardsDesc += "|" + plugin->GetName() + " (" + wildcards + ")|" + wildcards;
@@ -248,7 +244,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 +284,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" );
-#else
+
     auto plugin = GRAPHICS_IMPORT_MGR::GetPluginByExt( wxFileName( m_filename ).GetExt() );
-#endif
 
     if( plugin )
     {
diff --git a/pcbnew/import_gfx/graphics_import_mgr.cpp b/pcbnew/import_gfx/graphics_import_mgr.cpp
index c988584e8..44a0bff0a 100644
--- a/pcbnew/import_gfx/graphics_import_mgr.cpp
+++ b/pcbnew/import_gfx/graphics_import_mgr.cpp
@@ -27,6 +27,8 @@
 #include "dxf_import_plugin.h"
 #include "svg_import_plugin.h"
 
+#include <advanced_config.h>
+
 using namespace std;
 
 unique_ptr<GRAPHICS_IMPORT_PLUGIN> GRAPHICS_IMPORT_MGR::GetPlugin( GFX_FILE_T aType )
@@ -54,7 +56,7 @@ unique_ptr<GRAPHICS_IMPORT_PLUGIN> GRAPHICS_IMPORT_MGR::GetPlugin( GFX_FILE_T aT
 
 unique_ptr<GRAPHICS_IMPORT_PLUGIN> GRAPHICS_IMPORT_MGR::GetPluginByExt( const wxString& aExtension )
 {
-    for( auto fileType : GFX_FILE_TYPES )
+    for( auto fileType : GetImportableFileTypes() )
     {
         auto plugin = GetPlugin( fileType );
         auto fileExtensions = plugin->GetFileExtensions();
@@ -70,4 +72,18 @@ unique_ptr<GRAPHICS_IMPORT_PLUGIN> GRAPHICS_IMPORT_MGR::GetPluginByExt( const wx
 }
 
 
-const vector<GRAPHICS_IMPORT_MGR::GFX_FILE_T> GRAPHICS_IMPORT_MGR::GFX_FILE_TYPES = { DXF, SVG };
+GRAPHICS_IMPORT_MGR::TYPE_LIST GRAPHICS_IMPORT_MGR::GetImportableFileTypes()
+{
+    // Always available
+    TYPE_LIST list{
+        DXF,
+    };
+
+    // Only available sometimes
+    if( ADVANCED_CFG::GetCfg().m_enableSvgImport )
+    {
+        list.push_back( SVG );
+    }
+
+    return list;
+}
\ No newline at end of file
diff --git a/pcbnew/import_gfx/graphics_import_mgr.h b/pcbnew/import_gfx/graphics_import_mgr.h
index e312146a0..a07b785ca 100644
--- a/pcbnew/import_gfx/graphics_import_mgr.h
+++ b/pcbnew/import_gfx/graphics_import_mgr.h
@@ -44,8 +44,10 @@ public:
         SVG
     };
 
-    ///> Vector containing all GFX_FILE_T values.
-    static const std::vector<GFX_FILE_T> GFX_FILE_TYPES;
+    using TYPE_LIST = std::vector<GFX_FILE_T>;
+
+    ///> Vector containing all GFX_FILE_T values that can be imported.
+    static TYPE_LIST GetImportableFileTypes();
 
     ///> Returns a plugin that handles a specific file extension.
     static std::unique_ptr<GRAPHICS_IMPORT_PLUGIN> GetPluginByExt( const wxString& aExtension );
-- 
2.19.2

From b131b062a3fad0938bf5ace0c63b85b835ec6d78 Mon Sep 17 00:00:00 2001
From: John Beard <john.j.beard@xxxxxxxxx>
Date: Mon, 10 Dec 2018 10:56:28 +0000
Subject: [PATCH 1/3] Run-time config for advanced options

This can be used for "advanced" options which are for developers
to use for feature-flags and other configuration. Run time config
has some advantages over preprocessor defines:

* Can be changed without recompilation
   * Sensitive to XDG_CONFIG_DIR, so flipping configs is easy
* Better compiler coverage (less conditionally compiled code means
  less chance to break a different configuration). Also better
  analysis coverage.
* Type safe config params
* Centralised documentation: it's in doxygen, in one place

No advanced config should be required by a general users. If a general
user does use one of these configs, it's probably because:

* There is a bug and one of these configs is a workaround
* A config in here is generally useful and should be moved into the
  relevant application config and given UI.

For now, the config is read-only, and is read from the
"kicad_advanced" config file in the normal config dir.
---
 Documentation/development/testing.md |  13 ++-
 common/CMakeLists.txt                |   1 +
 common/advanced_config.cpp           | 161 +++++++++++++++++++++++++++
 include/advanced_config.h            |  85 ++++++++++++++
 4 files changed, 259 insertions(+), 1 deletion(-)
 create mode 100644 common/advanced_config.cpp
 create mode 100644 include/advanced_config.h

diff --git a/Documentation/development/testing.md b/Documentation/development/testing.md
index e0c3c04ae..6030d986d 100644
--- a/Documentation/development/testing.md
+++ b/Documentation/development/testing.md
@@ -259,9 +259,20 @@ Some available masks:
     * `KICAD_GEDA_PLUGIN`
     * `KICAD_PCB_PLUGIN`
 
+# Advanced configuration
+
+There are some advance configuration options, which are mostly used for
+development or testing purposes.
+
+To set these options, you can create the file `kicad_advanced` and set the keys
+as desired (the [advanced config documentation][] for a current list. You should
+never need to set these keys for normal usage - if you do, that's a bug.
+
 
 [CTest]: https://cmake.org/cmake/help/latest/module/CTest.html
 [Boost Unit Test framework]: https://www.boost.org/doc/libs/1_68_0/libs/test/doc/html/index.html
 [boost-test-functions]: https://www.boost.org/doc/libs/1_68_0/libs/test/doc/html/boost_test/utf_reference/testing_tool_ref.html
 [AFL fuzzing tool]: http://lcamtuf.coredump.cx/afl/
-[trace mask documentation]: http://docs.kicad-pcb.org/doxygen/group__trace__env__vars.html
\ No newline at end of file
+[trace mask documentation]: http://docs.kicad-pcb.org/doxygen/group__trace__env__vars.html
+[trace mask documentation]: http://docs.kicad-pcb.org/doxygen/group__trace__env__vars.html
+[advanced config documentation]: http://docs.kicad-pcb.org/doxygen/namespaceAC__KEYS.html
\ No newline at end of file
diff --git a/common/CMakeLists.txt b/common/CMakeLists.txt
index 569543a87..1ef0850b6 100644
--- a/common/CMakeLists.txt
+++ b/common/CMakeLists.txt
@@ -269,6 +269,7 @@ set( COMMON_SRCS
     ${COMMON_PAGE_LAYOUT_SRCS}
     ${COMMON_PREVIEW_ITEMS_SRCS}
     ${PLOTTERS_CONTROL_SRCS}
+    advanced_config.cpp
     base_struct.cpp
     bezier_curves.cpp
     bin_mod.cpp
diff --git a/common/advanced_config.cpp b/common/advanced_config.cpp
new file mode 100644
index 000000000..bf69424f2
--- /dev/null
+++ b/common/advanced_config.cpp
@@ -0,0 +1,161 @@
+/*
+ * This program source code file is part of KiCad, a free EDA CAD application.
+ *
+ * Copyright (C) 2018 KiCad Developers, see AUTHORS.txt for contributors.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, you may find one here:
+ * http://www.gnu.org/licenses/old-licenses/gpl-2.0.html
+ * or you may search the http://www.gnu.org website for the version 2 license,
+ * or you may write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA
+ */
+
+#include <advanced_config.h>
+
+#include <common.h>
+#include <config_params.h>
+
+#include <wx/config.h>
+#include <wx/filename.h>
+#include <wx/log.h>
+
+/*
+ * Flag to enable advanced config debugging
+ *
+ * Use "KICAD_ADVANCED_CONFIG" to enable.
+ *
+ * @ingroup trace_env_vars
+ */
+static const wxChar AdvancedConfigMask[] = wxT( "KICAD_ADVANCED_CONFIG" );
+
+/**
+ * List of known keys for advanced configuration options.
+ *
+ * Set these options in the file `kicad_advanced` in the
+ * KiCad config directory.
+ */
+namespace AC_KEYS
+{
+} // namespace KEYS
+
+
+/*
+ * Get a simple string for common parameters.
+ *
+ * This isn't exhaustive, but it covers most common types that might be
+ * used in the advance config
+ */
+wxString dumpParamCfg( const PARAM_CFG_BASE& aParam )
+{
+    wxString s = aParam.m_Ident + ": ";
+
+    /*
+     * This implementation is rather simplistic, but it is
+     * effective enough for simple uses. A better implementation would be
+     * some kind of visitor, but that's somewhat more work.
+     */
+    switch( aParam.m_Type )
+    {
+    case paramcfg_id::PARAM_INT:
+    case paramcfg_id::PARAM_INT_WITH_SCALE:
+        s << *static_cast<const PARAM_CFG_INT&>( aParam ).m_Pt_param;
+        break;
+    case paramcfg_id::PARAM_DOUBLE:
+        s << *static_cast<const PARAM_CFG_DOUBLE&>( aParam ).m_Pt_param;
+        break;
+    case paramcfg_id::PARAM_WXSTRING:
+        s << *static_cast<const PARAM_CFG_WXSTRING&>( aParam ).m_Pt_param;
+        break;
+    case paramcfg_id::PARAM_FILENAME:
+        s << *static_cast<const PARAM_CFG_FILENAME&>( aParam ).m_Pt_param;
+        break;
+    case paramcfg_id::PARAM_BOOL:
+        s << ( *static_cast<const PARAM_CFG_BOOL&>( aParam ).m_Pt_param ? "true" : "false" );
+        break;
+    default: s << "Unsupported PARAM_CFG variant: " << aParam.m_Type;
+    }
+
+    return s;
+}
+
+
+/**
+ * Dump the configs in the given array to trace.
+ */
+static void dumpCfg( const PARAM_CFG_ARRAY& aArray )
+{
+    // only dump if we need to
+    if( !wxLog::IsAllowedTraceMask( AdvancedConfigMask ) )
+        return;
+
+    for( const auto& param : aArray )
+    {
+        wxLogTrace( AdvancedConfigMask, dumpParamCfg( param ) );
+    }
+}
+
+
+/**
+ * Get the filename for the advanced config file
+ *
+ * The user must check the file exists if they care.
+ */
+static wxFileName getAdvancedCfgFilename()
+{
+    const static wxString cfg_filename{ "kicad_advanced" };
+    return wxFileName( GetKicadConfigPath(), cfg_filename );
+}
+
+
+ADVANCED_CFG::ADVANCED_CFG()
+{
+    wxLogTrace( AdvancedConfigMask, "Init advanced config" );
+    loadFromConfigFile();
+}
+
+
+const ADVANCED_CFG& ADVANCED_CFG::GetCfg()
+{
+    static ADVANCED_CFG instance;
+    return instance;
+}
+
+
+void ADVANCED_CFG::loadFromConfigFile()
+{
+    const auto k_advanced = getAdvancedCfgFilename();
+
+    if( !k_advanced.FileExists() )
+    {
+        wxLogTrace( AdvancedConfigMask, "File does not exist %s", k_advanced.GetFullPath() );
+        return;
+    }
+
+    wxLogTrace( AdvancedConfigMask, "Loading advanced config from: %s", k_advanced.GetFullPath() );
+
+    wxFileConfig file_cfg( "", "", k_advanced.GetFullPath() );
+    loadSettings( file_cfg );
+}
+
+
+void ADVANCED_CFG::loadSettings( wxConfigBase& aCfg )
+{
+    PARAM_CFG_ARRAY configParams;
+
+    // Add configs here
+
+    wxConfigLoadSetups( &aCfg, configParams );
+
+    dumpCfg( configParams );
+}
\ No newline at end of file
diff --git a/include/advanced_config.h b/include/advanced_config.h
new file mode 100644
index 000000000..07681722e
--- /dev/null
+++ b/include/advanced_config.h
@@ -0,0 +1,85 @@
+/*
+ * This program source code file is part of KiCad, a free EDA CAD application.
+ *
+ * Copyright (C) 2018 KiCad Developers, see AUTHORS.txt for contributors.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, you may find one here:
+ * http://www.gnu.org/licenses/old-licenses/gpl-2.0.html
+ * or you may search the http://www.gnu.org website for the version 2 license,
+ * or you may write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA
+ */
+
+#ifndef ADVANCED_CFG__H
+#define ADVANCED_CFG__H
+
+class wxConfigBase;
+
+/**
+ * Class containing "advanced" configuration options.
+ *
+ * Options set here are for developer or advanced users only. If a general user
+ * needs to set one of these for normal KiCad use, either:
+ * * They are working around some bug that should be fixed, or
+ * * The parameter they are setting is of general interest and should be in the
+ *   main application config, with UI provided.
+ *
+ * Options in this class are, in general, preferable to #defines, as they
+ * allow more flexible configuration by developers, and don't hide code from
+ * the compiler on other configurations, which can result in broken builds.
+ *
+ * Never use advanced configs in an untestable way. If a function depends on
+ * advanced config such that you cannot test it without changing the config,
+ * "lift" the config to a higher level and make pass it as parameter of the code
+ * under test. The tests can pass their own values as needed.
+ *
+ * This also applies to code that does not depend on "common" - it cannot
+ * use this class, so you must pass configuration in as proper parameters.
+ *
+ * Sometimes you can just use values directly, and sometimes helper functions
+ * might be provided to allow extra logic (for example when a advanced config
+ * applies only on certain platforms).
+ *
+ * For more information on what config keys set these parameters in the
+ * config files, and why you might want to set them, see #AC_KEYS
+ *
+ */
+class ADVANCED_CFG
+{
+public:
+    /**
+     * Get the singleton instance's config, which is shared by all
+     * consumers of advanced config.
+     *
+     * This configuration is read-only - to set options, users should
+     * add the parameters to their config files at ~/.config/kicad/advanced, or the
+     * platform equivalent.
+     */
+    static const ADVANCED_CFG& GetCfg();
+
+private:
+    ADVANCED_CFG();
+
+    /**
+     * Load the config from the normal config file
+     */
+    void loadFromConfigFile();
+
+    /*
+     * Load config from the given config base
+     */
+    void loadSettings( wxConfigBase& aCfg );
+};
+
+#endif // ADVANCED_CFG__H
-- 
2.19.2

From c9af83cc0b7a3fe2e3bcb40458d6a2463375572c Mon Sep 17 00:00:00 2001
From: John Beard <john.j.beard@xxxxxxxxx>
Date: Wed, 12 Dec 2018 12:26:03 +0000
Subject: [PATCH 2/3] 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                | 19 ++++++++++
 include/advanced_config.h                 |  5 +++
 pcbnew/CMakeLists.txt                     | 20 ----------
 pcbnew/import_gfx/dialog_import_gfx.cpp   | 32 +++++++++-------
 pcbnew/import_gfx/dialog_import_gfx.h     |  3 ++
 pcbnew/import_gfx/graphics_import_mgr.cpp | 45 +++++++++++++----------
 pcbnew/import_gfx/graphics_import_mgr.h   | 24 ++++++++----
 7 files changed, 88 insertions(+), 60 deletions(-)

diff --git a/common/advanced_config.cpp b/common/advanced_config.cpp
index bf69424f2..f11f6c9f9 100644
--- a/common/advanced_config.cpp
+++ b/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();
 }
 
diff --git a/include/advanced_config.h b/include/advanced_config.h
index 07681722e..20b10f4f9 100644
--- a/include/advanced_config.h
+++ b/include/advanced_config.h
@@ -68,6 +68,11 @@ public:
      */
     static const ADVANCED_CFG& GetCfg();
 
+    /**
+     * Enable SVG import.
+     */
+    bool m_enableSvgImport;
+
 private:
     ADVANCED_CFG();
 
diff --git a/pcbnew/CMakeLists.txt b/pcbnew/CMakeLists.txt
index a58dc89ba..8c518d1bf 100644
--- a/pcbnew/CMakeLists.txt
+++ b/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()
\ No newline at end of file
diff --git a/pcbnew/import_gfx/dialog_import_gfx.cpp b/pcbnew/import_gfx/dialog_import_gfx.cpp
index 48f098876..126b9cedd 100644
--- a/pcbnew/import_gfx/dialog_import_gfx.cpp
+++ b/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" );
-#else
-    auto plugin = GRAPHICS_IMPORT_MGR::GetPluginByExt( wxFileName( m_filename ).GetExt() );
-#endif
+
+    auto plugin = m_gfxImportMgr->GetPluginByExt( wxFileName( m_filename ).GetExt() );
 
     if( plugin )
     {
diff --git a/pcbnew/import_gfx/dialog_import_gfx.h b/pcbnew/import_gfx/dialog_import_gfx.h
index 77bc3f424..f4eb3079c 100644
--- a/pcbnew/import_gfx/dialog_import_gfx.h
+++ b/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
diff --git a/pcbnew/import_gfx/graphics_import_mgr.cpp b/pcbnew/import_gfx/graphics_import_mgr.cpp
index c988584e8..532e52b06 100644
--- a/pcbnew/import_gfx/graphics_import_mgr.cpp
+++ b/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:
-            ret.reset( new DXF_IMPORT_PLUGIN() );
-            break;
+    case DXF: ret = std::make_unique<DXF_IMPORT_PLUGIN>(); break;
 
-        case SVG:
-            ret.reset( new SVG_IMPORT_PLUGIN() );
-            break;
+    case SVG: ret = std::make_unique<SVG_IMPORT_PLUGIN>(); break;
 
-        default:
-            throw std::runtime_error( "Unhandled graphics format" );
-            break;
+    default: 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>();
-}
-
-
-const vector<GRAPHICS_IMPORT_MGR::GFX_FILE_T> GRAPHICS_IMPORT_MGR::GFX_FILE_TYPES = { DXF, SVG };
+    return {};
+}
\ No newline at end of file
diff --git a/pcbnew/import_gfx/graphics_import_mgr.h b/pcbnew/import_gfx/graphics_import_mgr.h
index e312146a0..d478c236b 100644
--- a/pcbnew/import_gfx/graphics_import_mgr.h
+++ b/pcbnew/import_gfx/graphics_import_mgr.h
@@ -44,19 +44,29 @@ public:
         SVG
     };
 
-    ///> Vector containing all GFX_FILE_T values.
-    static const std::vector<GFX_FILE_T> GFX_FILE_TYPES;
+    using TYPE_LIST = std::vector<GFX_FILE_T>;
+
+    /**
+     * 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 */
-- 
2.19.2

From 463a020c7004f6d590e1a822969a775afd85a368 Mon Sep 17 00:00:00 2001
From: John Beard <john.j.beard@xxxxxxxxx>
Date: Wed, 12 Dec 2018 14:16:50 +0000
Subject: [PATCH 3/3] Disable legacy canvas on GTK3

This make the use of legacy canvas on GTK3 a default-off
advanced config. Legacy is substantially broken on GTK3
and is of basically no use at all to general users on this
platform.

If the program starts with legacy canvas in the config,
it is forced into a GAL mode, as otherwise it could happen
that the user is stuck and unable to get into pcbnew to change
the setting.

Fixes: lp:1803156
* https://bugs.launchpad.net/kicad/+bug/1803156
---
 common/advanced_config.cpp           | 25 +++++++++++++++++++++++++
 common/legacy_gal/eda_draw_frame.cpp |  8 ++++++++
 common/legacy_wx/eda_draw_frame.cpp  | 10 +++++++++-
 gerbview/menubar.cpp                 | 14 +++++++++-----
 include/advanced_config.h            | 15 +++++++++++++++
 pcbnew/menubar_footprint_editor.cpp  | 14 +++++++++-----
 pcbnew/menubar_pcb_editor.cpp        | 20 +++++++++++---------
 7 files changed, 86 insertions(+), 20 deletions(-)

diff --git a/common/advanced_config.cpp b/common/advanced_config.cpp
index f11f6c9f9..d9c8e240d 100644
--- a/common/advanced_config.cpp
+++ b/common/advanced_config.cpp
@@ -61,6 +61,13 @@ namespace AC_KEYS
  */
 static const wxChar EnableSvgImport[] = wxT( "EnableSvgImport" );
 
+/**
+ * Allow legacy canvas to be shown in GTK3. Legacy canvas is generally pretty
+ * broken, but this avoids code in an ifdef where it could become broken
+ * on other platforms
+ */
+static const wxChar AllowLegacyCanvasInGtk3[] = wxT( "AllowLegacyCanvasInGtk3" );
+
 } // namespace KEYS
 
 
@@ -139,6 +146,7 @@ ADVANCED_CFG::ADVANCED_CFG()
     // Init defaults - this is done in case the config doesn't exist,
     // then the values will remain as set here.
     m_enableSvgImport = false;
+    m_allowLegacyCanvasInGtk3 = false;
 
     loadFromConfigFile();
 }
@@ -174,7 +182,24 @@ void ADVANCED_CFG::loadSettings( wxConfigBase& aCfg )
 
     // Add configs here
 
+    configParams.push_back( new PARAM_CFG_BOOL(
+            true, AC_KEYS::AllowLegacyCanvasInGtk3, &m_allowLegacyCanvasInGtk3, false ) );
+
     wxConfigLoadSetups( &aCfg, configParams );
 
     dumpCfg( configParams );
+}
+
+
+bool ADVANCED_CFG::AllowLegacyCanvas() const
+{
+    // default is to allow
+    bool allow = true;
+
+    // on GTK3, check the config
+#ifdef __WXGTK3__
+    allow = m_allowLegacyCanvasInGtk3;
+#endif
+
+    return allow;
 }
\ No newline at end of file
diff --git a/common/legacy_gal/eda_draw_frame.cpp b/common/legacy_gal/eda_draw_frame.cpp
index 83f63f5a9..78ba382d6 100644
--- a/common/legacy_gal/eda_draw_frame.cpp
+++ b/common/legacy_gal/eda_draw_frame.cpp
@@ -67,6 +67,7 @@
 #include <worksheet_shape_builder.h>
 #include <page_info.h>
 #include <title_block.h>
+#include <advanced_config.h>
 
 /**
  * Definition for enabling and disabling scroll bar setting trace output.  See the
@@ -1083,6 +1084,13 @@ EDA_DRAW_PANEL_GAL::GAL_TYPE EDA_DRAW_FRAME::LoadCanvasTypeSetting()
         canvasType = EDA_DRAW_PANEL_GAL::GAL_TYPE_NONE;
     }
 
+    // Coerce the value into a GAL type when Legacy is not available
+    if( canvasType == EDA_DRAW_PANEL_GAL::GAL_TYPE_NONE
+            && !ADVANCED_CFG::GetCfg().AllowLegacyCanvas() )
+    {
+        canvasType = EDA_DRAW_PANEL_GAL::GAL_TYPE_OPENGL;
+    }
+
     return canvasType;
 }
 
diff --git a/common/legacy_wx/eda_draw_frame.cpp b/common/legacy_wx/eda_draw_frame.cpp
index 0c4787f8d..9cc319d95 100644
--- a/common/legacy_wx/eda_draw_frame.cpp
+++ b/common/legacy_wx/eda_draw_frame.cpp
@@ -66,10 +66,11 @@
 #include <tool/tool_dispatcher.h>
 #include <tool/actions.h>
 
+#include <advanced_config.h>
 #include <menus_helpers.h>
-#include <worksheet_shape_builder.h>
 #include <page_info.h>
 #include <title_block.h>
+#include <worksheet_shape_builder.h>
 
 /**
  * Definition for enabling and disabling scroll bar setting trace output.  See the
@@ -1329,6 +1330,13 @@ EDA_DRAW_PANEL_GAL::GAL_TYPE EDA_DRAW_FRAME::LoadCanvasTypeSetting()
         canvasType = EDA_DRAW_PANEL_GAL::GAL_TYPE_NONE;
     }
 
+    // Coerce the value into a GAL type when Legacy is not available
+    if( canvasType == EDA_DRAW_PANEL_GAL::GAL_TYPE_NONE
+            && !ADVANCED_CFG::GetCfg().AllowLegacyCanvas() )
+    {
+        canvasType = EDA_DRAW_PANEL_GAL::GAL_TYPE_OPENGL;
+    }
+
     return canvasType;
 }
 
diff --git a/gerbview/menubar.cpp b/gerbview/menubar.cpp
index 792eaaf3a..1a1fdaddd 100644
--- a/gerbview/menubar.cpp
+++ b/gerbview/menubar.cpp
@@ -28,11 +28,12 @@
  * @brief (Re)Create the main menubar for GerbView
  */
 
+#include "gerbview_frame.h"
 
+#include <advanced_config.h>
 #include <kiface_i.h>
 #include <pgm_base.h>
 
-#include "gerbview_frame.h"
 #include "gerbview_id.h"
 #include "hotkeys.h"
 #include <menus_helpers.h>
@@ -300,10 +301,13 @@ void GERBVIEW_FRAME::ReCreateMenuBar()
     // Canvas selection
     configMenu->AppendSeparator();
 
-    text = AddHotkeyName( _( "Legacy Tool&set" ), GerbviewHokeysDescr, HK_CANVAS_LEGACY );
-    AddMenuItem( configMenu, ID_MENU_CANVAS_LEGACY,
-                 text, _( "Use Legacy Toolset (not all features will be available)" ),
-                 KiBitmap( tools_xpm ), wxITEM_RADIO );
+    if( ADVANCED_CFG::GetCfg().AllowLegacyCanvas() )
+    {
+        text = AddHotkeyName( _( "Legacy Tool&set" ), GerbviewHokeysDescr, HK_CANVAS_LEGACY );
+        AddMenuItem( configMenu, ID_MENU_CANVAS_LEGACY, text,
+                _( "Use Legacy Toolset (not all features will be available)" ),
+                KiBitmap( tools_xpm ), wxITEM_RADIO );
+    }
 
     text = AddHotkeyName( _( "Modern Toolset (&Accelerated)" ), GerbviewHokeysDescr, HK_CANVAS_OPENGL );
     AddMenuItem( configMenu, ID_MENU_CANVAS_OPENGL, text,
diff --git a/include/advanced_config.h b/include/advanced_config.h
index 20b10f4f9..a69669ebd 100644
--- a/include/advanced_config.h
+++ b/include/advanced_config.h
@@ -73,6 +73,21 @@ public:
      */
     bool m_enableSvgImport;
 
+    /**
+     * Helper to determine if legacy canvas is allowed (according to platform
+     * and config)
+     * @return true if legacy canvas should be shown
+     */
+    bool AllowLegacyCanvas() const;
+
+private:
+    /*
+     * These settings are private, as there is extra logic provide by helper
+     * functions above.
+     */
+
+    bool m_allowLegacyCanvasInGtk3;
+
 private:
     ADVANCED_CFG();
 
diff --git a/pcbnew/menubar_footprint_editor.cpp b/pcbnew/menubar_footprint_editor.cpp
index 7677950c8..ced90d147 100644
--- a/pcbnew/menubar_footprint_editor.cpp
+++ b/pcbnew/menubar_footprint_editor.cpp
@@ -29,13 +29,14 @@
  * @brief (Re)Create the main menubar for the footprint editor
  */
 
+#include "footprint_edit_frame.h"
 
+#include <advanced_config.h>
 #include <menus_helpers.h>
 #include <pgm_base.h>
 
 #include "help_common_strings.h"
 #include "hotkeys.h"
-#include "footprint_edit_frame.h"
 #include "pcbnew.h"
 #include "pcbnew_id.h"
 
@@ -431,10 +432,13 @@ void FOOTPRINT_EDIT_FRAME::ReCreateMenuBar()
 
     prefs_menu->AppendSeparator();
 
-    text = AddHotkeyName( _( "Legacy Tool&set" ), m_hotkeysDescrList, HK_CANVAS_LEGACY );
-    AddMenuItem( prefs_menu, ID_MENU_CANVAS_LEGACY, text,
-                 _( "Use Legacy Toolset (not all features will be available)" ),
-                 KiBitmap( tools_xpm ), wxITEM_RADIO );
+    if( ADVANCED_CFG::GetCfg().AllowLegacyCanvas() )
+    {
+        text = AddHotkeyName( _( "Legacy Tool&set" ), m_hotkeysDescrList, HK_CANVAS_LEGACY );
+        AddMenuItem( prefs_menu, ID_MENU_CANVAS_LEGACY, text,
+                _( "Use Legacy Toolset (not all features will be available)" ),
+                KiBitmap( tools_xpm ), wxITEM_RADIO );
+    }
 
     text = AddHotkeyName( _( "Modern Toolset (&Accelerated)" ), m_hotkeysDescrList, HK_CANVAS_OPENGL );
     AddMenuItem( prefs_menu, ID_MENU_CANVAS_OPENGL, text,
diff --git a/pcbnew/menubar_pcb_editor.cpp b/pcbnew/menubar_pcb_editor.cpp
index b104f2517..9683cfe80 100644
--- a/pcbnew/menubar_pcb_editor.cpp
+++ b/pcbnew/menubar_pcb_editor.cpp
@@ -28,12 +28,12 @@
  * @file menubar_pcb_editor.cpp
  * board editor menubars
  */
+#include <pcb_edit_frame.h>
 
-
-#include <menus_helpers.h>
+#include <advanced_config.h>
 #include <kiface_i.h>
+#include <menus_helpers.h>
 #include <pgm_base.h>
-#include <pcb_edit_frame.h>
 
 #include "help_common_strings.h"
 #include "hotkeys.h"
@@ -41,7 +41,6 @@
 #include "pcbnew_id.h"
 
 
-
 // Build the files menu. Because some commands are available only if
 // Pcbnew is run outside a project (run alone), aIsOutsideProject is false
 // when Pcbnew is run from Kicad manager, and true is run as stand alone app.
@@ -158,11 +157,14 @@ void preparePreferencesMenu( PCB_EDIT_FRAME* aFrame, wxMenu* aParentMenu )
                  _( "&Preferences..." ), _( "Show preferences for all open tools" ),
                  KiBitmap( preference_xpm ) );
 
-    text = AddHotkeyName( _( "Legacy Tool&set" ), g_Board_Editor_Hotkeys_Descr,
-                          HK_CANVAS_LEGACY );
-    AddMenuItem( aParentMenu, ID_MENU_CANVAS_LEGACY, text,
-                 _( "Use Legacy Toolset (not all features will be available)" ),
-                 KiBitmap( tools_xpm ), wxITEM_RADIO );
+    if( ADVANCED_CFG::GetCfg().AllowLegacyCanvas() )
+    {
+        text = AddHotkeyName(
+                _( "Legacy Tool&set" ), g_Board_Editor_Hotkeys_Descr, HK_CANVAS_LEGACY );
+        AddMenuItem( aParentMenu, ID_MENU_CANVAS_LEGACY, text,
+                _( "Use Legacy Toolset (not all features will be available)" ),
+                KiBitmap( tools_xpm ), wxITEM_RADIO );
+    }
 
     text = AddHotkeyName( _( "Modern Toolset (&Accelerated)" ), g_Board_Editor_Hotkeys_Descr,
                           HK_CANVAS_OPENGL );
-- 
2.19.2


Follow ups