Fixed problem in o3dConverter where it was not outputting all of the

vertex data in JSON format. Fixed problems in effect.js where GLSL
code path needed to name the texture coordinate attributes with the
semantic names and not the names like "diffuseUV" because otherwise
the backend did not know how to hook them up. Fixed problem with cube
map faces being flipped and bug in binding cube map textures.

At this point o3d-webgl can load the converted cube and teapot, though
the teapot is currently missing the bump map. Tested O3D plugin with
this sample to ensure no regression.

BUG=none
TEST=none

Review URL: http://codereview.chromium.org/1729012

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@45495 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 118 insertions, 33 deletions

diff --git a/o3d/samples/o3d-webgl/client.js b/o3d/samples/o3d-webgl/client.js
index 4e13450..fb54b79 100644
--- a/o3d/samples/o3d-webgl/client.js
+++ b/o3d/samples/o3d-webgl/client.js
@@ -752,7 +752,12 @@ o3d.Client.prototype.clearTickCallback = function() {
  */
 o3d.Client.prototype.setErrorCallback =
     function(error_callback) {
-  this.error_callback = error_callback;
+  // Other code expects to not see a null error callback.
+  if (error_callback) {
+    this.error_callback = error_callback;
+  } else {
+    this.error_callback = function(string) {};
+  }
 };
 
 
@@ -764,7 +769,7 @@ o3d.Client.prototype.setErrorCallback =
  * time or if you call ClearErrorCallback.
  */
 o3d.Client.prototype.clearErrorCallback = function() {
-  this.error_callback = null;
+  this.setErrorCallback(null);
 };
 
 
diff --git a/o3d/samples/o3d-webgl/pack.js b/o3d/samples/o3d-webgl/pack.js
index a9f79bb..4a72feb 100644
--- a/o3d/samples/o3d-webgl/pack.js
+++ b/o3d/samples/o3d-webgl/pack.js
@@ -205,6 +205,7 @@ o3d.Pack.prototype.createTexture2D =
   texture.height = height;
   texture.levels = levels;
   texture.texture_ = this.gl.createTexture();
+  texture.texture_target_ = this.gl.TEXTURE_2D;
 
   if (width != undefined && height != undefined) {
     this.gl.bindTexture(this.gl.TEXTURE_2D, texture.texture_);
@@ -246,6 +247,7 @@ o3d.Pack.prototype.createTextureCUBE =
   var texture = this.createObject('TextureCUBE');
   texture.edgeLength = edgeLength;
   texture.texture_ = this.gl.createTexture();
+  texture.texture_target_ = this.gl.TEXTURE_CUBE_MAP;
 
   this.gl.bindTexture(this.gl.TEXTURE_CUBE_MAP, texture.texture_);
   for (var ii = 0; ii < 6; ++ii) {
@@ -412,15 +414,14 @@ o3d.Pack.prototype.createBitmapsFromRawData =
 
   canvas.width = bitmap.width;
   canvas.height = bitmap.height;
-
-  bitmap.canvas_ = canvas;
   var context = canvas.getContext('2d');
-  // Flip it.
-  context.translate(0, bitmap.height);
-  context.scale(1, -1);
   context.drawImage(raw_data.image_,
       0, 0, bitmap.width, bitmap.height);
 
+  bitmap.canvas_ = canvas;
+  // Most images require a vertical flip.
+  bitmap.flipVertically();
+
   // TODO(petersont): I'm not sure how to get the format.
   bitmap.format = o3d.Texture.ARGB8;
   bitmap.numMipmaps = 1;
diff --git a/o3d/samples/o3d-webgl/param.js b/o3d/samples/o3d-webgl/param.js
index a32d004..731ae21 100644
--- a/o3d/samples/o3d-webgl/param.js
+++ b/o3d/samples/o3d-webgl/param.js
@@ -820,12 +820,14 @@ o3d.ParamSampler.prototype.applyToLocation = function(gl, location) {
   gl.activeTexture(gl.TEXTURE0 + i);
 
   var value = null;
+  var target = 0;
 
   if (this.value && this.value.texture && this.value.texture.texture_) {
     value = this.value.texture.texture_;
+    target = this.value.texture.texture_target_;
   }
 
-  gl.bindTexture(gl.TEXTURE_2D, value);
+  gl.bindTexture(target, value);
   gl.uniform1i(location, i);
   o3d.Param.texture_index_++;
 };
diff --git a/o3d/samples/o3d-webgl/texture.js b/o3d/samples/o3d-webgl/texture.js
index 53214d0..9f5db11 100644
--- a/o3d/samples/o3d-webgl/texture.js
+++ b/o3d/samples/o3d-webgl/texture.js
@@ -57,12 +57,18 @@ o3d.Texture = function() {
   this.alphaIsOne = true;
 
   /**
-   * The the associated gl texture.
+   * The associated gl texture.
    * @type {WebGLTexture}
    * @private
    */
   this.texture_ = null;
 
+  /**
+   * The associated GL texture target: TEXTURE_2D or TEXTURE_CUBE_MAP.
+   * @type {number}
+   * @private
+   */
+  this.texture_target_ = 0;
 };
 o3d.inherit('Texture', 'ParamObject');
 
@@ -274,7 +280,7 @@ o3d.Texture2D.prototype.setFromBitmap =
   this.gl.texImage2D(this.gl.TEXTURE_2D,
                      0,  // Level.
                      bitmap.canvas_,
-                     bitmap.defer_flip_verically_to_texture_);
+                     bitmap.defer_flip_vertically_to_texture_);
   if (bitmap.defer_mipmaps_to_texture_) {
     this.generateMips();
   }
diff --git a/o3d/samples/o3djs/effect.js b/o3d/samples/o3djs/effect.js
index 2213627..7c87cef 100644
--- a/o3d/samples/o3djs/effect.js
+++ b/o3d/samples/o3djs/effect.js
@@ -77,6 +77,7 @@ o3djs.effect.o3d = {
   VARYING_DECLARATION_PREFIX: '',
   VERTEX_VARYING_PREFIX: 'output.',
   PIXEL_VARYING_PREFIX: 'input.',
+  TEXTURE: 'tex',
   BEGIN_IN_STRUCT: 'struct InVertex {\n',
   BEGIN_OUT_STRUCT: 'struct OutVertex {\n',
   END_STRUCT: '};\n'
@@ -103,14 +104,31 @@ o3djs.effect.glsl = {
     VARYING_DECLARATION_PREFIX: 'v_',
     VERTEX_VARYING_PREFIX: 'v_',
     PIXEL_VARYING_PREFIX: 'v_',
+    TEXTURE: 'texture',
     BEGIN_IN_STRUCT: '',
     BEGIN_OUT_STRUCT: '',
-    END_STRUCT: ''
+    END_STRUCT: '',
+    // Only used in GLSL version of getAttributeName_.
+    semanticNameMap: {
+      'POSITION'  : 'position',
+      'NORMAL'    : 'normal',
+      'TANGENT'   : 'tangent',
+      'BINORMAL'  : 'binormal',
+      'COLOR'     : 'color',
+      'TEXCOORD0' : 'texCoord0',
+      'TEXCOORD1' : 'texCoord1',
+      'TEXCOORD2' : 'texCoord2',
+      'TEXCOORD3' : 'texCoord3',
+      'TEXCOORD4' : 'texCoord4',
+      'TEXCOORD5' : 'texCoord5',
+      'TEXCOORD6' : 'texCoord6',
+      'TEXCOORD7' : 'texCoord7'
+    }
 };
 
 
 /**
- * The string that goes between the stream name and the semicolon too indicate
+ * The string that goes between the stream name and the semicolon to indicate
  * the semantic.
  * @param {string} name Name of the semantic.
  * @return {string}
@@ -121,7 +139,7 @@ o3djs.effect.glsl.semanticSuffix = function(name) {
 
 
 /**
- * The string that goes between the stream name and the semicolon too indicate
+ * The string that goes between the stream name and the semicolon to indicate
  * the semantic.
  * @param {string} name Name of the semantic.
  * @return {string}
@@ -132,6 +150,26 @@ o3djs.effect.o3d.semanticSuffix = function(name) {
 
 
 /**
+ * Attribute variables in GLSL need to be named by their semantic in
+ * order for the implementation to hook them up correctly.
+ * @private
+ */
+o3djs.effect.glsl.getAttributeName_ = function(name, semantic) {
+  var p = o3djs.effect;
+  return p.semanticNameMap[semantic];
+};
+
+
+/**
+ * This passes through the name in the Cg implementation.
+ * @private
+ */
+o3djs.effect.o3d.getAttributeName_ = function(name, semantic) {
+  return name;
+};
+
+
+/**
  * Generates code to multiply two things.
  * @param {string} a One multiplicand.
  * @param {string} b The other multiplicand.
@@ -366,7 +404,7 @@ o3djs.effect.buildAttributeDecls =
     str += o3djs.effect.ATTRIBUTE + o3djs.effect.FLOAT3 + ' ' + 'normal' +
     o3djs.effect.semanticSuffix('NORMAL') + ';\n';
   }
-  str += o3djs.effect.buildTexCoords(material) +
+  str += o3djs.effect.buildTexCoords(material, false) +
          o3djs.effect.buildBumpInputCoords(bumpSampler) +
          o3djs.effect.END_STRUCT;
   return str;
@@ -397,7 +435,7 @@ o3djs.effect.buildVaryingDecls =
       p.VARYING + p.FLOAT4 + ' ' +
       p.VARYING_DECLARATION_PREFIX + 'position' +
       p.semanticSuffix('POSITION') + ';\n' +
-      p.buildTexCoords(material) +
+      p.buildTexCoords(material, true) +
       p.buildBumpOutputCoords(bumpSampler);
   if (diffuse || specular) {
     str += p.VARYING + p.FLOAT3 + ' ' +
@@ -430,18 +468,34 @@ o3djs.effect.interpolant_ = 0;
 
 /**
  * Builds the texture coordinate declaration for a given color input
- * (usually emissive, anmbient, diffuse or specular).  If the color
+ * (usually emissive, ambient, diffuse or specular).  If the color
  * input does not have a sampler, no TEXCOORD declaration is built.
  * @param {!o3d.Material} material The material to inspect.
+ * @param {boolean} varying Whether these vertex declarations should
+ *     be written as varying values.
  * @param {string} name The name of the color input.
  * @return {string} The code for the texture coordinate declaration.
  */
-o3djs.effect.buildTexCoord = function(material, name) {
+o3djs.effect.buildTexCoord = function(material, varying, name) {
   var p = o3djs.effect;
+  // In the GLSL version we need to name the incoming attributes by
+  // the semantic name in order for them to get hooked up correctly.
   if (material.getParam(name + 'Sampler')) {
-    return '  ' + p.FLOAT2 + ' ' + name + 'UV' +
-        p.semanticSuffix(
-            'TEXCOORD' + p.interpolant_++ + '') + ';\n';
+    if (varying) {
+      return '  ' + p.VARYING + p.FLOAT2 + ' ' +
+          p.VARYING_DECLARATION_PREFIX + name + 'UV' +
+          p.semanticSuffix(
+              'TEXCOORD' + p.interpolant_++ + '') + ';\n';
+    } else {
+      var desiredName = name + 'UV';
+      var semantic = 'TEXCOORD' + p.interpolant_++;
+      var outputName = p.getAttributeName_(desiredName, semantic);
+      if (p.semanticNameMap) {
+        p.nameToSemanticMap_[desiredName] = semantic;
+      }
+      return '  ' + p.ATTRIBUTE + p.FLOAT2 + ' ' + outputName +
+          p.semanticSuffix(semantic) + ';\n';
+    }
   } else {
     return '';
   }
@@ -451,15 +505,20 @@ o3djs.effect.buildTexCoord = function(material, name) {
  * Builds all the texture coordinate declarations for a vertex attribute
  * declaration.
  * @param {!o3d.Material} material The material to inspect.
+ * @param {boolean} varying Whether these vertex declarations should
+ *     be written as varying values.
  * @return {string} The code for the texture coordinate declarations.
  */
-o3djs.effect.buildTexCoords = function(material) {
+o3djs.effect.buildTexCoords = function(material, varying) {
   var p = o3djs.effect;
   p.interpolant_ = 0;
-  return p.buildTexCoord(material, 'emissive') +
-         p.buildTexCoord(material, 'ambient') +
-         p.buildTexCoord(material, 'diffuse') +
-         p.buildTexCoord(material, 'specular');
+  if (!varying) {
+    p.nameToSemanticMap_ = {};
+  }
+  return p.buildTexCoord(material, varying, 'emissive') +
+         p.buildTexCoord(material, varying, 'ambient') +
+         p.buildTexCoord(material, varying, 'diffuse') +
+         p.buildTexCoord(material, varying, 'specular');
 };
 
 
@@ -476,8 +535,14 @@ o3djs.effect.buildTexCoords = function(material) {
 o3djs.effect.buildUVPassthrough = function(material, name) {
   var p = o3djs.effect;
   if (material.getParam(name + 'Sampler')) {
-    return '  ' + p.VERTEX_VARYING_PREFIX + name + 'UV = ' +
-        p.ATTRIBUTE_PREFIX + name + 'UV;\n';
+    var sourceName = name + 'UV';
+    var destName = sourceName;
+    var semantic = p.nameToSemanticMap_[sourceName];
+    if (semantic) {
+      sourceName = p.getAttributeName_(sourceName, semantic);
+    }
+    return '  ' + p.VERTEX_VARYING_PREFIX + destName + ' = ' +
+        p.ATTRIBUTE_PREFIX + sourceName + ';\n';
   } else {
     return '';
   }
@@ -493,6 +558,11 @@ o3djs.effect.buildUVPassthrough = function(material, name) {
  */
 o3djs.effect.buildUVPassthroughs = function(material) {
   var p = o3djs.effect;
+  // TODO(petersont): in the GLSL implementation we need to generate
+  // the code for these attributes before we can pass their values
+  // through, because in this implementation their names must be their
+  // semantics (i.e., "texCoord4") rather than these chosen names.
+  // Currently bumpUV is the only one which does not obey this rule.
   return p.buildUVPassthrough(material, 'emissive') +
          p.buildUVPassthrough(material, 'ambient') +
          p.buildUVPassthrough(material, 'diffuse') +
@@ -839,7 +909,7 @@ o3djs.effect.buildStandardShaderString = function(material,
     if (samplerParam) {
       var type = getSamplerType(samplerParam);
       descriptions.push(name + type + 'Texture');
-      return 'sampler' + type + ' ' + name + 'Sampler;\n'
+      return 'uniform sampler' + type + ' ' + name + 'Sampler;\n'
     } else if (opt_addColorParam) {
       descriptions.push(name + 'Color');
       return 'uniform ' + p.FLOAT4 + ' ' + name + ';\n';
@@ -862,9 +932,9 @@ o3djs.effect.buildStandardShaderString = function(material,
     var samplerParam = material.getParam(name + 'Sampler');
     if (samplerParam) {
       var type = getSamplerType(samplerParam);
-      return '  ' + p.FLOAT4 + ' ' + name + ' = tex' + type +
+      return '  ' + p.FLOAT4 + ' ' + name + ' = ' + p.TEXTURE + type +
              '(' + name + 'Sampler, ' +
-             p.ATTRIBUTE_PREFIX + name + 'UV);\n'
+             p.PIXEL_VARYING_PREFIX + name + 'UV);\n'
     } else {
       return '';
     }
diff --git a/o3d/serializer/cross/serializer.cc b/o3d/serializer/cross/serializer.cc
index 0aff9de..ccdc7ef 100644
--- a/o3d/serializer/cross/serializer.cc
+++ b/o3d/serializer/cross/serializer.cc
@@ -458,11 +458,12 @@ class CustomVisitor : public VisitorBase<CustomVisitor> {
         Buffer* buffer = field->buffer();
         if (buffer) {
           unsigned num_elements = buffer->num_elements();
+          unsigned num_floats = num_elements * field->num_components();
           scoped_array<float> data(
-              new float[num_elements * field->num_components()]);
+              new float[num_floats]);
           field->GetAsFloats(
               0, data.get(), field->num_components(), num_elements);
-          for (size_t jj = 0; jj < num_elements; ++jj) {
+          for (size_t jj = 0; jj < num_floats; ++jj) {
             Serialize(writer_, data[jj]);
           }
         }
@@ -840,7 +841,7 @@ Serializer::Serializer(ServiceLocator* service_locator,
       new PropertiesVisitor(writer_, binary);
   sections_[CUSTOM_SECTION].name_ = "custom";
   sections_[CUSTOM_SECTION].visitor_ = new CustomVisitor(
-      writer_, binary ? &binary_archive_manager_ : false);
+      writer_, binary ? &binary_archive_manager_ : NULL);
   param_visitor_ = new ParamVisitor(writer_);
   binary_visitor_ = binary ? new BinaryVisitor(&binary_archive_manager_) : NULL;
 }