Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Support for building with Shiboken2 in Engine #697

Merged
merged 2 commits into from Nov 7, 2021
Merged

Support for building with Shiboken2 in Engine #697

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ba44387
Shiboken2 for NatronEngine
YakoYakoYokuYoku Nov 3, 2021
7a6b160
Preprocessor suggestions
YakoYakoYokuYoku Nov 4, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion Engine/AppManager.h
View file
Open in desktop
Expand Up @@ -44,7 +44,7 @@ CLANG_DIAG_ON(deprecated)
#include <QtCore/QProcess>
#include <QtCore/QMap>

#if !defined(Q_MOC_RUN) && !defined(SBK_RUN)
YakoYakoYokuYoku marked this conversation as resolved.
Show resolved Hide resolved
#if (!defined(Q_MOC_RUN) && !defined(SBK_RUN)) || defined(SBK2_GEN)
#include <boost/scoped_ptr.hpp>
#include <boost/shared_ptr.hpp>
#include <boost/noncopyable.hpp>
Expand Down
2 changes: 0 additions & 2 deletions Engine/Engine.pro
View file
Open in desktop
Expand Up @@ -238,7 +238,6 @@ SOURCES += \
NatronEngine/intparam_wrapper.cpp \
NatronEngine/itembase_wrapper.cpp \
NatronEngine/layer_wrapper.cpp \
NatronEngine/natron_namespace_wrapper.cpp \
NatronEngine/natronengine_module_wrapper.cpp \
NatronEngine/nodecreationproperty_wrapper.cpp \
NatronEngine/outputfileparam_wrapper.cpp \
Expand Down Expand Up @@ -383,7 +382,6 @@ HEADERS += \
PyParameter.h \
PyRoto.h \
PyTracker.h \
Pyside_Engine_Python.h \
ReadNode.h \
RectD.h \
RectDSerialization.h \
Expand Down
2 changes: 1 addition & 1 deletion Engine/ImagePlaneDesc.h
View file
Open in desktop
Expand Up @@ -31,7 +31,7 @@
#include <string>
#include <vector>

#if !defined(Q_MOC_RUN) && !defined(SBK_RUN)
YakoYakoYokuYoku marked this conversation as resolved.
Show resolved Hide resolved
#if (!defined(Q_MOC_RUN) && !defined(SBK_RUN)) || defined(SBK2_GEN)
GCC_DIAG_UNUSED_LOCAL_TYPEDEFS_OFF
GCC_DIAG_OFF(unused-parameter)
// /opt/local/include/boost/serialization/smart_cast.hpp:254:25: warning: unused parameter 'u' [-Wunused-parameter]
Expand Down
2 changes: 1 addition & 1 deletion Engine/Knob.h
View file
Open in desktop
Expand Up @@ -33,7 +33,7 @@
#include <set>
#include <map>

#if !defined(Q_MOC_RUN) && !defined(SBK_RUN)
YakoYakoYokuYoku marked this conversation as resolved.
Show resolved Hide resolved
#if (!defined(Q_MOC_RUN) && !defined(SBK_RUN)) || defined(SBK2_GEN)
#include <boost/enable_shared_from_this.hpp>
#include <boost/scoped_ptr.hpp>
#endif
Expand Down
6 changes: 6 additions & 0 deletions Engine/PySideCompat.cpp
View file
Open in desktop
Expand Up @@ -38,11 +38,17 @@
CLANG_DIAG_OFF(mismatched-tags)
GCC_DIAG_OFF(unused-parameter)
GCC_DIAG_OFF(missing-field-initializers)
#ifdef SBK2_GEN
#include <basewrapper.h>
#include <sbkconverter.h>
#include <gilstate.h>
#else
#include <basewrapper.h>
#include <conversions.h>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use #if/#ifdef, please don't remove

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The FreeCAD codebase uses #if/#ifdef for the Python wrapper, gonna abstain the removals and use the preprocessor instead.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK!
also, I would remommend putting shiboken-generated files and shiboken2 generated files in two separate directories, so that we can check in both in the sources. That would be great

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Problem is that the Engine.pro is gonna need a good amount of tinkering, cannot get any idea to what to do here

#include <sbkconverter.h>
#include <gilstate.h>
#include <typeresolver.h>
YakoYakoYokuYoku marked this conversation as resolved.
Show resolved Hide resolved
#endif
#include <bindingmanager.h>
CLANG_DIAG_ON(mismatched-tags)
GCC_DIAG_ON(unused-parameter)
Expand Down
10 changes: 7 additions & 3 deletions Engine/Pyside_Engine_Python.h
View file
Open in desktop
Expand Up @@ -17,6 +17,9 @@
* along with Natron. If not, see <http://www.gnu.org/licenses/gpl-2.0.html>
* ***** END LICENSE BLOCK ***** */

