From 80a6f91c553eaf9ed4d7d002a5abb442693e8e08 Mon Sep 17 00:00:00 2001 From: Christian Tismer Date: Thu, 23 May 2019 19:35:51 +0200 Subject: Support the qApp macro in "scriptable application" Renamed from "Fix scriptable application to support the qApp macro" because qApp was improved instead of scriptable application. The qApp macro needed some extra effort to support the qApp "macro" which is only defined in the Python wrappers. I took some generated code, created a QApplication instance in Python and used then reduced generated code to get at the object and adjust the refcount. This solution was then rejected, because I can do better, and in fact, scriptable application now has a correct qApp macro too, without any change to scriptable application. The central idea was to look into the module init function at import time and to see if a Q*Application already exists. I was not aware of that import. Many thanks for the rejection! :-) Update.. -------- After many attempts to make the qApp variable correctly behave like always, I recognized that pre-existing Q*Application instances have no wrappers or constructors at all! With that, it is not possible to create a sophisticated qApp macro as a singleton variable in the desired way. Fortunately, this is also not necessary, because a C++ Q*Application cannot be deleted from Python, and there is no point in supporting more that a simple variable. So in case of a pre-existing instance, the qApp variable now gets redirected to that instance. A small test was added to application_test.py that is triggered by an import. A weird effect when "qApp" was typed interactively before calling "QApplication()" was fixed, too. Change-Id: Ic69dd6a21c964838a90f63e316d299b62a54d612 Fixes: PYSIDE-571 Reviewed-by: Cristian Maureira-Fredes --- sources/pyside2/PySide2/glue/qtcore.cpp | 9 +++-- .../pyside2/tests/QtWidgets/application_test.py | 8 ++++- .../shiboken2/generator/shiboken2/cppgenerator.cpp | 2 +- sources/shiboken2/libshiboken/qapp_macro.cpp | 41 ++++++++++++++++++---- sources/shiboken2/libshiboken/qapp_macro.h | 2 +- 5 files changed, 47 insertions(+), 15 deletions(-) diff --git a/sources/pyside2/PySide2/glue/qtcore.cpp b/sources/pyside2/PySide2/glue/qtcore.cpp index f30607f6b..9db4e2e82 100644 --- a/sources/pyside2/PySide2/glue/qtcore.cpp +++ b/sources/pyside2/PySide2/glue/qtcore.cpp @@ -1340,18 +1340,17 @@ if (!PyTuple_SetItem(empty, 0, PyList_New(0))) { // @snippet qcoreapplication-2 // @snippet qcoreapplication-instance -QCoreApplication *app = QCoreApplication::instance(); PyObject *pyApp = Py_None; -if (app) { +if (qApp) { pyApp = reinterpret_cast( - Shiboken::BindingManager::instance().retrieveWrapper(app)); + Shiboken::BindingManager::instance().retrieveWrapper(qApp)); if (!pyApp) - pyApp = %CONVERTTOPYTHON[QCoreApplication*](app); + pyApp = %CONVERTTOPYTHON[QCoreApplication*](qApp); // this will keep app live after python exit (extra ref) } // PYSIDE-571: make sure that we return the singleton "None" if (pyApp == Py_None) - Py_DECREF(MakeSingletonQAppWrapper(0)); // here qApp and instance() diverge + Py_DECREF(MakeSingletonQAppWrapper(nullptr)); // here qApp and instance() diverge %PYARG_0 = pyApp; Py_XINCREF(%PYARG_0); // @snippet qcoreapplication-instance diff --git a/sources/pyside2/tests/QtWidgets/application_test.py b/sources/pyside2/tests/QtWidgets/application_test.py index bd0f94125..0b8f73cd6 100644 --- a/sources/pyside2/tests/QtWidgets/application_test.py +++ b/sources/pyside2/tests/QtWidgets/application_test.py @@ -31,19 +31,25 @@ import unittest from testbinding import TestObject from PySide2.QtWidgets import QApplication +from PySide2 import __all__ as all class QApplicationInstance(unittest.TestCase): def appDestroyed(self): - sefl.assertTrue(False) + self.assertTrue(False) def testInstanceObject(self): + self.assertEqual(type(qApp), type(None)) TestObject.createApp() app1 = QApplication.instance() app2 = QApplication.instance() app1.setObjectName("MyApp") self.assertEqual(app1, app2) self.assertEqual(app2.objectName(), app1.objectName()) + if len(all) > 3: + # an import triggers qApp initialization + __import__("PySide2." + all[-1]) + self.assertEqual(app1, qApp) app1.destroyed.connect(self.appDestroyed) if __name__ == '__main__': diff --git a/sources/shiboken2/generator/shiboken2/cppgenerator.cpp b/sources/shiboken2/generator/shiboken2/cppgenerator.cpp index 8ee0a9cf2..d1da3a4ba 100644 --- a/sources/shiboken2/generator/shiboken2/cppgenerator.cpp +++ b/sources/shiboken2/generator/shiboken2/cppgenerator.cpp @@ -5806,7 +5806,7 @@ bool CppGenerator::finishGeneration() if (usePySideExtensions()) { // initialize the qApp module. - s << INDENT << "NotifyModuleForQApp(module);" << endl; + s << INDENT << "NotifyModuleForQApp(module, qApp);" << endl; } s << endl; s << "SBK_MODULE_INIT_FUNCTION_END" << endl; diff --git a/sources/shiboken2/libshiboken/qapp_macro.cpp b/sources/shiboken2/libshiboken/qapp_macro.cpp index 19e985b20..ea9cf0c86 100644 --- a/sources/shiboken2/libshiboken/qapp_macro.cpp +++ b/sources/shiboken2/libshiboken/qapp_macro.cpp @@ -38,6 +38,7 @@ ****************************************************************************/ #include "basewrapper.h" +#include "autodecref.h" extern "C" { @@ -93,13 +94,14 @@ static int qApp_var_ref = 0; static int qApp_content_ref = 0; static int -reset_qApp_var() +reset_qApp_var(void) { PyObject **mod_ptr; - for (mod_ptr = qApp_moduledicts; *mod_ptr != NULL; mod_ptr++) { + for (mod_ptr = qApp_moduledicts; *mod_ptr != nullptr; mod_ptr++) { // We respect whatever the user may have set. - if (PyDict_GetItem(*mod_ptr, qApp_var) == NULL) { + PyObject *existing = PyDict_GetItem(*mod_ptr, qApp_var); + if (existing == nullptr || Py_TYPE(existing) == Py_NONE_TYPE) { if (PyDict_SetItem(*mod_ptr, qApp_var, qApp_content) < 0) return -1; } @@ -135,8 +137,13 @@ MakeSingletonQAppWrapper(PyTypeObject *type) if (Py_REFCNT(qApp_content) > qApp_content_ref) qApp_content_ref = Py_REFCNT(qApp_content); - if (Py_TYPE(qApp_content) != Py_NONE_TYPE) + if (Py_TYPE(qApp_content) != Py_NONE_TYPE) { + // Remove the "_" variable which might hold a reference to qApp. + Shiboken::AutoDecRef pymain(PyImport_ImportModule("__main__")); + if (pymain.object() && PyObject_HasAttrString(pymain.object(), "_")) + PyObject_DelAttrString(pymain.object(), "_"); Py_REFCNT(qApp_var) = 1; // fuse is armed... + } if (type == Py_NONE_TYPE) { // Debug mode showed that we need to do more than just remove the // reference. To keep everything in the right order, it is easiest @@ -149,8 +156,8 @@ MakeSingletonQAppWrapper(PyTypeObject *type) Py_TYPE(qApp_content) = Py_NONE_TYPE; Py_REFCNT(qApp_var) = qApp_var_ref; Py_REFCNT(qApp_content) = Py_REFCNT(Py_None); - if (__moduleShutdown != NULL) - Py_DECREF(PyObject_CallFunction(__moduleShutdown, (char *)"()")); + if (__moduleShutdown != nullptr) + Py_XDECREF(PyObject_CallFunction(__moduleShutdown, const_cast("()"))); } else (void)PyObject_INIT(qApp_content, type); @@ -216,9 +223,29 @@ setup_qApp_var(PyObject *module) } void -NotifyModuleForQApp(PyObject *module) +NotifyModuleForQApp(PyObject *module, void *qApp) { setup_qApp_var(module); + /* + * PYSIDE-571: Check if an QApplication instance exists before the import. + * This happens in scriptableapplication and application_test.py . + * + * Crucial Observation + * =================== + * + * A Q*Application object from C++ does not have a wrapper or constructor + * like instances created by Python. It makes no sense to support + * deletion or special features like qApp resurrection. + * + * Therefore, the implementation is very simple and just redirects the + * qApp_contents variable and assigns the instance, instead of vice-versa. + */ + if (qApp != nullptr) { + Shiboken::AutoDecRef pycore(PyImport_ImportModule("PySide2.QtCore")); + Shiboken::AutoDecRef coreapp(PyObject_GetAttrString(pycore, "QCoreApplication")); + qApp_content = PyObject_CallMethod(coreapp, "instance", ""); + reset_qApp_var(); + } } diff --git a/sources/shiboken2/libshiboken/qapp_macro.h b/sources/shiboken2/libshiboken/qapp_macro.h index ded892383..be45241de 100644 --- a/sources/shiboken2/libshiboken/qapp_macro.h +++ b/sources/shiboken2/libshiboken/qapp_macro.h @@ -46,7 +46,7 @@ extern "C" { LIBSHIBOKEN_API PyObject *MakeSingletonQAppWrapper(PyTypeObject *type); -LIBSHIBOKEN_API void NotifyModuleForQApp(PyObject *module); +LIBSHIBOKEN_API void NotifyModuleForQApp(PyObject *module, void *qApp); } // extern "C" -- cgit v1.2.3