kicad-developers team mailing list archive
-
kicad-developers team
-
Mailing list archive
-
Message #38667
Re: [PATCH] Run time advanced options
Hi Wayne,
Booleans are set as "0" or "1" by the wxConfigBase interface. I don't
think it translates "true" or "false" on read, so they'd probably be
defaults.
But I see I have somehow messed up the rebasing to make the SVG stuff
a separate commit and lost a line as well. Please try the attached
patches (1 and 3 should be the same) and a file with:
EnableLegacyCanvasWithGtk3 = 1
EnableSvgImport = 1
Sorry about that!
Cheers,
John
On Mon, Dec 17, 2018 at 5:13 PM Wayne Stambaugh <stambaughw@xxxxxxxxx> wrote:
>
> Hey John,
>
> I'm not sure this is working correctly at least on windows. I applied
> all 3 patches and created a kicad_advanced file (attached) but I'm not
> getting the debugging output I expect:
>
> [1588] (KICAD_ADVANCED_CONFIG) Init advanced config
> [1588] (KICAD_ADVANCED_CONFIG) Loading advanced config from:
> C:\Users\wstambaugh\AppData\Roaming\kicad\kicad_advanced
> [1588] (KICAD_ADVANCED_CONFIG) AllowLegacyCanvasInGtk3: false
>
> I don't see the EnableSvgImport option and the AllowLegacyCanvasInGtk3
> option is being read as false when it's set to true in the
> kicad_advanced file. Am I doing something wrong?
>
> Cheers,
>
> Wayne
>
> On 12/13/2018 8:30 AM, John Beard wrote:
> > Whoops: the 16kB 0001 patch should not be there, that's a very old draft!
> >
> > Sorry,
> >
> > John
> > On Thu, Dec 13, 2018 at 12:57 PM John Beard <john.j.beard@xxxxxxxxx> wrote:
> >>
> >> 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
> >
> > _______________________________________________
> > Mailing list: https://launchpad.net/~kicad-developers
> > Post to : kicad-developers@xxxxxxxxxxxxxxxxxxx
> > Unsubscribe : https://launchpad.net/~kicad-developers
> > More help : https://help.launchpad.net/ListHelp
> >
> _______________________________________________
> Mailing list: https://launchpad.net/~kicad-developers
> Post to : kicad-developers@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~kicad-developers
> More help : https://help.launchpad.net/ListHelp
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 4998696954543521219face4962f23b1a561300b 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 b9147a8de..4317d45a3 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();
}
@@ -175,7 +183,24 @@ void ADVANCED_CFG::loadSettings( wxConfigBase& aCfg )
configParams.push_back(
new PARAM_CFG_BOOL( true, AC_KEYS::EnableSvgImport, &m_enableSvgImport, false ) );
+ 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
From 492b857f75ae33c8eb909262ca5ec763421bf9ca 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 | 22 ++++++++++-
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, 90 insertions(+), 61 deletions(-)
diff --git a/common/advanced_config.cpp b/common/advanced_config.cpp
index bf69424f2..b9147a8de 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();
}
@@ -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 );
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
Follow ups
References