//Defined to avoid including some headers when running shiboken which may crash shiboken (particularly boost headers)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why move this here? this should be inside the include guard

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know exactly yet but Shiboken (2 and 6) does something strange that it makes "two passes" over this file, and at the second "pass" it ignores the SBK_RUN definition. Will see if something can be done instead.

#define SBK_RUN

#ifndef PYSIDE_ENGINE_PYTHON_H
#define PYSIDE_ENGINE_PYTHON_H

Expand All @@ -33,10 +36,11 @@
* Do not include it when compiling Natron.
**/

//Defined to avoid including some headers when running shiboken which may crash shiboken (particularly boost headers)
#define SBK_RUN

#ifdef SBK2_GEN
#include <pyside2_global.h>
YakoYakoYokuYoku marked this conversation as resolved.
Show resolved Hide resolved
#else
#include <pyside_global.h>
#endif
#include <string>
//Global
#include <GlobalDefines.h>
Expand Down
17 changes: 12 additions & 5 deletions Engine/typesystem_engine.xml
View file
Open in desktop
Expand Up @@ -18,10 +18,13 @@
- along with Natron. If not, see <http://www.gnu.org/licenses/gpl-2.0.html>
- ***** END LICENSE BLOCK ***** -->
<typesystem package="NatronEngine">


<inject-code class="native" position="beginning">
NATRON_NAMESPACE_USING NATRON_PYTHON_NAMESPACE_USING
</inject-code>

<!--Load QtCore typesystem-->
<load-typesystem name="typesystem_core.xml" generate="no" />

<!--Primitives-->
<primitive-type name="bool"/>
<primitive-type name="double"/>
Expand Down Expand Up @@ -209,8 +212,8 @@
</conversion-rule>
</container-type>

<!--Natron global enums-->
<namespace-type name="NATRON_NAMESPACE">
<!--Natron global enums
<namespace-type name="NATRON_NAMESPACE">-->
<enum-type name="StatusEnum"/>
<enum-type name="MergingFunctionEnum"/>
<enum-type name="StandardButtonEnum" flags="StandardButtons"/>
Expand All @@ -225,7 +228,7 @@
<enum-type name="OrientationEnum"/>
<enum-type name="PlaybackModeEnum"/>
<enum-type name="DisplayChannelsEnum"/>
</namespace-type>
<!--</namespace-type>-->


<object-type name="RectI">
Expand Down Expand Up @@ -1506,6 +1509,7 @@

