Merge ~pointhi/kicad:wx-python into ~kicad-product-committers/kicad:master

[Thomas Pointhuber]

Add optional wxPhoenix support

Based on the work of @mmccoo:
* https://kicad.mmccoo.com/2017/11/23/learnings-from-moving-kicad-to-wxpython-4-0/
and this additional patchset to remove wxpy_api.h dependency:
* http://mmccoo.com/random/0001-Remove-dependence-on-pywx_api.h.patch

It adds wxPhoenix support to KiCad while keeping support for the old wxPython binding. All known are resolved now. In my case testing took place with wx3.0.4+GTK3 on Python2+wxPython as well as Python3+wxPhoenix on Linux. It would be nice to get testing done on Windows as well as Mac at least with the old wxPython version to ensure nothing breaks. While there are currently some compiler problems to get wxPhoenix running on Windows, adding this would benefit especially Linux users on Python3 only systems to get shell support.

[Thomas Pointhuber (pointhi)]

Windows build can be found here: https://jenkins.simonrichter.eu/job/windows-kicad-msys2-patch/171/

[Seth Hillbrand (sethh)]

Looks good. Tested on Debian Buster and works smoothly.

review: Approve

[Maciej Suminski (orsonmmz)]

I have briefly tested the branch with Phoenix/Python3/GTK3 and wxPython/Python2/GTK3 on Linux - no problems observed so far.

review: Approve

[Wayne Stambaugh (stambaughw)]

It doesn't build on msys2 when python 3 is enabled. It finds the python 3 interpreter without any issue but it fails finding the python3 library path. Here is the config error:

-- Found PythonInterp: C:/msys64/mingw64/bin/python3.exe (found suitable version "3.7.1", minimum required is "3.3")
-- Check for installed Python Interpreter -- found
-- Python module install path: lib/python3.7/site-packages
CMake Error at C:/msys64/mingw64/share/cmake-3.12/Modules/FindPackageHandleStandardArgs.cmake:137 (message):
  Could NOT find PythonLibs (missing: PYTHON_INCLUDE_DIRS) (Required is at
  least version "3.3")
Call Stack (most recent call first):
  C:/msys64/mingw64/share/cmake-3.12/Modules/FindPackageHandleStandardArgs.cmake:378 (_FPHSA_FAILURE_MESSAGE)
  CMakeModules/FindPythonLibs.cmake:233 (find_package_handle_standard_args)
  CMakeLists.txt:750 (find_package)

-- Configuring incomplete, errors occurred!

review: Needs Fixing

[Thomas Pointhuber (pointhi)]

> It doesn't build on msys2 when python 3 is enabled. It finds the python 3
> interpreter without any issue but it fails finding the python3 library path.
> Here is the config error:
>
> -- Found PythonInterp: C:/msys64/mingw64/bin/python3.exe (found suitable
> version "3.7.1", minimum required is "3.3")
> -- Check for installed Python Interpreter -- found
> -- Python module install path: lib/python3.7/site-packages
> CMake Error at C:/msys64/mingw64/share/cmake-3.12/Modules/FindPackageHandleSta
> ndardArgs.cmake:137 (message):
> Could NOT find PythonLibs (missing: PYTHON_INCLUDE_DIRS) (Required is at
> least version "3.3")
> Call Stack (most recent call first):
> C:/msys64/mingw64/share/cmake-3.12/Modules/FindPackageHandleStandardArgs.cma
> ke:378 (_FPHSA_FAILURE_MESSAGE)
> CMakeModules/FindPythonLibs.cmake:233 (find_package_handle_standard_args)
> CMakeLists.txt:750 (find_package)
>
>
> -- Configuring incomplete, errors occurred!

That's weird because this patch should not change Python3 stuff. Can you verify this problem does not exist upstream?

[Nick Østergaard (nickoe)]

@Wayne, try to add -DPYTHON_INCLUDE_DIR=${MINGW_PREFIX}/include/python3.6m

[Nick Østergaard (nickoe)]

Or probably 3.7m in your case.

[Wayne Stambaugh (stambaughw)]

@Nick, thanks for the tip. That did it. Do you have any idea why the 'm' suffix is added to the python include path? This should be easy enough to fix by updating our FindPythonLib.cmake file to look in the python version path with the 'm' suffix. I also built and tested the python 3 support and to make sure the python 2 support still works on msys2/mingw32/mingw64 so I approve merging this.

