Mirrors/overte-thingvellir
3
0
Fork 0
mirror of https://thingvellir.net/git/overte synced 2025-03-27 23:52:03 +01:00
Code Issues Projects Releases Packages Wiki Activity Actions

V8 related cleanup

Browse source
This commit is contained in:
ksuprynowicz 2023-05-27 22:06:57 +02:00
parent b3997bff54
commit 6192416dce
12 changed files with 11 additions and 44 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
interface/src/raypick/ParabolaPick.cpp
View file

@ -23,8 +23,6 @@ ParabolaPick::ParabolaPick(const glm::vec3& position, const glm::vec3& direction
_speed(speed) {
}
//V8TODO: needs to be fixed due to 3D overlay support removal
PickParabola ParabolaPick::getMathematicalPick() const {
if (!parentTransform) {
PickParabola mathPick = _mathPick;

3
interface/src/raypick/PointerScriptingInterface.cpp
View file

@ -630,7 +630,6 @@ ScriptValue stylusPointerPropertiesToScriptValue(ScriptEngine* engine, const Sty
return engine->newVariant(QVariant(in.properties));
}
//V8TODO: adapt render states to what parabola expects
bool stylusPointerPropertiesFromScriptValue(const ScriptValue& value, StylusPointerProperties& out) {
// This copies properties from script value, but also converts entity properties of entities used in render states
// from JS objects into EntityItemProperties
@ -670,7 +669,6 @@ bool stylusPointerPropertiesFromScriptValue(const ScriptValue& value, StylusPoin
stateMap.insert("endPropertyIndex", QVariant(out.entityProperties.length()));
out.entityProperties.append(endProperties);
}
// V8TODO: Check if path is a polyline and if values are valid
renderStates[i].setValue(stateMap);
}
}
@ -727,7 +725,6 @@ bool parabolaPointerPropertiesFromScriptValue(const ScriptValue& value, Parabola
out.entityProperties.append(endProperties);
qDebug() << "parabolaPointerPropertiesFromScriptValue : added end entity";
}
// V8TODO: Check if path is a polyline and if values are valid
renderStates[i].setValue(stateMap);
}
}

1
libraries/animation/src/AnimVariant.cpp
View file

@ -124,7 +124,6 @@ void AnimVariantMap::animVariantMapFromScriptValue(const ScriptValue& source) {
}
}
qCWarning(animation) << "Ignoring unrecognized data " << value.toString() << " for animation property " << property->name();
// V8TODO this was spamming too much logs but needs to be fixed later
//Q_ASSERT(false);
}
}

4
libraries/script-engine/src/ScriptEngineCast.h
View file

