[education/rkward] /: Emergency port away from QScriptEngine, but for now on MacOS, only.
Thomas Friedrichsmeier
null at kde.org
Tue Apr 5 21:17:41 BST 2022
Git commit 83002d1bf908e981528aa0a06e5a89feed3899a0 by Thomas Friedrichsmeier.
Committed on 05/04/2022 at 20:15.
Pushed by tfry into branch 'master'.
Emergency port away from QScriptEngine, but for now on MacOS, only.
This is not really a change I want to make this close before release.
However, the scripting on Mac is so badly broken, it can only get better.
Will need cleanups and some performance testing, before this is enabled on all platforms.
M +12 -0 CMakeLists.txt
M +4 -0 rkward/scriptbackends/CMakeLists.txt
M +23 -9 rkward/scriptbackends/qtscriptbackend.cpp
M +28 -3 rkward/scriptbackends/qtscriptbackend.h
M +2 -4 rkward/scriptbackends/qtscripti18n.cpp
M +3 -2 rkward/scriptbackends/qtscripti18n.h
M +34 -20 rkward/scriptbackends/rkcomponentscripting.cpp
M +4 -3 rkward/scriptbackends/rkcomponentscripting.h
https://invent.kde.org/education/rkward/commit/83002d1bf908e981528aa0a06e5a89feed3899a0
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 5cb61bc6..b14f974e 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -39,6 +39,18 @@ IF(FORCE_PRETTY_MAKEFILE)
SET(CMAKE_VERBOSE_MAKEFILE OFF)
ENDIF(FORCE_PRETTY_MAKEFILE)
+# TODO: Temporary. Remove once we are sure that using QJSEngine has no negative performance impact on nested plugins, or we have mitigated it.
+# For now, meant to be used on Mac, where QScriptEngine is crashy.
+IF(APPLE)
+SET(USE_QJSENGINE TRUE)
+ELSE()
+SET(USE_QJSENGINE FALSE)
+ENDIF()
+IF(USE_QJSENGINE)
+ ADD_DEFINITIONS(-DUSE_QJSENGINE)
+ FIND_PACKAGE(Qt5 5.9 CONFIG REQUIRED COMPONENTS Qml)
+ENDIF(USE_QJSENGINE)
+
ADD_DEFINITIONS(${QT_DEFINITIONS} -DQT_NO_CAST_TO_ASCII)
ADD_DEFINITIONS(-DQT_USE_FAST_CONCATENATION -DQT_USE_FAST_OPERATOR_PLUS)
ADD_DEFINITIONS(-DQT_NO_URL_CAST_FROM_STRING)
diff --git a/rkward/scriptbackends/CMakeLists.txt b/rkward/scriptbackends/CMakeLists.txt
index f53d6b1d..308073e0 100644
--- a/rkward/scriptbackends/CMakeLists.txt
+++ b/rkward/scriptbackends/CMakeLists.txt
@@ -9,6 +9,10 @@ SET(scriptbackends_STAT_SRCS
)
ADD_LIBRARY(scriptbackends STATIC ${scriptbackends_STAT_SRCS})
+IF(USE_QJSENGINE)
+TARGET_LINK_LIBRARIES(scriptbackends Qt5::Qml Qt5::Widgets Qt5::Xml KF5::WidgetsAddons KF5::I18n)
+ELSE()
TARGET_LINK_LIBRARIES(scriptbackends Qt5::Script Qt5::Widgets Qt5::Xml KF5::WidgetsAddons KF5::I18n)
+ENDIF()
INSTALL(FILES common.js rkcomponentscripting.js DESTINATION ${DATA_INSTALL_DIR}/rkward/phpfiles )
diff --git a/rkward/scriptbackends/qtscriptbackend.cpp b/rkward/scriptbackends/qtscriptbackend.cpp
index 52187be9..f30aa906 100644
--- a/rkward/scriptbackends/qtscriptbackend.cpp
+++ b/rkward/scriptbackends/qtscriptbackend.cpp
@@ -224,9 +224,22 @@ QVariant QtScriptBackendThread::getUiLabelPair (const QString &identifier) {
return getValue (identifier, RKStandardComponent::UiLabelPair);
}
-bool QtScriptBackendThread::scriptError () {
+#ifdef USE_QJSENGINE
+bool QtScriptBackendThread::scriptError(const QJSValue &val) {
RK_TRACE (PHP);
+ if (!val.isError()) return false;
+
+ QString message = i18n("Script Error in %1, line %2: %3\nBacktrace:\n%4", val.property("fileName").toString(), val.property("lineNumber").toInt(), val.toString(), val.property("stack").toString()); // TODO: correct?
+ emit error(message);
+
+ return true;
+}
+#else
+bool QtScriptBackendThread::scriptError(const RKJSValue &val) {
+ RK_TRACE (PHP);
+ Q_UNUSED(val);
+
if (!engine.hasUncaughtException ()) return false;
QString message = i18n ("Script Error: %1\nBacktrace:\n%2", engine.uncaughtException ().toString (), engine.uncaughtExceptionBacktrace ().join ("\n"));
@@ -235,6 +248,7 @@ bool QtScriptBackendThread::scriptError () {
return true;
}
+#endif
bool QtScriptBackendThread::includeFile (const QString &filename) {
RK_TRACE (PHP);
@@ -251,11 +265,12 @@ bool QtScriptBackendThread::includeFile (const QString &filename) {
return false;
}
+#ifndef USE_QJSENGINE
// evaluate in global context
engine.currentContext ()->setActivationObject (engine.globalObject ());
- QScriptValue result = engine.evaluate (QString::fromUtf8 (file.readAll ()), _filename);
-
- if (scriptError ()) return false;
+#endif
+ RKJSValue result = engine.evaluate (QString::fromUtf8 (file.readAll ()), _filename);
+ if (scriptError(result)) return false;
return true;
}
@@ -263,17 +278,16 @@ bool QtScriptBackendThread::includeFile (const QString &filename) {
void QtScriptBackendThread::run () {
RK_TRACE (PHP);
- QScriptValue backend_object = engine.newQObject (this);
+ RKJSValue backend_object = engine.newQObject (this);
engine.globalObject ().setProperty ("_RK_backend", backend_object);
RKMessageCatalogObject::addI18nToScriptEngine (&engine, catalog);
- if (scriptError ()) return;
#ifdef USE_Q_SCRIPT_PROGRAM
if (!RKPrecompiledQtScripts::loadCommonScript (&engine, _commonfile)) {
if (!engine.hasUncaughtException ()) {
emit error (i18n ("Could not open common script file \"%1\"", _commonfile));
} else {
- scriptError ();
+ scriptError (QScriptValue());
}
return;
}
@@ -305,8 +319,8 @@ void QtScriptBackendThread::run () {
}
// do it!
- QScriptValue result = engine.evaluate (command);
- if (scriptError ()) return;
+ RKJSValue result = engine.evaluate (command);
+ if (scriptError(result)) return;
emit commandDone(result.toString());
command.clear ();
diff --git a/rkward/scriptbackends/qtscriptbackend.h b/rkward/scriptbackends/qtscriptbackend.h
index cfc16508..eb2aa396 100644
--- a/rkward/scriptbackends/qtscriptbackend.h
+++ b/rkward/scriptbackends/qtscriptbackend.h
@@ -49,7 +49,29 @@ private:
#include <QThread>
#include <QMutex>
+
+#ifdef USE_QJSENGINE
+#include <QtQml/QJSEngine>
+#define RKJSEngine QJSEngine
+#define RKJSValue QJSValue
+#define RKJSValueList QJSValueList
+template<typename T> RKJSValue rkJSMakeArray(RKJSEngine *engine, QVector<T> list) {
+ auto ret = engine->newArray(list.size());
+ for(int i = 0; i < list.size(); ++i) ret.setProperty(i, list.at(i));
+ return ret;
+}
+template<typename T> RKJSValue rkJSMakeArray(RKJSEngine *engine, QList<T> list) {
+ auto ret = engine->newArray(list.size());
+ for(int i = 0; i < list.size(); ++i) ret.setProperty(i, list.at(i));
+ return ret;
+}
+#else
#include <QScriptEngine>
+#define RKJSEngine QScriptEngine
+#define RKJSValue QScriptValue
+#define RKJSValueList QScriptValueList
+#define rkJSMakeArray(engine, val) qScriptValueFromSequence(engine, val)
+#endif
class QtScriptBackendThread : public QThread {
Q_OBJECT
@@ -75,8 +97,8 @@ protected slots:
protected:
void run () override;
private:
- /** for any script error in the last evaluation. If there was an error, a message is generated, and this function return true (and the thread should be made to exit!) */
- bool scriptError ();
+ /** for any script error in the last evaluation. If there was an error, a message is generated, and this function returns true (and the thread should be made to exit!) */
+ bool scriptError(const RKJSValue &val);
QVariant getValue (const QString &identifier, const int hint);
QString _command;
@@ -84,7 +106,7 @@ private:
QString _commonfile;
QString _scriptfile;
- QScriptEngine engine;
+ RKJSEngine engine;
const RKMessageCatalog *catalog;
bool killed;
@@ -95,10 +117,13 @@ private:
bool sleeping;
};
+#ifndef USE_QJSENGINE
+// TODO: support some level of pre-parsing with QJSEngine
#define USE_Q_SCRIPT_PROGRAM
#include <QScriptProgram>
namespace RKPrecompiledQtScripts {
bool loadCommonScript (QScriptEngine *engine, const QString &scriptfile);
};
+#endif
#endif
diff --git a/rkward/scriptbackends/qtscripti18n.cpp b/rkward/scriptbackends/qtscripti18n.cpp
index 42a4dae6..f52094c3 100644
--- a/rkward/scriptbackends/qtscripti18n.cpp
+++ b/rkward/scriptbackends/qtscripti18n.cpp
@@ -7,12 +7,10 @@ SPDX-License-Identifier: GPL-2.0-or-later
#include "qtscripti18n.h"
-#include <QScriptEngine>
-
#include "../debug.h"
-void RKMessageCatalogObject::addI18nToScriptEngine (QScriptEngine* engine, const RKMessageCatalog* catalog) {
- QScriptValue handle = engine->newQObject (new RKMessageCatalogObject (catalog, engine));
+void RKMessageCatalogObject::addI18nToScriptEngine (RKJSEngine* engine, const RKMessageCatalog* catalog) {
+ auto handle = engine->newQObject (new RKMessageCatalogObject (catalog, engine));
engine->globalObject ().setProperty ("_i18n", handle);
}
diff --git a/rkward/scriptbackends/qtscripti18n.h b/rkward/scriptbackends/qtscripti18n.h
index b6986969..08b90056 100644
--- a/rkward/scriptbackends/qtscripti18n.h
+++ b/rkward/scriptbackends/qtscripti18n.h
@@ -10,9 +10,10 @@ SPDX-License-Identifier: GPL-2.0-or-later
#include <QObject>
+#include "qtscriptbackend.h" // TODO: For RKJSEngine, only. Move back to .cpp-file
+
#include "../misc/rkmessagecatalog.h"
-class QScriptEngine;
/** A QObject wrapper around RKMessageCatalog. Meant for use in RKComponentScripting and QtScriptBackend */
class RKMessageCatalogObject : public QObject {
Q_OBJECT
@@ -33,7 +34,7 @@ public:
return (catalog->translate (msgctxt, msgid_singular, msgid_plural, count));
};
/** Add an RKMessageCatalog, and the required glue code to the given QScriptEngine. */
- static void addI18nToScriptEngine (QScriptEngine *engine, const RKMessageCatalog *catalog);
+ static void addI18nToScriptEngine (RKJSEngine *engine, const RKMessageCatalog *catalog);
private:
const RKMessageCatalog *catalog;
};
diff --git a/rkward/scriptbackends/rkcomponentscripting.cpp b/rkward/scriptbackends/rkcomponentscripting.cpp
index bbbc9c9f..0ae49a65 100644
--- a/rkward/scriptbackends/rkcomponentscripting.cpp
+++ b/rkward/scriptbackends/rkcomponentscripting.cpp
@@ -28,7 +28,7 @@ RKComponentScriptingProxy::RKComponentScriptingProxy (RKComponent *component) :
RK_ASSERT (component);
RKComponentScriptingProxy::component = component;
- QScriptValue backend_object = engine.newQObject (this);
+ RKJSValue backend_object = engine.newQObject(this);
engine.globalObject ().setProperty ("_rkward", backend_object);
RKMessageCatalogObject::addI18nToScriptEngine (&engine, component->xmlHelper ()->messageCatalog ());
}
@@ -63,17 +63,25 @@ void RKComponentScriptingProxy::initialize (const QString& file, const QString&
evaluate (_command);
}
-void RKComponentScriptingProxy::handleScriptError (const QString& current_file) {
+void RKComponentScriptingProxy::handleScriptError(const RKJSValue &val, const QString& current_file) {
RK_TRACE (PHP);
QString file = current_file;
if (file.isEmpty ()) file = _scriptfile;
+#ifdef USE_QJSENGINE
+ if (val.isError()) {
+ QString message = i18n("Script Error at '%1' line %2: %3\n", file.isEmpty() ? i18n("inlined code") : file, val.property("lineNumber").toInt(), val.toString());
+ KMessageBox::detailedError (0, message, val.property("stack").toString());
+ emit haveError();
+ }
+#else
if (engine.hasUncaughtException ()) {
QString message = i18n ("Script Error at '%1' line %2: %3\n", file.isEmpty() ? i18n ("inlined code") : file, engine.uncaughtExceptionLineNumber (), engine.uncaughtException ().toString ());
KMessageBox::detailedError (0, message, engine.uncaughtExceptionBacktrace ().join ("\n"));
engine.clearExceptions ();
emit haveError();
}
+#endif
}
void RKComponentScriptingProxy::include (const QString& filename) {
@@ -98,11 +106,13 @@ void RKComponentScriptingProxy::include (const QString& filename) {
void RKComponentScriptingProxy::evaluate (const QString &code) {
RK_TRACE (PHP);
+#ifndef USE_QJSENGINE
// evaluate in global context
engine.currentContext ()->setActivationObject (engine.globalObject ());
- QScriptValue result = engine.evaluate (code, _scriptfile);
+#endif
+ RKJSValue result = engine.evaluate (code, _scriptfile);
- handleScriptError ();
+ handleScriptError(result);
}
void RKComponentScriptingProxy::addChangeCommand (const QString& changed_id, const QString& command) {
@@ -148,18 +158,18 @@ QVariant RKComponentScriptingProxy::doRCommand (const QString& command, const QS
return (QVariant (com.command->id ()));
}
-static QScriptValue marshall (QScriptEngine *engine, RData *data) {
+static RKJSValue marshall (RKJSEngine *engine, RData *data) {
RK_TRACE (PHP);
- if (data->getDataType () == RData::StringVector) {
- return (qScriptValueFromSequence (engine, data->stringVector()));
- } else if (data->getDataType () == RData::IntVector) {
- return (qScriptValueFromSequence (engine, data->intVector()));
- } else if (data->getDataType () == RData::RealVector) {
- return (qScriptValueFromSequence (engine, data->realVector()));
- } else if (data->getDataType () == RData::StructureVector) {
- const RData::RDataStorage& rs = data->structureVector ();
- QScriptValue ret = engine->newArray (rs.size ());
+ if (data->getDataType() == RData::StringVector) {
+ return (rkJSMakeArray(engine, data->stringVector()));
+ } else if (data->getDataType() == RData::IntVector) {
+ return (rkJSMakeArray(engine, data->intVector()));
+ } else if (data->getDataType() == RData::RealVector) {
+ return (rkJSMakeArray(engine, data->realVector()));
+ } else if (data->getDataType() == RData::StructureVector) {
+ const RData::RDataStorage& rs = data->structureVector();
+ RKJSValue ret = engine->newArray (rs.size ());
for (int i = 0; i < rs.size (); ++i) {
ret.setProperty (i, marshall (engine, rs[i]));
}
@@ -167,7 +177,7 @@ static QScriptValue marshall (QScriptEngine *engine, RData *data) {
} else {
RK_ASSERT (false);
}
- return QScriptValue ();
+ return RKJSValue ();
}
void RKComponentScriptingProxy::scriptRCommandFinished (RCommand* command) {
@@ -187,12 +197,16 @@ void RKComponentScriptingProxy::scriptRCommandFinished (RCommand* command) {
if (command->wasCanceled ()) return;
if (command->failed ()) RK_DEBUG (PHP, DL_ERROR, "Plugin script R command %s failed. Full output wsa %s", qPrintable (command->command ()), qPrintable (command->fullOutput ()));
- QScriptValueList args;
+ RKJSValueList args;
args.append (marshall (&engine, command));
- args.append (QScriptValue (command->id ()));
- QScriptValue callback_obj = engine.globalObject ().property (callback);
- callback_obj.call (engine.globalObject (), args);
- handleScriptError ();
+ args.append (RKJSValue (command->id ()));
+ RKJSValue callback_obj = engine.globalObject ().property (callback);
+#ifdef USE_QJSENGINE
+ auto res = callback_obj.call(args);
+#else
+ auto res = callback_obj.call(engine.globalObject(), args);
+#endif
+ handleScriptError(res);
}
void RKComponentScriptingProxy::componentChanged (RKComponent* changed) {
diff --git a/rkward/scriptbackends/rkcomponentscripting.h b/rkward/scriptbackends/rkcomponentscripting.h
index 803e7dda..0ebe83a3 100644
--- a/rkward/scriptbackends/rkcomponentscripting.h
+++ b/rkward/scriptbackends/rkcomponentscripting.h
@@ -10,7 +10,8 @@ SPDX-License-Identifier: GPL-2.0-or-later
#include <QObject>
#include <QHash>
-#include <QScriptEngine>
+
+#include "qtscriptbackend.h" // TODO: For RKJSEngine, only
#include "../rbackend/rcommand.h"
@@ -57,7 +58,7 @@ private slots:
void scriptRCommandFinished (RCommand* command);
private:
RKComponent* component;
- QScriptEngine engine;
+ RKJSEngine engine;
struct OutstandingCommand {
RCommand *command;
QString callback;
@@ -69,7 +70,7 @@ private:
void handleChange (RKComponentBase* changed);
QHash<RKComponentBase*, QString> component_commands;
- void handleScriptError (const QString& current_file=QString ());
+ void handleScriptError (const RKJSValue &val, const QString& current_file=QString ());
};
#endif
More information about the rkward-tracker
mailing list