address code review feedback

raytranuk · raytranuk · commit 5bc6cb1e8136 · 2020-08-19T17:13:41.000+01:00
diff --git a/src/graphics/program-lib/programs/node.js b/src/graphics/program-lib/programs/node.js
@@ -21,23 +21,7 @@ var node = {
         return key;
     },
 
-    createShaderDefinition: function (device, options) {
-        // generate graph
-        var rootDeclGLSL = options.shaderGraph.generateRootDeclGlsl();
-        var rootCallGLSL = options.shaderGraph.generateRootCallGlsl();
-
-        // GENERATE ATTRIBUTES
-        var attributes = {
-            vertex_position: SEMANTIC_POSITION,
-            vertex_normal: SEMANTIC_NORMAL,
-            vertex_color: SEMANTIC_COLOR,
-            vertex_texCoord0: SEMANTIC_TEXCOORD0
-        };
-        if (options.skin) {
-            attributes.vertex_boneWeights = SEMANTIC_BLENDWEIGHT;
-            attributes.vertex_boneIndices = SEMANTIC_BLENDINDICES;
-        }
-
+    _generateVertexShader: function (device, options, rootDeclGLSL, rootCallGLSL) {
         var chunks = shaderChunks;
 
         // GENERATE VERTEX SHADER
@@ -109,6 +93,12 @@ var node = {
             vshader = startCode + vshader;
         }
 
+        return vshader;
+    },
+
+    _generateFragmentShader: function (device, options, rootDeclGLSL, rootCallGLSL) {
+        var chunks = shaderChunks;
+
         // GENERATE FRAGMENT SHADER
         if (options.forceFragmentPrecision && options.forceFragmentPrecision != "highp" &&
             options.forceFragmentPrecision !== "mediump" && options.forceFragmentPrecision !== "lowp")
@@ -119,7 +109,7 @@ var node = {
             if (options.forceFragmentPrecision === "mediump" && device.maxPrecision === "lowp") options.forceFragmentPrecision = "lowp";
         }
 
-        code = '';
+        var code = '';
 
         if (device.webgl2) {
             code += versionCode(device);
@@ -179,7 +169,30 @@ var node = {
 
         code += end();
 
-        var fshader = code;
+        return code;
+    },
+
+    createShaderDefinition: function (device, options) {
+        // generate graph
+        // TODO: support generation of shader variants based on options
+        var rootDeclGLSL = options.shaderGraph.generateRootDeclGlsl();
+        var rootCallGLSL = options.shaderGraph.generateRootCallGlsl();
+
+        // GENERATE ATTRIBUTES
+        var attributes = {
+            vertex_position: SEMANTIC_POSITION,
+            vertex_normal: SEMANTIC_NORMAL,
+            vertex_color: SEMANTIC_COLOR,
+            vertex_texCoord0: SEMANTIC_TEXCOORD0
+        };
+        if (options.skin) {
+            attributes.vertex_boneWeights = SEMANTIC_BLENDWEIGHT;
+            attributes.vertex_boneIndices = SEMANTIC_BLENDINDICES;
+        }
+
+        var vshader = this._generateVertexShader(device, options, rootDeclGLSL, rootCallGLSL);
+
+        var fshader = this._generateFragmentShader(device, options, rootDeclGLSL, rootCallGLSL);
 
         return {
             attributes: attributes,
diff --git a/src/resources/node-material.js b/src/resources/node-material.js
@@ -8,12 +8,13 @@ import { NodeMaterial } from '../scene/materials/node-material.js';
  * @class
  * @name pc.NodeMaterialBinder
  * @classdesc Resource binder used for binding {@link pc.NodeMaterial} resources.
- * @param {pc.Application} app - The running {@link pc.Application}.
+ * @param {pc.AssetRegistry} assets - The asset registry.
+ * @param {pc.GraphicsDevice} device - The graphics device of the application.
  * @param {pc.JsonNodeMaterialParser} parser - JSON parser for {@link pc.NodeMaterial} owned by global {@link pc.MaterialHandler}
  */
-function NodeMaterialBinder(app, parser) {
-    this._assets = app.assets;
-    this._device = app.graphicsDevice;
+function NodeMaterialBinder(assets, device, parser) {
+    this._assets = assets;
+    this._device = device;
     this._parser = parser;
 
     this._placeholderGraph = null;
diff --git a/src/scene/materials/node-material.js b/src/scene/materials/node-material.js
@@ -254,7 +254,8 @@ Object.assign(NodeMaterial.prototype, {
         } else if (typeof(value) === 'number') {
             graphVar = { type: type, name: name, valueX: value };
         } else {
-            graphVar = { type: type, name: name };
+            // currently unsupported value type - should not be possible in editor 
+            // TODO: deal with this case when script interface is completed.
         }
 
         this.graphData.graphVars.push(graphVar);
@@ -389,12 +390,14 @@ Object.assign(NodeMaterial.prototype, {
         return callString;
     },
 
-    _getGraphVarByName: function (name) {
+    // this is currently not used - but will be used by the shadergraph script interface
+    // TODO: re-enable and optimize using transient name map
+    // _getGraphVarByName: function (name) {
         // convienient but not fast - TODO: optimize?
-        return this.graphData.graphVars.filter(function (graphVar) {
-            return graphVar.name === name;
-        })[0];
-    },
+        // return this.graphData.graphVars.filter(function (graphVar) {
+        //    return graphVar.name === name;
+        // })[0];
+    // },
 
     _generateSubGraphFuncs: function (depGraphFuncs, depGraphVarList) {
         var i;
