diff --git a/interface/src/raypick/ParabolaPick.cpp b/interface/src/raypick/ParabolaPick.cpp index 081c68def2..378a46b96b 100644 --- a/interface/src/raypick/ParabolaPick.cpp +++ b/interface/src/raypick/ParabolaPick.cpp @@ -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; diff --git a/interface/src/raypick/PointerScriptingInterface.cpp b/interface/src/raypick/PointerScriptingInterface.cpp index 17a1c7e184..6ea3272572 100644 --- a/interface/src/raypick/PointerScriptingInterface.cpp +++ b/interface/src/raypick/PointerScriptingInterface.cpp @@ -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); } } diff --git a/libraries/animation/src/AnimVariant.cpp b/libraries/animation/src/AnimVariant.cpp index 3d38252407..872051bcf0 100644 --- a/libraries/animation/src/AnimVariant.cpp +++ b/libraries/animation/src/AnimVariant.cpp @@ -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); } } diff --git a/libraries/script-engine/src/ScriptEngineCast.h b/libraries/script-engine/src/ScriptEngineCast.h index ea8a9de312..29746b84b7 100644 --- a/libraries/script-engine/src/ScriptEngineCast.h +++ b/libraries/script-engine/src/ScriptEngineCast.h @@ -44,7 +44,6 @@ inline ScriptValue scriptValueFromValue(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 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(const ScriptValue& value) { return value.toVariant(); } -//#define MARSHAL_MACRO(FUNCTION, TYPE) +[FUNCTION](ScriptEngine* engine, const void* p) -> ScriptValue { FUNCTION(engine, *(static_cast(p)) ); } - template ScriptValue toScriptValueWrapper(ScriptEngine* engine, const void *p) { Q_ASSERT(p != NULL); diff --git a/libraries/script-engine/src/v8/FastScriptValueUtils.cpp b/libraries/script-engine/src/v8/FastScriptValueUtils.cpp index 07cae34688..66c5fce2e4 100644 --- a/libraries/script-engine/src/v8/FastScriptValueUtils.cpp +++ b/libraries/script-engine/src/v8/FastScriptValueUtils.cpp @@ -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; } diff --git a/libraries/script-engine/src/v8/ScriptContextV8Wrapper.cpp b/libraries/script-engine/src/v8/ScriptContextV8Wrapper.cpp index 0a2bdf15bb..2b92a9ae8a 100644 --- a/libraries/script-engine/src/v8/ScriptContextV8Wrapper.cpp +++ b/libraries/script-engine/src/v8/ScriptContextV8Wrapper.cpp @@ -57,7 +57,6 @@ v8::Local 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 stackTrace = v8::StackTrace::CurrentStackTrace(isolate, 40); QStringList backTrace; - //V8TODO nicer formatting for (int i = 0; i < stackTrace->GetFrameCount(); i++) { v8::Local 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 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 stackTrace = v8::StackTrace::CurrentStackTrace(_engine->getIsolate(), 1); v8::Local stackFrame = stackTrace->GetFrame(_engine->getIsolate(), 0); v8::Local 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); diff --git a/libraries/script-engine/src/v8/ScriptEngineV8.cpp b/libraries/script-engine/src/v8/ScriptEngineV8.cpp index c39995af41..3c7e3fb49f 100644 --- a/libraries/script-engine/src/v8/ScriptEngineV8.cpp +++ b/libraries/script-engine/src/v8/ScriptEngineV8.cpp @@ -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 diff --git a/libraries/script-engine/src/v8/ScriptEngineV8.h b/libraries/script-engine/src/v8/ScriptEngineV8.h index 147b76f549..68907b14a8 100644 --- a/libraries/script-engine/src/v8/ScriptEngineV8.h +++ b/libraries/script-engine/src/v8/ScriptEngineV8.h @@ -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 context); void popContext(); - // V8TODO: call this after initializing global object void storeGlobalObjectContents(); #ifdef OVERTE_V8_MEMORY_DEBUG void incrementScriptValueCounter() { scriptValueCount++; }; diff --git a/libraries/script-engine/src/v8/ScriptEngineV8_cast.cpp b/libraries/script-engine/src/v8/ScriptEngineV8_cast.cpp index 8b96fb2cbf..8dead23ddb 100644 --- a/libraries/script-engine/src/v8/ScriptEngineV8_cast.cpp +++ b/libraries/script-engine/src/v8/ScriptEngineV8_cast.cpp @@ -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"; diff --git a/libraries/script-engine/src/v8/ScriptObjectV8Proxy.cpp b/libraries/script-engine/src/v8/ScriptObjectV8Proxy.cpp index 8a727559f2..e2fd9dfaa6 100644 --- a/libraries/script-engine/src/v8/ScriptObjectV8Proxy.cpp +++ b/libraries/script-engine/src/v8/ScriptObjectV8Proxy.cpp @@ -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; diff --git a/libraries/script-engine/src/v8/ScriptObjectV8Proxy.h b/libraries/script-engine/src/v8/ScriptObjectV8Proxy.h index a1241a54f8..eb1e51ccdc 100644 --- a/libraries/script-engine/src/v8/ScriptObjectV8Proxy.h +++ b/libraries/script-engine/src/v8/ScriptObjectV8Proxy.h @@ -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 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); diff --git a/libraries/shared/src/Transform.h b/libraries/shared/src/Transform.h index 3da436bfc8..c0c0a7ed94 100644 --- a/libraries/shared/src/Transform.h +++ b/libraries/shared/src/Transform.h @@ -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; }