From e4a2a589a51ba243568f9553966870b150c28a85 Mon Sep 17 00:00:00 2001
From: humbletim <humbletim@gmail.com>
Date: Thu, 22 Feb 2018 07:56:09 -0500
Subject: [PATCH] CR fedback and cleanup

---
 interface/src/Application.cpp                 |   9 +-
 interface/src/ui/overlays/ModelOverlay.cpp    |   1 +
 .../src/RenderableModelEntityItem.cpp         |   6 -
 libraries/fbx/src/FBX.h                       |   3 +
 libraries/fbx/src/OBJWriter.cpp               |  21 +--
 libraries/graphics-scripting/CMakeLists.txt   |   1 -
 .../BufferViewScripting.cpp                   |   4 +-
 .../GraphicsScriptingInterface.cpp            |  29 ++--
 .../GraphicsScriptingUtil.h                   |  52 ++-----
 .../src/graphics-scripting/ScriptableMesh.cpp |  39 ++---
 .../src/graphics-scripting/ScriptableMesh.h   |  26 ++--
 .../src/graphics}/BufferViewHelpers.cpp       | 145 ++++++++++--------
 .../src/graphics}/BufferViewHelpers.h         |   6 +-
 libraries/render-utils/src/Model.cpp          |  22 +--
 .../src/Model_temporary_hack.cpp.h            |  16 +-
 15 files changed, 159 insertions(+), 221 deletions(-)
 rename libraries/{graphics-scripting/src/graphics-scripting => graphics/src/graphics}/BufferViewHelpers.cpp (87%)
 rename libraries/{graphics-scripting/src/graphics-scripting => graphics/src/graphics}/BufferViewHelpers.h (91%)

diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp
index 0a0c6e15d1..1be22d5087 100644
--- a/interface/src/Application.cpp
+++ b/interface/src/Application.cpp
@@ -607,7 +607,7 @@ void messageHandler(QtMsgType type, const QMessageLogContext& context, const QSt
 
 class ApplicationMeshProvider : public scriptable::ModelProviderFactory  {
 public:
-    virtual scriptable::ModelProviderPointer lookupModelProvider(QUuid uuid) {
+    virtual scriptable::ModelProviderPointer lookupModelProvider(const QUuid& uuid) {
         QString error;
 
         scriptable::ModelProviderPointer provider;
@@ -631,7 +631,7 @@ public:
         if (auto entity = entityTree->findEntityByID(entityID)) {
             if (auto renderer = entityTreeRenderer->renderableForEntityId(entityID)) {
                 provider = std::dynamic_pointer_cast<scriptable::ModelProvider>(renderer);
-                provider->metadata["providerType"] = "entity";
+                provider->modelProviderType = NestableType::Entity;
             } else {
                 qCWarning(interfaceapp) << "no renderer for entity ID" << entityID.toString();
             }
@@ -645,7 +645,7 @@ public:
         if (auto overlay = overlays.getOverlay(overlayID)) {
             if (auto base3d = std::dynamic_pointer_cast<Base3DOverlay>(overlay)) {
                 provider = std::dynamic_pointer_cast<scriptable::ModelProvider>(base3d);
-                provider->metadata["providerType"] = "overlay";
+                provider->modelProviderType = NestableType::Overlay;
             } else {
                 qCWarning(interfaceapp) << "no renderer for overlay ID" << overlayID.toString();
             }
@@ -659,7 +659,7 @@ public:
         if (auto avatar = avatarManager->getAvatarBySessionID(sessionUUID)) {
             if (avatar->getSessionUUID() == sessionUUID) {
                 provider = std::dynamic_pointer_cast<scriptable::ModelProvider>(avatar);
-                provider->metadata["providerType"] = "avatar";
+                provider->modelProviderType = NestableType::Avatar;
             }
         }
         return provider;
@@ -811,6 +811,7 @@ bool setupEssentials(int& argc, char** argv, bool runningMarkerExisted) {
     DependencyManager::set<ResourceCacheSharedItems>();
     DependencyManager::set<DesktopScriptingInterface>();
     DependencyManager::set<EntityScriptingInterface>(true);
+    DependencyManager::set<GraphicsScriptingInterface>();
     DependencyManager::registerInheritance<scriptable::ModelProviderFactory, ApplicationMeshProvider>();
     DependencyManager::set<ApplicationMeshProvider>();
     DependencyManager::set<RecordingScriptingInterface>();
diff --git a/interface/src/ui/overlays/ModelOverlay.cpp b/interface/src/ui/overlays/ModelOverlay.cpp
index e610fa5c2e..b6b79fd8c9 100644
--- a/interface/src/ui/overlays/ModelOverlay.cpp
+++ b/interface/src/ui/overlays/ModelOverlay.cpp
@@ -119,6 +119,7 @@ bool ModelOverlay::addToScene(Overlay::Pointer overlay, const render::ScenePoint
 void ModelOverlay::removeFromScene(Overlay::Pointer overlay, const render::ScenePointer& scene, render::Transaction& transaction) {
     Volume3DOverlay::removeFromScene(overlay, scene, transaction);
     _model->removeFromScene(scene, transaction);
+    emit DependencyManager::get<scriptable::ModelProviderFactory>()->modelRemovedFromScene(getID(), NestableType::Overlay, _model);
     transaction.updateItem<Overlay>(getRenderItemID(), [](Overlay& data) {
         auto modelOverlay = static_cast<ModelOverlay*>(&data);
         modelOverlay->clearSubRenderItemIDs();
diff --git a/libraries/entities-renderer/src/RenderableModelEntityItem.cpp b/libraries/entities-renderer/src/RenderableModelEntityItem.cpp
index 1c4a7f1055..c93bcb2055 100644
--- a/libraries/entities-renderer/src/RenderableModelEntityItem.cpp
+++ b/libraries/entities-renderer/src/RenderableModelEntityItem.cpp
@@ -1073,14 +1073,8 @@ void ModelEntityRenderer::removeFromScene(const ScenePointer& scene, Transaction
 
 void ModelEntityRenderer::onRemoveFromSceneTyped(const TypedEntityPointer& entity) {
     entity->setModel({});
-    //emit DependencyManager::get<scriptable::ModelProviderFactory>()->modelRemovedFromScene(entity->getID(), NestableType::Entity, _model);
 }
 
-void ModelEntityRenderer::onAddToSceneTyped(const TypedEntityPointer& entity) {
-    //emit DependencyManager::get<scriptable::ModelProviderFactory>()->modelAddedToScene(entity->getID(), NestableType::Entity, _model);
-}
-
-
 void ModelEntityRenderer::animate(const TypedEntityPointer& entity) {
     if (!_animation || !_animation->isLoaded()) {
         return;
diff --git a/libraries/fbx/src/FBX.h b/libraries/fbx/src/FBX.h
index 224d19fe96..a609d85fc8 100644
--- a/libraries/fbx/src/FBX.h
+++ b/libraries/fbx/src/FBX.h
@@ -242,6 +242,9 @@ public:
 
     graphics::MeshPointer _mesh;
     bool wasCompressed { false };
+
+    void createMeshTangents(bool generateFromTexCoords);
+    void createBlendShapeTangents(bool generateTangents);
 };
 
 class ExtractedMesh {
diff --git a/libraries/fbx/src/OBJWriter.cpp b/libraries/fbx/src/OBJWriter.cpp
index 621852f591..a53a3c7c5d 100644
--- a/libraries/fbx/src/OBJWriter.cpp
+++ b/libraries/fbx/src/OBJWriter.cpp
@@ -9,15 +9,14 @@
 //  See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html
 //
 
+#include "OBJWriter.h"
+
 #include <QFile>
 #include <QFileInfo>
-#include "graphics/Geometry.h"
-#include "OBJWriter.h"
+#include <graphics/BufferViewHelpers.h>
+#include <graphics/Geometry.h>
 #include "ModelFormatLogging.h"
 
-// FIXME: should this live in shared? (it depends on gpu/)
-#include <../graphics-scripting/src/graphics-scripting/BufferViewHelpers.h>
-
 static QString formatFloat(double n) {
     // limit precision to 6, but don't output trailing zeros.
     QString s = QString::number(n, 'f', 6);
@@ -71,8 +70,10 @@ bool writeOBJToTextStream(QTextStream& out, QList<MeshPointer> meshes) {
             out << formatFloat(v[1]) << " ";
             out << formatFloat(v[2]);
             if (colorIndex < numColors) {
-                glm::vec3 color = glmVecFromVariant<glm::vec3>(buffer_helpers::toVariant(colorsBufferView, colorIndex));
-                //glm::vec3 color = colorsBufferView.get<glm::vec3>(colorIndex);
+                glm::vec3 color = buffer_helpers::convert<glm::vec3>(colorsBufferView, colorIndex);
+                // TODO: still verifying that the above decodes properly; previous variations were:
+                // glm::vec3 color = buffer_helpers::glmVecFromVariant<glm::vec3>(buffer_helpers::toVariant(colorsBufferView, colorIndex));
+                // glm::vec3 color = colorsBufferView.get<glm::vec3>(colorIndex);
                 out << " " << formatFloat(color[0]);
                 out << " " << formatFloat(color[1]);
                 out << " " << formatFloat(color[2]);
@@ -95,8 +96,10 @@ bool writeOBJToTextStream(QTextStream& out, QList<MeshPointer> meshes) {
         const gpu::BufferView& normalsBufferView = mesh->getAttributeBuffer(gpu::Stream::InputSlot::NORMAL);
         gpu::BufferView::Index numNormals = (gpu::BufferView::Index)normalsBufferView.getNumElements();
         for (gpu::BufferView::Index i = 0; i < numNormals; i++) {
-            glm::vec3 normal = glmVecFromVariant<glm::vec3>(buffer_helpers::toVariant(normalsBufferView, i));
-            //glm::vec3 normal = normalsBufferView.get<glm::vec3>(i);
+            glm::vec3 normal = buffer_helpers::convert<glm::vec3>(normalsBufferView, i);
+            // TODO: still verifying that the above decodes properly; previous variations were:
+            // glm::vec3 normal = buffer_helpers::glmVecFromVariant<glm::vec3>(buffer_helpers::toVariant(normalsBufferView, i));
+            // glm::vec3 normal = normalsBufferView.get<glm::vec3>(i);
             out << "vn ";
             out << formatFloat(normal[0]) << " ";
             out << formatFloat(normal[1]) << " ";
diff --git a/libraries/graphics-scripting/CMakeLists.txt b/libraries/graphics-scripting/CMakeLists.txt
index e7fa3de155..ad8055b647 100644
--- a/libraries/graphics-scripting/CMakeLists.txt
+++ b/libraries/graphics-scripting/CMakeLists.txt
@@ -2,4 +2,3 @@ set(TARGET_NAME graphics-scripting)
 setup_hifi_library()
 link_hifi_libraries(shared networking graphics fbx model-networking script-engine)
 include_hifi_library_headers(gpu)
-include_hifi_library_headers(graphics-scripting)
diff --git a/libraries/graphics-scripting/src/graphics-scripting/BufferViewScripting.cpp b/libraries/graphics-scripting/src/graphics-scripting/BufferViewScripting.cpp
index 775aedad52..31cf5ff8f3 100644
--- a/libraries/graphics-scripting/src/graphics-scripting/BufferViewScripting.cpp
+++ b/libraries/graphics-scripting/src/graphics-scripting/BufferViewScripting.cpp
@@ -9,10 +9,10 @@
 #include <gpu/Format.h>
 #include <gpu/Stream.h>
 
-#include <graphics-scripting/BufferViewHelpers.h>
+#include <graphics/BufferViewHelpers.h>
 
 #ifdef DEBUG_BUFFERVIEW_SCRIPTING
-    #include <graphics/DebugNames.h>
+    #include "DebugNames.h"
 #endif
 
 namespace {
diff --git a/libraries/graphics-scripting/src/graphics-scripting/GraphicsScriptingInterface.cpp b/libraries/graphics-scripting/src/graphics-scripting/GraphicsScriptingInterface.cpp
index 71ef57bd82..58fcd7a064 100644
--- a/libraries/graphics-scripting/src/graphics-scripting/GraphicsScriptingInterface.cpp
+++ b/libraries/graphics-scripting/src/graphics-scripting/GraphicsScriptingInterface.cpp
@@ -1,8 +1,7 @@
 //
 //  GraphicsScriptingInterface.cpp
-//  libraries/script-engine/src
+//  libraries/graphics-scripting/src
 //
-//  Created by Seth Alves on 2017-1-27.
 //  Copyright 2017 High Fidelity, Inc.
 //
 //  Distributed under the Apache License, Version 2.0.
@@ -10,28 +9,24 @@
 //
 
 #include "GraphicsScriptingInterface.h"
-#include <QtScript/QScriptEngine>
-#include <QtScript/QScriptValueIterator>
-#include <QtScript/QScriptValue>
-#include <QUuid>
 #include "BaseScriptEngine.h"
-#include "ScriptEngineLogging.h"
+#include "BufferViewScripting.h"
+#include "DebugNames.h"
+#include "GraphicsScriptingUtil.h"
 #include "OBJWriter.h"
-
+#include "RegisteredMetaTypes.h"
+#include "ScriptEngineLogging.h"
+#include "ScriptableMesh.h"
 #include <GeometryUtil.h>
+#include <QUuid>
+#include <QtScript/QScriptEngine>
+#include <QtScript/QScriptValue>
+#include <QtScript/QScriptValueIterator>
+#include <graphics/BufferViewHelpers.h>
 #include <shared/QtHelpers.h>
 
-#include <graphics-scripting/DebugNames.h>
-#include <graphics-scripting/BufferViewHelpers.h>
-
-#include "BufferViewScripting.h"
-#include "ScriptableMesh.h"
-#include "GraphicsScriptingUtil.h"
-
 #include "GraphicsScriptingInterface.moc"
 
-#include "RegisteredMetaTypes.h"
-
 GraphicsScriptingInterface::GraphicsScriptingInterface(QObject* parent) : QObject(parent) {
     if (auto scriptEngine = qobject_cast<QScriptEngine*>(parent)) {
         this->registerMetaTypes(scriptEngine);
diff --git a/libraries/graphics-scripting/src/graphics-scripting/GraphicsScriptingUtil.h b/libraries/graphics-scripting/src/graphics-scripting/GraphicsScriptingUtil.h
index cfa510f87f..594e09bb32 100644
--- a/libraries/graphics-scripting/src/graphics-scripting/GraphicsScriptingUtil.h
+++ b/libraries/graphics-scripting/src/graphics-scripting/GraphicsScriptingUtil.h
@@ -12,12 +12,15 @@ Q_DECLARE_LOGGING_CATEGORY(graphics_scripting)
 
 namespace scriptable {
     // derive current context's C++ QObject (based on current JS "this" value)
-    template <typename T> T this_qobject_cast(QScriptEngine* engine) {
+    template <typename T>
+    T this_qobject_cast(QScriptEngine* engine) {
         auto context = engine ? engine->currentContext() : nullptr;
         return qscriptvalue_cast<T>(context ? context->thisObject() : QScriptValue::NullValue);
     }
+
     // JS => QPointer<QObject>
-    template <typename T> QPointer<T> qpointer_qobject_cast(const QScriptValue& value) {
+    template <typename T>
+    QPointer<T> qpointer_qobject_cast(const QScriptValue& value) {
         auto obj = value.toQObject();
 #ifdef SCRIPTABLE_MESH_DEBUG
         qCInfo(graphics_scripting) << "qpointer_qobject_cast" << obj << value.toString();
@@ -41,54 +44,21 @@ namespace scriptable {
     }
 
     // C++ > QtOwned instance
-    template <typename T, class... Rest> std::shared_ptr<T> make_qtowned(Rest... rest) {
+    template <typename T, class... Rest>
+    std::shared_ptr<T> make_qtowned(Rest... rest) {
         T* tmp = new T(rest...);
-#ifdef SCRIPTABLE_MESH_DEBUG
-        qCInfo(graphics_scripting) << "scriptable::make_qtowned" << toDebugString(tmp);
-#endif
-        QString debug = toDebugString(tmp);
         if (tmp) {
             tmp->metadata["__ownership__"] = QScriptEngine::QtOwnership;
-#ifdef SCRIPTABLE_MESH_DEBUG
-            QObject::connect(tmp, &QObject::destroyed, [=]() {  qCInfo(graphics_scripting) << "-------- ~scriptable::make_qtowned" << debug; });
-#endif
-            auto ptr = std::shared_ptr<T>(tmp, [debug](T* tmp) {
-                    //qDebug() << "~std::shared_ptr<T>" << debug;
-                delete tmp;
-            });
-            return ptr;
-        } else {
-            return std::shared_ptr<T>(tmp);
         }
+        return std::shared_ptr<T>(tmp);
     }
+
     // C++ > ScriptOwned JS instance
-    template <typename T, class... Rest> QPointer<T> make_scriptowned(Rest... rest) {
+    template <typename T, class... Rest>
+    QPointer<T> make_scriptowned(Rest... rest) {
         T* tmp = new T(rest...);
-#ifdef SCRIPTABLE_MESH_DEBUG
-        qCInfo(graphics_scripting) << "scriptable::make_scriptowned" << toDebugString(tmp);
-#endif
         if (tmp) {
             tmp->metadata["__ownership__"] = QScriptEngine::ScriptOwnership;
-            //auto blah = (DeleterFunction)[](void* delme) { };
-            return add_scriptowned_destructor(tmp);
-        } else {
-            return QPointer<T>(tmp);
-        }
-    }
-    // C++ > ScriptOwned JS instance
-    template <typename T> QPointer<T> add_scriptowned_destructor(T* tmp) {
-        QString debug = toDebugString(tmp);
-        if (tmp) {
-#ifdef SCRIPTABLE_MESH_DEBUG
-            QObject::connect(tmp, &QObject::destroyed, [=]() {
-                    qCInfo(graphics_scripting) << "-------- ~scriptable::make_scriptowned" << debug;// << !!customDeleter;
-                //if (customDeleter) {
-                    // customDeleter(tmp);
-                //}
-            });
-#endif
-        } else {
-            qCInfo(graphics_scripting) << "add_scriptowned_destructor -- not connecting to null value" << debug;
         }
         return QPointer<T>(tmp);
     }
diff --git a/libraries/graphics-scripting/src/graphics-scripting/ScriptableMesh.cpp b/libraries/graphics-scripting/src/graphics-scripting/ScriptableMesh.cpp
index c662371c89..28e57692f6 100644
--- a/libraries/graphics-scripting/src/graphics-scripting/ScriptableMesh.cpp
+++ b/libraries/graphics-scripting/src/graphics-scripting/ScriptableMesh.cpp
@@ -1,41 +1,26 @@
 //
-//  SimpleMeshProxy.cpp
-//  libraries/model-networking/src/model-networking/
-//
-//  Created by Seth Alves on 2017-3-22.
-//  Copyright 2017 High Fidelity, Inc.
+//  Copyright 2018 High Fidelity, Inc.
 //
 //  Distributed under the Apache License, Version 2.0.
 //  See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html
 //
 
-#include "GraphicsScriptingUtil.h"
 #include "ScriptableMesh.h"
 
-#include <glm/glm.hpp>
-#include <glm/gtx/transform.hpp>
-#include <glm/gtx/norm.hpp>
-#include <graphics/Geometry.h>
-#include <graphics-scripting/DebugNames.h>
-#include <graphics-scripting/BufferViewHelpers.h>
-#include <graphics-scripting/BufferViewScripting.h>
-
-#include "ScriptableMesh.moc"
-
-#include <RegisteredMetaTypes.h>
+#include "BufferViewScripting.h"
+#include "DebugNames.h"
+#include "GraphicsScriptingUtil.h"
+#include "OBJWriter.h"
 #include <BaseScriptEngine.h>
 #include <QtScript/QScriptValue>
+#include <RegisteredMetaTypes.h>
+#include <glm/glm.hpp>
+#include <glm/gtx/norm.hpp>
+#include <glm/gtx/transform.hpp>
+#include <graphics/BufferViewHelpers.h>
+#include <graphics/Geometry.h>
 
-#include "OBJWriter.h"
-
-// #define SCRIPTABLE_MESH_DEBUG
-
-namespace scriptable {
-    // QScriptValue jsBindCallback(QScriptValue callback);
-    // template <typename T> QPointer<T> qpointer_qobject_cast(const QScriptValue& value);
-    // template <typename T> T this_qobject_cast(QScriptEngine* engine);
-    // template <typename T, class... Rest> QPointer<T> make_scriptowned(Rest... rest);
-}
+#include "ScriptableMesh.moc"
 
 scriptable::ScriptableMeshPart::ScriptableMeshPart(scriptable::ScriptableMeshPointer parentMesh, int partIndex)
     : parentMesh(parentMesh), partIndex(partIndex)  {
diff --git a/libraries/graphics-scripting/src/graphics-scripting/ScriptableMesh.h b/libraries/graphics-scripting/src/graphics-scripting/ScriptableMesh.h
index ba0efa007d..2b9399d6ef 100644
--- a/libraries/graphics-scripting/src/graphics-scripting/ScriptableMesh.h
+++ b/libraries/graphics-scripting/src/graphics-scripting/ScriptableMesh.h
@@ -1,26 +1,22 @@
 #pragma once
 
-#include <glm/glm.hpp>
-#include <QtCore/QObject>
-#include <QtCore/QVector>
-#include <QtCore/QList>
-#include <QtCore/QVariant>
-#include <QtCore/QUuid>
-#include <QPointer>
-#include <memory>
+#include "ScriptableModel.h"
 
+#include <glm/glm.hpp>
+#include <graphics/BufferViewHelpers.h>
 #include <DependencyManager.h>
 
-//#include <graphics-scriping/Forward.h>
-#include <graphics-scripting/ScriptableModel.h>
-#include <graphics-scripting/BufferViewHelpers.h>
-
-#include <QtScript/QScriptable>
+#include <memory>
+#include <QPointer>
+#include <QtCore/QList>
+#include <QtCore/QObject>
+#include <QtCore/QUuid>
+#include <QtCore/QVariant>
+#include <QtCore/QVector>
 #include <QtScript/QScriptValue>
+#include <QtScript/QScriptable>
 
 namespace scriptable {
-
-    QScriptValue jsBindCallback(QScriptValue callback);
     class ScriptableMesh : public ScriptableMeshBase, QScriptable {
         Q_OBJECT
     public:
diff --git a/libraries/graphics-scripting/src/graphics-scripting/BufferViewHelpers.cpp b/libraries/graphics/src/graphics/BufferViewHelpers.cpp
similarity index 87%
rename from libraries/graphics-scripting/src/graphics-scripting/BufferViewHelpers.cpp
rename to libraries/graphics/src/graphics/BufferViewHelpers.cpp
index e4eeee856e..ea511d82f4 100644
--- a/libraries/graphics-scripting/src/graphics-scripting/BufferViewHelpers.cpp
+++ b/libraries/graphics/src/graphics/BufferViewHelpers.cpp
@@ -1,4 +1,11 @@
-#include "./graphics-scripting/BufferViewHelpers.h"
+//
+//  Copyright 2018 High Fidelity, Inc.
+//
+//  Distributed under the Apache License, Version 2.0.
+//  See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html
+//
+
+#include "BufferViewHelpers.h"
 
 #include <QDebug>
 #include <QVariant>
@@ -7,7 +14,7 @@
 #include <gpu/Format.h>
 #include <gpu/Stream.h>
 
-#include <graphics/Geometry.h>
+#include "Geometry.h"
 
 #include <Extents.h>
 #include <AABox.h>
@@ -15,25 +22,26 @@
 #include <glm/gtx/string_cast.hpp>
 #include <glm/gtc/packing.hpp>
 #include <glm/detail/type_vec.hpp>
+
 namespace glm {
     using hvec2 = glm::tvec2<glm::detail::hdata>;
     using hvec4 = glm::tvec4<glm::detail::hdata>;
 }
 
-//#define DEBUG_BUFFERVIEW_SCRIPTING
-//#ifdef DEBUG_BUFFERVIEW_SCRIPTING
-#include "DebugNames.h"
-//#endif
+#ifdef DEBUG_BUFFERVIEW_SCRIPTING
+#include "../../graphics-scripting/src/graphics-scripting/DebugNames.h"
+#endif
 
 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" }};
+    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" } };
 }
 
 gpu::BufferView buffer_helpers::getBufferView(graphics::MeshPointer mesh, gpu::Stream::Slot slot) {
     return slot == gpu::Stream::POSITION ? mesh->getVertexBuffer() : mesh->getAttributeBuffer(slot);
 }
+
 QMap<QString,int> buffer_helpers::ATTRIBUTES{
     {"position", gpu::Stream::POSITION },
     {"normal", gpu::Stream::NORMAL },
@@ -49,16 +57,23 @@ QMap<QString,int> buffer_helpers::ATTRIBUTES{
 };
 
 
-template <typename T>
-QVariant getBufferViewElement(const gpu::BufferView& view, quint32 index, bool asArray = false) {
-    return glmVecToVariant(view.get<T>(index), asArray);
-}
+namespace {
+    bool boundsCheck(const gpu::BufferView& view, quint32 index) {
+        const auto byteLength = view._element.getSize();
+        return (
+            index < view.getNumElements() &&
+            index * byteLength < (view._size - 1) * byteLength
+        );
+    }
 
-template <typename T>
-void setBufferViewElement(const gpu::BufferView& view, quint32 index, const QVariant& v) {
-    view.edit<T>(index) = glmVecFromVariant<T>(v);
-}
+    template <typename T> QVariant getBufferViewElement(const gpu::BufferView& view, quint32 index, bool asArray = false) {
+        return buffer_helpers::glmVecToVariant(view.get<T>(index), asArray);
+    }
 
+    template <typename T> void setBufferViewElement(const gpu::BufferView& view, quint32 index, const QVariant& v) {
+        view.edit<T>(index) = buffer_helpers::glmVecFromVariant<T>(v);
+    }
+}
 
 void buffer_helpers::packNormalAndTangent(glm::vec3 normal, glm::vec3 tangent, glm::uint32& packedNormal, glm::uint32& packedTangent) {
     auto absNormal = glm::abs(normal);
@@ -147,14 +162,6 @@ bool buffer_helpers::fromVariant(const gpu::BufferView& view, quint32 index, con
     return false;
 }
 
-bool boundsCheck(const gpu::BufferView& view, quint32 index) {
-    const auto byteLength = view._element.getSize();
-    return (
-        index < view.getNumElements() &&
-        index * byteLength < (view._size - 1) * byteLength
-    );
-}
-
 QVariant buffer_helpers::toVariant(const gpu::BufferView& view, quint32 index, bool asArray, const char* hint) {
     const auto& element = view._element;
     const auto vecN = element.getScalarCount();
@@ -167,14 +174,17 @@ QVariant buffer_helpers::toVariant(const gpu::BufferView& view, quint32 index, b
         auto byteOffset = index * vecN * BYTES_PER_ELEMENT;
         auto maxByteOffset = (view._size - 1) * vecN * BYTES_PER_ELEMENT;
         if (byteOffset > maxByteOffset) {
-            qDebug() << "bufferViewElementToVariant -- byteOffset out of range " << byteOffset << " < " << maxByteOffset << DebugNames::stringFrom(dataType);
-            qDebug() << "bufferViewElementToVariant -- index: " << index << "numElements" << view.getNumElements();
-            qDebug() << "bufferViewElementToVariant -- vecN: " << vecN << "byteLength" << byteLength << "BYTES_PER_ELEMENT" << BYTES_PER_ELEMENT;
+#ifdef DEBUG_BUFFERVIEW_SCRIPTING
+            qDebug() << "toVariant -- " << DebugNames::stringFrom(dataType)
+#endif
+            qDebug() << "toVariant -- byteOffset out of range " << byteOffset << " < " << maxByteOffset;
+            qDebug() << "toVariant -- index: " << index << "numElements" << view.getNumElements();
+            qDebug() << "toVariant -- vecN: " << vecN << "byteLength" << byteLength << "BYTES_PER_ELEMENT" << BYTES_PER_ELEMENT;
         }
         Q_ASSERT(byteOffset <= maxByteOffset);
     }
 #ifdef DEBUG_BUFFERVIEW_SCRIPTING
-    qCDebug(bufferhelper_logging) << "bufferViewElementToVariant" << index << DebugNames::stringFrom(dataType) << BYTES_PER_ELEMENT << vecN;
+    qCDebug(bufferhelper_logging) << "toVariant -- " << index << DebugNames::stringFrom(dataType) << BYTES_PER_ELEMENT << vecN;
 #endif
     if (BYTES_PER_ELEMENT == 1) {
         switch(vecN) {
@@ -223,7 +233,7 @@ QVariant buffer_helpers::toVariant(const gpu::BufferView& view, quint32 index, b
 }
 
 template <typename T>
-QVariant glmVecToVariant(const T& v, bool asArray /*= false*/) {
+QVariant buffer_helpers::glmVecToVariant(const T& v, bool asArray /*= false*/) {
     static const auto len = T().length();
     if (asArray) {
         QVariantList list;
@@ -239,8 +249,9 @@ QVariant glmVecToVariant(const T& v, bool asArray /*= false*/) {
         return obj;
     }
 }
+
 template <typename T>
-const T glmVecFromVariant(const QVariant& v) {
+const T buffer_helpers::glmVecFromVariant(const QVariant& v) {
     auto isMap = v.type() == (QVariant::Type)QMetaType::QVariantMap;
     static const auto len = T().length();
     const auto& components = isMap ? XYZW : ZERO123;
@@ -255,9 +266,11 @@ const T glmVecFromVariant(const QVariant& v) {
         } else {
             value = list.value(i).toFloat();
         }
+#ifdef DEBUG_BUFFERVIEW_SCRIPTING
         if (value != value) { // NAN
             qWarning().nospace()<< "vec" << len << "." << components[i] << " NAN received from script.... " << v.toString();
         }
+#endif        
         result[i] = value;
     }
     return result;
@@ -268,17 +281,26 @@ gpu::BufferView buffer_helpers::fromVector(const QVector<T>& elements, const gpu
     auto vertexBuffer = std::make_shared<gpu::Buffer>(elements.size() * sizeof(T), (gpu::Byte*)elements.data());
     return { vertexBuffer, 0, vertexBuffer->getSize(),sizeof(T), elementType };
 }
-template<> gpu::BufferView buffer_helpers::fromVector<unsigned int>(const QVector<unsigned int>& elements, const gpu::Element& elementType) { return fromVector(elements, elementType); }
-template<> gpu::BufferView buffer_helpers::fromVector<glm::vec3>(const QVector<glm::vec3>& elements, const gpu::Element& elementType) { return fromVector(elements, elementType); }
+template<> gpu::BufferView buffer_helpers::fromVector<unsigned int>(
+    const QVector<unsigned int>& elements, const gpu::Element& elementType
+) { return fromVector(elements, elementType); }
 
-template <typename T> struct GpuVec4ToGlm;// { static T get(const gpu::BufferView& view, quint32 index, const char *hint); };
-template <typename T> struct getScalar;// { static T get(const gpu::BufferView& view, quint32 index, const char *hint); };
+template<> gpu::BufferView buffer_helpers::fromVector<glm::vec3>(
+    const QVector<glm::vec3>& elements, const gpu::Element& elementType
+) { return fromVector(elements, elementType); }
+
+template <typename T> struct GpuVec4ToGlm;
+template <typename T> struct GpuScalarToGlm;
 
 struct GpuToGlmAdapter {
     static float error(const QString& name, const gpu::BufferView& view, quint32 index, const char *hint) {
+        QString debugName;
+#ifdef DEBUG_BUFFERVIEW_SCRIPTING
+        debugName = DebugNames::stringFrom(view._element.getType())
+#endif
         qDebug() << QString("GpuToGlmAdapter:: unhandled type=%1(element=%2(%3)) size=%4(per=%5) vec%6 hint=%7 #%8")
             .arg(name)
-            .arg(DebugNames::stringFrom(view._element.getType()))
+            .arg(debugName)
             .arg(view._element.getType())
             .arg(view._element.getSize())
             .arg(view._element.getSize() / view._element.getScalarCount())
@@ -290,7 +312,8 @@ struct GpuToGlmAdapter {
         return NAN;
     }
 };
-template <typename T> struct getScalar : GpuToGlmAdapter {
+
+template <typename T> struct GpuScalarToGlm : GpuToGlmAdapter {
     static T get(const gpu::BufferView& view, quint32 index, const char *hint) { switch(view._element.getType()) {
         case gpu::UINT32: return view.get<glm::uint32>(index);
         case gpu::UINT16: return view.get<glm::uint16>(index);
@@ -301,7 +324,7 @@ template <typename T> struct getScalar : GpuToGlmAdapter {
         case gpu::FLOAT: return view.get<glm::float32>(index);
         case gpu::HALF: return T(glm::unpackSnorm1x8(view.get<glm::int8>(index)));
         default: break;
-        } return T(error("getScalar", view, index, hint));
+        } return T(error("GpuScalarToGlm", view, index, hint));
     }
 };
 
@@ -376,8 +399,9 @@ struct getVec {
     }
 };
 
+// BufferView => QVector<T>
 template <> QVector<int> buffer_helpers::toVector<int>(const gpu::BufferView& view, const char *hint) {
-    return getVec<getScalar<int>,int>::__to_vector__(view, hint);
+    return getVec<GpuScalarToGlm<int>,int>::__to_vector__(view, hint);
 }
 template <> QVector<glm::vec2> buffer_helpers::toVector<glm::vec2>(const gpu::BufferView& view, const char *hint) {
     return getVec<GpuVec2ToGlm<glm::vec2>,glm::vec2>::__to_vector__(view, hint);
@@ -390,8 +414,9 @@ template <> QVector<glm::vec4> buffer_helpers::toVector<glm::vec4>(const gpu::Bu
 }
 
 
+// indexed conversion accessors (similar to "view.convert<T>(i)" existed)
 template <> int buffer_helpers::convert<int>(const gpu::BufferView& view, quint32 index, const char *hint) {
-    return getVec<getScalar<int>,int>::__to_scalar__(view, index, hint);
+    return getVec<GpuScalarToGlm<int>,int>::__to_scalar__(view, index, hint);
 }
 template <> glm::vec2 buffer_helpers::convert<glm::vec2>(const gpu::BufferView& view, quint32 index, const char *hint) {
     return getVec<GpuVec2ToGlm<glm::vec2>,glm::vec2>::__to_scalar__(view, index, hint);
@@ -407,30 +432,25 @@ gpu::BufferView buffer_helpers::clone(const gpu::BufferView& input) {
     return gpu::BufferView(
         std::make_shared<gpu::Buffer>(input._buffer->getSize(), input._buffer->getData()),
         input._offset, input._size, input._stride, input._element
-        );
+    );
 }
 
+// TODO: preserve existing data
 gpu::BufferView buffer_helpers::resize(const gpu::BufferView& input, quint32 numElements) {
     auto effectiveSize = input._buffer->getSize() / input.getNumElements();
-    qDebug() << "resize input" << input.getNumElements() << input._buffer->getSize() << "effectiveSize" << effectiveSize;
+    qCDebug(bufferhelper_logging) << "resize input" << input.getNumElements() << input._buffer->getSize() << "effectiveSize" << effectiveSize;
     auto vsize = input._element.getSize() * numElements;
-    gpu::Byte *data = new gpu::Byte[vsize];
-    memset(data, 0, vsize);
-    auto buffer = new gpu::Buffer(vsize, (gpu::Byte*)data);
-    delete[] data;
+    std::unique_ptr<gpu::Byte[]> data{ new gpu::Byte[vsize] };
+    memset(data.get(), 0, vsize);
+    auto buffer = new gpu::Buffer(vsize, data.get());
     auto output = gpu::BufferView(buffer, input._element);
-    qDebug() << "resized output" << output.getNumElements() << output._buffer->getSize();
+    qCDebug(bufferhelper_logging) << "resized output" << output.getNumElements() << output._buffer->getSize();
     return output;
 }
 
 graphics::MeshPointer buffer_helpers::cloneMesh(graphics::MeshPointer mesh) {
     auto clone = std::make_shared<graphics::Mesh>();
-    //[](graphics::Mesh* blah) {
-    //qCDebug(bufferhelper_logging) << "--- DELETING MESH POINTER" << blah;
-    //  delete blah;
-    //});
     clone->displayName = (QString::fromStdString(mesh->displayName) + "-clone").toStdString();
-    //qCInfo(bufferhelper_logging) << "+++ ALLOCATED MESH POINTER ScriptableMesh::cloneMesh" << clone->displayName << clone.get() << !!mesh;
     clone->setIndexBuffer(buffer_helpers::clone(mesh->getIndexBuffer()));
     clone->setPartBuffer(buffer_helpers::clone(mesh->getPartBuffer()));
     auto attributeViews = buffer_helpers::gatherBufferViews(mesh);
@@ -447,18 +467,11 @@ graphics::MeshPointer buffer_helpers::cloneMesh(graphics::MeshPointer mesh) {
     return clone;
 }
 
-
-/// --- buffer view <-> variant helpers
-
 namespace {
     // expand the corresponding attribute buffer (creating it if needed) so that it matches POSITIONS size and specified element type
     gpu::BufferView _expandedAttributeBuffer(const graphics::MeshPointer mesh, gpu::Stream::Slot slot) {
         gpu::BufferView bufferView = buffer_helpers::getBufferView(mesh, slot);
         const auto& elementType = bufferView._element;
-        //auto vecN = element.getScalarCount();
-        //auto type = element.getType();
-        //gpu::Element elementType = getVecNElement(type, vecN);
-
         gpu::Size elementSize = elementType.getSize();
         auto nPositions = mesh->getNumVertices();
         auto vsize = nPositions * elementSize;
@@ -478,10 +491,9 @@ namespace {
         if (bufferView.getNumElements() < nPositions || diffTypes) {
             if (!bufferView._buffer || bufferView.getNumElements() == 0) {
                 qCInfo(bufferhelper_logging).nospace() << "ScriptableMesh -- adding missing mesh attribute '" << hint << "' for BufferView";
-                gpu::Byte *data = new gpu::Byte[vsize];
-                memset(data, 0, vsize);
-                auto buffer = new gpu::Buffer(vsize, (gpu::Byte*)data);
-                delete[] data;
+                std::unique_ptr<gpu::Byte[]> data{ new gpu::Byte[vsize] };
+                memset(data.get(), 0, vsize);
+                auto buffer = new gpu::Buffer(vsize, data.get());
                 bufferView = gpu::BufferView(buffer, elementType);
                 mesh->addAttribute(slot, bufferView);
             } else {
@@ -553,7 +565,6 @@ std::map<QString, gpu::BufferView> buffer_helpers::gatherBufferViews(graphics::M
     return attributeViews;
 }
 
-
 bool buffer_helpers::recalculateNormals(graphics::MeshPointer mesh) {
     qCInfo(bufferhelper_logging) << "Recalculating normals" << !!mesh;
     if (!mesh) {
@@ -567,10 +578,8 @@ bool buffer_helpers::recalculateNormals(graphics::MeshPointer mesh) {
     auto numPoints = indices.getNumElements();
     const auto TRIANGLE = 3;
     quint32 numFaces = (quint32)numPoints / TRIANGLE;
-    //QVector<Triangle> faces;
     QVector<glm::vec3> faceNormals;
     QMap<QString,QVector<quint32>> vertexToFaces;
-    //faces.resize(numFaces);
     faceNormals.resize(numFaces);
     auto numNormals = normals.getNumElements();
     qCInfo(bufferhelper_logging) << QString("numFaces: %1, numNormals: %2, numPoints: %3").arg(numFaces).arg(numNormals).arg(numPoints);
@@ -590,7 +599,9 @@ bool buffer_helpers::recalculateNormals(graphics::MeshPointer mesh) {
         };
         faceNormals[i] = face.getNormal();
         if (glm::isnan(faceNormals[i].x)) {
+#ifdef DEBUG_BUFFERVIEW_SCRIPTING
             qCInfo(bufferhelper_logging) << i << i0 << i1 << i2 << glmVecToVariant(face.v0) << glmVecToVariant(face.v1) << glmVecToVariant(face.v2);
+#endif
             break;
         }
         vertexToFaces[glm::to_string(face.v0).c_str()] << i;
@@ -615,10 +626,12 @@ bool buffer_helpers::recalculateNormals(graphics::MeshPointer mesh) {
             normal = verts.get<glm::vec3>(j);
         }
         if (glm::isnan(normal.x)) {
+#ifdef DEBUG_BUFFERVIEW_SCRIPTING
             static int logged = 0;
             if (logged++ < 10) {
                 qCInfo(bufferhelper_logging) << "isnan(normal.x)" << j << glmVecToVariant(normal);
             }
+#endif
             break;
         }
         buffer_helpers::fromVariant(normals, j, glmVecToVariant(glm::normalize(normal)));
diff --git a/libraries/graphics-scripting/src/graphics-scripting/BufferViewHelpers.h b/libraries/graphics/src/graphics/BufferViewHelpers.h
similarity index 91%
rename from libraries/graphics-scripting/src/graphics-scripting/BufferViewHelpers.h
rename to libraries/graphics/src/graphics/BufferViewHelpers.h
index e0c2e1eee1..53fddfe581 100644
--- a/libraries/graphics-scripting/src/graphics-scripting/BufferViewHelpers.h
+++ b/libraries/graphics/src/graphics/BufferViewHelpers.h
@@ -15,9 +15,6 @@ namespace gpu {
     class Element;
 }
 
-template <typename T> QVariant glmVecToVariant(const T& v, bool asArray = false);
-template <typename T> const T glmVecFromVariant(const QVariant& v);
-
 namespace graphics {
     class Mesh;
     using MeshPointer = std::shared_ptr<Mesh>;
@@ -27,6 +24,9 @@ class Extents;
 class AABox;
 
 struct buffer_helpers {
+    template <typename T> static QVariant glmVecToVariant(const T& v, bool asArray = false);
+    template <typename T> static const T glmVecFromVariant(const QVariant& v);
+
     static graphics::MeshPointer cloneMesh(graphics::MeshPointer mesh);
     static QMap<QString,int> ATTRIBUTES;
     static std::map<QString, gpu::BufferView> gatherBufferViews(graphics::MeshPointer mesh, const QStringList& expandToMatchPositions = QStringList());
diff --git a/libraries/render-utils/src/Model.cpp b/libraries/render-utils/src/Model.cpp
index ffc93340b0..9dbef5c6fc 100644
--- a/libraries/render-utils/src/Model.cpp
+++ b/libraries/render-utils/src/Model.cpp
@@ -9,10 +9,6 @@
 //  See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html
 //
 
-#include <QtCore/QLoggingCategory>
-namespace { QLoggingCategory wtf{ "tim.Model.cpp" }; }
-
-
 #include "Model.h"
 
 #include <QMetaType>
@@ -31,7 +27,7 @@ namespace { QLoggingCategory wtf{ "tim.Model.cpp" }; }
 #include <TBBHelpers.h>
 
 #include <graphics-scripting/Forward.h>
-#include <graphics-scripting/BufferViewHelpers.h>
+#include <graphics/BufferViewHelpers.h>
 #include <DualQuaternion.h>
 
 #include <glm/gtc/packing.hpp>
@@ -372,7 +368,7 @@ bool Model::updateGeometry() {
 #if FBX_PACK_NORMALS
                     glm::uint32 finalNormal;
                     glm::uint32 finalTangent;
-                    packNormalAndTangent(*normalIt, *tangentIt, finalNormal, finalTangent);
+                    buffer_helpers::packNormalAndTangent(*normalIt, *tangentIt, finalNormal, finalTangent);
 #else
                     const auto finalNormal = *normalIt;
                     const auto finalTangent = *tangentIt;
@@ -569,25 +565,15 @@ bool Model::replaceScriptableModelMeshPart(scriptable::ScriptableModelBasePointe
         auto newRenderGeometry = new MyGeometryMappingResource(
             _url, _renderGeometry, _newModel ? scriptable::make_qtowned<scriptable::ScriptableModelBase>(*_newModel) : nullptr
         );
-        //_needsUpdateTextures = true;
         _visualGeometryRequestFailed = false;
-        //invalidCalculatedMeshBoxes();
         deleteGeometry();
         _renderGeometry.reset(newRenderGeometry);
-        //onInvalidate();
-        //reset();
         _rig.destroyAnimGraph();
-        //assert(_rig.jointStatesEmpty());
         updateGeometry();
         calculateTriangleSets();
-        //computeMeshPartLocalBounds();
-        //_needsReload = false;
+        _needsReload = false;
         _needsFixupInScene = true;
-        //invalidCalculatedMeshBoxes();
         setRenderItemsNeedUpdate();
-        //_hasCalculatedTextureInfo = false;
-        //calculateTextureInfo();
-        //updateRenderItems();
     }
     return true;
 }
@@ -597,7 +583,7 @@ scriptable::ScriptableModelBase Model::getScriptableModel(bool* ok) {
     scriptable::ScriptableModelBase result;
 
     if (!isLoaded()) {
-        qCDebug(wtf) << "Model::getScriptableModel -- !isLoaded";
+        qCDebug(renderutils) << "Model::getScriptableModel -- !isLoaded";
         return scriptable::ModelProvider::modelUnavailableError(ok);
     }
 
diff --git a/libraries/render-utils/src/Model_temporary_hack.cpp.h b/libraries/render-utils/src/Model_temporary_hack.cpp.h
index bc31abcb37..9b0e0bcf9a 100644
--- a/libraries/render-utils/src/Model_temporary_hack.cpp.h
+++ b/libraries/render-utils/src/Model_temporary_hack.cpp.h
@@ -1,15 +1,7 @@
-#include <graphics-scripting/BufferViewHelpers.h>
+#include <graphics/BufferViewHelpers.h>
 #include <graphics-scripting/GraphicsScriptingUtil.h>
 class MyGeometryMappingResource : public GeometryResource {
-//    Q_OBJECT
 public:
-    virtual void init(bool resetLoaded = true) override {
-        qCDebug(wtf) << "############################# Snarfing init()...";
-    }
-
-    virtual void deleter() override {
-        qCDebug(wtf) << "############################# Snarfing deleter()...";
-    }
     shared_ptr<FBXGeometry> fbxGeometry;
     MyGeometryMappingResource(const QUrl& url, Geometry::Pointer originalGeometry, std::shared_ptr<scriptable::ScriptableModelBase> newModel) : GeometryResource(url) {
         fbxGeometry = std::make_shared<FBXGeometry>();
@@ -77,6 +69,8 @@ public:
                 mesh.colors = buffer_helpers::toVector<glm::vec3>(buffer_helpers::getBufferView(mesh._mesh, gpu::Stream::COLOR), "mesh.colors");
                 mesh.texCoords = buffer_helpers::toVector<glm::vec2>(buffer_helpers::getBufferView(mesh._mesh, gpu::Stream::TEXCOORD0), "mesh.texCoords");
                 mesh.texCoords1 = buffer_helpers::toVector<glm::vec2>(buffer_helpers::getBufferView(mesh._mesh, gpu::Stream::TEXCOORD1), "mesh.texCoords1");
+                mesh.createMeshTangents(true);
+                mesh.createBlendShapeTangents(false);
                 geometry.meshes << mesh;
                 // Copy mesh pointers
                 meshes->emplace_back(newMesh.getMeshPointer());//buffer_helpers::cloneMesh(ptr));
@@ -84,10 +78,9 @@ public:
                 const auto oldParts = mesh.parts;
                 mesh.parts.clear();
                 for (const FBXMeshPart& fbxPart : oldParts) {
-                    FBXMeshPart part; // copy;
+                    FBXMeshPart part; // new copy
                     part.materialID = fbxPart.materialID;
                     // Construct local parts
-                    ///qCDebug(wtf) << "GeometryMappingResource -- meshes part" << meshID << partID << part.materialID;
                     part.triangleIndices = buffer_helpers::toVector<int>(mesh._mesh->getIndexBuffer(), "part.triangleIndices");
                     mesh.parts << part;
                     auto p = std::make_shared<MeshPart>(meshID, partID, (int)materialIDAtlas[part.materialID]);
@@ -115,7 +108,6 @@ public:
         _animGraphOverrideUrl = originalGeometry ? originalGeometry->getAnimGraphOverrideUrl() : QUrl();
         _loaded = true;
         _fbxGeometry = fbxGeometry;
-        finishedLoading(true);
     };    
 };