review: Approve

[Maciej Suminski (orsonmmz)]

Thank you Thomas!

diff --git a/CMakeLists.txt b/CMakeLists.txt
index cf55d49..42e4c60 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -82,6 +82,10 @@ option( KICAD_SCRIPTING_WXPYTHON
     "Build wxPython implementation for wx interface building in Python and py.shell (default ON)."
     ON )
 
+option( KICAD_SCRIPTING_WXPYTHON_PHOENIX
+    "Use new wxPython binding (default OFF)."
+    OFF )
+
 option( KICAD_SCRIPTING_ACTION_MENU
     "Build a tools menu with registered python plugins: actions plugins (default ON)."
     ON )
@@ -389,6 +393,10 @@ if( KICAD_SCRIPTING_WXPYTHON )
     add_definitions( -DKICAD_SCRIPTING_WXPYTHON )
 endif()
 
+if( KICAD_SCRIPTING_WXPYTHON_PHOENIX )
+    add_definitions( -DKICAD_SCRIPTING_WXPYTHON_PHOENIX )
+endif()
+
 if( KICAD_SCRIPTING_ACTION_MENU )
     add_definitions( -DKICAD_SCRIPTING_ACTION_MENU )
 endif()
@@ -747,8 +755,24 @@ if( KICAD_SCRIPTING OR KICAD_SCRIPTING_MODULES )
     if( KICAD_SCRIPTING_WXPYTHON )
         # Check to see if the correct version of wxPython is installed based on the version of
         # wxWidgets found.  At least the major an minor version should match.
-        set( _wxpy_version "${wxWidgets_VERSION_MAJOR}.${wxWidgets_VERSION_MINOR}" )
-        set( _py_cmd "import wxversion;print(wxversion.checkInstalled('${_wxpy_version}'))" )
+        if( KICAD_SCRIPTING_WXPYTHON_PHOENIX )
+            set( _py_cmd "import wx;print(wx.version())" )
+            execute_process( COMMAND ${PYTHON_EXECUTABLE} -c "${_py_cmd}"
+                RESULT_VARIABLE WXPYTHON_VERSION_RESULT
+                OUTPUT_VARIABLE WXPYTHON_VERSION_FOUND
+                OUTPUT_STRIP_TRAILING_WHITESPACE
+            )
+            # Check to see if any version of wxPython is installed on the system.
+            if( WXPYTHON_VERSION_RESULT GREATER 0 )
+                message( FATAL_ERROR "wxPython does not appear to be installed on the system." )
+            endif()
+            set( _wxpy_version ${WXPYTHON_VERSION_FOUND} )
+            set( _py_cmd "import wx;import re;print(re.search('wxWidgets ${wxWidgets_VERSION_MAJOR}.${wxWidgets_VERSION_MINOR}', wx.wxWidgets_version) != None)" )
+        else()
+            set( _wxpy_version "${wxWidgets_VERSION_MAJOR}.${wxWidgets_VERSION_MINOR}" )
+            set( _py_cmd "import wxversion;print(wxversion.checkInstalled('${_wxpy_version}'))" )
+        endif()
+
 
         # Add user specified Python site package path.
         if( PYTHON_SITE_PACKAGE_PATH )
diff --git a/Documentation/development/compiling.md b/Documentation/development/compiling.md
index 4de0def..9af2fe5 100644
--- a/Documentation/development/compiling.md
+++ b/Documentation/development/compiling.md
@@ -166,6 +166,11 @@ of Python 2.  This option is disabled by default.
 The KICAD_SCRIPTING_WXPYTHON option is used to enable building the wxPython interface into
 Pcbnew including the wxPython console.  This option is enabled by default.
 
+## wxPython Phoenix Scripting Support ## {#wxpython_phoenix}
+
+The KICAD_SCRIPTING_WXPYTHON_PHOENIX option is used to enable building the wxPython interface with
+the new Phoenix binding instead of the legacy one.  This option is disabled by default.
+
 ## GitHub Plugin ## {#github_opt}
 
 The BUILD_GITHUB_PLUGIN option is used to control if the GitHub plug in is built.  This option is
diff --git a/common/dialog_about/dialog_about.cpp b/common/dialog_about/dialog_about.cpp
index 0af42f8..9fc8233 100644
--- a/common/dialog_about/dialog_about.cpp
+++ b/common/dialog_about/dialog_about.cpp
@@ -554,6 +554,13 @@ void DIALOG_ABOUT::buildVersionInfoData( wxString& aMsg, bool aFormatHtml )
     aMsg << OFF;
 #endif
 