@ -44,7 +44,6 @@ inline ScriptValue scriptValueFromValue<QVariant>(ScriptEngine* engine, const QV
return engine->create(v.userType(), v.data());
}
// V8TODO: check if it's typesafe
// V8TODO: run through debugger for AnimationPointer/AnimationObject
template <typename T>
inline T scriptvalue_cast(const ScriptValue& value) {
@ -64,14 +63,11 @@ inline T scriptvalue_cast(const ScriptValue& value) {
return T();
}
// V8TODO: check if it's typesafe
template <>
inline QVariant scriptvalue_cast<QVariant>(const ScriptValue& value) {
return value.toVariant();
}
//#define MARSHAL_MACRO(FUNCTION, TYPE) +[FUNCTION](ScriptEngine* engine, const void* p) -> ScriptValue { FUNCTION(engine, *(static_cast<const TYPE*>(p)) ); }
template <typename T, ScriptValue (*f)(ScriptEngine*, const T&)>
ScriptValue toScriptValueWrapper(ScriptEngine* engine, const void *p) {
Q_ASSERT(p != NULL);

1
libraries/script-engine/src/v8/FastScriptValueUtils.cpp
View file

@ -136,7 +136,6 @@ bool vec3FromScriptValue(const ScriptValue& object, glm::vec3& vec3) {
if (!array->Get(context, 2).ToLocal(&zValue)) {
return false;
}
//V8TODO:
if (xValue->IsNullOrUndefined() || yValue->IsNullOrUndefined() || zValue->IsNullOrUndefined()) {
return false;
}

24
libraries/script-engine/src/v8/ScriptContextV8Wrapper.cpp
View file

@ -57,7 +57,6 @@ v8::Local<v8::Context> ScriptContextV8Wrapper::toV8Value() const {
}
int ScriptContextV8Wrapper::argumentCount() const {
// V8TODO
auto isolate = _engine->getIsolate();
v8::Locker locker(isolate);
v8::Isolate::Scope isolateScope(isolate);
@ -70,9 +69,6 @@ int ScriptContextV8Wrapper::argumentCount() const {
} else {
return Q_METAMETHOD_INVOKE_MAX_ARGS;
}
// This was wrong, in function registration it seems to be used as maximum number od arguments instead?
// Is it also used for something else?
//return _functionCallbackInfo->Length();
}
ScriptValue ScriptContextV8Wrapper::argument(int index) const {
@ -103,7 +99,6 @@ QStringList ScriptContextV8Wrapper::backtrace() const {
v8::Context::Scope contextScope(_context.Get(isolate));
v8::Local<v8::StackTrace> stackTrace = v8::StackTrace::CurrentStackTrace(isolate, 40);
QStringList backTrace;
//V8TODO nicer formatting
for (int i = 0; i < stackTrace->GetFrameCount(); i++) {
v8::Local<v8::StackFrame> stackFrame = stackTrace->GetFrame(isolate, i);
backTrace.append(QString(*v8::String::Utf8Value(isolate, stackFrame->GetScriptNameOrSourceURL())) +
@ -166,7 +161,7 @@ ScriptValue ScriptContextV8Wrapper::thisObject() const {
ScriptValue ScriptContextV8Wrapper::throwError(const QString& text) {
auto isolate = _engine->getIsolate();
// V8TODO: I have no idea how to do this yet because it happens on another thread
// V8TODO: I have no idea how to do this yet when it happens on another thread
if(isolate->IsCurrent()) {
v8::Locker locker(isolate);
v8::Isolate::Scope isolateScope(isolate);
@ -178,7 +173,6 @@ ScriptValue ScriptContextV8Wrapper::throwError(const QString& text) {
return ScriptValue(new ScriptValueV8Wrapper(_engine, std::move(result)));
} else {
qCWarning(scriptengine_v8) << "throwError on a different thread not implemented yet, error value: " << text;
//return _engine->undefinedValue();
return ScriptValue();
}
}
@ -213,33 +207,24 @@ ScriptFunctionContextV8Wrapper::~ScriptFunctionContextV8Wrapper() {
}
QString ScriptFunctionContextV8Wrapper::fileName() const {
//V8TODO: It's not exactly like in QtScript, because there's no such context object in V8, let's return the current one for now
//Maybe fetch data on creation or store stack frame?
// It's not exactly like in QtScript, because there's no such context object in V8, let's return the current one for now
auto isolate = _engine->getIsolate();
v8::Locker locker(isolate);
v8::Isolate::Scope isolateScope(isolate);
v8::HandleScope handleScope(isolate);
v8::Context::Scope contextScope(_context.Get(isolate));
// V8TODO: does this work for context selected with contextScope?
v8::Local<v8::String> name = v8::StackTrace::CurrentScriptNameOrSourceURL(_engine->getIsolate());
/*auto stackTrace = v8::StackTrace::CurrentStackTrace(isolate, 1);
if (stackTrace.IsEmpty() || stackTrace->GetFrameCount() == 0) {
return "";
}
auto name = stackTrace->GetFrame(isolate, 0)->GetScriptNameOrSourceURL();*/
v8::String::Utf8Value nameUTF(_engine->getIsolate(), name);
return QString(*nameUTF);
}
QString ScriptFunctionContextV8Wrapper::functionName() const {
//V8TODO: It's not exactly like in QtScript, because there's no such context object in V8, let's return the current one for now
//Maybe fetch data on creation?
// It's not exactly like in QtScript, because there's no such context object in V8, let's return the current one for now
auto isolate = _engine->getIsolate();
v8::Locker locker(isolate);
v8::Isolate::Scope isolateScope(isolate);
v8::HandleScope handleScope(isolate);
v8::Context::Scope contextScope(_context.Get(isolate));
// V8TODO: does this work for context selected with contextScope?
v8::Local<v8::StackTrace> stackTrace = v8::StackTrace::CurrentStackTrace(_engine->getIsolate(), 1);
v8::Local<v8::StackFrame> stackFrame = stackTrace->GetFrame(_engine->getIsolate(), 0);
v8::Local<v8::String> name = stackFrame->GetFunctionName();
@ -254,8 +239,7 @@ ScriptFunctionContext::FunctionType ScriptFunctionContextV8Wrapper::functionType
}
int ScriptFunctionContextV8Wrapper::lineNumber() const {
//V8TODO: It's not exactly like in QtScript, because there's no such context object in V8, let's return the current one for now
//Maybe fetch data on creation?
// It's not exactly like in QtScript, because there's no such context object in V8, let's return the current one for now
auto isolate = _engine->getIsolate();
v8::Locker locker(isolate);
v8::Isolate::Scope isolateScope(isolate);

2
libraries/script-engine/src/v8/ScriptEngineV8.cpp
View file

@ -188,7 +188,7 @@ ScriptEngineV8::ScriptEngineV8(ScriptManager *manager) : ScriptEngine(manager),
// Based on that, going with 256K for stacks for now. That seems like a reasonable value.
// We'll probably need a more complex system on the longer term, with configurable limits.
// Flags to try:
// V8TODO --single-threaded is to check if it fixes random crashes
// --single-threaded is to check if it fixes random crashes
// --jitless - might improve debugging performance due to no JIT?
// --assert-types

1
libraries/script-engine/src/v8/ScriptEngineV8.h
View file

@ -206,7 +206,6 @@ public: // not for public use, but I don't like how Qt strings this along with p
ScriptContextV8Pointer pushContext(v8::Local<v8::Context> context);
void popContext();
// V8TODO: call this after initializing global object
void storeGlobalObjectContents();
#ifdef OVERTE_V8_MEMORY_DEBUG
void incrementScriptValueCounter() { scriptValueCount++; };

9
libraries/script-engine/src/v8/ScriptEngineV8_cast.cpp
View file

@ -345,12 +345,10 @@ bool ScriptEngineV8::castValueToVariant(const V8ScriptValue& v8Val, QVariant& de
break;
}
if (val->IsBoolean()) {
//V8TODO is it right isolate? What if value from different script engine is used here
dest = QVariant::fromValue(val->ToBoolean(_v8Isolate)->Value());
break;
}
if (val->IsString()) {
//V8TODO is it right context? What if value from different script engine is used here
v8::String::Utf8Value string(_v8Isolate, val);
Q_ASSERT(*string != nullptr);
dest = QVariant::fromValue(QString(*string));
@ -389,8 +387,7 @@ bool ScriptEngineV8::castValueToVariant(const V8ScriptValue& v8Val, QVariant& de
+ "to variant. Destination type: " + QMetaType::typeName(destTypeId) +" details: "+ scriptValueDebugDetailsV8(v8Val);
qCDebug(scriptengine_v8) << errorMessage;
//Q_ASSERT(false);
// V8TODO
// V8TODO: this doesn't seem to be necessary anymore but I'm keeping it until all the API is tested
//dest = val->ToVariant();
break;
case QMetaType::Bool:
@ -402,7 +399,6 @@ bool ScriptEngineV8::castValueToVariant(const V8ScriptValue& v8Val, QVariant& de
double timeMs = v8::Date::Cast(*val)->NumberValue(context).ToChecked();
dest = QVariant::fromValue(QDateTime::fromMSecsSinceEpoch(timeMs));
} else if (val->IsNumber()) {
//V8TODO should we automatically cast numbers to datetime?
dest = QVariant::fromValue(QDateTime::fromMSecsSinceEpoch(val->ToNumber(context).ToLocalChecked()->Value()));
} else {
return false;
@ -448,7 +444,8 @@ bool ScriptEngineV8::castValueToVariant(const V8ScriptValue& v8Val, QVariant& de
break;
case QMetaType::QObjectStar:
{
// V8TODO: this is to diagnose a really weird segfault where it looks like only half of the QPointer is set to null upon object deletion
// V8TODO: This is to diagnose a really weird segfault where it looks like only half of the QPointer is set
// to null upon object deletion, it doesn't seem to happen anymore, but I'm keeping it for now
uint64_t ptrVal = (uint64_t)(ScriptObjectV8Proxy::unwrap(v8Val));
if ((uint32_t)(ptrVal) == 0 && ptrVal != 0) {
qCDebug(scriptengine_v8) << "ScriptEngineV8::castValueToVariant pointer bug happened";

1
libraries/script-engine/src/v8/ScriptObjectV8Proxy.cpp
View file

@ -173,7 +173,6 @@ ScriptObjectV8Proxy* ScriptObjectV8Proxy::unwrapProxy(v8::Isolate* isolate, v8::
v8::Locker locker(isolate);
v8::Isolate::Scope isolateScope(isolate);
v8::HandleScope handleScope(isolate);
//V8TODO: should there be context scope here?
if (value->IsNullOrUndefined()) {
return nullptr;

5
libraries/script-engine/src/v8/ScriptObjectV8Proxy.h
View file

@ -35,7 +35,7 @@ class ScriptSignalV8Proxy;
// V8TODO: Current implementation relies on weak handle callbacks for destroying objects on C++ side
// this is fine for avoiding memory leaks while script engine runs, but there's no guarantee that these will be called
// when script engine shutd down, so memory leaks may happen
// when script engine shuts down, so memory leaks may happen
// To avoid this handle visitor needs to be added (it's a feature of V8)
/// [V8] (re-)implements the translation layer between ScriptValue and QObject. This object
@ -106,7 +106,6 @@ public:
virtual V8ScriptValue property(const V8ScriptValue& object, const V8ScriptString& name, uint id);
virtual ScriptValue::PropertyFlags propertyFlags(const V8ScriptValue& object, const V8ScriptString& name, uint id);
//V8TODO
virtual QueryFlags queryProperty(const V8ScriptValue& object, const V8ScriptString& name, QueryFlags flags, uint* id);
virtual void setProperty(V8ScriptValue& object, const V8ScriptString& name, uint id, const V8ScriptValue& value);
v8::Local<v8::Array> getPropertyNames();
@ -151,7 +150,7 @@ private: // storage
new AnimationObject(), ScriptEngine::ScriptOwnership));
*
*/
// V8TODO: there may be memory leaks in these
// V8TODO: there may be memory leaks in these, it's worth checking if the proxy actually gets garbage-collected and destroyed
class ScriptVariantV8Proxy final {
public: // construction
ScriptVariantV8Proxy(ScriptEngineV8* engine, const QVariant& variant, V8ScriptValue scriptProto, ScriptObjectV8Proxy* proto);

2
libraries/shared/src/Transform.h
View file

@ -29,10 +29,10 @@ class QJsonValue;
inline bool isValidScale(glm::vec3 scale) {
bool result = scale.x != 0.0f && scale.y != 0.0f && scale.z != 0.0f;
// V8TODO: commented out for now
if(!result){
qWarning() << "Scale is equal to 0";
}
// V8TODO: commented out for now
// assert(result);
return result;
}