for (int j = 0; j &lt; subSize; ++j) {
PyObject* pyString = PyList_GET_ITEM(subList,j);
#if PY_VERSION_MAJOR &lt; 3
if ( PyString_Check(pyString) ) {
char* buf = PyString_AsString(pyString);
if (buf) {
Expand All @@ -1514,6 +1518,9 @@
rowVec[j] = ret;
}
} else if (PyUnicode_Check(pyString) ) {
#else
if (PyUnicode_Check(pyString) ) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of removing,

  • replace PyString_Check by PyBytes_Check
  • replace PyString_AsString by PyObject_Bytes
  • put the if ( PyBytes_Check(pyString) ) { section after the if (PyUnicode_Check(pyString) section

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(this is to keep python2 compatibility)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gonna follow PEP-393 by adapting what you've suggested. May be better for Shiboken to be smart enough for #if PY_MAJOR_VERSION > 3/#else.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the PyBytes_Check works both in python2 and 3 (PyString_Check disappeared from Python3), so that makes the code 2/3 compatible. Are you suggesting it should be done differently? There is similar code at several places in Natron

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Luckily enough one call to PyUnicode_Check is needed to check if the type is suitable for Python strings, according to the official docs. Although the PyBytes_Check method works too.

#endif
PyObject* utf8pyobj = PyUnicode_AsUTF8String(pyString); // newRef
if (utf8pyobj) {
char* cstr = PyBytes_AS_STRING(utf8pyobj); // Borrowed pointer
Expand Down
4 changes: 2 additions & 2 deletions Global/Enums.h
View file
Open in desktop
Expand Up @@ -28,7 +28,7 @@ CLANG_DIAG_OFF(deprecated)
CLANG_DIAG_ON(deprecated)

NATRON_NAMESPACE_ENTER
#ifdef SBK_RUN
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why remove?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Newer C++ implementations let the preprocessor to expand the name of a namespace if it was defined. Wasn't portable to begin with.

#if defined(SBK_RUN) && !defined(SBK2_GEN)
// shiboken doesn't generate SbkNatronEngine_StandardButtonEnum_as_number unless it is put in a class or namespace
NATRON_NAMESPACE_EXIT
namespace NATRON_NAMESPACE {
Expand Down Expand Up @@ -684,7 +684,7 @@ enum MergingFunctionEnum
//typedef QFlags<StandardButtonEnum> StandardButtons;
Q_DECLARE_FLAGS(StandardButtons, StandardButtonEnum)

#ifdef SBK_RUN
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why remove?

#if defined(SBK_RUN) && !defined(SBK2_GEN)
}
NATRON_NAMESPACE_ENTER
#endif
Expand Down
8 changes: 4 additions & 4 deletions Global/GlobalDefines.h
View file
Open in desktop
Expand Up @@ -66,10 +66,10 @@ CLANG_DIAG_ON(deprecated)
//#define foreach Q_FOREACH


typedef boost::uint32_t U32;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unrelated change. file a different PR if you think this is useful

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The later versions of Shiboken were ignoring the boost::*_t types. Given that C++11 has fixed width integers it compiled without nuisance. Can remain here in the case of being accepted, otherwise I can file a different PR.

typedef boost::uint64_t U64;
typedef boost::uint8_t U8;
typedef boost::uint16_t U16;
typedef std::uint32_t U32;
typedef std::uint64_t U64;
typedef std::uint8_t U8;
typedef std::uint16_t U16;


NATRON_NAMESPACE_ENTER
Expand Down
1 change: 1 addition & 0 deletions Global/Pyside_Shiboken2_Macros.h
View file
Open in desktop
@@ -0,0 +1 @@
#define SBK2_GEN
2 changes: 0 additions & 2 deletions global.pri
View file
Open in desktop
Expand Up @@ -41,8 +41,6 @@ run-without-python {
# from <https://docs.python.org/3/c-api/intro.html#include-files>:
# "Since Python may define some pre-processor definitions which affect the standard headers on some systems, you must include Python.h before any standard headers are included."
CONFIG += python
QMAKE_CFLAGS += -include Python.h
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was giving me build errors to preinclude that file. If the current Natron build can do it without these flags, which I think it can and it is already included in many places of the codebase where it's needed.

QMAKE_CXXFLAGS += -include Python.h
python3 {
PYV=3
PY_PKG_SUFFIX=-embed
Expand Down
88 changes: 88 additions & 0 deletions tools/utils/runPostShiboken2.sh
View file
Open in desktop
@@ -0,0 +1,88 @@
#!/bin/sh
# ***** BEGIN LICENSE BLOCK *****
# This file is part of Natron <http://www.natron.fr/>,
# Copyright (C) 2016 INRIA and Alexandre Gauthier
#
# Natron 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.
#
# Natron 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 Natron. If not, see <http://www.gnu.org/licenses/gpl-2.0.html>
# ***** END LICENSE BLOCK *****

# exit if a command returns an error status
set -e

# PySide version override, add 2 for Qt5/PySide2
PYV="${1:-2}"

# To be run after shiboken to fix errors

sed -e '/<destroylistener.h>/d' -i'.bak' Engine/NatronEngine/*.cpp
sed -e '/<destroylistener.h>/d' -i'.bak' Gui/NatronGui/*.cpp
sed -e "/SbkPySide${PYV}_QtCoreTypes;/d" -i'.bak' Gui/NatronGui/natrongui_module_wrapper.cpp
sed -e "/SbkPySide${PYV}_QtCoreTypeConverters;/d" -i'.bak' Gui/NatronGui/natrongui_module_wrapper.cpp
sed -e '/SbkNatronEngineTypes;/d' -i'.bak' Gui/NatronGui/natrongui_module_wrapper.cpp
sed -e '/SbkNatronEngineTypeConverters;/d' -i'.bak' Gui/NatronGui/natrongui_module_wrapper.cpp
sed -e 's/cleanTypesAttributes/cleanGuiTypesAttributes/g' -i'.bak' Gui/NatronGui/natrongui_module_wrapper.cpp
sed -e '/Py_BEGIN_ALLOW_THREADS/d' -i'.bak' Engine/NatronEngine/*.cpp
sed -e '/Py_BEGIN_ALLOW_THREADS/d' -i'.bak' Gui/NatronGui/*.cpp
sed -e '/Py_END_ALLOW_THREADS/d' -i'.bak' Engine/NatronEngine/*.cpp
sed -e '/Py_END_ALLOW_THREADS/d' -i'.bak' Gui/NatronGui/*.cpp

# fix warnings
sed -e 's@^#include <shiboken.h>$@#include "Global/Macros.h"\
CLANG_DIAG_OFF(mismatched-tags)\
GCC_DIAG_OFF(unused-parameter)\
GCC_DIAG_OFF(missing-field-initializers)\
GCC_DIAG_OFF(missing-declarations)\
GCC_DIAG_OFF(uninitialized)\
GCC_DIAG_UNUSED_LOCAL_TYPEDEFS_OFF\
#include <shiboken.h> // produces many warnings@' -i'.bak' Engine/NatronEngine/*.cpp Gui/NatronGui/*.cpp

sed -e 's@// inner classes@// inner classes\
NATRON_NAMESPACE_USING NATRON_PYTHON_NAMESPACE_USING@' -i'.bak' Engine/NatronEngine/*.cpp Gui/NatronGui/*.cpp

# replace NATRON_NAMESPACE with Natron for enums with flags (e.g. StandardButtonEnum)
sed -e 's@"NatronEngine\.NATRON_NAMESPACE@"NatronEngine.Natron@g' -e 's@, NatronEngine\.NATRON_NAMESPACE@, NatronEngine.Natron@g' -e 's@"NatronGui\.NATRON_NAMESPACE@"NatronGui.Natron@g' -e 's@"NATRON_NAMESPACE@"Natron@g' -i'.bak' Engine/NatronEngine/*_wrapper.cpp

# re-add the Natron namespace
#sed -e 's@" ::\([^s][^t][^d]\)@ NATRON_NAMESPACE::\1@g' -i'.bak' Engine/NatronEngine/*.cpp Engine/NatronEngine/*.h Gui/NatronGui/*.cpp Gui/NatronGui/*.h

sed -e '/AnimatedParam/ {:y;n;by};s@SbkType< ::\(.*\) >@SbkType<NATRON_NAMESPACE::\1 >@g' -i Engine/NatronEngine/natronengine_python.h
sed -e 's@SbkType< ::@SbkType<NATRON_NAMESPACE::NATRON_PYTHON_NAMESPACE::@g' -e 's@SbkType<NATRON_NAMESPACE::QFlags<@SbkType< ::QFlags<NATRON_NAMESPACE::@g' -e's@NATRON_NAMESPACE::NATRON_PYTHON_NAMESPACE::Rect@NATRON_NAMESPACE::Rect@g' -i'.bak' Engine/NatronEngine/natronengine_python.h Gui/NatronGui/natrongui_python.h
sed -e 's@^class @NATRON_NAMESPACE_ENTER NATRON_PYTHON_NAMESPACE_ENTER\
class @g;T;{:y;/^};/! {n;by}};s@^};@};\
NATRON_PYTHON_NAMESPACE_EXIT NATRON_NAMESPACE_EXIT@g' -i'.bak' Engine/NatronEngine/*.h Gui/NatronGui/*.h

# replace NATRON_NAMESPACE::NATRON_NAMESPACE with NATRON_NAMESPACE in the enums wrappers
sed -e 's@NATRON_NAMESPACE::NATRON_PYTHON_NAMESPACE::NATRON_NAMESPACE@NATRON_NAMESPACE@g' -i'.bak' Engine/NatronEngine/natronengine_python.h Gui/NatronGui/natrongui_python.h

sed -e 's@^#include <pysidemetafunction.h>$@CLANG_DIAG_OFF(header-guard)\
#include <pysidemetafunction.h> // has wrong header guards in pyside 1.2.2@' -i'.bak' Engine/NatronEngine/*.cpp Gui/NatronGui/*.cpp

sed -e 's@^#include <pyside'${PYV}'_qtcore_python.h>$@CLANG_DIAG_OFF(deprecated)\
CLANG_DIAG_OFF(uninitialized)\
CLANG_DIAG_OFF(keyword-macro)\
#include <pyside'${PYV}'_qtcore_python.h> // produces warnings\
CLANG_DIAG_ON(deprecated)\
CLANG_DIAG_ON(uninitialized)\
CLANG_DIAG_ON(keyword-macro)@' -i'.bak' Engine/NatronEngine/*.cpp Gui/NatronGui/*.cpp Engine/NatronEngine/*.h Gui/NatronGui/*.h

sed -e 's@^#include <pyside'${PYV}'_qtgui_python.h>$@CLANG_DIAG_OFF(deprecated)\
CLANG_DIAG_OFF(uninitialized)\
CLANG_DIAG_OFF(keyword-macro)\
#include <pyside'${PYV}'_qtgui_python.h> // produces warnings\
CLANG_DIAG_ON(deprecated)\
CLANG_DIAG_ON(uninitialized)\
CLANG_DIAG_ON(keyword-macro)@' -i'.bak' Engine/NatronEngine/*.cpp Gui/NatronGui/*.cpp Engine/NatronEngine/*.h Gui/NatronGui/*.h

# clean up
rm Gui/NatronGui/*.bak Engine/NatronEngine/*.bak