* changes per CR feedback

* make MeshPart.topology read-only
* JS Graphics API starter documentation
* remove redundant qscriptvalue sequence registrations

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

@@ -94,7 +94,12 @@ namespace scriptable {
     using ScriptableMeshPointer = QPointer<ScriptableMesh>;
     class ScriptableMeshPart;
     using ScriptableMeshPartPointer = QPointer<ScriptableMeshPart>;
-    bool registerMetaTypes(QScriptEngine* engine);
 }
 
+Q_DECLARE_METATYPE(glm::uint32)
+Q_DECLARE_METATYPE(QVector<glm::uint32>)
 Q_DECLARE_METATYPE(NestableType)
+Q_DECLARE_METATYPE(scriptable::MeshPointer)
+Q_DECLARE_METATYPE(scriptable::WeakMeshPointer)
+Q_DECLARE_METATYPE(scriptable::ModelProviderPointer)
+

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

@@ -26,9 +26,6 @@
 #include <SpatiallyNestable.h>
 
 GraphicsScriptingInterface::GraphicsScriptingInterface(QObject* parent) : QObject(parent), QScriptable() {
-    if (auto scriptEngine = qobject_cast<QScriptEngine*>(parent)) {
-        this->registerMetaTypes(scriptEngine);
-    }
 }
 
 void GraphicsScriptingInterface::jsThrowError(const QString& error) {
@@ -179,12 +176,17 @@ scriptable::ScriptableMeshPointer GraphicsScriptingInterface::newMesh(const QVar
 
     const auto numVertices = vertices.size();
     const auto numIndices = indices.size();
-    const auto topology = graphics::Mesh::TRIANGLES;
+    const auto topology = graphics::TOPOLOGIES.key(topologyName);
+
+    // TODO: support additional topologies (POINTS and LINES ought to "just work" --
+    //   if MeshPartPayload::drawCall is updated to actually check the Mesh::Part::_topology value
+    //   (TRIANGLE_STRIP, TRIANGLE_FAN, QUADS, QUAD_STRIP may need additional conversion code though)
+    static const QStringList acceptableTopologies{ "triangles" };
 
     // sanity checks
     QString error;
-    if (!topologyName.isEmpty() && topologyName != "triangles") {
+    if (!topologyName.isEmpty() && !acceptableTopologies.contains(topologyName)) {
-        error = "expected 'triangles' or undefined for .topology";
+        error = QString("expected .topology to be %1").arg(acceptableTopologies.join(" | "));
     } else if (!numIndices) {
         error = QString("expected non-empty [uint32,...] array for .indices (got type=%1)").arg(ifsMeshData.value("indices").typeName());
     } else if (numIndices % 3 != 0) {
@@ -227,8 +229,8 @@ scriptable::ScriptableMeshPointer GraphicsScriptingInterface::newMesh(const QVar
     mesh->modelName = "graphics::newMesh";
     mesh->displayName = meshName.toStdString();
 
-    // TODO: newFromVector does no conversion -- later we could autodetect if fitting into gpu::INDEX_UINT16
+    // TODO: newFromVector does inbound type conversion, but not compression or packing
-    //  and also pack other values (like NORMAL / TEXCOORD0 where relevant)
+    //  (later we should autodetect if fitting into gpu::INDEX_UINT16 and reduce / pack normals etc.)
     mesh->setIndexBuffer(buffer_helpers::newFromVector(indices, gpu::Format::INDEX_INT32));
     mesh->setVertexBuffer(buffer_helpers::newFromVector(vertices, gpu::Format::VEC3F_XYZ));
     if (normals.size()) {
@@ -240,7 +242,7 @@ scriptable::ScriptableMeshPointer GraphicsScriptingInterface::newMesh(const QVar
     if (texCoords0.size()) {
         mesh->addAttribute(gpu::Stream::TEXCOORD0, buffer_helpers::newFromVector(texCoords0, gpu::Format::VEC2F_UV));
     }
-    QVector<graphics::Mesh::Part> parts = {{ 0, indices.size(), 0, topology}};
+    QVector<graphics::Mesh::Part> parts = {{ 0, indices.size(), 0, topology }};
     mesh->setPartBuffer(buffer_helpers::newFromVector(parts, gpu::Element::PART_DRAWCALL));
     return scriptable::make_scriptowned<scriptable::ScriptableMesh>(mesh, nullptr);
 }
@@ -262,10 +264,6 @@ QString GraphicsScriptingInterface::exportModelToOBJ(const scriptable::Scriptabl
     return QString();
 }
 
-void GraphicsScriptingInterface::registerMetaTypes(QScriptEngine* engine) {
-    scriptable::registerMetaTypes(engine);
-}
-
 MeshPointer GraphicsScriptingInterface::getMeshPointer(const scriptable::ScriptableMesh& meshProxy) {
     return meshProxy.getMeshPointer();
 }
@@ -287,14 +285,6 @@ MeshPointer GraphicsScriptingInterface::getMeshPointer(scriptable::ScriptableMes
 }
 
 namespace {
-    QScriptValue qVectorUInt32ToScriptValue(QScriptEngine* engine, const QVector<glm::uint32>& vector) {
-        return qScriptValueFromSequence(engine, vector);
-    }
-
-    void qVectorUInt32FromScriptValue(const QScriptValue& array, QVector<glm::uint32>& result) {
-        qScriptValueToSequence(array, result);
-    }
-
     QVector<int> metaTypeIds{
         qRegisterMetaType<glm::uint32>("uint32"),
         qRegisterMetaType<glm::uint32>("glm::uint32"),
@@ -312,7 +302,7 @@ namespace {
 }
 
 namespace scriptable {
-    template <typename T> int registerQPointerThing(QScriptEngine* engine) {
+    template <typename T> int registerQPointerMetaType(QScriptEngine* engine) {
         qScriptRegisterSequenceMetaType<QVector<QPointer<T>>>(engine);
         return qScriptRegisterMetaType<QPointer<T>>(
             engine,
@@ -346,44 +336,32 @@ namespace scriptable {
     }
 
     template <typename T> int registerDebugEnum(QScriptEngine* engine, const DebugEnums<T>& debugEnums) {
-        static const DebugEnums<T>& poop = debugEnums;
+        static const DebugEnums<T>& instance = debugEnums;
         return qScriptRegisterMetaType<T>(
             engine,
             [](QScriptEngine* engine, const T& topology) -> QScriptValue {
-                return poop.value(topology);
+                return instance.value(topology);
             },
             [](const QScriptValue& value, T& topology) {
-                topology = poop.key(value.toString());
+                topology = instance.key(value.toString());
             }
         );
     }
+}
 
-    bool registerMetaTypes(QScriptEngine* engine) {
+void GraphicsScriptingInterface::registerMetaTypes(QScriptEngine* engine) {
-        qScriptRegisterSequenceMetaType<QVector<glm::uint32>>(engine);
+    qScriptRegisterSequenceMetaType<QVector<glm::uint32>>(engine);
 
-        qScriptRegisterMetaType(engine, qVectorUInt32ToScriptValue, qVectorUInt32FromScriptValue);
+    scriptable::registerQPointerMetaType<scriptable::ScriptableModel>(engine);
+    scriptable::registerQPointerMetaType<scriptable::ScriptableMesh>(engine);
+    scriptable::registerQPointerMetaType<scriptable::ScriptableMeshPart>(engine);
 
-        registerQPointerThing<scriptable::ScriptableModel>(engine);
+    scriptable::registerDebugEnum<graphics::Mesh::Topology>(engine, graphics::TOPOLOGIES);
-        registerQPointerThing<scriptable::ScriptableMesh>(engine);
+    scriptable::registerDebugEnum<gpu::Type>(engine, gpu::TYPES);
-        registerQPointerThing<scriptable::ScriptableMeshPart>(engine);
+    scriptable::registerDebugEnum<gpu::Semantic>(engine, gpu::SEMANTICS);
-        qScriptRegisterMetaType<QVector<scriptable::ScriptableMeshPointer>>(
+    scriptable::registerDebugEnum<gpu::Dimension>(engine, gpu::DIMENSIONS);
-            engine,
-            [](QScriptEngine* engine, const QVector<scriptable::ScriptableMeshPointer>& vector) -> QScriptValue {
-                return qScriptValueFromSequence(engine, vector);
-            },
-            [](const QScriptValue& array, QVector<scriptable::ScriptableMeshPointer>& result) {
-                qScriptValueToSequence(array, result);
-            }
-        );
-
-        registerDebugEnum<graphics::Mesh::Topology>(engine, graphics::TOPOLOGIES);
-        registerDebugEnum<gpu::Type>(engine, gpu::TYPES);
-        registerDebugEnum<gpu::Semantic>(engine, gpu::SEMANTICS);
-        registerDebugEnum<gpu::Dimension>(engine, gpu::DIMENSIONS);
-
-        return metaTypeIds.size();
-    }
 
+    Q_UNUSED(metaTypeIds);
 }
 
 #include "GraphicsScriptingInterface.moc"

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

@@ -20,6 +20,12 @@
 #include "ScriptableMesh.h"
 #include <DependencyManager.h>
 
+
+/**jsdoc
+ * The experimental Graphics API <em>(experimental)</em> lets you query and manage certain graphics-related structures (like underlying meshes and textures) from scripting.
+ * @namespace Graphics
+ */
+
 class GraphicsScriptingInterface : public QObject, public QScriptable, public Dependency {
     Q_OBJECT
 
@@ -29,15 +35,36 @@ public:
 
 public slots:
     /**jsdoc
-     * Returns the model/meshes associated with a UUID (entityID, overlayID, or avatarID)
+     * Returns a model reference object associated with the specified UUID ({@link EntityID}, {@link OverlayID}, or {@link AvatarID}).
      *
-     * @function GraphicsScriptingInterface.getModel
+     * @function Graphics.getModel
-     * @param {UUID} The objectID of the model whose meshes are to be retrieve
+     * @param {UUID} The objectID of the model whose meshes are to be retrieved.
+     * @return {Graphics.Model} the resulting Model object
      */
     scriptable::ScriptableModelPointer getModel(QUuid uuid);
+
     bool updateModel(QUuid uuid, const scriptable::ScriptableModelPointer& model);
+
     bool canUpdateModel(QUuid uuid, int meshIndex = -1, int partNumber = -1);
+
     scriptable::ScriptableModelPointer newModel(const scriptable::ScriptableMeshes& meshes);
+
+    /**jsdoc
+     * Create a new Mesh / Mesh Part with the specified data buffers.
+     *
+     * @function Graphics.newMesh
+     * @param {Graphics.IFSData} ifsMeshData Index-Faced Set (IFS) arrays used to create the new mesh.
+     * @return {Graphics.Mesh} the resulting Mesh / Mesh Part object
+     */
+    /**jsdoc
+    * @typedef {object} Graphics.IFSData
+    * @property {string} [name] - mesh name (useful for debugging / debug prints).
+    * @property {number[]} indices - vertex indices to use for the mesh faces.
+    * @property {Vec3[]} vertices - vertex positions (model space)
+    * @property {Vec3[]} [normals] - vertex normals (normalized)
+    * @property {Vec3[]} [colors] - vertex colors (normalized)
+    * @property {Vec2[]} [texCoords0] - vertex texture coordinates (normalized)
+    */
     scriptable::ScriptableMeshPointer newMesh(const QVariantMap& ifsMeshData);
 
 #ifdef SCRIPTABLE_MESH_TODO
@@ -58,6 +85,4 @@ private:
 
 };
 
-Q_DECLARE_METATYPE(scriptable::ModelProviderPointer)
-
 #endif // hifi_GraphicsScriptingInterface_h

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

@@ -28,6 +28,15 @@
 #include <graphics/Geometry.h>
 
 namespace scriptable {
+    /**jsdoc
+     * @typedef {object} Graphics.Mesh
+     * @property {Graphics.MeshPart[]} parts - Array of submesh part references.
+     * @property {string[]} attributeNames - Vertex attribute names (color, normal, etc.)
+     * @property {number} numParts - The number of parts contained in the mesh.
+     * @property {number} numIndices - Total number of vertex indices in the mesh.
+     * @property {number} numVertices - Total number of vertices in the Mesh.
+     * @property {number} numAttributes - Number of currently defined vertex attributes.
+     */
     class ScriptableMesh : public ScriptableMeshBase, QScriptable {
         Q_OBJECT
     public:
@@ -97,5 +106,3 @@ namespace scriptable {
 
 Q_DECLARE_METATYPE(scriptable::ScriptableMeshPointer)
 Q_DECLARE_METATYPE(QVector<scriptable::ScriptableMeshPointer>)
-Q_DECLARE_METATYPE(glm::uint32)
-Q_DECLARE_METATYPE(QVector<glm::uint32>)

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

@@ -21,6 +21,7 @@
 #include <graphics/GpuHelpers.h>
 #include <graphics/Geometry.h>
 
+
 QString scriptable::ScriptableMeshPart::toOBJ() {
     if (!getMeshPointer()) {
         if (context()) {
@@ -371,6 +372,7 @@ bool scriptable::ScriptableMeshPart::setIndices(const QVector<glm::uint32>& indi
     if (len != getNumVertices()) {
         context()->throwError(QString("setIndices: currently new indicies must be assign 1:1 across old indicies (indicies.size()=%1, numIndices=%2)")
                               .arg(len).arg(getNumIndices()));
+        return false;
     }
     auto mesh = getMeshPointer();
     auto indexBuffer = mesh->getIndexBuffer();
@@ -397,18 +399,24 @@ const graphics::Mesh::Part& scriptable::ScriptableMeshPart::getMeshPart() const
     return getMeshPointer()->getPartBuffer().get<graphics::Mesh::Part>(partIndex);
 }
 
+// FIXME: how we handle topology will need to be reworked if wanting to support TRIANGLE_STRIP, QUADS and QUAD_STRIP
 bool scriptable::ScriptableMeshPart::setTopology(graphics::Mesh::Topology topology) {
     if (!isValid()) {
         return false;
     }
     auto& part = getMeshPointer()->getPartBuffer().edit<graphics::Mesh::Part>(partIndex);
-    if (topology == graphics::Mesh::Topology::POINTS ||
+    switch (topology) {
-        topology == graphics::Mesh::Topology::LINES ||
+#ifdef DEV_BUILD
-        topology == graphics::Mesh::Topology::TRIANGLES) {
+    case graphics::Mesh::Topology::POINTS:
+    case graphics::Mesh::Topology::LINES:
+#endif
+    case graphics::Mesh::Topology::TRIANGLES:
         part._topology = topology;
         return true;
+    default:
+        context()->throwError("changing topology to " + graphics::toString(topology) + " is not yet supported");
+        return false;
     }
-    return false;
 }
 
 glm::uint32 scriptable::ScriptableMeshPart::getTopologyLength() const {
@@ -416,16 +424,23 @@ glm::uint32 scriptable::ScriptableMeshPart::getTopologyLength() const {
     case graphics::Mesh::Topology::POINTS: return 1;
     case graphics::Mesh::Topology::LINES: return 2;
     case graphics::Mesh::Topology::TRIANGLES: return 3;
+    case graphics::Mesh::Topology::QUADS: return 4;
     default: qCDebug(graphics_scripting) << "getTopologyLength -- unrecognized topology" << getTopology();
     }
     return 0;
 }
 
 QVector<glm::uint32> scriptable::ScriptableMeshPart::getFace(glm::uint32 faceIndex) const {
-    if (faceIndex < getNumFaces()) {
+    switch (getTopology()) {
-        return getIndices().mid(faceIndex * getTopologyLength(), getTopologyLength());
+    case graphics::Mesh::Topology::POINTS:
+    case graphics::Mesh::Topology::LINES:
+    case graphics::Mesh::Topology::TRIANGLES:
+    case graphics::Mesh::Topology::QUADS:
+        if (faceIndex < getNumFaces()) {
+            return getIndices().mid(faceIndex * getTopologyLength(), getTopologyLength());
+        }
+    default: return QVector<glm::uint32>();
     }
-    return QVector<glm::uint32>();
 }
 
 QVariantMap scriptable::ScriptableMeshPart::getPartExtents() const {

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

@@ -10,6 +10,18 @@
 #include "ScriptableMesh.h"
 
 namespace scriptable {
+    /**jsdoc
+     * @typedef {object} Graphics.MeshPart
+     * @property {number} partIndex - The part index (within the containing Mesh).
+     * @property {Graphics.Topology} topology - element interpretation (currently only 'triangles' is supported).
+     * @property {string[]} attributeNames - Vertex attribute names (color, normal, etc.)
+     * @property {number} numIndices - Number of vertex indices that this mesh part refers to.
+     * @property {number} numVerticesPerFace - Number of vertices per face (eg: 3 when topology is 'triangles').
+     * @property {number} numFaces - Number of faces represented by the mesh part (numIndices / numVerticesPerFace).
+     * @property {number} numVertices - Total number of vertices in the containing Mesh.
+     * @property {number} numAttributes - Number of currently defined vertex attributes.
+     */
+
     class ScriptableMeshPart : public QObject, QScriptable {
         Q_OBJECT
         Q_PROPERTY(bool valid READ isValid)
@@ -18,7 +30,8 @@ namespace scriptable {
         Q_PROPERTY(glm::uint32 baseVertexIndex READ getBaseVertexIndex WRITE setBaseVertexIndex)
         Q_PROPERTY(glm::uint32 lastVertexIndex READ getLastVertexIndex WRITE setLastVertexIndex)
         Q_PROPERTY(int numVerticesPerFace READ getTopologyLength)
-        Q_PROPERTY(graphics::Mesh::Topology topology READ getTopology WRITE setTopology)
+        // NOTE: making read-only for now (see also GraphicsScriptingInterface::newMesh and MeshPartPayload::drawCall)
+        Q_PROPERTY(graphics::Mesh::Topology topology READ getTopology)
 
         Q_PROPERTY(glm::uint32 numFaces READ getNumFaces)
         Q_PROPERTY(glm::uint32 numAttributes READ getNumAttributes)

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

@@ -15,6 +15,14 @@ class QScriptValue;
 namespace scriptable {
 
     using ScriptableMeshes = QVector<scriptable::ScriptableMeshPointer>;
+
+    /**jsdoc
+     * @typedef {object} Graphics.Model
+     * @property {Uuid} objectID - UUID of corresponding inworld object (if model is associated)
+     * @property {number} numMeshes - The number of submeshes contained in the model.
+     * @property {Graphics.Mesh[]} meshes - Array of submesh references.
+     */
+
     class ScriptableModel : public ScriptableModelBase {
         Q_OBJECT
         Q_PROPERTY(QUuid objectID MEMBER objectID CONSTANT)
@@ -45,8 +53,6 @@ namespace scriptable {
 
 }
 
-Q_DECLARE_METATYPE(scriptable::MeshPointer)
-Q_DECLARE_METATYPE(scriptable::WeakMeshPointer)
 Q_DECLARE_METATYPE(scriptable::ScriptableModelPointer)
 Q_DECLARE_METATYPE(scriptable::ScriptableModelBase)
 Q_DECLARE_METATYPE(scriptable::ScriptableModelBasePointer)

libraries/graphics/src/graphics/GpuHelpers.cpp

@@ -21,8 +21,7 @@ namespace graphics {
     };
 }
 namespace gpu {
-
+    DebugEnums<Type> TYPES{
-     DebugEnums<Type> TYPES{
         { Type::FLOAT, "float" },
         { Type::INT32, "int32" },
         { Type::UINT32, "uint32" },