Fix renderbuffer width, height issues

TEST=unit tests
BUG=76497

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

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

7 files changed, 81 insertions, 17 deletions

diff --git a/gpu/command_buffer/service/framebuffer_manager.cc b/gpu/command_buffer/service/framebuffer_manager.cc
index 333b681..190a910 100644
--- a/gpu/command_buffer/service/framebuffer_manager.cc
+++ b/gpu/command_buffer/service/framebuffer_manager.cc
@@ -169,6 +169,17 @@ void FramebufferManager::FramebufferInfo::MarkAttachedRenderbuffersAsCleared() {
   }
 }
 
+bool FramebufferManager::FramebufferInfo::IsNotComplete() const {
+  for (AttachmentMap::const_iterator it = attachments_.begin();
+       it != attachments_.end(); ++it) {
+    Attachment* attachment = it->second;
+    if (attachment->width() == 0 || attachment->height() == 0) {
+      return true;
+    }
+  }
+  return false;
+}
+
 FramebufferManager::FramebufferInfo* FramebufferManager::GetFramebufferInfo(
     GLuint client_id) {
   FramebufferInfoMap::iterator it = framebuffer_infos_.find(client_id);
diff --git a/gpu/command_buffer/service/framebuffer_manager.h b/gpu/command_buffer/service/framebuffer_manager.h
index 82dd152..1f55673 100644
--- a/gpu/command_buffer/service/framebuffer_manager.h
+++ b/gpu/command_buffer/service/framebuffer_manager.h
@@ -72,6 +72,12 @@ class FramebufferManager {
       return has_been_bound_ && !IsDeleted();
     }
 
+    // We can't know if the frame buffer is complete since that is
+    // implementation  dependent and we'd have to check after every glTexImage
+    // call but we can know in certain cases that it's NOT complete which we
+    // need to enforce the OpenGL ES 2.0 spec on top of DesktopGL.
+    bool IsNotComplete() const;
+
    private:
     friend class FramebufferManager;
     friend class base::RefCounted<FramebufferInfo>;
diff --git a/gpu/command_buffer/service/framebuffer_manager_unittest.cc b/gpu/command_buffer/service/framebuffer_manager_unittest.cc
index cb30d8e..d3c3879 100644
--- a/gpu/command_buffer/service/framebuffer_manager_unittest.cc
+++ b/gpu/command_buffer/service/framebuffer_manager_unittest.cc
@@ -147,6 +147,7 @@ TEST_F(FramebufferInfoTest, AttachRenderbuffer) {
   EXPECT_FALSE(info_->HasUnclearedAttachment(GL_DEPTH_ATTACHMENT));
   EXPECT_FALSE(info_->HasUnclearedAttachment(GL_STENCIL_ATTACHMENT));
   EXPECT_FALSE(info_->HasUnclearedAttachment(GL_DEPTH_STENCIL_ATTACHMENT));
+  EXPECT_FALSE(info_->IsNotComplete());
 
   RenderbufferManager rb_manager(kMaxRenderbufferSize);
   rb_manager.CreateRenderbufferInfo(
@@ -159,16 +160,19 @@ TEST_F(FramebufferInfoTest, AttachRenderbuffer) {
   info_->AttachRenderbuffer(GL_COLOR_ATTACHMENT0, rb_info1);
   EXPECT_TRUE(info_->HasUnclearedAttachment(GL_COLOR_ATTACHMENT0));
   EXPECT_FALSE(info_->HasUnclearedAttachment(GL_DEPTH_ATTACHMENT));
+  EXPECT_TRUE(info_->IsNotComplete());
 
   // check adding another
   info_->AttachRenderbuffer(GL_DEPTH_ATTACHMENT, rb_info1);
   EXPECT_TRUE(info_->HasUnclearedAttachment(GL_COLOR_ATTACHMENT0));
   EXPECT_TRUE(info_->HasUnclearedAttachment(GL_DEPTH_ATTACHMENT));
+  EXPECT_TRUE(info_->IsNotComplete());
 
   // check marking them as cleared.
   info_->MarkAttachedRenderbuffersAsCleared();
   EXPECT_FALSE(info_->HasUnclearedAttachment(GL_COLOR_ATTACHMENT0));
   EXPECT_FALSE(info_->HasUnclearedAttachment(GL_DEPTH_ATTACHMENT));
+  EXPECT_TRUE(info_->IsNotComplete());
 
   // Check adding one that is already cleared.
   info_->AttachRenderbuffer(GL_STENCIL_ATTACHMENT, rb_info1);
@@ -176,6 +180,7 @@ TEST_F(FramebufferInfoTest, AttachRenderbuffer) {
 
   // Check marking the renderbuffer as unclared.
   rb_info1->SetInfo(kSamples1, kFormat1, kWidth1, kHeight1);
+  EXPECT_FALSE(info_->IsNotComplete());
 
   const FramebufferManager::FramebufferInfo::Attachment* attachment =
       info_->GetAttachment(GL_COLOR_ATTACHMENT0);
@@ -259,6 +264,7 @@ TEST_F(FramebufferInfoTest, AttachTexture) {
   EXPECT_FALSE(info_->HasUnclearedAttachment(GL_DEPTH_ATTACHMENT));
   EXPECT_FALSE(info_->HasUnclearedAttachment(GL_STENCIL_ATTACHMENT));
   EXPECT_FALSE(info_->HasUnclearedAttachment(GL_DEPTH_STENCIL_ATTACHMENT));
+  EXPECT_FALSE(info_->IsNotComplete());
 
   FeatureInfo feature_info;
   TextureManager tex_manager(kMaxTextureSize, kMaxTextureSize);
@@ -267,14 +273,17 @@ TEST_F(FramebufferInfoTest, AttachTexture) {
   TextureManager::TextureInfo* tex_info1 =
       tex_manager.GetTextureInfo(kTextureClient1Id);
   ASSERT_TRUE(tex_info1 != NULL);
-  tex_manager.SetInfoTarget(tex_info1, GL_TEXTURE_2D);
-  tex_manager.SetLevelInfo(
-      &feature_info, tex_info1, GL_TEXTURE_2D, kLevel1,
-      kFormat1, kWidth1, kHeight1, kDepth, kBorder, kFormat1, kType);
 
   // check adding one attachment
   info_->AttachTexture(GL_COLOR_ATTACHMENT0, tex_info1, kTarget1, kLevel1);
   EXPECT_FALSE(info_->HasUnclearedAttachment(GL_COLOR_ATTACHMENT0));
+  EXPECT_TRUE(info_->IsNotComplete());
+
+  tex_manager.SetInfoTarget(tex_info1, GL_TEXTURE_2D);
+  tex_manager.SetLevelInfo(
+      &feature_info, tex_info1, GL_TEXTURE_2D, kLevel1,
+      kFormat1, kWidth1, kHeight1, kDepth, kBorder, kFormat1, kType);
+  EXPECT_FALSE(info_->IsNotComplete());
 
   const FramebufferManager::FramebufferInfo::Attachment* attachment =
       info_->GetAttachment(GL_COLOR_ATTACHMENT0);
diff --git a/gpu/command_buffer/service/gles2_cmd_decoder.cc b/gpu/command_buffer/service/gles2_cmd_decoder.cc
index c263eae..40bc479 100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder.cc
+++ b/gpu/command_buffer/service/gles2_cmd_decoder.cc
@@ -989,6 +989,9 @@ class GLES2DecoderImpl : public base::SupportsWeakPtr<GLES2DecoderImpl>,
   // Remembers the state of some capabilities.
   void SetCapabilityState(GLenum cap, bool enabled);
 
+  // Check that the current frame buffer is complete. Generates error if not.
+  bool CheckFramebufferComplete(const char* func_name);
+
   // Checks if the current program exists and is valid. If not generates the
   // appropriate GL error.  Returns true if the current program is in a usable
   // state.
@@ -1050,6 +1053,9 @@ class GLES2DecoderImpl : public base::SupportsWeakPtr<GLES2DecoderImpl>,
   // Wrapper for glCheckFramebufferStatus
   GLenum DoCheckFramebufferStatus(GLenum target);
 
+  // Wrapper for glClear
+  void DoClear(GLbitfield mask);
+
   // Wrappers for clear and mask settings functions.
   void DoClearColor(
       GLclampf red, GLclampf green, GLclampf blue, GLclampf alpha);
@@ -2218,6 +2224,15 @@ void GLES2DecoderImpl::RestoreCurrentTexture2DBindings() {
   glActiveTexture(GL_TEXTURE0 + active_texture_unit_);
 }
 
+bool GLES2DecoderImpl::CheckFramebufferComplete(const char* func_name) {
+  if (bound_draw_framebuffer_ && bound_draw_framebuffer_->IsNotComplete()) {
+    SetGLError(GL_FRAMEBUFFER_INCOMPLETE_ATTACHMENT,
+               (std::string(func_name) + " framebuffer incomplete").c_str());
+    return false;
+  }
+  return true;
+}
+
 gfx::Size GLES2DecoderImpl::GetBoundReadFrameBufferSize() {
   if (bound_read_framebuffer_ != 0) {
     const FramebufferManager::FramebufferInfo::Attachment* attachment =
@@ -3268,8 +3283,17 @@ error::Error GLES2DecoderImpl::HandleRegisterSharedIdsCHROMIUM(
   return error::kNoError;
 }
 
+void GLES2DecoderImpl::DoClear(GLbitfield mask) {
+  if (CheckFramebufferComplete("glClear")) {
+    glClear(mask);
+  }
+}
+
 void GLES2DecoderImpl::DoDrawArrays(
     GLenum mode, GLint first, GLsizei count) {
+  if (!CheckFramebufferComplete("glDrawArrays")) {
+    return;
+  }
   // We have to check this here because the prototype for glDrawArrays
   // is GLint not GLsizei.
   if (first < 0) {
@@ -3539,11 +3563,20 @@ void GLES2DecoderImpl::DoGetRenderbufferParameteriv(
                "glGetRenderbufferParameteriv: no renderbuffer bound");
     return;
   }
-  if (pname == GL_RENDERBUFFER_INTERNAL_FORMAT) {
+  switch (pname) {
+  case GL_RENDERBUFFER_INTERNAL_FORMAT:
     *params = bound_renderbuffer_->internal_format();
-    return;
+    break;
+  case GL_RENDERBUFFER_WIDTH:
+    *params = bound_renderbuffer_->width();
+    break;
+  case GL_RENDERBUFFER_HEIGHT:
+    *params = bound_renderbuffer_->height();
+    break;
+  default:
+    glGetRenderbufferParameterivEXT(target, pname, params);
+    break;
   }
-  glGetRenderbufferParameterivEXT(target, pname, params);
 }
 
 void GLES2DecoderImpl::DoBlitFramebufferEXT(
@@ -4175,6 +4208,10 @@ error::Error GLES2DecoderImpl::HandleDrawElements(
     return error::kNoError;
   }
 
+  if (!CheckFramebufferComplete("glDrawElements")) {
+    return error::kNoError;
+  }
+
   if (count == 0) {
     return error::kNoError;
   }
diff --git a/gpu/command_buffer/service/gles2_cmd_decoder_autogen.h b/gpu/command_buffer/service/gles2_cmd_decoder_autogen.h
index 78bf68d..06ebf50 100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder_autogen.h
+++ b/gpu/command_buffer/service/gles2_cmd_decoder_autogen.h
@@ -221,7 +221,7 @@ error::Error GLES2DecoderImpl::HandleCheckFramebufferStatus(
 error::Error GLES2DecoderImpl::HandleClear(
     uint32 immediate_data_size, const gles2::Clear& c) {
   GLbitfield mask = static_cast<GLbitfield>(c.mask);
-  glClear(mask);
+  DoClear(mask);
   return error::kNoError;
 }
 
diff --git a/gpu/command_buffer/service/gles2_cmd_decoder_unittest_1_autogen.h b/gpu/command_buffer/service/gles2_cmd_decoder_unittest_1_autogen.h
index 0607a02..477c3bf 100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder_unittest_1_autogen.h
+++ b/gpu/command_buffer/service/gles2_cmd_decoder_unittest_1_autogen.h
@@ -1491,15 +1491,15 @@ TEST_F(GLES2DecoderTest1, GetRenderbufferParameterivValidArgs) {
   Result* result = static_cast<Result*>(shared_memory_address_);
   EXPECT_CALL(
       *gl_, GetRenderbufferParameterivEXT(
-          GL_RENDERBUFFER, GL_RENDERBUFFER_WIDTH, result->GetData()));
+          GL_RENDERBUFFER, GL_RENDERBUFFER_RED_SIZE, result->GetData()));
   result->size = 0;
   GetRenderbufferParameteriv cmd;
   cmd.Init(
-      GL_RENDERBUFFER, GL_RENDERBUFFER_WIDTH, shared_memory_id_,
+      GL_RENDERBUFFER, GL_RENDERBUFFER_RED_SIZE, shared_memory_id_,
       shared_memory_offset_);
   EXPECT_EQ(error::kNoError, ExecuteCmd(cmd));
   EXPECT_EQ(decoder_->GetGLES2Util()->GLGetNumValuesReturned(
-                GL_RENDERBUFFER_WIDTH),
+                GL_RENDERBUFFER_RED_SIZE),
             result->GetNumResults());
   EXPECT_EQ(GL_NO_ERROR, GetGLError());
 }
@@ -1512,7 +1512,7 @@ TEST_F(GLES2DecoderTest1, GetRenderbufferParameterivInvalidArgs0_0) {
   result->size = 0;
   GetRenderbufferParameteriv cmd;
   cmd.Init(
-      GL_FRAMEBUFFER, GL_RENDERBUFFER_WIDTH, shared_memory_id_,
+      GL_FRAMEBUFFER, GL_RENDERBUFFER_RED_SIZE, shared_memory_id_,
       shared_memory_offset_);
   EXPECT_EQ(error::kNoError, ExecuteCmd(cmd));
   EXPECT_EQ(0u, result->size);
@@ -1526,7 +1526,8 @@ TEST_F(GLES2DecoderTest1, GetRenderbufferParameterivInvalidArgs2_0) {
       static_cast<GetRenderbufferParameteriv::Result*>(shared_memory_address_);
   result->size = 0;
   GetRenderbufferParameteriv cmd;
-  cmd.Init(GL_RENDERBUFFER, GL_RENDERBUFFER_WIDTH, kInvalidSharedMemoryId, 0);
+  cmd.Init(
+      GL_RENDERBUFFER, GL_RENDERBUFFER_RED_SIZE, kInvalidSharedMemoryId, 0);
   EXPECT_EQ(error::kOutOfBounds, ExecuteCmd(cmd));
   EXPECT_EQ(0u, result->size);
 }
@@ -1539,7 +1540,7 @@ TEST_F(GLES2DecoderTest1, GetRenderbufferParameterivInvalidArgs2_1) {
   result->size = 0;
   GetRenderbufferParameteriv cmd;
   cmd.Init(
-      GL_RENDERBUFFER, GL_RENDERBUFFER_WIDTH, shared_memory_id_,
+      GL_RENDERBUFFER, GL_RENDERBUFFER_RED_SIZE, shared_memory_id_,
       kInvalidSharedMemoryOffset);
   EXPECT_EQ(error::kOutOfBounds, ExecuteCmd(cmd));
   EXPECT_EQ(0u, result->size);
diff --git a/gpu/command_buffer/service/gles2_cmd_validation_implementation_autogen.h b/gpu/command_buffer/service/gles2_cmd_validation_implementation_autogen.h
index f4ef6d8..62c71df 100644
--- a/gpu/command_buffer/service/gles2_cmd_validation_implementation_autogen.h
+++ b/gpu/command_buffer/service/gles2_cmd_validation_implementation_autogen.h
@@ -270,15 +270,15 @@ static GLenum valid_render_buffer_format_table[] = {
 };
 
 static GLenum valid_render_buffer_parameter_table[] = {
-  GL_RENDERBUFFER_WIDTH,
-  GL_RENDERBUFFER_HEIGHT,
-  GL_RENDERBUFFER_INTERNAL_FORMAT,
   GL_RENDERBUFFER_RED_SIZE,
   GL_RENDERBUFFER_GREEN_SIZE,
   GL_RENDERBUFFER_BLUE_SIZE,
   GL_RENDERBUFFER_ALPHA_SIZE,
   GL_RENDERBUFFER_DEPTH_SIZE,
   GL_RENDERBUFFER_STENCIL_SIZE,
+  GL_RENDERBUFFER_WIDTH,
+  GL_RENDERBUFFER_HEIGHT,
+  GL_RENDERBUFFER_INTERNAL_FORMAT,
 };
 
 static GLenum valid_render_buffer_target_table[] = {