CR feedback / cleanup

libraries/graphics-scripting/src/graphics-scripting/BufferViewScripting.cpp

@@ -11,15 +11,6 @@
 
 #include <graphics/BufferViewHelpers.h>
 
-#ifdef DEBUG_BUFFERVIEW_SCRIPTING
-    #include "DebugNames.h"
-#endif
-
-namespace {
-    const std::array<const char*, 4> XYZW = {{ "x", "y", "z", "w" }};
-    const std::array<const char*, 4> ZERO123 = {{ "0", "1", "2", "3" }};
-}
-
 template <typename T>
 QScriptValue getBufferViewElement(QScriptEngine* js, const gpu::BufferView& view, quint32 index, bool asArray = false) {
     return glmVecToScriptValue(js, view.get<T>(index), asArray);
@@ -45,7 +36,7 @@ bool bufferViewElementFromScriptValue(const QScriptValue& v, const gpu::BufferVi
 template <typename T>
 QScriptValue glmVecToScriptValue(QScriptEngine *js, const T& v, bool asArray) {
     static const auto len = T().length();
-    const auto& components = asArray ? ZERO123 : XYZW;
+    const auto& components = asArray ? buffer_helpers::ZERO123 : buffer_helpers::XYZW;
     auto obj = asArray ? js->newArray() : js->newObject();
     for (int i = 0; i < len ; i++) {
         const auto key = components[i];
@@ -65,7 +56,7 @@ QScriptValue glmVecToScriptValue(QScriptEngine *js, const T& v, bool asArray) {
 template <typename T>
 const T glmVecFromScriptValue(const QScriptValue& v) {
     static const auto len = T().length();
-    const auto& components = v.property("x").isValid() ? XYZW : ZERO123;
+    const auto& components = v.property("x").isValid() ? buffer_helpers::XYZW : buffer_helpers::ZERO123;
     T result;
     for (int i = 0; i < len ; i++) {
         const auto key = components[i];

libraries/graphics-scripting/src/graphics-scripting/DebugNames.h

@@ -1,72 +0,0 @@
-#pragma once
-#include <QtCore>
-#include <QVariant>
-#include <QObject>
-#include <gpu/Format.h>
-#include <gpu/Stream.h>
-//#include <gpu/Buffer.h>
-
-Q_DECLARE_METATYPE(gpu::Type);
-#ifdef QT_MOC_RUN
-class DebugNames {
-        Q_OBJECT
-public:
-#else
-  namespace DebugNames {
-    Q_NAMESPACE
-        #endif
-    
-enum Type : uint8_t {
-
-    FLOAT = 0,
-    INT32,
-    UINT32,
-    HALF,
-    INT16,
-    UINT16,
-    INT8,
-    UINT8,
-
-    NINT32,
-    NUINT32,
-    NINT16,
-    NUINT16,
-    NINT8,
-    NUINT8,
-
-    COMPRESSED,
-
-    NUM_TYPES,
-
-    BOOL = UINT8,
-    NORMALIZED_START = NINT32,
-};
-    
-    Q_ENUM_NS(Type)
-    enum InputSlot {
-        POSITION = 0,
-        NORMAL = 1,
-        COLOR = 2,
-        TEXCOORD0 = 3,
-        TEXCOORD = TEXCOORD0,
-        TANGENT = 4,
-        SKIN_CLUSTER_INDEX = 5,
-        SKIN_CLUSTER_WEIGHT = 6,
-        TEXCOORD1 = 7,
-        TEXCOORD2 = 8,
-        TEXCOORD3 = 9,
-        TEXCOORD4 = 10,
-
-        NUM_INPUT_SLOTS,
-
-        DRAW_CALL_INFO = 15, // Reserve last input slot for draw call infos
-    };
-
-    Q_ENUM_NS(InputSlot)
-    inline QString stringFrom(Type t) { return QVariant::fromValue(t).toString(); }
-    inline QString stringFrom(InputSlot t) { return QVariant::fromValue(t).toString(); }
-    inline QString stringFrom(gpu::Type t) { return stringFrom((Type)t); }
-    inline QString stringFrom(gpu::Stream::Slot t) { return stringFrom((InputSlot)t); }
-
-    extern const QMetaObject staticMetaObject;
-  };

libraries/graphics-scripting/src/graphics-scripting/GraphicsScriptingInterface.cpp

@@ -11,7 +11,6 @@
 #include "GraphicsScriptingInterface.h"
 #include "BaseScriptEngine.h"
 #include "BufferViewScripting.h"
-#include "DebugNames.h"
 #include "GraphicsScriptingUtil.h"
 #include "OBJWriter.h"
 #include "RegisteredMetaTypes.h"

libraries/graphics-scripting/src/graphics-scripting/GraphicsScriptingInterface.h

@@ -24,6 +24,7 @@ class GraphicsScriptingInterface : public QObject, public QScriptable, public De
     Q_OBJECT
 
 public:
+    static void registerMetaTypes(QScriptEngine* engine);
     GraphicsScriptingInterface(QObject* parent = nullptr);
 
 public slots:
@@ -39,8 +40,6 @@ public slots:
 
     QString meshToOBJ(const scriptable::ScriptableModel& in);
 
-    static void registerMetaTypes(QScriptEngine* engine);
-
 private:
     scriptable::MeshPointer getMeshPointer(scriptable::ScriptableMeshPointer meshProxy);
     scriptable::MeshPointer getMeshPointer(scriptable::ScriptableMesh& meshProxy);

libraries/graphics-scripting/src/graphics-scripting/ScriptableMesh.cpp

@@ -8,7 +8,6 @@
 #include "ScriptableMesh.h"
 
 #include "BufferViewScripting.h"
-#include "DebugNames.h"
 #include "GraphicsScriptingUtil.h"
 #include "OBJWriter.h"
 #include <BaseScriptEngine.h>
@@ -508,14 +507,11 @@ scriptable::ScriptableMeshPointer scriptable::ScriptableMesh::cloneMesh(bool rec
         qCInfo(graphics_scripting) << "ScriptableMesh::cloneMesh -- !meshPointer";
         return nullptr;
     }
-    // qCInfo(graphics_scripting) << "ScriptableMesh::cloneMesh...";
     auto clone = buffer_helpers::cloneMesh(mesh);
     
-    // qCInfo(graphics_scripting) << "ScriptableMesh::cloneMesh...";
     if (recalcNormals) {
         buffer_helpers::recalculateNormals(clone);
     }
-    //qCDebug(graphics_scripting) << clone.get();// << metadata;
     auto meshPointer = scriptable::make_scriptowned<scriptable::ScriptableMesh>(provider, model, clone, metadata);
     clone.reset(); // free local reference
     // qCInfo(graphics_scripting) << "========= ScriptableMesh::cloneMesh..." << meshPointer << meshPointer->ownedMesh.use_count();
@@ -549,7 +545,6 @@ scriptable::ScriptableMeshBase::ScriptableMeshBase(scriptable::MeshPointer mesh,
     : ScriptableMeshBase(WeakModelProviderPointer(), nullptr, mesh, metadata) {
     ownedMesh = mesh;
 }
-//scriptable::ScriptableMeshBase::ScriptableMeshBase(const scriptable::ScriptableMeshBase& other) { *this = other; }
 scriptable::ScriptableMeshBase& scriptable::ScriptableMeshBase::operator=(const scriptable::ScriptableMeshBase& view) {
     provider = view.provider;
     model = view.model;
@@ -617,22 +612,6 @@ namespace {
     void meshPartPointerFromScriptValue(const QScriptValue& value, scriptable::ScriptableMeshPartPointer &out) {
         out = scriptable::qpointer_qobject_cast<scriptable::ScriptableMeshPart>(value);
     }
-    
-    // FIXME: MESHFACES:
-    // QScriptValue meshFaceToScriptValue(QScriptEngine* engine, const mesh::MeshFace &meshFace) {
-    //     QScriptValue obj = engine->newObject();
-    //     obj.setProperty("vertices", qVectorIntToScriptValue(engine, meshFace.vertexIndices));
-    //     return obj;
-    // }
-    // void meshFaceFromScriptValue(const QScriptValue &object, mesh::MeshFace& meshFaceResult) {
-    //     qScriptValueToSequence(object.property("vertices"), meshFaceResult.vertexIndices);
-    // }
-    // QScriptValue qVectorMeshFaceToScriptValue(QScriptEngine* engine, const QVector<mesh::MeshFace>& vector) {
-    //     return qScriptValueFromSequence(engine, vector);
-    // }
-    // void qVectorMeshFaceFromScriptValue(const QScriptValue& array, QVector<mesh::MeshFace>& result) {
-    //     qScriptValueToSequence(array, result);
-    // }
 
     QScriptValue qVectorUInt32ToScriptValue(QScriptEngine* engine, const QVector<scriptable::uint32>& vector) {
         return qScriptValueFromSequence(engine, vector);
@@ -700,11 +679,15 @@ bool scriptable::GraphicsScriptingInterface::updateMeshPart(ScriptableMeshPointe
     Q_ASSERT(part->parentMesh);
     auto tmp = exportMeshPart(mesh, part->partIndex);
     if (part->parentMesh == mesh) {
+#ifdef SCRIPTABLE_MESH_DEBUG
         qCInfo(graphics_scripting) << "updateMeshPart -- update via clone" << mesh << part;
+#endif
         tmp->replaceMeshData(part->cloneMeshPart());
         return false;
     } else {
+#ifdef SCRIPTABLE_MESH_DEBUG
         qCInfo(graphics_scripting) << "updateMeshPart -- update via inplace" << mesh << part;
+#endif
         tmp->replaceMeshData(part);
         return true;
     }

libraries/graphics-scripting/src/graphics-scripting/ScriptableMesh.h

@@ -16,6 +16,8 @@
 #include <QtScript/QScriptValue>
 #include <QtScript/QScriptable>
 
+#include "GraphicsScriptingUtil.h"
+
 namespace scriptable {
     class ScriptableMesh : public ScriptableMeshBase, QScriptable {
         Q_OBJECT

libraries/graphics-scripting/src/graphics-scripting/ScriptableModel.cpp

@@ -41,34 +41,29 @@ scriptable::ScriptableModelBase::~ScriptableModelBase() {
 #ifdef SCRIPTABLE_MESH_DEBUG
     qCDebug(graphics_scripting) << "~ScriptableModelBase" << this;
 #endif
+    // makes cleanup order more deterministic to help with debugging
     for (auto& m : meshes) {
         m.ownedMesh.reset();
-        //qCDebug(graphics_scripting) << "~~~~ScriptableModelBase" << &m << m.mesh.use_count();
     }
     meshes.clear();
-    //qCDebug(graphics_scripting) << "//~ScriptableModelBase" << this;
 }
 
 void scriptable::ScriptableModelBase::append(scriptable::WeakMeshPointer mesh, const QVariantMap& metadata) {
-    //qCDebug(graphics_scripting) << "+ APPEND WeakMeshPointer" << mesh.lock().get();
     meshes << ScriptableMeshBase{ provider, this, mesh, metadata };
 }
 
 void scriptable::ScriptableModelBase::append(const ScriptableMeshBase& mesh, const QVariantMap& modelMetaData) {
-    //qCDebug(graphics_scripting) << "+ APPEND ScriptableMeshBase" << &mesh;
     if (mesh.provider.lock().get() != provider.lock().get()) {
         qCDebug(graphics_scripting) << "warning: appending mesh from different provider..." << mesh.provider.lock().get() << " != " << provider.lock().get();
     }
-    //if (mesh.model && mesh.model != this) {
-    //    qCDebug(graphics_scripting) << "warning: appending mesh from different model..." << mesh.model << " != " << this;
-    //}                    
     meshes << mesh;
     mixin(modelMetaData);
 }
 
 void scriptable::ScriptableModelBase::append(const ScriptableModelBase& other, const QVariantMap& modelMetaData) {
-    //qCDebug(graphics_scripting) << "+ APPEND ScriptableModelBase" << &other;
+    for (const auto& mesh : other.meshes) {
-    for (const auto& mesh : other.meshes) { append(mesh); }
+        append(mesh);
+    }
     mixin(other.metadata);
     mixin(modelMetaData);
 }
@@ -82,21 +77,16 @@ QString scriptable::ScriptableModel::toString() const {
 
 scriptable::ScriptableModelPointer scriptable::ScriptableModel::cloneModel(const QVariantMap& options) {
     scriptable::ScriptableModelPointer clone = scriptable::ScriptableModelPointer(new scriptable::ScriptableModel(*this));
-    qCDebug(graphics_scripting) << "clone->getNumMeshes" << clone->getNumMeshes();
     clone->meshes.clear();
-    qCDebug(graphics_scripting) << "..clone->getNumMeshes" << clone->getNumMeshes();
     for (const auto &mesh : getConstMeshes()) {
         auto cloned = mesh->cloneMesh(options.value("recalculateNormals").toBool());
         if (auto tmp = qobject_cast<scriptable::ScriptableMeshBase*>(cloned)) {
-            qCDebug(graphics_scripting) << "++ APPEND"  << tmp << tmp->ownedMesh.use_count() << tmp->metadata.value("__ownership__") << tmp->metadata.value("__native__");
             clone->meshes << *tmp;
-            tmp->deleteLater();
+            tmp->deleteLater(); // schedule our copy for cleanup
-            qCDebug(graphics_scripting) << "//++ APPEND"  << clone->meshes.constLast().ownedMesh.use_count();;
         } else {
             qCDebug(graphics_scripting) << "error cloning mesh" << cloned;
         }
     }
-    qCDebug(graphics_scripting) << "//clone->getNumMeshes" << clone->getNumMeshes();
     return clone;
 }
 

libraries/graphics-scripting/src/graphics-scripting/ScriptableModel.h

@@ -1,6 +1,7 @@
 #pragma once
 
 #include "Forward.h"
+#include "GraphicsScriptingUtil.h"
 
 class QScriptValue;
 namespace scriptable {

libraries/graphics/src/graphics/BufferViewHelpers.cpp

@@ -34,10 +34,12 @@ namespace glm {
 
 namespace {
     QLoggingCategory bufferhelper_logging{ "hifi.bufferview" };
-    const std::array<const char*, 4> XYZW = { { "x", "y", "z", "w" } };
-    const std::array<const char*, 4> ZERO123 = { { "0", "1", "2", "3" } };
 }
 
+
+const std::array<const char*, 4> buffer_helpers::XYZW = { { "x", "y", "z", "w" } };
+const std::array<const char*, 4> buffer_helpers::ZERO123 = { { "0", "1", "2", "3" } };
+
 gpu::BufferView buffer_helpers::getBufferView(graphics::MeshPointer mesh, gpu::Stream::Slot slot) {
     return slot == gpu::Stream::POSITION ? mesh->getVertexBuffer() : mesh->getAttributeBuffer(slot);
 }

libraries/graphics/src/graphics/BufferViewHelpers.h

@@ -49,4 +49,7 @@ struct buffer_helpers {
     static gpu::BufferView resize(const gpu::BufferView& input, quint32 numElements);
 
     static void packNormalAndTangent(glm::vec3 normal, glm::vec3 tangent, glm::uint32& packedNormal, glm::uint32& packedTangent);
+
+    static const std::array<const char*, 4> XYZW;
+    static const std::array<const char*, 4> ZERO123;
 };