From 0dd367216278a5bf640eca89774ddf1043c62b57 Mon Sep 17 00:00:00 2001 From: humbletim Date: Thu, 22 Feb 2018 21:20:56 -0500 Subject: [PATCH] CR feedback / cleanup --- .../BufferViewScripting.cpp | 13 +--- .../src/graphics-scripting/DebugNames.h | 72 ------------------- .../GraphicsScriptingInterface.cpp | 1 - .../GraphicsScriptingInterface.h | 3 +- .../src/graphics-scripting/ScriptableMesh.cpp | 25 ++----- .../src/graphics-scripting/ScriptableMesh.h | 2 + .../graphics-scripting/ScriptableModel.cpp | 20 ++---- .../src/graphics-scripting/ScriptableModel.h | 1 + .../src/graphics/BufferViewHelpers.cpp | 6 +- .../graphics/src/graphics/BufferViewHelpers.h | 3 + 10 files changed, 22 insertions(+), 124 deletions(-) delete mode 100644 libraries/graphics-scripting/src/graphics-scripting/DebugNames.h diff --git a/libraries/graphics-scripting/src/graphics-scripting/BufferViewScripting.cpp b/libraries/graphics-scripting/src/graphics-scripting/BufferViewScripting.cpp index 31cf5ff8f3..9d7a0e1f30 100644 --- a/libraries/graphics-scripting/src/graphics-scripting/BufferViewScripting.cpp +++ b/libraries/graphics-scripting/src/graphics-scripting/BufferViewScripting.cpp @@ -11,15 +11,6 @@ #include -#ifdef DEBUG_BUFFERVIEW_SCRIPTING - #include "DebugNames.h" -#endif - -namespace { - const std::array XYZW = {{ "x", "y", "z", "w" }}; - const std::array ZERO123 = {{ "0", "1", "2", "3" }}; -} - template QScriptValue getBufferViewElement(QScriptEngine* js, const gpu::BufferView& view, quint32 index, bool asArray = false) { return glmVecToScriptValue(js, view.get(index), asArray); @@ -45,7 +36,7 @@ bool bufferViewElementFromScriptValue(const QScriptValue& v, const gpu::BufferVi template 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 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]; diff --git a/libraries/graphics-scripting/src/graphics-scripting/DebugNames.h b/libraries/graphics-scripting/src/graphics-scripting/DebugNames.h deleted file mode 100644 index e5edf1c9d8..0000000000 --- a/libraries/graphics-scripting/src/graphics-scripting/DebugNames.h +++ /dev/null @@ -1,72 +0,0 @@ -#pragma once -#include -#include -#include -#include -#include -//#include - -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; - }; diff --git a/libraries/graphics-scripting/src/graphics-scripting/GraphicsScriptingInterface.cpp b/libraries/graphics-scripting/src/graphics-scripting/GraphicsScriptingInterface.cpp index 28d5d0edfc..01e68c5328 100644 --- a/libraries/graphics-scripting/src/graphics-scripting/GraphicsScriptingInterface.cpp +++ b/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" diff --git a/libraries/graphics-scripting/src/graphics-scripting/GraphicsScriptingInterface.h b/libraries/graphics-scripting/src/graphics-scripting/GraphicsScriptingInterface.h index 6afa549a19..ab2e5467db 100644 --- a/libraries/graphics-scripting/src/graphics-scripting/GraphicsScriptingInterface.h +++ b/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); diff --git a/libraries/graphics-scripting/src/graphics-scripting/ScriptableMesh.cpp b/libraries/graphics-scripting/src/graphics-scripting/ScriptableMesh.cpp index 3678c54b7b..643debf475 100644 --- a/libraries/graphics-scripting/src/graphics-scripting/ScriptableMesh.cpp +++ b/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 @@ -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(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(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& vector) { - // return qScriptValueFromSequence(engine, vector); - // } - // void qVectorMeshFaceFromScriptValue(const QScriptValue& array, QVector& result) { - // qScriptValueToSequence(array, result); - // } QScriptValue qVectorUInt32ToScriptValue(QScriptEngine* engine, const QVector& 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; } diff --git a/libraries/graphics-scripting/src/graphics-scripting/ScriptableMesh.h b/libraries/graphics-scripting/src/graphics-scripting/ScriptableMesh.h index 2cba33edde..459613135a 100644 --- a/libraries/graphics-scripting/src/graphics-scripting/ScriptableMesh.h +++ b/libraries/graphics-scripting/src/graphics-scripting/ScriptableMesh.h @@ -16,6 +16,8 @@ #include #include +#include "GraphicsScriptingUtil.h" + namespace scriptable { class ScriptableMesh : public ScriptableMeshBase, QScriptable { Q_OBJECT diff --git a/libraries/graphics-scripting/src/graphics-scripting/ScriptableModel.cpp b/libraries/graphics-scripting/src/graphics-scripting/ScriptableModel.cpp index cc882aa7bb..229d56adab 100644 --- a/libraries/graphics-scripting/src/graphics-scripting/ScriptableModel.cpp +++ b/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) { append(mesh); } + for (const auto& mesh : other.meshes) { + 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(cloned)) { - qCDebug(graphics_scripting) << "++ APPEND" << tmp << tmp->ownedMesh.use_count() << tmp->metadata.value("__ownership__") << tmp->metadata.value("__native__"); clone->meshes << *tmp; - tmp->deleteLater(); - qCDebug(graphics_scripting) << "//++ APPEND" << clone->meshes.constLast().ownedMesh.use_count();; + tmp->deleteLater(); // schedule our copy for cleanup } else { qCDebug(graphics_scripting) << "error cloning mesh" << cloned; } } - qCDebug(graphics_scripting) << "//clone->getNumMeshes" << clone->getNumMeshes(); return clone; } diff --git a/libraries/graphics-scripting/src/graphics-scripting/ScriptableModel.h b/libraries/graphics-scripting/src/graphics-scripting/ScriptableModel.h index bdfaac5d7c..a78c9a4ef5 100644 --- a/libraries/graphics-scripting/src/graphics-scripting/ScriptableModel.h +++ b/libraries/graphics-scripting/src/graphics-scripting/ScriptableModel.h @@ -1,6 +1,7 @@ #pragma once #include "Forward.h" +#include "GraphicsScriptingUtil.h" class QScriptValue; namespace scriptable { diff --git a/libraries/graphics/src/graphics/BufferViewHelpers.cpp b/libraries/graphics/src/graphics/BufferViewHelpers.cpp index a15ce1b998..ddf3d84c89 100644 --- a/libraries/graphics/src/graphics/BufferViewHelpers.cpp +++ b/libraries/graphics/src/graphics/BufferViewHelpers.cpp @@ -34,10 +34,12 @@ namespace glm { namespace { QLoggingCategory bufferhelper_logging{ "hifi.bufferview" }; - const std::array XYZW = { { "x", "y", "z", "w" } }; - const std::array ZERO123 = { { "0", "1", "2", "3" } }; } + +const std::array buffer_helpers::XYZW = { { "x", "y", "z", "w" } }; +const std::array 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); } diff --git a/libraries/graphics/src/graphics/BufferViewHelpers.h b/libraries/graphics/src/graphics/BufferViewHelpers.h index 53fddfe581..6d4908a2c7 100644 --- a/libraries/graphics/src/graphics/BufferViewHelpers.h +++ b/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 XYZW; + static const std::array ZERO123; };