Validate TexParameter for GL_TEXTURE_MAX_ANISOTROPY_EXT

BUG=164337


Review URL: https://chromiumcodereview.appspot.com/11437012

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

4 files changed, 84 insertions, 35 deletions

diff --git a/gpu/command_buffer/service/gles2_cmd_decoder.cc b/gpu/command_buffer/service/gles2_cmd_decoder.cc
index b1292f0..515474d 100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder.cc
+++ b/gpu/command_buffer/service/gles2_cmd_decoder.cc
@@ -23,6 +23,7 @@
 #endif
 #include "base/memory/scoped_ptr.h"
 #include "base/string_number_conversions.h"
+#include "base/stringprintf.h"
 #include "build/build_config.h"
 #define GLES2_GPU_SERVICE 1
 #include "gpu/command_buffer/common/gles2_cmd_format.h"
@@ -1219,6 +1220,13 @@ class GLES2DecoderImpl : public GLES2Decoder {
   // errors caused by GL calls that it was not responsible for issuing.
   void ClearRealGLErrors();
 
+  // Generates a GL error for a bad parameter.
+  void SetGLErrorInvalidParam(
+      GLenum error,
+      const char* function_name,
+      GLenum pname,
+      GLint param);
+
   // Checks if the current program and vertex attributes are valid for drawing.
   bool IsDrawValid(
       const char* function_name, GLuint max_vertex_accessed, GLsizei primcount);
@@ -4666,9 +4674,11 @@ void GLES2DecoderImpl::DoTexParameterf(
     return;
   }
 
-  if (!texture_manager()->SetParameter(
-      info, pname, static_cast<GLint>(param))) {
-    SetGLErrorInvalidEnum("glTexParameterf", pname, "pname");
+  GLenum error = texture_manager()->SetParameter(
+      info, pname, static_cast<GLint>(param));
+  if (error != GL_NO_ERROR) {
+    SetGLErrorInvalidParam(
+        error, "glTexParameterf", pname, static_cast<GLint>(param));
     return;
   }
   glTexParameterf(target, pname, param);
@@ -4682,8 +4692,9 @@ void GLES2DecoderImpl::DoTexParameteri(
     return;
   }
 
-  if (!texture_manager()->SetParameter(info, pname, param)) {
-    SetGLErrorInvalidEnum("glTexParameteri", pname, "pname");
+  GLenum error =  texture_manager()->SetParameter(info, pname, param);
+  if(error != GL_NO_ERROR) {
+    SetGLErrorInvalidParam(error, "glTexParameteri", pname, param);
     return;
   }
   glTexParameteri(target, pname, param);
@@ -4697,9 +4708,11 @@ void GLES2DecoderImpl::DoTexParameterfv(
     return;
   }
 
-  if (!texture_manager()->SetParameter(
-      info, pname, static_cast<GLint>(params[0]))) {
-    SetGLErrorInvalidEnum("glTexParameterfv", pname, "pname");
+  GLenum error =texture_manager()->SetParameter(
+      info, pname, static_cast<GLint>(params[0]));
+  if (error != GL_NO_ERROR) {
+    SetGLErrorInvalidParam(
+        error, "glTexParameterfv", pname, static_cast<GLint>(params[0]));
     return;
   }
   glTexParameterfv(target, pname, params);
@@ -4713,8 +4726,9 @@ void GLES2DecoderImpl::DoTexParameteriv(
     return;
   }
 
-  if (!texture_manager()->SetParameter(info, pname, *params)) {
-    SetGLErrorInvalidEnum("glTexParameteriv", pname, "pname");
+  GLenum error = texture_manager()->SetParameter(info, pname, *params);
+  if (error != GL_NO_ERROR) {
+    SetGLErrorInvalidParam(error, "glTexParameteriv", pname, *params);
     return;
   }
   glTexParameteriv(target, pname, params);
@@ -5024,6 +5038,26 @@ void GLES2DecoderImpl::SetGLErrorInvalidEnum(
               GLES2Util::GetStringEnum(value)).c_str());
 }
 
+void GLES2DecoderImpl::SetGLErrorInvalidParam(
+    GLenum error,
+    const char* function_name,
+    GLenum pname,
+    GLint param) {
+  if (error == GL_INVALID_ENUM) {
+    SetGLError(
+        GL_INVALID_ENUM, function_name,
+        (std::string("trying to set ") +
+         GLES2Util::GetStringEnum(pname) + " to " +
+         GLES2Util::GetStringEnum(param)).c_str());
+  } else {
+    SetGLError(
+        error, function_name,
+        (std::string("trying to set ") +
+         GLES2Util::GetStringEnum(pname) + " to " +
+         base::StringPrintf("%d", param)).c_str());
+  }
+}
+
 const std::string& GLES2DecoderImpl::GetLogPrefix() const {
   const std::string& prefix(debug_marker_manager_.GetMarker());
   return prefix.empty() ? this_in_hex_ : prefix;
diff --git a/gpu/command_buffer/service/texture_manager.cc b/gpu/command_buffer/service/texture_manager.cc
index 3627582..1c36999 100644
--- a/gpu/command_buffer/service/texture_manager.cc
+++ b/gpu/command_buffer/service/texture_manager.cc
@@ -442,7 +442,7 @@ bool TextureManager::TextureInfo::GetLevelType(
   return false;
 }
 
-bool TextureManager::TextureInfo::SetParameter(
+GLenum TextureManager::TextureInfo::SetParameter(
     const FeatureInfo* feature_info, GLenum pname, GLint param) {
   DCHECK(feature_info);
 
@@ -450,53 +450,55 @@ bool TextureManager::TextureInfo::SetParameter(
       target_ == GL_TEXTURE_RECTANGLE_ARB) {
     if (pname == GL_TEXTURE_MIN_FILTER &&
         (param != GL_NEAREST && param != GL_LINEAR))
-      return false;
+      return GL_INVALID_ENUM;
     if ((pname == GL_TEXTURE_WRAP_S || pname == GL_TEXTURE_WRAP_T) &&
         param != GL_CLAMP_TO_EDGE)
-      return false;
+      return GL_INVALID_ENUM;
   }
 
   switch (pname) {
     case GL_TEXTURE_MIN_FILTER:
       if (!feature_info->validators()->texture_min_filter_mode.IsValid(param)) {
-        return false;
+        return GL_INVALID_ENUM;
       }
       min_filter_ = param;
       break;
     case GL_TEXTURE_MAG_FILTER:
       if (!feature_info->validators()->texture_mag_filter_mode.IsValid(param)) {
-        return false;
+        return GL_INVALID_ENUM;
       }
       mag_filter_ = param;
       break;
     case GL_TEXTURE_WRAP_S:
       if (!feature_info->validators()->texture_wrap_mode.IsValid(param)) {
-        return false;
+        return GL_INVALID_ENUM;
       }
       wrap_s_ = param;
       break;
     case GL_TEXTURE_WRAP_T:
       if (!feature_info->validators()->texture_wrap_mode.IsValid(param)) {
-        return false;
+        return GL_INVALID_ENUM;
       }
       wrap_t_ = param;
       break;
     case GL_TEXTURE_MAX_ANISOTROPY_EXT:
-      // Nothing to do for this case at the moment.
+      if (param < 1) {
+        return GL_INVALID_VALUE;
+      }
       break;
     case GL_TEXTURE_USAGE_ANGLE:
       if (!feature_info->validators()->texture_usage.IsValid(param)) {
-        return false;
+        return GL_INVALID_ENUM;
       }
       usage_ = param;
       break;
     default:
       NOTREACHED();
-      return false;
+      return GL_INVALID_ENUM;
   }
   Update(feature_info);
   UpdateCleared();
-  return true;
+  return GL_NO_ERROR;
 }
 
 void TextureManager::TextureInfo::Update(const FeatureInfo* feature_info) {
@@ -1028,7 +1030,7 @@ bool TextureManager::Restore(TextureInfo* info,
   return true;
 }
 
-bool TextureManager::SetParameter(
+GLenum TextureManager::SetParameter(
     TextureManager::TextureInfo* info, GLenum pname, GLint param) {
   DCHECK(info);
   if (!info->CanRender(feature_info_)) {
@@ -1039,7 +1041,7 @@ bool TextureManager::SetParameter(
     DCHECK_NE(0, num_unsafe_textures_);
     --num_unsafe_textures_;
   }
-  bool result = info->SetParameter(feature_info_, pname, param);
+  GLenum result = info->SetParameter(feature_info_, pname, param);
   if (!info->CanRender(feature_info_)) {
     ++num_unrenderable_textures_;
   }
diff --git a/gpu/command_buffer/service/texture_manager.h b/gpu/command_buffer/service/texture_manager.h
index ed4c8c9..5113499 100644
--- a/gpu/command_buffer/service/texture_manager.h
+++ b/gpu/command_buffer/service/texture_manager.h
@@ -241,8 +241,8 @@ class GPU_EXPORT TextureManager {
 
     // Sets a texture parameter.
     // TODO(gman): Expand to SetParameteri,f,iv,fv
-    // Returns false if param was INVALID_ENUN
-    bool SetParameter(
+    // Returns GL_NO_ERROR on success. Otherwise the error to generate.
+    GLenum SetParameter(
         const FeatureInfo* feature_info, GLenum pname, GLint param);
 
     // Makes each of the mip levels as though they were generated.
@@ -435,8 +435,9 @@ class GPU_EXPORT TextureManager {
   void SetLevelCleared(TextureInfo* info, GLenum target, GLint level);
 
   // Sets a texture parameter of a TextureInfo
+  // Returns GL_NO_ERROR on success. Otherwise the error to generate.
   // TODO(gman): Expand to SetParameteri,f,iv,fv
-  bool SetParameter(
+  GLenum SetParameter(
       TextureInfo* info, GLenum pname, GLint param);
 
   // Makes each of the mip levels as though they were generated.
diff --git a/gpu/command_buffer/service/texture_manager_unittest.cc b/gpu/command_buffer/service/texture_manager_unittest.cc
index da56669..a761c03 100644
--- a/gpu/command_buffer/service/texture_manager_unittest.cc
+++ b/gpu/command_buffer/service/texture_manager_unittest.cc
@@ -104,24 +104,36 @@ TEST_F(TextureManagerTest, SetParameter) {
   // Check texture got created.
   TextureManager::TextureInfo* info = manager_.GetTextureInfo(kClient1Id);
   ASSERT_TRUE(info != NULL);
-  EXPECT_TRUE(manager_.SetParameter(info, GL_TEXTURE_MIN_FILTER, GL_NEAREST));
+  EXPECT_EQ(static_cast<GLenum>(GL_NO_ERROR), manager_.SetParameter(
+      info, GL_TEXTURE_MIN_FILTER, GL_NEAREST));
   EXPECT_EQ(static_cast<GLenum>(GL_NEAREST), info->min_filter());
-  EXPECT_TRUE(manager_.SetParameter(info, GL_TEXTURE_MAG_FILTER, GL_NEAREST));
+  EXPECT_EQ(static_cast<GLenum>(GL_NO_ERROR), manager_.SetParameter(
+      info, GL_TEXTURE_MAG_FILTER, GL_NEAREST));
   EXPECT_EQ(static_cast<GLenum>(GL_NEAREST), info->mag_filter());
-  EXPECT_TRUE(manager_.SetParameter(info, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE));
+  EXPECT_EQ(static_cast<GLenum>(GL_NO_ERROR), manager_.SetParameter(
+      info, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE));
   EXPECT_EQ(static_cast<GLenum>(GL_CLAMP_TO_EDGE), info->wrap_s());
-  EXPECT_TRUE(manager_.SetParameter(info, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE));
+  EXPECT_EQ(static_cast<GLenum>(GL_NO_ERROR), manager_.SetParameter(
+      info, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE));
   EXPECT_EQ(static_cast<GLenum>(GL_CLAMP_TO_EDGE), info->wrap_t());
-  EXPECT_FALSE(manager_.SetParameter(
+  EXPECT_EQ(static_cast<GLenum>(GL_NO_ERROR), manager_.SetParameter(
+      info, GL_TEXTURE_MAX_ANISOTROPY_EXT, 1));
+  EXPECT_EQ(static_cast<GLenum>(GL_NO_ERROR), manager_.SetParameter(
+      info, GL_TEXTURE_MAX_ANISOTROPY_EXT, 2));
+  EXPECT_EQ(static_cast<GLenum>(GL_INVALID_ENUM), manager_.SetParameter(
       info, GL_TEXTURE_MIN_FILTER, GL_CLAMP_TO_EDGE));
   EXPECT_EQ(static_cast<GLenum>(GL_NEAREST), info->min_filter());
-  EXPECT_FALSE(manager_.SetParameter(
+  EXPECT_EQ(static_cast<GLenum>(GL_INVALID_ENUM), manager_.SetParameter(
       info, GL_TEXTURE_MAG_FILTER, GL_CLAMP_TO_EDGE));
   EXPECT_EQ(static_cast<GLenum>(GL_NEAREST), info->min_filter());
-  EXPECT_FALSE(manager_.SetParameter(info, GL_TEXTURE_WRAP_S, GL_NEAREST));
+  EXPECT_EQ(static_cast<GLenum>(GL_INVALID_ENUM), manager_.SetParameter(
+      info, GL_TEXTURE_WRAP_S, GL_NEAREST));
   EXPECT_EQ(static_cast<GLenum>(GL_CLAMP_TO_EDGE), info->wrap_s());
-  EXPECT_FALSE(manager_.SetParameter(info, GL_TEXTURE_WRAP_T, GL_NEAREST));
+  EXPECT_EQ(static_cast<GLenum>(GL_INVALID_ENUM), manager_.SetParameter(
+      info, GL_TEXTURE_WRAP_T, GL_NEAREST));
   EXPECT_EQ(static_cast<GLenum>(GL_CLAMP_TO_EDGE), info->wrap_t());
+  EXPECT_EQ(static_cast<GLenum>(GL_INVALID_VALUE), manager_.SetParameter(
+      info, GL_TEXTURE_MAX_ANISOTROPY_EXT, 0));
 }
 
 TEST_F(TextureManagerTest, TextureUsageExt) {
@@ -137,7 +149,7 @@ TEST_F(TextureManagerTest, TextureUsageExt) {
   // Check texture got created.
   TextureManager::TextureInfo* info = manager.GetTextureInfo(kClient1Id);
   ASSERT_TRUE(info != NULL);
-  EXPECT_TRUE(manager.SetParameter(
+  EXPECT_EQ(static_cast<GLenum>(GL_NO_ERROR), manager.SetParameter(
       info, GL_TEXTURE_USAGE_ANGLE, GL_FRAMEBUFFER_ATTACHMENT_ANGLE));
   EXPECT_EQ(static_cast<GLenum>(GL_FRAMEBUFFER_ATTACHMENT_ANGLE),
             info->usage());