+    aMsg << indent4 << "KICAD_SCRIPTING_WXPYTHON_PHOENIX=";
+#ifdef KICAD_SCRIPTING_WXPYTHON_PHOENIX
+    aMsg << ON;
+#else
+    aMsg << OFF;
+#endif
+
     aMsg << indent4 << "KICAD_SCRIPTING_ACTION_MENU=";
 #ifdef KICAD_SCRIPTING_ACTION_MENU
     aMsg << ON;
diff --git a/pcbnew/python/kicad_pyshell/__init__.py b/pcbnew/python/kicad_pyshell/__init__.py
index 7f476d5..91bca5b 100644
--- a/pcbnew/python/kicad_pyshell/__init__.py
+++ b/pcbnew/python/kicad_pyshell/__init__.py
@@ -86,6 +86,11 @@ class PcbnewPyShell(editor.EditorNotebookFrame):
         self.autoSaveHistory = False
         self.LoadSettings()
 
+        # in case of wxPhoenix we need to create a wxApp first and store it
+        # to prevent removal by gabage collector
+        if 'phoenix' in wx.PlatformInfo:
+            self.theApp = wx.App()
+
         self.crust = crust.Crust(parent=self.notebook,
                                  intro=intro, locals=namespace,
                                  rootLabel="locals()",
diff --git a/pcbnew/swig/python_scripting.cpp b/pcbnew/swig/python_scripting.cpp
index 4577406..17c3ee0 100644
--- a/pcbnew/swig/python_scripting.cpp
+++ b/pcbnew/swig/python_scripting.cpp
@@ -30,6 +30,7 @@
 #include <python_scripting.h>
 #include <stdlib.h>
 #include <string.h>
+#include <sstream>
 
 #include <fctsys.h>
 #include <eda_base_frame.h>
@@ -162,6 +163,7 @@ bool pcbnewInitPythonScripting( const char * aUserScriptingPath )
 #ifdef KICAD_SCRIPTING_WXPYTHON
     PyEval_InitThreads();
 
+#ifndef KICAD_SCRIPTING_WXPYTHON_PHOENIX
 #ifndef __WINDOWS__     // import wxversion.py currently not working under winbuilder, and not useful.
     char cmd[1024];
     // Make sure that that the correct version of wxPython is loaded. In systems where there
@@ -190,13 +192,14 @@ bool pcbnewInitPythonScripting( const char * aUserScriptingPath )
         Py_Finalize();
         return false;
     }
+#endif
 
     wxPythonLoaded = true;
 
     // Save the current Python thread state and release the
     // Global Interpreter Lock.
+    g_PythonMainTState = PyEval_SaveThread();
 
-    g_PythonMainTState = wxPyBeginAllowThreads();
 #endif  // ifdef KICAD_SCRIPTING_WXPYTHON
 
     // load pcbnew inside python, and load all the user plugins, TODO: add system wide plugins
@@ -298,7 +301,7 @@ void pcbnewGetWizardsBackTrace( wxString& aNames )
 void pcbnewFinishPythonScripting()
 {
 #ifdef KICAD_SCRIPTING_WXPYTHON
-    wxPyEndAllowThreads( g_PythonMainTState );
+    PyEval_RestoreThread( g_PythonMainTState );
 #endif
     Py_Finalize();
 }
