Add support for Param Arrays for Effect parameters.

Arrays of floats, float2, float3, float4, Matrix4,
int, bool and sampler are all supported.

I'll enable the test for selenium and check in
a screenshot in another build.

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

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

5 files changed, 746 insertions, 101 deletions

diff --git a/o3d/core/cross/effect.cc b/o3d/core/cross/effect.cc
index 9c271fb..5aba914 100644
--- a/o3d/core/cross/effect.cc
+++ b/o3d/core/cross/effect.cc
@@ -125,12 +125,11 @@ void Effect::CreateSpecifiedParameters(ParamObject* param_object, bool sas) {
           param = param_object->CreateParamByClass(
               param_info.name(),
               param_info.sas_class_type() ? param_info.sas_class_type() :
-              param_info.class_type());
+                                            param_info.class_type());
         } else {
           // Array type
-          param = param_object->CreateParamByClass(
-              param_info.name(),
-              ParamParamArray::GetApparentClass());
+          param =
+              param_object->CreateParam<ParamParamArray>(param_info.name());
         }
         if (!param) {
           errors += String(errors.empty() ? "" : "\n") +
@@ -188,7 +187,7 @@ bool GetIdentifierAfterString(const String& original,
 
 }  // anonymous namespace
 
-// TODO: Replace this with the runtime shader parser.
+// TODO(gman): Replace this with the runtime shader parser.
 //    For now it's very stupid. It requires the word "techinque" not appear
 //    anywhere inside the file. Then it searches for
 //
diff --git a/o3d/core/cross/effect_test.cc b/o3d/core/cross/effect_test.cc
index af028e6..b3a245c 100644
--- a/o3d/core/cross/effect_test.cc
+++ b/o3d/core/cross/effect_test.cc
@@ -34,6 +34,7 @@
 #include "core/cross/effect.h"
 #include "core/cross/primitive.h"
 #include "core/cross/standard_param.h"
+#include "core/cross/param_array.h"
 #include "core/cross/stream.h"
 #include "tests/common/win/testing_common.h"
 
@@ -161,7 +162,7 @@ uint32 kIndexBlock[4] = {
   0, 1, 2, 3
 };
 
-bool IsExpectedParam(const EffectParameterInfo& info) {
+bool IsExpectedParamInfo(const EffectParameterInfo& info) {
   for (unsigned ii = 0; ii < arraysize(expected_params); ++ii) {
     const ParamInfo& expected_info = expected_params[ii];
     if (info.name().compare(expected_info.name) == 0) {
@@ -187,7 +188,7 @@ bool IsExpectedStream(const EffectStreamInfo& info) {
 }  // anonymous namespace.
 
 TEST_F(EffectTest, LogOpenGLCalls) {
-  // TODO: Find a way to implement a Mocklog object under
+  // TODO(o3d): Find a way to implement a Mocklog object under
   // Googleclient. The code would be of the usual form:
   //
   //    ScopedMockLog log;
@@ -215,7 +216,7 @@ TEST_F(EffectTest, LogOpenGLCalls) {
   material->set_effect(fx);
   primitive->set_material(material);
 
-  // TODO: Test the log file.
+  // TODO(o3d): Test the log file.
 
   // Clean up.
   object_manager()->DestroyPack(pack);
@@ -333,7 +334,75 @@ TEST_F(EffectTest, GetEffectParameters) {
   EXPECT_EQ(arraysize(expected_params), info.size());
 
   for (EffectParameterInfoArray::size_type ii = 0; ii < info.size(); ++ii) {
-    EXPECT_TRUE(IsExpectedParam(info[ii]));
+    EXPECT_TRUE(IsExpectedParamInfo(info[ii]));
+  }
+
+  // Clean up.
+  object_manager()->DestroyPack(pack);
+}
+
+TEST_F(EffectTest, CreateUniformParameters) {
+  Pack* pack = object_manager()->CreatePack();
+  ASSERT_TRUE(pack != NULL);
+
+  // load an effect
+  Effect *fx = pack->Create<Effect>();
+  ASSERT_TRUE(fx != NULL);
+  EXPECT_TRUE(fx->LoadFromFXString(String(kLambertEffect)));
+
+  ParamObject* param_object = pack->Create<ParamObject>();
+  ASSERT_TRUE(param_object != NULL);
+
+  // Check that we get the correct params
+  fx->CreateUniformParameters(param_object);
+
+  for (unsigned ii = 0; ii < arraysize(expected_params); ++ii) {
+    const ParamInfo& expected_info = expected_params[ii];
+    Param* param = param_object->GetUntypedParam(expected_info.name);
+    if (expected_info.sas_type) {
+      ASSERT_TRUE(param == NULL);
+    } else {
+      ASSERT_TRUE(param != NULL);
+      if (expected_info.num_elements > 0) {
+        ASSERT_TRUE(param->IsA(ParamParamArray::GetApparentClass()));
+      } else {
+        EXPECT_TRUE(param->IsA(expected_info.type));
+      }
+    }
+  }
+
+  // Clean up.
+  object_manager()->DestroyPack(pack);
+}
+
+TEST_F(EffectTest, CreateSASParameters) {
+  Pack* pack = object_manager()->CreatePack();
+  ASSERT_TRUE(pack != NULL);
+
+  // load an effect
+  Effect *fx = pack->Create<Effect>();
+  ASSERT_TRUE(fx != NULL);
+  EXPECT_TRUE(fx->LoadFromFXString(String(kLambertEffect)));
+
+  ParamObject* param_object = pack->Create<ParamObject>();
+  ASSERT_TRUE(param_object != NULL);
+
+  // Check that we get the correct params
+  fx->CreateSASParameters(param_object);
+
+  for (unsigned ii = 0; ii < arraysize(expected_params); ++ii) {
+    const ParamInfo& expected_info = expected_params[ii];
+    Param* param = param_object->GetUntypedParam(expected_info.name);
+    if (expected_info.sas_type) {
+      ASSERT_TRUE(param != NULL);
+      if (expected_info.num_elements > 0) {
+        ASSERT_TRUE(param->IsA(ParamParamArray::GetApparentClass()));
+      } else {
+        EXPECT_TRUE(param->IsA(expected_info.sas_type));
+      }
+    } else {
+      ASSERT_TRUE(param == NULL);
+    }
   }
 
   // Clean up.
diff --git a/o3d/core/cross/gl/effect_gl.cc b/o3d/core/cross/gl/effect_gl.cc
index 9fe940e..f20d704 100644
--- a/o3d/core/cross/gl/effect_gl.cc
+++ b/o3d/core/cross/gl/effect_gl.cc
@@ -75,6 +75,7 @@ static const ObjectBase::Class* CgTypeToParamType(CGtype cg_type) {
     case CG_FLOAT2      : return ParamFloat2::GetApparentClass();
     case CG_FLOAT3      : return ParamFloat3::GetApparentClass();
     case CG_FLOAT4      : return ParamFloat4::GetApparentClass();
+    case CG_INT         : return ParamInteger::GetApparentClass();
     case CG_INT1        : return ParamInteger::GetApparentClass();
     case CG_FLOAT4x4    : return ParamMatrix4::GetApparentClass();
     case CG_BOOL        :
@@ -82,6 +83,7 @@ static const ObjectBase::Class* CgTypeToParamType(CGtype cg_type) {
     case CG_SAMPLER     :
     case CG_SAMPLER1D   :
     case CG_SAMPLER2D   :
+    case CG_SAMPLER3D   :
     case CG_SAMPLERCUBE : return ParamSampler::GetApparentClass();
     default : {
       DLOG(ERROR) << "Cannot convert CGtype "
@@ -204,7 +206,7 @@ bool EffectGL::LoadFromFXString(const String& effect) {
   String vertex_shader_entry_point;
   String fragment_shader_entry_point;
   MatrixLoadOrder matrix_load_order;
-  // TODO: Check for failure once shader parser is in.
+  // TODO(gman): Check for failure once shader parser is in.
   if (!ValidateFX(effect,
                   &vertex_shader_entry_point,
                   &fragment_shader_entry_point,
@@ -309,7 +311,7 @@ bool EffectGL::LoadFromFXString(const String& effect) {
     return false;
   }
 
-  // TODO: remove this (OLD path for textures).
+  // TODO(o3d): remove this (OLD path for textures).
   FillSamplerToTextureMap(effect);
 
   CHECK_GL_ERROR();
@@ -359,7 +361,7 @@ void EffectGL::FillSamplerToTextureMap(const String &effect) {
   cgDestroyEffect(cg_effect);
 }
 
-// TODO: remove this (OLD path for textures).
+// TODO(o3d): remove this (OLD path for textures).
 String EffectGL::GetTextureNameFromSamplerParamName(
     const String &sampler_name) {
   std::map<String, String>::iterator it =
@@ -428,12 +430,13 @@ void EffectGL::GetShaderParamInfo(
     // by keeping an internal collection of Texture-Sampler mappings
     // that is built here, so we can later to do the reverse lookup.
     //
-    // TODO: This will not solve the one-to-many problem of one
+    // TODO(o3d): This will not solve the one-to-many problem of one
     // Texture being used by many Sampler params, but it's enough to get
     // us up and running.
-    // TODO: Once we start using samplers exclusively, this special
-    // treatment of textures should go away. For the time being though, we
-    // do end up creating a texture param on the param_object.
+    //
+    // TODO(o3d): Once we start using samplers exclusively, this special
+    // treatment of textures should go away. For the time being though, we do
+    // end up creating a texture param on the param_object.
     if (cg_type == CG_SAMPLER1D ||
         cg_type == CG_SAMPLER2D ||
         cg_type == CG_SAMPLER3D ||
@@ -549,7 +552,7 @@ void EffectGL::GetStreamInfo(
 // Loop over all the CG_SAMPLER objects and attach the GLuint handle for the
 // GL texture object that we discovered earlier. Then execute the
 // CGstateassignments in the sampler_state to set up the texture unit.
-// TODO: remove this (OLD path for textures).
+// TODO(o3d): remove this (OLD path for textures).
 void EffectGL::SetTexturesFromEffect(ParamCacheGL* param_cache_gl) {
   DLOG_FIRST_N(INFO, kNumLoggedEvents) << "EffectGL EnableTexturesFromEffect";
   ParamCacheGL::SamplerParameterMap& map = param_cache_gl->sampler_map();
@@ -610,7 +613,7 @@ void EffectGL::PrepareForDraw(ParamCacheGL* param_cache_gl) {
     cgGLBindProgram(cg_fragment_);
     UpdateShaderUniformsFromEffect(param_cache_gl);
 
-    // TODO: remove this (OLD path for textures).
+    // TODO(o3d): remove this (OLD path for textures).
     SetTexturesFromEffect(param_cache_gl);
   } else {
     DLOG_FIRST_N(ERROR, kNumLoggedEvents)
diff --git a/o3d/core/cross/gl/param_cache_gl.cc b/o3d/core/cross/gl/param_cache_gl.cc
index b91245e..c950f3f 100644
--- a/o3d/core/cross/gl/param_cache_gl.cc
+++ b/o3d/core/cross/gl/param_cache_gl.cc
@@ -179,8 +179,7 @@ class EffectParamHandlerForSamplersGL : public EffectParamHandlerGL {
     SamplerGL* sampler_gl = down_cast<SamplerGL*>(param_->value());
     if (!sampler_gl) {
       // Use the error sampler.
-      sampler_gl = down_cast<SamplerGL*>(
-          renderer->error_sampler());
+      sampler_gl = down_cast<SamplerGL*>(renderer->error_sampler());
       // If no error texture is set then generate an error.
       if (!renderer->error_texture()) {
         O3D_ERROR(param_->service_locator())
@@ -191,41 +190,220 @@ class EffectParamHandlerForSamplersGL : public EffectParamHandlerGL {
   }
   virtual void ResetEffectParam(RendererGL* renderer, CGparameter cg_param) {
     SamplerGL* sampler_gl = down_cast<SamplerGL*>(param_->value());
-    if (sampler_gl) {
-      sampler_gl->ResetTexture(cg_param);
+    if (!sampler_gl) {
+      sampler_gl = down_cast<SamplerGL*>(renderer->error_sampler());
     }
+    sampler_gl->ResetTexture(cg_param);
   }
  private:
   ParamSampler* param_;
 };
 
-class EffectParamFloatArrayHandlerGL : public EffectParamHandlerGL {
+template <typename T>
+class EffectParamArrayHandlerGL : public EffectParamHandlerGL {
  public:
-  explicit EffectParamFloatArrayHandlerGL(ParamParamArray* param)
+  explicit EffectParamArrayHandlerGL(ParamParamArray* param)
       : param_(param) {
   }
-  virtual void SetEffectParam(RendererGL* renderer, CGparameter cg_param);
+  virtual void SetEffectParam(RendererGL* renderer, CGparameter cg_param) {
+    ParamArray* param = param_->value();
+    if (param) {
+      int size = cgGetArraySize(cg_param, 0);
+      if (size != static_cast<int>(param->size())) {
+        O3D_ERROR(param->service_locator())
+            << "number of params in ParamArray does not match number of params "
+            << "needed by shader array";
+      } else {
+        for (int i = 0; i < size; ++i) {
+          Param* untyped_element = param->GetUntypedParam(i);
+          // TODO(gman): Make this check happen when building the param cache.
+          //    To do that would require that ParamParamArray mark it's owner
+          //    as changed if a Param in it's ParamArray changes.
+          if (untyped_element->IsA(T::GetApparentClass())) {
+            CGparameter cg_element = cgGetArrayParameter(cg_param, i);
+            SetElement(cg_element, down_cast<T*>(untyped_element));
+          } else {
+            O3D_ERROR(param->service_locator())
+                << "Param in ParamArray at index " << i << " is not a "
+                << T::GetApparentClassName();
+          }
+        }
+      }
+    }
+  }
+  void SetElement(CGparameter cg_element, T* param);
+
  private:
   ParamParamArray* param_;
 };
 
-void EffectParamFloatArrayHandlerGL::SetEffectParam(
-    RendererGL* renderer,
-    CGparameter cg_param) {
-  ParamArray* param = param_->value();
-  if (param) {
-    int cg_size = cgGetArraySize(cg_param, 0);
-    int size = std::min(static_cast<int>(param->size()), cg_size);
-    for (int i = 0; i < size; ++i) {
-      ParamFloat* element = param->GetParam<ParamFloat>(i);
-      CGparameter cg_element = cgGetArrayParameter(cg_param, i);
-      if (element) {
-        cgSetParameter1f(cg_element, element->value());
+template <bool column_major>
+class EffectParamArrayMatrix4HandlerGL : public EffectParamHandlerGL {
+ public:
+  explicit EffectParamArrayMatrix4HandlerGL(ParamParamArray* param)
+      : param_(param) {
+  }
+  virtual void SetEffectParam(RendererGL* renderer, CGparameter cg_param) {
+    ParamArray* param = param_->value();
+    if (param) {
+      int size = cgGetArraySize(cg_param, 0);
+      if (size != static_cast<int>(param->size())) {
+        O3D_ERROR(param->service_locator())
+            << "number of params in ParamArray does not match number of params "
+            << "needed by shader array";
       } else {
-        cgSetParameter1f(cg_element, 0.0f);
+        for (int i = 0; i < size; ++i) {
+          Param* untyped_element = param->GetUntypedParam(i);
+          // TODO(gman): Make this check happen when building the param cache.
+          //    To do that would require that ParamParamArray mark it's owner
+          //    as changed if a Param in it's ParamArray changes.
+          if (untyped_element->IsA(ParamMatrix4::GetApparentClass())) {
+            CGparameter cg_element = cgGetArrayParameter(cg_param, i);
+            SetElement(cg_element, down_cast<ParamMatrix4*>(untyped_element));
+          } else {
+            O3D_ERROR(param->service_locator())
+                << "Param in ParamArray at index " << i
+                << " is not a ParamMatrix4";
+          }
+        }
+      }
+    }
+  }
+  void SetElement(CGparameter cg_element, ParamMatrix4* param);
+
+ private:
+  ParamParamArray* param_;
+};
+
+class EffectParamArraySamplerHandlerGL : public EffectParamHandlerGL {
+ public:
+  explicit EffectParamArraySamplerHandlerGL(ParamParamArray* param)
+      : param_(param) {
+  }
+  virtual void SetEffectParam(RendererGL* renderer, CGparameter cg_param) {
+    ParamArray* param = param_->value();
+    if (param) {
+      int size = cgGetArraySize(cg_param, 0);
+      if (size != static_cast<int>(param->size())) {
+        O3D_ERROR(param->service_locator())
+            << "number of params in ParamArray does not match number of params "
+            << "needed by shader array";
+      } else {
+        for (int i = 0; i < size; ++i) {
+          Param* untyped_element = param->GetUntypedParam(i);
+          // TODO(gman): Make this check happen when building the param cache.
+          //    To do that would require that ParamParamArray mark it's owner
+          //    as changed if a Param in it's ParamArray changes.
+          if (untyped_element->IsA(ParamSampler::GetApparentClass())) {
+            CGparameter cg_element = cgGetArrayParameter(cg_param, i);
+            ParamSampler* element = down_cast<ParamSampler*>(untyped_element);
+            SamplerGL* sampler_gl = down_cast<SamplerGL*>(element->value());
+            if (!sampler_gl) {
+              // Use the error sampler.
+              sampler_gl = down_cast<SamplerGL*>(renderer->error_sampler());
+              // If no error texture is set then generate an error.
+              if (!renderer->error_texture()) {
+                O3D_ERROR(param_->service_locator())
+                    << "Missing Sampler for ParamSampler '" << param_->name()
+                    << "' index " << i;
+              }
+            }
+            sampler_gl->SetTextureAndStates(cg_element);
+          } else {
+            O3D_ERROR(param->service_locator())
+                << "Param in ParamArray at index " << i
+                << " is not a ParamSampler";
+          }
+        }
+      }
+    }
+  }
+  virtual void ResetEffectParam(RendererGL* renderer, CGparameter cg_param) {
+    ParamArray* param = param_->value();
+    if (param) {
+      int size = cgGetArraySize(cg_param, 0);
+      if (size == static_cast<int>(param->size())) {
+        for (int i = 0; i < size; ++i) {
+          Param* untyped_element = param->GetUntypedParam(i);
+          if (untyped_element->IsA(ParamSampler::GetApparentClass())) {
+            CGparameter cg_element = cgGetArrayParameter(cg_param, i);
+            ParamSampler* element = down_cast<ParamSampler*>(untyped_element);
+            SamplerGL* sampler_gl = down_cast<SamplerGL*>(element->value());
+            if (!sampler_gl) {
+              sampler_gl = down_cast<SamplerGL*>(renderer->error_sampler());
+            }
+            sampler_gl->ResetTexture(cg_param);
+          }
+        }
       }
     }
   }
+
+ private:
+  ParamParamArray* param_;
+};
+
+template<>
+void EffectParamArrayHandlerGL<ParamFloat>::SetElement(
+    CGparameter cg_element,
+    ParamFloat* param) {
+  cgSetParameter1f(cg_element, param->value());
+}
+
+template<>
+void EffectParamArrayHandlerGL<ParamFloat2>::SetElement(
+    CGparameter cg_element,
+    ParamFloat2* param) {
+  Float2 f = param->value();
+  cgSetParameter2fv(cg_element, f.GetFloatArray());
+}
+
+template<>
+void EffectParamArrayHandlerGL<ParamFloat3>::SetElement(
+    CGparameter cg_element,
+    ParamFloat3* param) {
+  Float3 f = param->value();
+  cgSetParameter3fv(cg_element, f.GetFloatArray());
+}
+
+template<>
+void EffectParamArrayHandlerGL<ParamFloat4>::SetElement(
+    CGparameter cg_element,
+    ParamFloat4* param) {
+  Float4 f = param->value();
+  cgSetParameter4fv(cg_element, f.GetFloatArray());
+}
+
+template<>
+void EffectParamArrayMatrix4HandlerGL<false>::SetElement(
+    CGparameter cg_element,
+    ParamMatrix4* param) {
+  // set the data as floats in row major order.
+  Matrix4 mat = param->value();
+  cgSetMatrixParameterfr(cg_element, &mat[0][0]);
+}
+
+template<>
+void EffectParamArrayMatrix4HandlerGL<true>::SetElement(
+    CGparameter cg_element,
+    ParamMatrix4* param) {
+  // set the data as floats in column major order.
+  Matrix4 mat = param->value();
+  cgSetMatrixParameterfc(cg_element, &mat[0][0]);
+}
+
+template<>
+void EffectParamArrayHandlerGL<ParamInteger>::SetElement(
+    CGparameter cg_element,
+    ParamInteger* param) {
+  cgSetParameter1i(cg_element, param->value());
+}
+
+template<>
+void EffectParamArrayHandlerGL<ParamBoolean>::SetElement(
+    CGparameter cg_element,
+    ParamBoolean* param) {
+  cgSetParameter1i(cg_element, param->value());
 }
 
 static EffectParamHandlerGL::Ref GetHandlerFromParamAndCgType(
@@ -234,10 +412,52 @@ static EffectParamHandlerGL::Ref GetHandlerFromParamAndCgType(
     CGtype cg_type) {
   EffectParamHandlerGL::Ref handler;
   if (param->IsA(ParamParamArray::GetApparentClass())) {
-    if (cg_type == CG_FLOAT) {
-      handler = EffectParamHandlerGL::Ref(
-          new EffectParamFloatArrayHandlerGL(
-              down_cast<ParamParamArray*>(param)));
+    ParamParamArray* param_param_array = down_cast<ParamParamArray*>(param);
+    switch (cg_type) {
+      case CG_FLOAT:
+      case CG_FLOAT1:
+        handler = EffectParamHandlerGL::Ref(
+            new EffectParamArrayHandlerGL<ParamFloat>(param_param_array));
+        break;
+      case CG_FLOAT2:
+        handler = EffectParamHandlerGL::Ref(
+            new EffectParamArrayHandlerGL<ParamFloat2>(param_param_array));
+        break;
+      case CG_FLOAT3:
+        handler = EffectParamHandlerGL::Ref(
+            new EffectParamArrayHandlerGL<ParamFloat3>(param_param_array));
+        break;
+      case CG_FLOAT4:
+        handler = EffectParamHandlerGL::Ref(
+            new EffectParamArrayHandlerGL<ParamFloat4>(param_param_array));
+        break;
+      case CG_FLOAT4x4:
+        if (effect_gl->matrix_load_order() == Effect::COLUMN_MAJOR) {
+          handler = EffectParamHandlerGL::Ref(
+              new EffectParamArrayMatrix4HandlerGL<true>(param_param_array));
+        } else {
+          handler = EffectParamHandlerGL::Ref(
+              new EffectParamArrayMatrix4HandlerGL<false>(param_param_array));
+        }
+        break;
+      case CG_INT:
+      case CG_INT1:
+        handler = EffectParamHandlerGL::Ref(
+            new EffectParamArrayHandlerGL<ParamInteger>(param_param_array));
+        break;
+      case CG_BOOL:
+      case CG_BOOL1:
+        handler = EffectParamHandlerGL::Ref(
+            new EffectParamArrayHandlerGL<ParamBoolean>(param_param_array));
+        break;
+      case CG_SAMPLER:
+      case CG_SAMPLER1D:
+      case CG_SAMPLER2D:
+      case CG_SAMPLER3D:
+      case CG_SAMPLERCUBE:
+        handler = EffectParamHandlerGL::Ref(
+            new EffectParamArraySamplerHandlerGL(param_param_array));
+        break;
     }
   } else if (param->IsA(ParamMatrix4::GetApparentClass())) {
     if (cg_type == CG_FLOAT4x4) {
@@ -279,13 +499,13 @@ static EffectParamHandlerGL::Ref GetHandlerFromParamAndCgType(
               down_cast<ParamFloat4*>(param)));
     }
   }  else if (param->IsA(ParamInteger::GetApparentClass())) {
-    if (cg_type == CG_INT) {
+    if (cg_type == CG_INT || cg_type == CG_INT1) {
       handler = EffectParamHandlerGL::Ref(
           new TypedEffectParamHandlerGL<ParamInteger>(
               down_cast<ParamInteger*>(param)));
     }
   } else if (param->IsA(ParamBoolean::GetApparentClass())) {
-    if (cg_type == CG_BOOL) {
+    if (cg_type == CG_BOOL || cg_type == CG_BOOL1) {
       handler = EffectParamHandlerGL::Ref(
           new TypedEffectParamHandlerGL<ParamBoolean>(
               down_cast<ParamBoolean*>(param)));
@@ -358,12 +578,12 @@ static void ScanUniformParameters(SemanticManager* semantic_manager,
         continue;
       }
 
-      // TODO: The following code block should be removed once we
-      // start creating sampler params for all effects coming in via the
-      // importer.  For the time being, we keep an extra ParamTexture
-      // that does the job it used to do.  If we are using a ParamSampler
-      // on the object then the ParamTexture will have no value and
-      // therefore its handler will have no side-effects.
+      // TODO(o3d): The following code block should be removed once we start
+      // creating sampler params for all effects coming in via the importer. For
+      // the time being, we keep an extra ParamTexture that does the job it used
+      // to do.  If we are using a ParamSampler on the object then the
+      // ParamTexture will have no value and therefore its handler will have no
+      // side-effects.
       if (cg_type == CG_SAMPLER ||
           cg_type == CG_SAMPLER1D ||
           cg_type == CG_SAMPLER2D ||
diff --git a/o3d/core/win/d3d9/effect_d3d9.cc b/o3d/core/win/d3d9/effect_d3d9.cc
index 216e6b9..2bac2d9 100644
--- a/o3d/core/win/d3d9/effect_d3d9.cc
+++ b/o3d/core/win/d3d9/effect_d3d9.cc
@@ -32,6 +32,8 @@
 
 // This file contains the definition of EffectD3D9.
 
+// TODO(gman): Most of the D3DXHANDLE lookup could be cached.
+
 #include "core/cross/precompile.h"
 
 #include "core/win/d3d9/effect_d3d9.h"
@@ -53,6 +55,18 @@
 
 namespace o3d {
 
+namespace {
+
+inline bool IsSamplerType(D3DXPARAMETER_TYPE type) {
+  return type == D3DXPT_SAMPLER ||
+         type == D3DXPT_SAMPLER1D ||
+         type == D3DXPT_SAMPLER2D ||
+         type == D3DXPT_SAMPLER3D ||
+         type == D3DXPT_SAMPLERCUBE;
+}
+
+}  // anonymous namespace
+
 // A 'mostly' typesafe class to set an effect parameter from an O3D
 // Param. The phandle must match the type of Param to be typesafe. That is
 // handled when these are created.
@@ -69,22 +83,329 @@ class TypedEffectParamHandlerD3D9 : public EffectParamHandlerD3D9 {
   D3DXHANDLE phandle_;
 };
 
-class EffectParamFloatArrayHandlerD3D9 : public EffectParamHandlerD3D9 {
+template <typename T>
+class EffectParamArrayHandlerD3D9 : public EffectParamHandlerD3D9 {
  public:
-  EffectParamFloatArrayHandlerD3D9(ParamParamArray* param, D3DXHANDLE phandle)
+  EffectParamArrayHandlerD3D9(ParamParamArray* param,
+                              D3DXHANDLE phandle,
+                              unsigned num_elements)
       : param_(param),
-        phandle_(phandle) {
+        phandle_(phandle),
+        num_elements_(num_elements) {
   }
-  virtual void SetEffectParam(RendererD3D9* renderer, ID3DXEffect* d3d_effect);
+  virtual void SetEffectParam(RendererD3D9* renderer, ID3DXEffect* d3d_effect) {
+    ParamArray* param = param_->value();
+    if (param) {
+      int size = param->size();
+      if (size != num_elements_) {
+        O3D_ERROR(param->service_locator())
+            << "number of params in ParamArray does not match number of params "
+            << "needed by shader array";
+      } else {
+        for (int i = 0; i < size; ++i) {
+          Param* untyped_element = param->GetUntypedParam(i);
+          // TODO(gman): Make this check happen when building the param cache.
+          //    To do that would require that ParamParamArray mark it's owner
+          //    as changed if a Param in it's ParamArray changes.
+          if (untyped_element->IsA(T::GetApparentClass())) {
+            D3DXHANDLE dx_element =
+                d3d_effect->GetParameterElement(phandle_, i);
+            SetElement(d3d_effect, dx_element, down_cast<T*>(untyped_element));
+          } else {
+            O3D_ERROR(param->service_locator())
+                << "Param in ParamArray at index " << i << " is not a "
+                << T::GetApparentClassName();
+          }
+        }
+      }
+    }
+  }
+  void SetElement(ID3DXEffect* d3dx_effect,
+                  D3DXHANDLE dx_element,
+                  T* element);
+
  private:
   ParamParamArray* param_;
   D3DXHANDLE phandle_;
+  unsigned num_elements_;
 };
 
 // Number of h/w sampler units in the same shader using a single sampler.
 // Eight should be enough!
 static const int kMaxUnitsPerSampler = 8;
 
+template <bool column_major>
+class EffectParamMatrix4ArrayHandlerD3D9 : public EffectParamHandlerD3D9 {
+ public:
+  EffectParamMatrix4ArrayHandlerD3D9(ParamParamArray* param,
+                                     D3DXHANDLE phandle,
+                                     unsigned num_elements)
+      : param_(param),
+        phandle_(phandle),
+        num_elements_(num_elements) {
+  }
+  virtual void SetEffectParam(RendererD3D9* renderer, ID3DXEffect* d3d_effect) {
+    ParamArray* param = param_->value();
+    if (param) {
+      int size = param->size();
+      if (size != num_elements_) {
+        O3D_ERROR(param->service_locator())
+            << "number of params in ParamArray does not match number of params "
+            << "needed by shader array";
+      } else {
+        for (int i = 0; i < size; ++i) {
+          Param* untyped_element = param->GetUntypedParam(i);
+          // TODO(gman): Make this check happen when building the param cache.
+          //    To do that would require that ParamParamArray mark it's owner
+          //    as changed if a Param in it's ParamArray changes.
+          if (untyped_element->IsA(ParamMatrix4::GetApparentClass())) {
+            D3DXHANDLE dx_element =
+                d3d_effect->GetParameterElement(phandle_, i);
+            SetElement(d3d_effect,
+                       dx_element,
+                       down_cast<ParamMatrix4*>(untyped_element));
+          } else {
+            O3D_ERROR(param->service_locator())
+                << "Param in ParamArray at index " << i << " is not a "
+                << ParamMatrix4::GetApparentClassName();
+          }
+        }
+      }
+    }
+  }
+  void SetElement(ID3DXEffect* d3dx_effect,
+                  D3DXHANDLE dx_element,
+                  ParamMatrix4* element);
+
+ private:
+  ParamParamArray* param_;
+  D3DXHANDLE phandle_;
+  unsigned num_elements_;
+};
+
+// A class for setting the the appropriate d3d sampler states from an array of
+// o3d Sampler object.
+class EffectParamSamplerArrayHandlerD3D9 : public EffectParamHandlerD3D9 {
+ public:
+  EffectParamSamplerArrayHandlerD3D9(ParamParamArray* param,
+                                     D3DXHANDLE phandle,
+                                     const D3DXPARAMETER_DESC& pdesc,
+                                     LPD3DXCONSTANTTABLE fs_constant_table,
+                                     LPDIRECT3DDEVICE9 d3d_device)
+      : param_(param),
+        phandle_(phandle),
+        sampler_unit_index_arrays_(pdesc.Elements) {
+    if (!fs_constant_table) {
+      DLOG(ERROR) << "Fragment shader constant table is NULL";
+      return;
+    }
+    D3DXHANDLE sampler_array_handle = fs_constant_table->GetConstantByName(
+        NULL,
+        pdesc.Name);
+    if (!sampler_array_handle) {
+      DLOG(ERROR) << "Sampler " << pdesc.Name <<
+          " not found in fragment shader";
+      return;
+    }
+    for (unsigned ii = 0; ii < pdesc.Elements; ++ii) {
+      D3DXHANDLE sampler_handle = fs_constant_table->GetConstantElement(
+          sampler_array_handle,
+          ii);
+      if (!sampler_handle) {
+        DLOG(ERROR) << "Sampler " << pdesc.Name << " index " << ii
+                    << " not found in fragment shader";
+      } else {
+        D3DXCONSTANT_DESC desc_array[kMaxUnitsPerSampler];
+        UINT num_desc = kMaxUnitsPerSampler;
+        fs_constant_table->GetConstantDesc(
+            sampler_handle, desc_array, &num_desc);
+        // We have no good way of querying how many descriptions would really be
+        // returned as we're capping the number to kMaxUnitsPerSampler (which
+        // should be more than sufficient).  If however we do end up with the
+        // max number there's a chance that there were actually more so let's
+        // log it.
+        if (num_desc == kMaxUnitsPerSampler) {
+          DLOG(WARNING) << "Number of constant descriptions might have "
+                        << "exceeded the maximum of " << kMaxUnitsPerSampler;
+        }
+        SamplerUnitIndexArray& index_array = sampler_unit_index_arrays_[ii];
+
+        for (UINT desc_index = 0; desc_index < num_desc; desc_index++) {
+          D3DXCONSTANT_DESC constant_desc = desc_array[desc_index];
+          if (constant_desc.Class == D3DXPC_OBJECT &&
+              IsSamplerType(constant_desc.Type)) {
+            index_array.push_back(constant_desc.RegisterIndex);
+          }
+        }
+        if (index_array.empty()) {
+          DLOG(ERROR) << "No matching sampler units found for " <<
+              pdesc.Name;
+        }
+      }
+    }
+  }
+
+  virtual void SetEffectParam(RendererD3D9* renderer, ID3DXEffect* d3d_effect) {
+    ParamArray* param = param_->value();
+    if (param) {
+      unsigned size = param->size();
+      if (size != sampler_unit_index_arrays_.size()) {
+        O3D_ERROR(param->service_locator())
+            << "number of params in ParamArray does not match number of params "
+            << "needed by shader array";
+      } else {
+        for (int i = 0; i < size; ++i) {
+          SamplerUnitIndexArray& index_array = sampler_unit_index_arrays_[i];
+          Param* untyped_element = param->GetUntypedParam(i);
+          // TODO(gman): Make this check happen when building the param cache.
+          //    To do that would require that ParamParamArray mark it's owner
+          //    as changed if a Param in it's ParamArray changes.
+          if (untyped_element->IsA(ParamSampler::GetApparentClass())) {
+            D3DXHANDLE dx_element =
+                d3d_effect->GetParameterElement(phandle_,  i);
+            // Find the texture associated with the sampler first.
+            Sampler* sampler =
+                down_cast<ParamSampler*>(untyped_element)->value();
+            if (!sampler) {
+              sampler = renderer->error_sampler();
+              if (!renderer->error_texture()) {
+                O3D_ERROR(param->service_locator())
+                    << "Missing Sampler for ParamSampler "
+                    << param->name();
+              }
+            }
+
+            SamplerD3D9* d3d_sampler = down_cast<SamplerD3D9*>(sampler);
+            for (unsigned stage = 0; stage < index_array.size(); stage++) {
+              d3d_sampler->SetTextureAndStates(index_array[stage]);
+            }
+          } else {
+            O3D_ERROR(param->service_locator())
+                << "Param in ParamArray at index " << i << " is not a "
+                << ParamSampler::GetApparentClassName();
+          }
+        }
+      }
+    }
+  }
+
+  // Resets the value of the parameter to default.  Currently this is used
+  // to unbind textures contained in Sampler params.
+  virtual void ResetEffectParam(RendererD3D9* renderer,
+                                ID3DXEffect* d3d_effect) {
+    ParamArray* param = param_->value();
+    if (param) {
+      unsigned size = param->size();
+      if (size == sampler_unit_index_arrays_.size()) {
+        for (int i = 0; i < size; ++i) {
+          SamplerUnitIndexArray& index_array = sampler_unit_index_arrays_[i];
+          Param* untyped_element = param->GetUntypedParam(i);
+          // TODO(gman): Make this check happen when building the param cache.
+          //    To do that would require that ParamParamArray mark it's owner
+          //    as changed if a Param in it's ParamArray changes.
+          if (untyped_element->IsA(ParamSampler::GetApparentClass())) {
+            D3DXHANDLE dx_element =
+                d3d_effect->GetParameterElement(phandle_,  i);
+            // Find the texture associated with the sampler first.
+            Sampler* sampler =
+                down_cast<ParamSampler*>(untyped_element)->value();
+            if (!sampler) {
+              sampler = renderer->error_sampler();
+            }
+
+            SamplerD3D9* d3d_sampler = down_cast<SamplerD3D9*>(sampler);
+            for (unsigned stage = 0; stage < index_array.size(); stage++) {
+              d3d_sampler->ResetTexture(index_array[stage]);
+            }
+          }
+        }
+      }
+    }
+  }
+
+ private:
+  typedef std::vector<int> SamplerUnitIndexArray;
+  typedef std::vector<SamplerUnitIndexArray> SamplerIndexArrayArray;
+
+  ParamParamArray* param_;
+  D3DXHANDLE phandle_;
+  // An array of arrays of sampler unit indices.
+  SamplerIndexArrayArray sampler_unit_index_arrays_;
+};
+
+template<>
+void EffectParamArrayHandlerD3D9<ParamFloat>::SetElement(
+    ID3DXEffect* d3dx_effect,
+    D3DXHANDLE dx_element,
+    ParamFloat* element) {
+  d3dx_effect->SetFloat(dx_element, element->value());
+}
+
+template<>
+void EffectParamArrayHandlerD3D9<ParamFloat2>::SetElement(
+    ID3DXEffect* d3dx_effect,
+    D3DXHANDLE dx_element,
+    ParamFloat2* element) {
+  Float2 float2 = element->value();
+  HR(d3dx_effect->SetFloatArray(dx_element, float2.GetFloatArray(), 2));
+}
+
+template<>
+void EffectParamArrayHandlerD3D9<ParamFloat3>::SetElement(
+    ID3DXEffect* d3dx_effect,
+    D3DXHANDLE dx_element,
+    ParamFloat3* element) {
+  Float3 float3 = element->value();
+  HR(d3dx_effect->SetFloatArray(dx_element, float3.GetFloatArray(), 3));
+}
+
+template<>
+void EffectParamArrayHandlerD3D9<ParamFloat4>::SetElement(
+    ID3DXEffect* d3dx_effect,
+    D3DXHANDLE dx_element,
+    ParamFloat4* element) {
+  Float4 float4 = element->value();
+  HR(d3dx_effect->SetFloatArray(dx_element, float4.GetFloatArray(), 4));
+}
+
+template<>
+void EffectParamArrayHandlerD3D9<ParamBoolean>::SetElement(
+    ID3DXEffect* d3dx_effect,
+    D3DXHANDLE dx_element,
+    ParamBoolean* element) {
+  HR(d3dx_effect->SetBool(dx_element, element->value()));
+}
+
+template<>
+void EffectParamArrayHandlerD3D9<ParamInteger>::SetElement(
+    ID3DXEffect* d3dx_effect,
+    D3DXHANDLE dx_element,
+    ParamInteger* element) {
+  HR(d3dx_effect->SetInt(dx_element, element->value()));
+}
+
+template<>
+void EffectParamMatrix4ArrayHandlerD3D9<false>::SetElement(
+    ID3DXEffect* d3dx_effect,
+    D3DXHANDLE dx_element,
+    ParamMatrix4* element) {
+  Matrix4 param_matrix = element->value();
+  HR(d3dx_effect->SetMatrix(
+         dx_element,
+         reinterpret_cast<D3DXMATRIX*>(&param_matrix[0][0])));
+}
+
+template<>
+void EffectParamMatrix4ArrayHandlerD3D9<true>::SetElement(
+    ID3DXEffect* d3dx_effect,
+    D3DXHANDLE dx_element,
+    ParamMatrix4* element) {
+  Matrix4 param_matrix = transpose(element->value());
+  HR(d3dx_effect->SetMatrix(
+         dx_element,
+         reinterpret_cast<D3DXMATRIX*>(&param_matrix[0][0])));
+}
+
 // A class for setting the the appropriate d3d sampler states from a
 // o3d Sampler object.
 class EffectParamHandlerForSamplersD3D9 : public EffectParamHandlerD3D9 {
@@ -144,16 +465,13 @@ static const ObjectBase::Class* D3DXPDescToParamType(
              pdesc.Columns == 1) {
     return ParamBoolean::GetApparentClass();
   // Texture param
-  // TODO Texture params should be removed once we switch over to
+  // TODO(o3d): Texture params should be removed once we switch over to
   // using samplers only.
   } else if (pdesc.Type == D3DXPT_TEXTURE &&
              pdesc.Class == D3DXPC_OBJECT) {
     return ParamTexture::GetApparentClass();
   // Sampler param
-  } else if (pdesc.Class == D3DXPC_OBJECT &&
-             (pdesc.Type == D3DXPT_SAMPLER ||
-              pdesc.Type == D3DXPT_SAMPLER2D ||
-              pdesc.Type == D3DXPT_SAMPLERCUBE)) {
+  } else if (pdesc.Class == D3DXPC_OBJECT && IsSamplerType(pdesc.Type)) {
     return ParamSampler::GetApparentClass();
   }
   return NULL;
@@ -178,13 +496,13 @@ bool EffectD3D9::PrepareFX(const String& effect,
   String fragment_shader_entry_point;
   MatrixLoadOrder matrix_load_order;
 
-  // TODO: Temporary fix to make GL and D3D match until the shader parser
+  // TODO(o3d): Temporary fix to make GL and D3D match until the shader parser
   //     is written.
   if (!ValidateFX(effect,
                   &vertex_shader_entry_point,
                   &fragment_shader_entry_point,
                   &matrix_load_order)) {
-    // TODO: Remove this but for now just let bad ones pass so collada
+    // TODO(o3d): Remove this but for now just let bad ones pass so collada
     //     importer works.
     *prepared_effect = effect;
     return false;
@@ -219,7 +537,7 @@ bool EffectD3D9::LoadFromFXString(const String& effect) {
   LPD3DXBUFFER error_buffer;
 
   String prepared_effect;
-  // TODO: Check for failure once shader parser is in.
+  // TODO(o3d): Check for failure once shader parser is in.
   PrepareFX(effect, &prepared_effect);
 
   if (!HR(o3d::D3DXCreateEffect(d3d_device_,
@@ -356,12 +674,69 @@ bool EffectD3D9::AddParameterMapping(
     D3DXHANDLE phandle,
     EffectParamHandlerCacheD3D9* effect_param_cache) {
   // Array param
-  if (param->IsA(ParamParamArray::GetApparentClass()) &&
-      pdesc.Elements > 1) {
-    if (pdesc.Class == D3DXPC_SCALAR && pdesc.Type == D3DXPT_FLOAT) {
+  if (param->IsA(ParamParamArray::GetApparentClass()) && pdesc.Elements > 0) {
+    ParamParamArray* param_param_array = down_cast<ParamParamArray*>(param);
+    if (pdesc.Class == D3DXPC_SCALAR &&
+        pdesc.Type == D3DXPT_FLOAT) {
+      effect_param_cache->AddElement(
+          new EffectParamArrayHandlerD3D9<ParamFloat>(
+              param_param_array, phandle, pdesc.Elements));
+    } else if (pdesc.Class == D3DXPC_VECTOR &&
+               pdesc.Type == D3DXPT_FLOAT &&
+               pdesc.Columns == 2) {
+      effect_param_cache->AddElement(
+          new EffectParamArrayHandlerD3D9<ParamFloat2>(
+              param_param_array, phandle, pdesc.Elements));
+    } else if (pdesc.Class == D3DXPC_VECTOR &&
+               pdesc.Type == D3DXPT_FLOAT &&
+               pdesc.Columns == 3) {
+      effect_param_cache->AddElement(
+          new EffectParamArrayHandlerD3D9<ParamFloat3>(
+              param_param_array, phandle, pdesc.Elements));
+    } else if (pdesc.Class == D3DXPC_VECTOR &&
+               pdesc.Type == D3DXPT_FLOAT &&
+               pdesc.Columns == 4) {
+      effect_param_cache->AddElement(
+          new EffectParamArrayHandlerD3D9<ParamFloat4>(
+              param_param_array, phandle, pdesc.Elements));
+    } else if (pdesc.Class == D3DXPC_SCALAR &&
+               pdesc.Type == D3DXPT_INT &&
+               pdesc.Columns == 1) {
       effect_param_cache->AddElement(
-          new EffectParamFloatArrayHandlerD3D9(
-              down_cast<ParamParamArray*>(param), phandle));
+          new EffectParamArrayHandlerD3D9<ParamInteger>(
+              param_param_array, phandle, pdesc.Elements));
+    } else if (pdesc.Class == D3DXPC_SCALAR &&
+               pdesc.Type == D3DXPT_BOOL &&
+               pdesc.Columns == 1) {
+      effect_param_cache->AddElement(
+          new EffectParamArrayHandlerD3D9<ParamBoolean>(
+              param_param_array, phandle, pdesc.Elements));
+    } else if (pdesc.Class == D3DXPC_MATRIX_COLUMNS) {
+      effect_param_cache->AddElement(
+          new EffectParamMatrix4ArrayHandlerD3D9<true>(
+              param_param_array, phandle, pdesc.Elements));
+    } else if (pdesc.Class == D3DXPC_MATRIX_ROWS) {
+      if (matrix_load_order() == COLUMN_MAJOR) {
+        // D3D has already created a uniform of type MATRIX_ROWS, but the
+        // effect wants column major matrices, so we create a handler
+        // for MATRIX_COLUMNS.  This will cause the matrix to be transposed
+        // on load.
+        effect_param_cache->AddElement(
+            new EffectParamMatrix4ArrayHandlerD3D9<true>(
+                param_param_array, phandle, pdesc.Elements));
+      } else {
+        effect_param_cache->AddElement(
+            new EffectParamMatrix4ArrayHandlerD3D9<false>(
+                param_param_array, phandle, pdesc.Elements));
+      }
+    } else if (pdesc.Class == D3DXPC_OBJECT && IsSamplerType(pdesc.Type)) {
+      effect_param_cache->AddElement(
+          new EffectParamSamplerArrayHandlerD3D9(
+             param_param_array,
+             phandle,
+             pdesc,
+             fs_constant_table_,
+             d3d_device_));
     }
   // Matrix4 Param
   } else if (param->IsA(ParamMatrix4::GetApparentClass()) &&
@@ -375,7 +750,7 @@ bool EffectD3D9::AddParameterMapping(
     if (matrix_load_order() == COLUMN_MAJOR) {
       // D3D has already created a uniform of type MATRIX_ROWS, but the
       // effect wants column major matrices, so we create a handler
-      // for MATRIX_COLUMNS.  This will cause the matrix to be tranposed
+      // for MATRIX_COLUMNS.  This will cause the matrix to be transposed
       // on load.
       effect_param_cache->AddElement(
           new TypedEffectParamHandlerD3D9<ParamMatrix4,
@@ -441,7 +816,7 @@ bool EffectD3D9::AddParameterMapping(
                                         D3DXPC_SCALAR>(
             down_cast<ParamBoolean*>(param), phandle));
     // Texture param
-    // TODO The texture param block should be removed once we start
+    // TODO(o3d): The texture param block should be removed once we start
     // using samplers only.  In the meantime, we need to create a texture param
     // to be able to handle collada files referencing external fx .
   } else if (param->IsA(ParamTexture::GetApparentClass()) &&
@@ -453,10 +828,7 @@ bool EffectD3D9::AddParameterMapping(
             down_cast<ParamTexture*>(param), phandle));
   // Sampler param
   } else if (param->IsA(ParamSampler::GetApparentClass()) &&
-             pdesc.Class == D3DXPC_OBJECT &&
-             (pdesc.Type == D3DXPT_SAMPLER ||
-              pdesc.Type == D3DXPT_SAMPLER2D ||
-              pdesc.Type == D3DXPT_SAMPLERCUBE)) {
+             pdesc.Class == D3DXPC_OBJECT && IsSamplerType(pdesc.Type)) {
     effect_param_cache->AddElement(
         new EffectParamHandlerForSamplersD3D9(down_cast<ParamSampler*>(param),
                                               pdesc,
@@ -663,7 +1035,7 @@ void TypedEffectParamHandlerD3D9<ParamBoolean,
   HR(d3dx_effect->SetBool(phandle_, param_->value()));
 }
 
-// TODO: The following handler should be removed once we switch to
+// TODO(o3d): The following handler should be removed once we switch to
 // using Samplers exclusively.
 template<>
 void TypedEffectParamHandlerD3D9<ParamTexture,
@@ -671,7 +1043,7 @@ void TypedEffectParamHandlerD3D9<ParamTexture,
                                      RendererD3D9* renderer,
                                      ID3DXEffect* d3dx_effect) {
   Texture* texture = param_->value();
-  // TODO: If texture is NULL then we don't set the texture on the
+  // TODO(o3d): If texture is NULL then we don't set the texture on the
   // effect to avoid clobbering texture set by the corresponding sampler in
   // the cases where we use samplers.  The side-effect of this is that if
   // the texture is not set, we could end up using whatever texture was used
@@ -693,24 +1065,6 @@ void TypedEffectParamHandlerD3D9<ParamTexture,
   }
 }
 
-void EffectParamFloatArrayHandlerD3D9::SetEffectParam(
-    RendererD3D9* renderer,
-    ID3DXEffect* d3dx_effect) {
-  ParamArray* param = param_->value();
-  if (param) {
-    int size = param->size();
-    for (int i = 0; i < size; ++i) {
-      ParamFloat* element = param->GetParam<ParamFloat>(i);
-      D3DXHANDLE dx_element = d3dx_effect->GetParameterElement(phandle_, i);
-      if (element) {
-        d3dx_effect->SetFloat(dx_element, element->value());
-      } else {
-        d3dx_effect->SetFloat(dx_element, 0.0);
-      }
-    }
-  }
-}
-
 void EffectParamHandlerForSamplersD3D9::SetEffectParam(
     RendererD3D9* renderer,
     ID3DXEffect* d3dx_effect) {
@@ -734,11 +1088,13 @@ void EffectParamHandlerForSamplersD3D9::ResetEffectParam(
     RendererD3D9* renderer,
     ID3DXEffect* d3dx_effect) {
   Sampler* sampler = sampler_param_->value();
-  if (sampler) {
-    SamplerD3D9* d3d_sampler = down_cast<SamplerD3D9*>(sampler);
-    for (int stage = 0; stage < number_sampler_units_; stage++) {
-      d3d_sampler->ResetTexture(sampler_unit_index_array_[stage]);
-    }
+  if (!sampler) {
+    sampler = renderer->error_sampler();
+  }
+
+  SamplerD3D9* d3d_sampler = down_cast<SamplerD3D9*>(sampler);
+  for (int stage = 0; stage < number_sampler_units_; stage++) {
+    d3d_sampler->ResetTexture(sampler_unit_index_array_[stage]);
   }
 }
 
@@ -781,9 +1137,7 @@ EffectParamHandlerForSamplersD3D9::EffectParamHandlerForSamplersD3D9(
   for (UINT desc_index = 0; desc_index < num_desc; desc_index++) {
     D3DXCONSTANT_DESC constant_desc = desc_array[desc_index];
     if (constant_desc.Class == D3DXPC_OBJECT &&
-        (constant_desc.Type == D3DXPT_SAMPLER ||
-         constant_desc.Type == D3DXPT_SAMPLER2D ||
-         constant_desc.Type == D3DXPT_SAMPLERCUBE)) {
+        IsSamplerType(constant_desc.Type)) {
       sampler_unit_index_array_[number_sampler_units_++] =
           constant_desc.RegisterIndex;
     }