diff options
author | nsylvain@chromium.org <nsylvain@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-09-09 16:37:34 +0000 |
---|---|---|
committer | nsylvain@chromium.org <nsylvain@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-09-09 16:37:34 +0000 |
commit | 3c7303a2fa6b76aa415e3e51241288f95eb80923 (patch) | |
tree | 09ab65cc003b39b34e083dd722355342c977965f /gpu/command_buffer | |
parent | ef96330868a8cd27d62e29ec77ba83b5a2f66588 (diff) | |
download | chromium_src-3c7303a2fa6b76aa415e3e51241288f95eb80923.zip chromium_src-3c7303a2fa6b76aa415e3e51241288f95eb80923.tar.gz chromium_src-3c7303a2fa6b76aa415e3e51241288f95eb80923.tar.bz2 |
Revert 100384 - Fix bug in SimulateAttrib0 that did not check for out of memory.
It also did not correctly check for math overflow. Also fixed
similar bugs in SimulateFixedAttribs
TEST=unit tests
BUG=95625
R=apatrick@chromium.org
Review URL: http://codereview.chromium.org/7845017
TBR=gman@chromium.org
Review URL: http://codereview.chromium.org/7857037
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@100411 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'gpu/command_buffer')
-rw-r--r-- | gpu/command_buffer/service/gles2_cmd_decoder.cc | 63 | ||||
-rw-r--r-- | gpu/command_buffer/service/gles2_cmd_decoder_unittest.cc | 90 |
2 files changed, 32 insertions, 121 deletions
diff --git a/gpu/command_buffer/service/gles2_cmd_decoder.cc b/gpu/command_buffer/service/gles2_cmd_decoder.cc index 61be8de..465bff8 100644 --- a/gpu/command_buffer/service/gles2_cmd_decoder.cc +++ b/gpu/command_buffer/service/gles2_cmd_decoder.cc @@ -1064,9 +1064,8 @@ class GLES2DecoderImpl : public base::SupportsWeakPtr<GLES2DecoderImpl>, // Checks if the current program and vertex attributes are valid for drawing. bool IsDrawValid(GLuint max_vertex_accessed); - // Returns true if successful, simulated will be true if attrib0 was - // simulated. - bool SimulateAttrib0(GLuint max_vertex_accessed, bool* simulated); + // Returns true if attrib0 was simulated. + bool SimulateAttrib0(GLuint max_vertex_accessed); void RestoreStateForSimulatedAttrib0(); // Returns true if textures were set. @@ -4263,46 +4262,30 @@ bool GLES2DecoderImpl::IsDrawValid(GLuint max_vertex_accessed) { return true; } -bool GLES2DecoderImpl::SimulateAttrib0( - GLuint max_vertex_accessed, bool* simulated) { - DCHECK(simulated); - *simulated = false; - +bool GLES2DecoderImpl::SimulateAttrib0(GLuint max_vertex_accessed) { if (gfx::GetGLImplementation() == gfx::kGLImplementationEGLGLES2) - return true; + return false; const VertexAttribManager::VertexAttribInfo* info = vertex_attrib_manager_.GetVertexAttribInfo(0); // If it's enabled or it's not used then we don't need to do anything. bool attrib_0_used = current_program_->GetAttribInfoByLocation(0) != NULL; if (info->enabled() && attrib_0_used) { - return true; + return false; } - // Make a buffer with a single repeated vec4 value enough to - // simulate the constant value that is supposed to be here. - // This is required to emulate GLES2 on GL. typedef VertexAttribManager::VertexAttribInfo::Vec4 Vec4; - GLsizei num_vertices = max_vertex_accessed + 1; - GLsizei size_needed = 0; - - if (num_vertices < 0 || !SafeMultiply( - num_vertices, static_cast<GLsizei>(sizeof(Vec4)), &size_needed)) { - SetGLError(GL_OUT_OF_MEMORY, "glDrawXXX: Simulating attrib 0"); - return false; - } - - CopyRealGLErrorsToWrapper(); glBindBuffer(GL_ARRAY_BUFFER, attrib_0_buffer_id_); + // Make a buffer with a single repeated vec4 value enough to + // simulate the constant value that is supposed to be here. + // This is required to emulate GLES2 on GL. + GLsizei num_vertices = max_vertex_accessed + 1; + GLsizei size_needed = num_vertices * sizeof(Vec4); // NOLINT if (size_needed > attrib_0_size_) { glBufferData(GL_ARRAY_BUFFER, size_needed, NULL, GL_DYNAMIC_DRAW); - GLenum error = glGetError(); - if (error != GL_NO_ERROR) { - SetGLError(GL_OUT_OF_MEMORY, "glDrawXXX: Simulating attrib 0"); - return false; - } + // TODO(gman): check for error here? attrib_0_buffer_matches_value_ = false; } if (attrib_0_used && @@ -4320,7 +4303,6 @@ bool GLES2DecoderImpl::SimulateAttrib0( glVertexAttribPointer(0, 4, GL_FLOAT, GL_FALSE, 0, NULL); - *simulated = true; return true; } @@ -4355,12 +4337,7 @@ bool GLES2DecoderImpl::SimulateFixedAttribs( // tests so we just add to the buffer attrib used. // Compute the number of elements needed. - GLsizei num_vertices = max_vertex_accessed + 1; - if (num_vertices < 0) { - SetGLError(GL_OUT_OF_MEMORY, "glDrawXXX: Simulating attrib 0"); - return false; - } - + int num_vertices = max_vertex_accessed + 1; int elements_needed = 0; const VertexAttribManager::VertexAttribInfoList& infos = vertex_attrib_manager_.GetEnabledVertexAttribInfos(); @@ -4390,16 +4367,10 @@ bool GLES2DecoderImpl::SimulateFixedAttribs( return false; } - CopyRealGLErrorsToWrapper(); glBindBuffer(GL_ARRAY_BUFFER, fixed_attrib_buffer_id_); if (size_needed > fixed_attrib_buffer_size_) { glBufferData(GL_ARRAY_BUFFER, size_needed, NULL, GL_DYNAMIC_DRAW); - GLenum error = glGetError(); - if (error != GL_NO_ERROR) { - SetGLError(GL_OUT_OF_MEMORY, "glDrawXXX: simulating GL_FIXED attribs"); - return false; - } } // Copy the elements and convert to float @@ -4469,10 +4440,7 @@ error::Error GLES2DecoderImpl::HandleDrawArrays( GLuint max_vertex_accessed = first + count - 1; if (IsDrawValid(max_vertex_accessed)) { - bool simulated_attrib_0 = false; - if (!SimulateAttrib0(max_vertex_accessed, &simulated_attrib_0)) { - return error::kNoError; - } + bool simulated_attrib_0 = SimulateAttrib0(max_vertex_accessed); bool simulated_fixed_attribs = false; if (SimulateFixedAttribs(max_vertex_accessed, &simulated_fixed_attribs)) { bool textures_set = SetBlackTextureForNonRenderableTextures(); @@ -4543,10 +4511,7 @@ error::Error GLES2DecoderImpl::HandleDrawElements( } if (IsDrawValid(max_vertex_accessed)) { - bool simulated_attrib_0 = false; - if (!SimulateAttrib0(max_vertex_accessed, &simulated_attrib_0)) { - return error::kNoError; - } + bool simulated_attrib_0 = SimulateAttrib0(max_vertex_accessed); bool simulated_fixed_attribs = false; if (SimulateFixedAttribs(max_vertex_accessed, &simulated_fixed_attribs)) { bool textures_set = SetBlackTextureForNonRenderableTextures(); diff --git a/gpu/command_buffer/service/gles2_cmd_decoder_unittest.cc b/gpu/command_buffer/service/gles2_cmd_decoder_unittest.cc index de27194..d0c63bd 100644 --- a/gpu/command_buffer/service/gles2_cmd_decoder_unittest.cc +++ b/gpu/command_buffer/service/gles2_cmd_decoder_unittest.cc @@ -55,13 +55,8 @@ class GLES2DecoderWithShaderTest : public GLES2DecoderWithShaderTestBase { : GLES2DecoderWithShaderTestBase() { } - - void AddExpectationsForSimulatedAttrib0WithError( - GLsizei num_vertices, GLuint buffer_id, GLenum error) { - EXPECT_CALL(*gl_, GetError()) - .WillOnce(Return(GL_NO_ERROR)) - .WillOnce(Return(error)) - .RetiresOnSaturation(); + void AddExpectationsForSimulatedAttrib0( + GLsizei num_vertices, GLuint buffer_id) { EXPECT_CALL(*gl_, BindBuffer(GL_ARRAY_BUFFER, kServiceAttrib0BufferId)) .Times(1) .RetiresOnSaturation(); @@ -70,30 +65,22 @@ class GLES2DecoderWithShaderTest : public GLES2DecoderWithShaderTestBase { _, GL_DYNAMIC_DRAW)) .Times(1) .RetiresOnSaturation(); - if (error == GL_NO_ERROR) { - EXPECT_CALL(*gl_, BufferSubData( - GL_ARRAY_BUFFER, 0, num_vertices * sizeof(GLfloat) * 4, _)) - .Times(1) - .RetiresOnSaturation(); - EXPECT_CALL(*gl_, VertexAttribPointer(0, 4, GL_FLOAT, GL_FALSE, 0, NULL)) - .Times(1) - .RetiresOnSaturation(); - EXPECT_CALL(*gl_, BindBuffer(GL_ARRAY_BUFFER, 0)) - .Times(1) - .RetiresOnSaturation(); - EXPECT_CALL(*gl_, VertexAttribPointer(0, 4, GL_FLOAT, GL_FALSE, 0, NULL)) - .Times(1) - .RetiresOnSaturation(); - EXPECT_CALL(*gl_, BindBuffer(GL_ARRAY_BUFFER, buffer_id)) - .Times(1) - .RetiresOnSaturation(); - } - } - - void AddExpectationsForSimulatedAttrib0( - GLsizei num_vertices, GLuint buffer_id) { - AddExpectationsForSimulatedAttrib0WithError( - num_vertices, buffer_id, GL_NO_ERROR); + EXPECT_CALL(*gl_, BufferSubData( + GL_ARRAY_BUFFER, 0, num_vertices * sizeof(GLfloat) * 4, _)) + .Times(1) + .RetiresOnSaturation(); + EXPECT_CALL(*gl_, VertexAttribPointer(0, 4, GL_FLOAT, GL_FALSE, 0, NULL)) + .Times(1) + .RetiresOnSaturation(); + EXPECT_CALL(*gl_, BindBuffer(GL_ARRAY_BUFFER, 0)) + .Times(1) + .RetiresOnSaturation(); + EXPECT_CALL(*gl_, VertexAttribPointer(0, 4, GL_FLOAT, GL_FALSE, 0, NULL)) + .Times(1) + .RetiresOnSaturation(); + EXPECT_CALL(*gl_, BindBuffer(GL_ARRAY_BUFFER, buffer_id)) + .Times(1) + .RetiresOnSaturation(); } }; @@ -138,47 +125,6 @@ TEST_F(GLES2DecoderWithShaderTest, DrawArraysNoAttributesSucceeds) { EXPECT_EQ(GL_NO_ERROR, GetGLError()); } -// Tests when the math overflows (0x40000000 * sizeof GLfloat) -TEST_F(GLES2DecoderWithShaderTest, DrawArraysSimulatedAttrib0OverflowFails) { - const GLsizei kLargeCount = 0x40000000; - SetupTexture(); - EXPECT_CALL(*gl_, DrawArrays(_, _, _)) - .Times(0) - .RetiresOnSaturation(); - DrawArrays cmd; - cmd.Init(GL_TRIANGLES, 0, kLargeCount); - EXPECT_EQ(error::kNoError, ExecuteCmd(cmd)); - EXPECT_EQ(GL_OUT_OF_MEMORY, GetGLError()); -} - -// Tests when the math overflows (0x7FFFFFFF + 1 = 0x8000000 verts) -TEST_F(GLES2DecoderWithShaderTest, DrawArraysSimulatedAttrib0PosToNegFails) { - const GLsizei kLargeCount = 0x7FFFFFFF; - SetupTexture(); - EXPECT_CALL(*gl_, DrawArrays(_, _, _)) - .Times(0) - .RetiresOnSaturation(); - DrawArrays cmd; - cmd.Init(GL_TRIANGLES, 0, kLargeCount); - EXPECT_EQ(error::kNoError, ExecuteCmd(cmd)); - EXPECT_EQ(GL_OUT_OF_MEMORY, GetGLError()); -} - -// Tests when the driver returns an error -TEST_F(GLES2DecoderWithShaderTest, DrawArraysSimulatedAttrib0OOMFails) { - const GLsizei kFakeLargeCount = 0x1234; - SetupTexture(); - AddExpectationsForSimulatedAttrib0WithError( - kFakeLargeCount, 0, GL_OUT_OF_MEMORY); - EXPECT_CALL(*gl_, DrawArrays(_, _, _)) - .Times(0) - .RetiresOnSaturation(); - DrawArrays cmd; - cmd.Init(GL_TRIANGLES, 0, kFakeLargeCount); - EXPECT_EQ(error::kNoError, ExecuteCmd(cmd)); - EXPECT_EQ(GL_OUT_OF_MEMORY, GetGLError()); -} - TEST_F(GLES2DecoderWithShaderTest, DrawArraysBadTextureUsesBlack) { DoBindTexture(GL_TEXTURE_2D, client_texture_id_, kServiceTextureId); // This is an NPOT texture. As the default filtering requires mips |