@@ -325,15 +328,25 @@ void RedirectStdio()
 
 wxWindow* CreatePythonShellWindow( wxWindow* parent, const wxString& aFramenameId )
 {
-    const char* pcbnew_pyshell =
-        "import kicad_pyshell\n"
-        "\n"
-        "def makeWindow(parent):\n"
-        "    return kicad_pyshell.makePcbnewShellWindow(parent)\n"
-        "\n";
+    // parent is actually *PCB_EDIT_FRAME
+    const int parentId = parent->GetId();
+    {
+        wxWindow* parent2 = wxWindow::FindWindowById( parentId );
+        wxASSERT( parent2 == parent );
+    }
 
-    wxWindow*   window = NULL;
-    PyObject*   result;
+    // passing window ids instead of pointers is because wxPython is not
+    // exposing the needed c++ apis to make that possible.
+    std::stringstream pcbnew_pyshell_one_step;
+    pcbnew_pyshell_one_step << "import kicad_pyshell\n";
+    pcbnew_pyshell_one_step << "import wx\n";
+    pcbnew_pyshell_one_step << "\n";
+    pcbnew_pyshell_one_step << "parent = wx.FindWindowById( " << parentId << " )\n";
+    pcbnew_pyshell_one_step << "newshell = kicad_pyshell.makePcbnewShellWindow( parent )\n";
+    pcbnew_pyshell_one_step << "newshell.SetName( \"" << aFramenameId << "\" )\n";
+    // return value goes into a "global". It's not actually global, but rather
+    // the dict that is passed to PyRun_String
+    pcbnew_pyshell_one_step << "retval = newshell.GetId()\n";
 
     // As always, first grab the GIL
     PyLOCK      lock;
@@ -354,7 +367,7 @@ wxWindow* CreatePythonShellWindow( wxWindow* parent, const wxString& aFramenameI
     Py_DECREF( builtins );
 
     // Execute the code to make the makeWindow function we defined above
-    result = PyRun_String( pcbnew_pyshell, Py_file_input, globals, globals );
+    PyObject*   result = PyRun_String( pcbnew_pyshell_one_step.str().c_str(), Py_file_input, globals, globals );
 
     // Was there an exception?
     if( !result )
@@ -365,43 +378,37 @@ wxWindow* CreatePythonShellWindow( wxWindow* parent, const wxString& aFramenameI
 
     Py_DECREF( result );
 
-    // Now there should be an object named 'makeWindow' in the dictionary that
-    // we can grab a pointer to:
-    PyObject* func = PyDict_GetItemString( globals, "makeWindow" );
-    wxASSERT( PyCallable_Check( func ) );
+    result = PyDict_GetItemString( globals, "retval" );
 
-    // Now build an argument tuple and call the Python function.  Notice the
-    // use of another wxPython API to take a wxWindows object and build a
-    // wxPython object that wraps it.
+#if PY_MAJOR_VERSION >= 3
+    if( !PyLong_Check( result ) )
+#else
+    if( !PyInt_Check( result ) )
+#endif
+    {
+        wxLogError("creation of scripting window didn't return a number");
+        return NULL;
+    }
 
-    PyObject*   arg = wxPyMake_wxObject( parent, false );
-    wxASSERT( arg != NULL );
+#if PY_MAJOR_VERSION >= 3
+    const long windowId = PyLong_AsLong( result );
+#else
+    const long windowId = PyInt_AsLong( result );
+#endif
 
-    PyObject*   tuple = PyTuple_New( 1 );
-    PyTuple_SET_ITEM( tuple, 0, arg );
+    // It's important not to decref globals before extracting the window id.
+    // If you do it early, globals, and the retval int it contains, may/will be garbage collected.
+    // We do not need to decref result, because GetItemString returns a borrowed reference.
+    Py_DECREF( globals );
 
-    result = PyEval_CallObject( func, tuple );
+    wxWindow* window = wxWindow::FindWindowById( windowId );
 
-    // Was there an exception?
-    if( !result )
-        PyErr_Print();
-    else
+    if( !window )
     {
-        // Otherwise, get the returned window out of Python-land and
-        // into C++-ville...
-        bool success = wxPyConvertSwigPtr( result, (void**) &window, "wxWindow" );
-        (void) success;
-
-        wxASSERT_MSG( success, "Returned object was not a wxWindow!" );
-        Py_DECREF( result );
-
-        window->SetName( aFramenameId );
+        wxLogError("unable to find pyshell window with id %d", windowId);
+        return NULL;
     }
 
-    // Release the python objects we still have
-    Py_DECREF( globals );
-    Py_DECREF( tuple );
-
     return window;
 }
 
diff --git a/pcbnew/swig/python_scripting.h b/pcbnew/swig/python_scripting.h
index 373cc66..2cc53b8 100644
--- a/pcbnew/swig/python_scripting.h
+++ b/pcbnew/swig/python_scripting.h
@@ -39,7 +39,11 @@
 
 #ifndef NO_WXPYTHON_EXTENSION_HEADERS
 #ifdef KICAD_SCRIPTING_WXPYTHON
-    #include <wx/wxPython/wxPython.h>
+    #ifdef KICAD_SCRIPTING_WXPYTHON_PHOENIX
+        #include <wx/window.h>
+    #else
+        #include <wx/wxPython/wxPython.h>
+    #endif
 #endif
 #endif