diff options
author | gman@chromium.org <gman@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-11-11 10:43:10 +0000 |
---|---|---|
committer | gman@chromium.org <gman@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-11-11 10:43:10 +0000 |
commit | a0b78dc409eff99d0134b03029032039ad1771a8 (patch) | |
tree | fd844d95aeb3d15d7d68005dc7d40b3a72e83c9a /gpu | |
parent | 23e03468930e5e042b22be9ef07ed00c4c5c1337 (diff) | |
download | chromium_src-a0b78dc409eff99d0134b03029032039ad1771a8.zip chromium_src-a0b78dc409eff99d0134b03029032039ad1771a8.tar.gz chromium_src-a0b78dc409eff99d0134b03029032039ad1771a8.tar.bz2 |
Allow deleted resources to be used
This is to make the command buffer more OpenGL ES 2.0 spec
compliant. Not that to work around bugs in drivers we
will likely have to change all or some resources to not actually
get deleted until the last reference is freed.
That can wait for another CL
TEST=unit tests and ran both ES and WebGL conformance tests
BUG=102412
R=apatrick@chromium.org
Review URL: http://codereview.chromium.org/8515023
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@109612 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'gpu')
-rw-r--r-- | gpu/command_buffer/service/buffer_manager.cc | 3 | ||||
-rw-r--r-- | gpu/command_buffer/service/buffer_manager.h | 2 | ||||
-rw-r--r-- | gpu/command_buffer/service/framebuffer_manager.cc | 48 | ||||
-rw-r--r-- | gpu/command_buffer/service/framebuffer_manager.h | 10 | ||||
-rw-r--r-- | gpu/command_buffer/service/framebuffer_manager_unittest.cc | 73 | ||||
-rw-r--r-- | gpu/command_buffer/service/gles2_cmd_decoder.cc | 153 | ||||
-rw-r--r-- | gpu/command_buffer/service/gles2_cmd_decoder_unittest.cc | 26 | ||||
-rw-r--r-- | gpu/command_buffer/service/texture_manager.cc | 22 | ||||
-rw-r--r-- | gpu/command_buffer/service/texture_manager_unittest.cc | 10 | ||||
-rw-r--r-- | gpu/command_buffer/service/vertex_attrib_manager.cc | 5 | ||||
-rw-r--r-- | gpu/command_buffer/service/vertex_attrib_manager.h | 7 | ||||
-rw-r--r-- | gpu/command_buffer/service/vertex_attrib_manager_unittest.cc | 34 |
12 files changed, 327 insertions, 66 deletions
diff --git a/gpu/command_buffer/service/buffer_manager.cc b/gpu/command_buffer/service/buffer_manager.cc index 3c1d09b..fb9f215 100644 --- a/gpu/command_buffer/service/buffer_manager.cc +++ b/gpu/command_buffer/service/buffer_manager.cc @@ -66,7 +66,6 @@ BufferManager::BufferInfo::~BufferInfo() { } void BufferManager::BufferInfo::SetInfo( GLsizeiptr size, GLenum usage, bool shadow) { - DCHECK(!IsDeleted()); usage_ = usage; if (size != size_ || shadow != shadowed_) { shadowed_ = shadow; @@ -81,7 +80,6 @@ void BufferManager::BufferInfo::SetInfo( bool BufferManager::BufferInfo::SetRange( GLintptr offset, GLsizeiptr size, const GLvoid * data) { - DCHECK(!IsDeleted()); if (offset < 0 || offset + size < offset || offset + size > size_) { return false; } @@ -123,7 +121,6 @@ GLuint GetMaxValue(const void* data, GLuint offset, GLsizei count) { bool BufferManager::BufferInfo::GetMaxValueForRange( GLuint offset, GLsizei count, GLenum type, GLuint* max_value) { - DCHECK(!IsDeleted()); Range range(offset, count, type); RangeToMaxValueMap::iterator it = range_set_.find(range); if (it != range_set_.end()) { diff --git a/gpu/command_buffer/service/buffer_manager.h b/gpu/command_buffer/service/buffer_manager.h index 6f67e53..dedf5d1 100644 --- a/gpu/command_buffer/service/buffer_manager.h +++ b/gpu/command_buffer/service/buffer_manager.h @@ -115,8 +115,6 @@ class BufferManager { void MarkAsDeleted() { service_id_ = 0; - shadow_.reset(); - ClearCache(); } void SetInfo(GLsizeiptr size, GLenum usage, bool shadow); diff --git a/gpu/command_buffer/service/framebuffer_manager.cc b/gpu/command_buffer/service/framebuffer_manager.cc index 0ce9df07..7ca3e65 100644 --- a/gpu/command_buffer/service/framebuffer_manager.cc +++ b/gpu/command_buffer/service/framebuffer_manager.cc @@ -49,6 +49,11 @@ class RenderbufferAttachment return false; } + virtual bool IsRenderbuffer( + RenderbufferManager::RenderbufferInfo* renderbuffer) const { + return renderbuffer_ == renderbuffer; + } + virtual bool CanRenderTo() const { return true; } @@ -123,6 +128,11 @@ class TextureAttachment return texture == texture_.get(); } + virtual bool IsRenderbuffer( + RenderbufferManager::RenderbufferInfo* /* renderbuffer */) const { + return false; + } + TextureManager::TextureInfo* texture() const { return texture_.get(); } @@ -284,6 +294,44 @@ bool FramebufferManager::FramebufferInfo::IsCleared() const { return true; } +void FramebufferManager::FramebufferInfo::UnbindRenderbuffer( + GLenum target, RenderbufferManager::RenderbufferInfo* renderbuffer) { + bool done; + do { + done = true; + for (AttachmentMap::const_iterator it = attachments_.begin(); + it != attachments_.end(); ++it) { + Attachment* attachment = it->second; + if (attachment->IsRenderbuffer(renderbuffer)) { + // TODO(gman): manually detach renderbuffer. + // glFramebufferRenderbufferEXT(target, it->first, GL_RENDERBUFFER, 0); + AttachRenderbuffer(it->first, NULL); + done = false; + break; + } + } + } while (!done); +} + +void FramebufferManager::FramebufferInfo::UnbindTexture( + GLenum target, TextureManager::TextureInfo* texture) { + bool done; + do { + done = true; + for (AttachmentMap::const_iterator it = attachments_.begin(); + it != attachments_.end(); ++it) { + Attachment* attachment = it->second; + if (attachment->IsTexture(texture)) { + // TODO(gman): manually detach texture. + // glFramebufferTexture2DEXT(target, it->first, GL_TEXTURE_2D, 0, 0); + AttachTexture(it->first, NULL, GL_TEXTURE_2D, 0); + done = false; + break; + } + } + } while (!done); +} + 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 78b3e2f..1082c94 100644 --- a/gpu/command_buffer/service/framebuffer_manager.h +++ b/gpu/command_buffer/service/framebuffer_manager.h @@ -39,6 +39,8 @@ class FramebufferManager { RenderbufferManager* renderbuffer_manager, TextureManager* texture_manager) = 0; virtual bool IsTexture(TextureManager::TextureInfo* texture) const = 0; + virtual bool IsRenderbuffer( + RenderbufferManager::RenderbufferInfo* renderbuffer) const = 0; virtual bool CanRenderTo() const = 0; virtual void DetachFromFramebuffer() = 0; virtual bool ValidForAttachmentType(GLenum attachment_type) = 0; @@ -62,6 +64,14 @@ class FramebufferManager { GLenum attachment, TextureManager::TextureInfo* texture, GLenum target, GLint level); + // Unbinds the given renderbuffer if it is bound. + void UnbindRenderbuffer( + GLenum target, RenderbufferManager::RenderbufferInfo* renderbuffer); + + // Unbinds the given texture if it is bound. + void UnbindTexture( + GLenum target, TextureManager::TextureInfo* texture); + void MarkAttachmentsAsCleared( RenderbufferManager* renderbuffer_manager, TextureManager* texture_manager); diff --git a/gpu/command_buffer/service/framebuffer_manager_unittest.cc b/gpu/command_buffer/service/framebuffer_manager_unittest.cc index 264245f..f7aa23a 100644 --- a/gpu/command_buffer/service/framebuffer_manager_unittest.cc +++ b/gpu/command_buffer/service/framebuffer_manager_unittest.cc @@ -492,6 +492,79 @@ TEST_F(FramebufferInfoTest, AttachTexture) { EXPECT_TRUE(info_->IsCleared()); } +TEST_F(FramebufferInfoTest, UnbindRenderbuffer) { + const GLuint kRenderbufferClient1Id = 33; + const GLuint kRenderbufferService1Id = 333; + const GLuint kRenderbufferClient2Id = 34; + const GLuint kRenderbufferService2Id = 334; + + renderbuffer_manager_.CreateRenderbufferInfo( + kRenderbufferClient1Id, kRenderbufferService1Id); + RenderbufferManager::RenderbufferInfo* rb_info1 = + renderbuffer_manager_.GetRenderbufferInfo(kRenderbufferClient1Id); + ASSERT_TRUE(rb_info1 != NULL); + renderbuffer_manager_.CreateRenderbufferInfo( + kRenderbufferClient2Id, kRenderbufferService2Id); + RenderbufferManager::RenderbufferInfo* rb_info2 = + renderbuffer_manager_.GetRenderbufferInfo(kRenderbufferClient2Id); + ASSERT_TRUE(rb_info2 != NULL); + + // Attach to 2 attachment points. + info_->AttachRenderbuffer(GL_COLOR_ATTACHMENT0, rb_info1); + info_->AttachRenderbuffer(GL_DEPTH_ATTACHMENT, rb_info1); + // Check they were attached. + EXPECT_TRUE(info_->GetAttachment(GL_COLOR_ATTACHMENT0) != NULL); + EXPECT_TRUE(info_->GetAttachment(GL_DEPTH_ATTACHMENT) != NULL); + // Unbind unattached renderbuffer. + info_->UnbindRenderbuffer(GL_RENDERBUFFER, rb_info2); + // Should be no-op. + EXPECT_TRUE(info_->GetAttachment(GL_COLOR_ATTACHMENT0) != NULL); + EXPECT_TRUE(info_->GetAttachment(GL_DEPTH_ATTACHMENT) != NULL); + // Unbind renderbuffer. + info_->UnbindRenderbuffer(GL_RENDERBUFFER, rb_info1); + // Check they were detached + EXPECT_TRUE(info_->GetAttachment(GL_COLOR_ATTACHMENT0) == NULL); + EXPECT_TRUE(info_->GetAttachment(GL_DEPTH_ATTACHMENT) == NULL); +} + +TEST_F(FramebufferInfoTest, UnbindTexture) { + const GLuint kTextureClient1Id = 33; + const GLuint kTextureService1Id = 333; + const GLuint kTextureClient2Id = 34; + const GLuint kTextureService2Id = 334; + const GLenum kTarget1 = GL_TEXTURE_2D; + const GLint kLevel1 = 0; + + FeatureInfo feature_info; + texture_manager_.CreateTextureInfo( + &feature_info, kTextureClient1Id, kTextureService1Id); + TextureManager::TextureInfo* tex_info1 = + texture_manager_.GetTextureInfo(kTextureClient1Id); + ASSERT_TRUE(tex_info1 != NULL); + texture_manager_.CreateTextureInfo( + &feature_info, kTextureClient2Id, kTextureService2Id); + TextureManager::TextureInfo* tex_info2 = + texture_manager_.GetTextureInfo(kTextureClient2Id); + ASSERT_TRUE(tex_info2 != NULL); + + // Attach to 2 attachment points. + info_->AttachTexture(GL_COLOR_ATTACHMENT0, tex_info1, kTarget1, kLevel1); + info_->AttachTexture(GL_DEPTH_ATTACHMENT, tex_info1, kTarget1, kLevel1); + // Check they were attached. + EXPECT_TRUE(info_->GetAttachment(GL_COLOR_ATTACHMENT0) != NULL); + EXPECT_TRUE(info_->GetAttachment(GL_DEPTH_ATTACHMENT) != NULL); + // Unbind unattached texture. + info_->UnbindTexture(kTarget1, tex_info2); + // Should be no-op. + EXPECT_TRUE(info_->GetAttachment(GL_COLOR_ATTACHMENT0) != NULL); + EXPECT_TRUE(info_->GetAttachment(GL_DEPTH_ATTACHMENT) != NULL); + // Unbind texture. + info_->UnbindTexture(kTarget1, tex_info1); + // Check they were detached + EXPECT_TRUE(info_->GetAttachment(GL_COLOR_ATTACHMENT0) == NULL); + EXPECT_TRUE(info_->GetAttachment(GL_DEPTH_ATTACHMENT) == NULL); +} + } // namespace gles2 } // namespace gpu diff --git a/gpu/command_buffer/service/gles2_cmd_decoder.cc b/gpu/command_buffer/service/gles2_cmd_decoder.cc index eb7c6df..f31effa 100644 --- a/gpu/command_buffer/service/gles2_cmd_decoder.cc +++ b/gpu/command_buffer/service/gles2_cmd_decoder.cc @@ -566,6 +566,18 @@ class GLES2DecoderImpl : public base::SupportsWeakPtr<GLES2DecoderImpl>, (type == GL_SAMPLER_EXTERNAL_OES ? bound_texture_external_oes : bound_texture_cube_map); } + + void Unbind(TextureManager::TextureInfo* texture) { + if (bound_texture_2d == texture) { + bound_texture_2d = NULL; + } + if (bound_texture_cube_map == texture) { + bound_texture_cube_map = NULL; + } + if (bound_texture_external_oes == texture) { + bound_texture_external_oes = NULL; + } + } }; // Initialize or re-initialize the shader translator. @@ -623,7 +635,7 @@ class GLES2DecoderImpl : public base::SupportsWeakPtr<GLES2DecoderImpl>, TextureManager::TextureInfo* GetTextureInfo(GLuint client_id) { TextureManager::TextureInfo* info = texture_manager()->GetTextureInfo(client_id); - return (info && !info->IsDeleted()) ? info : NULL; + return info; } // Deletes the texture info for the given texture. @@ -782,7 +794,7 @@ class GLES2DecoderImpl : public base::SupportsWeakPtr<GLES2DecoderImpl>, BufferManager::BufferInfo* GetBufferInfo(GLuint client_id) { BufferManager::BufferInfo* info = buffer_manager()->GetBufferInfo(client_id); - return (info && !info->IsDeleted()) ? info : NULL; + return info; } // Removes any buffers in the VertexAtrribInfos and BufferInfos. This is used @@ -800,7 +812,7 @@ class GLES2DecoderImpl : public base::SupportsWeakPtr<GLES2DecoderImpl>, GLuint client_id) { FramebufferManager::FramebufferInfo* info = framebuffer_manager()->GetFramebufferInfo(client_id); - return (info && !info->IsDeleted()) ? info : NULL; + return info; } // Removes the framebuffer info for the given framebuffer. @@ -819,7 +831,7 @@ class GLES2DecoderImpl : public base::SupportsWeakPtr<GLES2DecoderImpl>, GLuint client_id) { RenderbufferManager::RenderbufferInfo* info = renderbuffer_manager()->GetRenderbufferInfo(client_id); - return (info && !info->IsDeleted()) ? info : NULL; + return info; } // Removes the renderbuffer info for the given renderbuffer. @@ -1145,7 +1157,7 @@ class GLES2DecoderImpl : public base::SupportsWeakPtr<GLES2DecoderImpl>, DCHECK(target == GL_ARRAY_BUFFER || target == GL_ELEMENT_ARRAY_BUFFER); BufferManager::BufferInfo* info = target == GL_ARRAY_BUFFER ? bound_array_buffer_ : bound_element_array_buffer_; - return (info && !info->IsDeleted()) ? info : NULL; + return info; } // Gets the texture id for a given target. @@ -1176,7 +1188,7 @@ class GLES2DecoderImpl : public base::SupportsWeakPtr<GLES2DecoderImpl>, NOTREACHED(); return NULL; } - return (info && !info->IsDeleted()) ? info : NULL; + return info; } GLenum GetBindTargetForSamplerType(GLenum type) { @@ -1203,7 +1215,7 @@ class GLES2DecoderImpl : public base::SupportsWeakPtr<GLES2DecoderImpl>, NOTREACHED(); break; } - return (info && !info->IsDeleted()) ? info : NULL; + return info; } RenderbufferManager::RenderbufferInfo* GetRenderbufferInfoForTarget( @@ -1217,7 +1229,7 @@ class GLES2DecoderImpl : public base::SupportsWeakPtr<GLES2DecoderImpl>, NOTREACHED(); break; } - return (info && !info->IsDeleted()) ? info : NULL; + return info; } // Validates the program and location for a glGetUniform call and returns @@ -2186,9 +2198,16 @@ bool GLES2DecoderImpl::GenTexturesHelper(GLsizei n, const GLuint* client_ids) { void GLES2DecoderImpl::DeleteBuffersHelper( GLsizei n, const GLuint* client_ids) { for (GLsizei ii = 0; ii < n; ++ii) { - BufferManager::BufferInfo* info = GetBufferInfo(client_ids[ii]); - if (info) { - GLuint service_id = info->service_id(); + BufferManager::BufferInfo* buffer = GetBufferInfo(client_ids[ii]); + if (buffer && !buffer->IsDeleted()) { + vertex_attrib_manager_.Unbind(buffer); + if (bound_array_buffer_ == buffer) { + bound_array_buffer_ = NULL; + } + if (bound_element_array_buffer_ == buffer) { + bound_element_array_buffer_ = NULL; + } + GLuint service_id = buffer->service_id(); glDeleteBuffersARB(1, &service_id); RemoveBufferInfo(client_ids[ii]); } @@ -2201,23 +2220,23 @@ void GLES2DecoderImpl::DeleteFramebuffersHelper( feature_info_->feature_flags().chromium_framebuffer_multisample; for (GLsizei ii = 0; ii < n; ++ii) { - FramebufferManager::FramebufferInfo* info = + FramebufferManager::FramebufferInfo* framebuffer = GetFramebufferInfo(client_ids[ii]); - if (info) { - if (info == bound_draw_framebuffer_) { + if (framebuffer && !framebuffer->IsDeleted()) { + if (framebuffer == bound_draw_framebuffer_) { bound_draw_framebuffer_ = NULL; state_dirty_ = true; GLenum target = supports_seperate_framebuffer_binds ? GL_DRAW_FRAMEBUFFER : GL_FRAMEBUFFER; glBindFramebufferEXT(target, GetBackbufferServiceId()); } - if (info == bound_read_framebuffer_) { + if (framebuffer == bound_read_framebuffer_) { bound_read_framebuffer_ = NULL; GLenum target = supports_seperate_framebuffer_binds ? GL_READ_FRAMEBUFFER : GL_FRAMEBUFFER; glBindFramebufferEXT(target, GetBackbufferServiceId()); } - GLuint service_id = info->service_id(); + GLuint service_id = framebuffer->service_id(); glDeleteFramebuffersEXT(1, &service_id); RemoveFramebufferInfo(client_ids[ii]); } @@ -2226,12 +2245,33 @@ void GLES2DecoderImpl::DeleteFramebuffersHelper( void GLES2DecoderImpl::DeleteRenderbuffersHelper( GLsizei n, const GLuint* client_ids) { + bool supports_seperate_framebuffer_binds = + feature_info_->feature_flags().chromium_framebuffer_multisample; for (GLsizei ii = 0; ii < n; ++ii) { - RenderbufferManager::RenderbufferInfo* info = + RenderbufferManager::RenderbufferInfo* renderbuffer = GetRenderbufferInfo(client_ids[ii]); - if (info) { + if (renderbuffer && !renderbuffer->IsDeleted()) { + if (bound_renderbuffer_ == renderbuffer) { + bound_renderbuffer_ = NULL; + } + // Unbind from current framebuffers. + if (supports_seperate_framebuffer_binds) { + if (bound_read_framebuffer_) { + bound_read_framebuffer_->UnbindRenderbuffer( + GL_READ_FRAMEBUFFER, renderbuffer); + } + if (bound_draw_framebuffer_) { + bound_draw_framebuffer_->UnbindRenderbuffer( + GL_DRAW_FRAMEBUFFER, renderbuffer); + } + } else { + if (bound_draw_framebuffer_) { + bound_draw_framebuffer_->UnbindRenderbuffer( + GL_FRAMEBUFFER, renderbuffer); + } + } state_dirty_ = true; - GLuint service_id = info->service_id(); + GLuint service_id = renderbuffer->service_id(); glDeleteRenderbuffersEXT(1, &service_id); RemoveRenderbufferInfo(client_ids[ii]); } @@ -2240,14 +2280,33 @@ void GLES2DecoderImpl::DeleteRenderbuffersHelper( void GLES2DecoderImpl::DeleteTexturesHelper( GLsizei n, const GLuint* client_ids) { + bool supports_seperate_framebuffer_binds = + feature_info_->feature_flags().chromium_framebuffer_multisample; for (GLsizei ii = 0; ii < n; ++ii) { - TextureManager::TextureInfo* info = GetTextureInfo(client_ids[ii]); - if (info) { - if (info->IsAttachedToFramebuffer()) { + TextureManager::TextureInfo* texture = GetTextureInfo(client_ids[ii]); + if (texture && !texture->IsDeleted()) { + if (texture->IsAttachedToFramebuffer()) { state_dirty_ = true; } - GLuint service_id = info->service_id(); - if (info->IsStreamTexture() && stream_texture_manager_) { + // Unbind texture from texture units. + for (size_t jj = 0; jj < group_->max_texture_units(); ++jj) { + texture_units_[ii].Unbind(texture); + } + // Unbind from current framebuffers. + if (supports_seperate_framebuffer_binds) { + if (bound_read_framebuffer_) { + bound_read_framebuffer_->UnbindTexture(GL_READ_FRAMEBUFFER, texture); + } + if (bound_draw_framebuffer_) { + bound_draw_framebuffer_->UnbindTexture(GL_DRAW_FRAMEBUFFER, texture); + } + } else { + if (bound_draw_framebuffer_) { + bound_draw_framebuffer_->UnbindTexture(GL_FRAMEBUFFER, texture); + } + } + GLuint service_id = texture->service_id(); + if (texture->IsStreamTexture() && stream_texture_manager_) { stream_texture_manager_->DestroyStreamTexture(service_id); } glDeleteTextures(1, &service_id); @@ -2329,7 +2388,7 @@ void GLES2DecoderImpl::RestoreCurrentTexture2DBindings() { bool GLES2DecoderImpl::CheckFramebufferValid( FramebufferManager::FramebufferInfo* framebuffer, GLenum target, const char* func_name) { - if (!framebuffer || framebuffer->IsDeleted()) { + if (!framebuffer) { return true; } @@ -3798,7 +3857,6 @@ void GLES2DecoderImpl::DoStencilMaskSeparate(GLenum face, GLuint mask) { // Assumes framebuffer is complete. void GLES2DecoderImpl::ClearUnclearedAttachments( GLenum target, FramebufferManager::FramebufferInfo* info) { - DCHECK(!info->IsDeleted()); if (target == GL_READ_FRAMEBUFFER_EXT) { // bind this to the DRAW point, clear then bind back to READ // TODO(gman): I don't think there is any guarantee that an FBO that @@ -4564,7 +4622,7 @@ bool GLES2DecoderImpl::IsDrawValid(GLuint max_vertex_accessed) { } } else { // This attrib is not used in the current program. - if (!info->buffer() || info->buffer()->IsDeleted()) { + if (!info->buffer()) { SetGLError( GL_INVALID_OPERATION, "glDrawXXX: attempt to render with no buffer attached to enabled " @@ -4817,8 +4875,7 @@ error::Error GLES2DecoderImpl::HandleDrawArrays( error::Error GLES2DecoderImpl::HandleDrawElements( uint32 immediate_data_size, const gles2::DrawElements& c) { - if (!bound_element_array_buffer_ || - bound_element_array_buffer_->IsDeleted()) { + if (!bound_element_array_buffer_) { SetGLError(GL_INVALID_OPERATION, "glDrawElements: No element array buffer bound"); return error::kNoError; @@ -5117,37 +5174,39 @@ error::Error GLES2DecoderImpl::HandleGetShaderInfoLog( } bool GLES2DecoderImpl::DoIsBuffer(GLuint client_id) { - const BufferManager::BufferInfo* info = GetBufferInfo(client_id); - return info && info->IsValid(); + const BufferManager::BufferInfo* buffer = GetBufferInfo(client_id); + return buffer && buffer->IsValid() && !buffer->IsDeleted(); } bool GLES2DecoderImpl::DoIsFramebuffer(GLuint client_id) { - const FramebufferManager::FramebufferInfo* info = + const FramebufferManager::FramebufferInfo* framebuffer = GetFramebufferInfo(client_id); - return info && info->IsValid(); + return framebuffer && framebuffer->IsValid() && !framebuffer->IsDeleted(); } bool GLES2DecoderImpl::DoIsProgram(GLuint client_id) { // IsProgram is true for programs as soon as they are created, until they are // deleted and no longer in use. - return GetProgramInfo(client_id) != NULL; + const ProgramManager::ProgramInfo* program = GetProgramInfo(client_id); + return program != NULL && !program->IsDeleted(); } bool GLES2DecoderImpl::DoIsRenderbuffer(GLuint client_id) { - const RenderbufferManager::RenderbufferInfo* info = + const RenderbufferManager::RenderbufferInfo* renderbuffer = GetRenderbufferInfo(client_id); - return info && info->IsValid(); + return renderbuffer && renderbuffer->IsValid() && !renderbuffer->IsDeleted(); } bool GLES2DecoderImpl::DoIsShader(GLuint client_id) { // IsShader is true for shaders as soon as they are created, until they // are deleted and not attached to any programs. - return GetShaderInfo(client_id) != NULL; + const ShaderManager::ShaderInfo* shader = GetShaderInfo(client_id); + return shader != NULL && !shader->IsDeleted(); } bool GLES2DecoderImpl::DoIsTexture(GLuint client_id) { - const TextureManager::TextureInfo* info = GetTextureInfo(client_id); - return info && info->IsValid(); + const TextureManager::TextureInfo* texture = GetTextureInfo(client_id); + return texture && texture->IsValid() && !texture->IsDeleted(); } void GLES2DecoderImpl::DoAttachShader( @@ -5529,10 +5588,6 @@ error::Error GLES2DecoderImpl::HandleReadPixels( return error::kNoError; } - CopyRealGLErrorsToWrapper(); - - ScopedResolvedFrameBufferBinder binder(this, false, true); - // Get the size of the current fbo or backbuffer. gfx::Size max_size = GetBoundReadFrameBufferSize(); @@ -5547,6 +5602,10 @@ error::Error GLES2DecoderImpl::HandleReadPixels( return error::kNoError; } + CopyRealGLErrorsToWrapper(); + + ScopedResolvedFrameBufferBinder binder(this, false, true); + if (x < 0 || y < 0 || max_x > max_size.width() || max_y > max_size.height()) { // The user requested an out of range area. Get the results 1 line // at a time. @@ -6387,6 +6446,10 @@ void GLES2DecoderImpl::DoCopyTexImage2D( return; } + if (!CheckBoundFramebuffersValid("glCopyTexImage2D")) { + return; + } + CopyRealGLErrorsToWrapper(); ScopedResolvedFrameBufferBinder binder(this, false, true); gfx::Size size = GetBoundReadFrameBufferSize(); @@ -6471,6 +6534,10 @@ void GLES2DecoderImpl::DoCopyTexSubImage2D( return; } + if (!CheckBoundFramebuffersValid("glCopyTexSubImage2D")) { + return; + } + ScopedResolvedFrameBufferBinder binder(this, false, true); gfx::Size size = GetBoundReadFrameBufferSize(); GLint copyX = 0; diff --git a/gpu/command_buffer/service/gles2_cmd_decoder_unittest.cc b/gpu/command_buffer/service/gles2_cmd_decoder_unittest.cc index 3b206f7..cb52457 100644 --- a/gpu/command_buffer/service/gles2_cmd_decoder_unittest.cc +++ b/gpu/command_buffer/service/gles2_cmd_decoder_unittest.cc @@ -5392,6 +5392,32 @@ TEST_F(GLES2DecoderWithShaderTest, EXPECT_EQ(GL_NO_ERROR, GetGLError()); } +TEST_F(GLES2DecoderWithShaderTest, CopyTexImageWithInCompleteFBOFails) { + GLenum target = GL_TEXTURE_2D; + GLint level = 0; + GLenum internal_format = GL_RGBA; + GLsizei width = 2; + GLsizei height = 4; + GLint border = 0; + SetupTexture(); + DoBindRenderbuffer(GL_RENDERBUFFER, client_renderbuffer_id_, + kServiceRenderbufferId); + DoBindFramebuffer(GL_FRAMEBUFFER, client_framebuffer_id_, + kServiceFramebufferId); + DoRenderbufferStorage( + GL_RENDERBUFFER, GL_RGBA4, GL_RGBA, 0, 0, GL_NO_ERROR); + DoFramebufferRenderbuffer( + GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_RENDERBUFFER, + client_renderbuffer_id_, kServiceRenderbufferId, GL_NO_ERROR); + + EXPECT_CALL(*gl_, CopyTexImage2D(_, _, _, _, _, _, _, _)) + .Times(0) + .RetiresOnSaturation(); + CopyTexImage2D cmd; + cmd.Init(target, level, internal_format, 0, 0, width, height, border); + EXPECT_EQ(error::kNoError, ExecuteCmd(cmd)); + EXPECT_EQ(GL_INVALID_FRAMEBUFFER_OPERATION, GetGLError()); +} // TODO(gman): Complete this test. diff --git a/gpu/command_buffer/service/texture_manager.cc b/gpu/command_buffer/service/texture_manager.cc index e5ace9f..3a433d3 100644 --- a/gpu/command_buffer/service/texture_manager.cc +++ b/gpu/command_buffer/service/texture_manager.cc @@ -88,7 +88,7 @@ void TextureManager::Destroy(bool have_context) { bool TextureManager::TextureInfo::CanRender( const FeatureInfo* feature_info) const { - if (target_ == 0 || IsDeleted()) { + if (target_ == 0) { return false; } bool needs_mips = NeedsMips(); @@ -160,7 +160,7 @@ void TextureManager::TextureInfo::SetTarget(GLenum target, GLint max_levels) { bool TextureManager::TextureInfo::CanGenerateMipmaps( const FeatureInfo* feature_info) const { if ((npot() && !feature_info->feature_flags().npot_ok) || - level_infos_.empty() || IsDeleted() || + level_infos_.empty() || target_ == GL_TEXTURE_EXTERNAL_OES) { return false; } @@ -272,7 +272,7 @@ bool TextureManager::TextureInfo::ValidForTexture( GLenum format, GLenum type) const { size_t face_index = GLTargetToFaceIndex(face); - if (!IsDeleted() && level >= 0 && face_index < level_infos_.size() && + if (level >= 0 && face_index < level_infos_.size() && static_cast<size_t>(level) < level_infos_[face_index].size()) { const LevelInfo& info = level_infos_[GLTargetToFaceIndex(face)][level]; GLint right; @@ -294,7 +294,7 @@ bool TextureManager::TextureInfo::GetLevelSize( DCHECK(width); DCHECK(height); size_t face_index = GLTargetToFaceIndex(face); - if (!IsDeleted() && level >= 0 && face_index < level_infos_.size() && + if (level >= 0 && face_index < level_infos_.size() && static_cast<size_t>(level) < level_infos_[face_index].size()) { const LevelInfo& info = level_infos_[GLTargetToFaceIndex(face)][level]; if (info.target != 0) { @@ -311,7 +311,7 @@ bool TextureManager::TextureInfo::GetLevelType( DCHECK(type); DCHECK(internal_format); size_t face_index = GLTargetToFaceIndex(face); - if (!IsDeleted() && level >= 0 && face_index < level_infos_.size() && + if (level >= 0 && face_index < level_infos_.size() && static_cast<size_t>(level) < level_infos_[face_index].size()) { const LevelInfo& info = level_infos_[GLTargetToFaceIndex(face)][level]; if (info.target != 0) { @@ -479,8 +479,7 @@ bool TextureManager::TextureInfo::ClearRenderableLevels(GLES2Decoder* decoder) { bool TextureManager::TextureInfo::IsLevelCleared(GLenum target, GLint level) { size_t face_index = GLTargetToFaceIndex(target); - if (IsDeleted() || - face_index >= level_infos_.size() || + if (face_index >= level_infos_.size() || level >= static_cast<GLint>(level_infos_[face_index].size())) { return true; } @@ -494,8 +493,7 @@ bool TextureManager::TextureInfo::ClearLevel( GLES2Decoder* decoder, GLenum target, GLint level) { DCHECK(decoder); size_t face_index = GLTargetToFaceIndex(target); - if (IsDeleted() || - face_index >= level_infos_.size() || + if (face_index >= level_infos_.size() || level >= static_cast<GLint>(level_infos_[face_index].size())) { return true; } @@ -650,7 +648,6 @@ void TextureManager::SetInfoTarget( void TextureManager::SetLevelCleared( TextureManager::TextureInfo* info, GLenum target, GLint level) { DCHECK(info); - DCHECK(!info->IsDeleted()); if (!info->SafeToRenderFrom()) { DCHECK_NE(0, num_unsafe_textures_); --num_unsafe_textures_; @@ -667,7 +664,6 @@ void TextureManager::SetLevelCleared( bool TextureManager::ClearRenderableLevels( GLES2Decoder* decoder,TextureManager::TextureInfo* info) { DCHECK(info); - DCHECK(!info->IsDeleted()); if (info->SafeToRenderFrom()) { return true; } @@ -687,7 +683,6 @@ bool TextureManager::ClearTextureLevel( GLES2Decoder* decoder,TextureManager::TextureInfo* info, GLenum target, GLint level) { DCHECK(info); - DCHECK(!info->IsDeleted()); if (info->num_uncleared_mips() == 0) { return true; } @@ -720,7 +715,6 @@ void TextureManager::SetLevelInfo( GLenum type, bool cleared) { DCHECK(info); - DCHECK(!info->IsDeleted()); if (!info->CanRender(feature_info)) { DCHECK_NE(0, num_unrenderable_textures_); --num_unrenderable_textures_; @@ -748,7 +742,6 @@ bool TextureManager::SetParameter( TextureManager::TextureInfo* info, GLenum pname, GLint param) { DCHECK(feature_info); DCHECK(info); - DCHECK(!info->IsDeleted()); if (!info->CanRender(feature_info)) { DCHECK_NE(0, num_unrenderable_textures_); --num_unrenderable_textures_; @@ -771,7 +764,6 @@ bool TextureManager::MarkMipmapsGenerated( const FeatureInfo* feature_info, TextureManager::TextureInfo* info) { DCHECK(info); - DCHECK(!info->IsDeleted()); if (!info->CanRender(feature_info)) { DCHECK_NE(0, num_unrenderable_textures_); --num_unrenderable_textures_; diff --git a/gpu/command_buffer/service/texture_manager_unittest.cc b/gpu/command_buffer/service/texture_manager_unittest.cc index 44ce830..15fc02e 100644 --- a/gpu/command_buffer/service/texture_manager_unittest.cc +++ b/gpu/command_buffer/service/texture_manager_unittest.cc @@ -572,7 +572,9 @@ TEST_F(TextureInfoTest, GetLevelSize) { EXPECT_EQ(4, width); EXPECT_EQ(5, height); manager_.RemoveTextureInfo(&feature_info_, kClient1Id); - EXPECT_FALSE(info_->GetLevelSize(GL_TEXTURE_2D, 1, &width, &height)); + EXPECT_TRUE(info_->GetLevelSize(GL_TEXTURE_2D, 1, &width, &height)); + EXPECT_EQ(4, width); + EXPECT_EQ(5, height); } TEST_F(TextureInfoTest, GetLevelType) { @@ -588,7 +590,9 @@ TEST_F(TextureInfoTest, GetLevelType) { EXPECT_EQ(static_cast<GLenum>(GL_UNSIGNED_BYTE), type); EXPECT_EQ(static_cast<GLenum>(GL_RGBA), format); manager_.RemoveTextureInfo(&feature_info_, kClient1Id); - EXPECT_FALSE(info_->GetLevelType(GL_TEXTURE_2D, 1, &type, &format)); + EXPECT_TRUE(info_->GetLevelType(GL_TEXTURE_2D, 1, &type, &format)); + EXPECT_EQ(static_cast<GLenum>(GL_UNSIGNED_BYTE), type); + EXPECT_EQ(static_cast<GLenum>(GL_RGBA), format); } TEST_F(TextureInfoTest, ValidForTexture) { @@ -633,7 +637,7 @@ TEST_F(TextureInfoTest, ValidForTexture) { EXPECT_TRUE(info_->ValidForTexture( GL_TEXTURE_2D, 1, 1, 1, 2, 3, GL_RGBA, GL_UNSIGNED_BYTE)); manager_.RemoveTextureInfo(&feature_info_, kClient1Id); - EXPECT_FALSE(info_->ValidForTexture( + EXPECT_TRUE(info_->ValidForTexture( GL_TEXTURE_2D, 1, 0, 0, 4, 5, GL_RGBA, GL_UNSIGNED_BYTE)); } diff --git a/gpu/command_buffer/service/vertex_attrib_manager.cc b/gpu/command_buffer/service/vertex_attrib_manager.cc index 7ba709b..9df444c 100644 --- a/gpu/command_buffer/service/vertex_attrib_manager.cc +++ b/gpu/command_buffer/service/vertex_attrib_manager.cc @@ -88,6 +88,11 @@ bool VertexAttribManager::Enable(GLuint index, bool enable) { return true; } +void VertexAttribManager::Unbind(BufferManager::BufferInfo* buffer) { + for (uint32 vv = 0; vv < max_vertex_attribs_; ++vv) { + vertex_attrib_infos_[vv].Unbind(buffer); + } +} } // namespace gles2 } // namespace gpu diff --git a/gpu/command_buffer/service/vertex_attrib_manager.h b/gpu/command_buffer/service/vertex_attrib_manager.h index ef24696..9cbc18f 100644 --- a/gpu/command_buffer/service/vertex_attrib_manager.h +++ b/gpu/command_buffer/service/vertex_attrib_manager.h @@ -111,6 +111,12 @@ class VertexAttribManager { offset_ = offset; } + void Unbind(BufferManager::BufferInfo* buffer) { + if (buffer_ == buffer) { + buffer_ = NULL; + } + } + // The index of this attrib. GLuint index_; @@ -195,6 +201,7 @@ class VertexAttribManager { } } + void Unbind(BufferManager::BufferInfo* buffer); private: uint32 max_vertex_attribs_; diff --git a/gpu/command_buffer/service/vertex_attrib_manager_unittest.cc b/gpu/command_buffer/service/vertex_attrib_manager_unittest.cc index 49c1883..ab51f83 100644 --- a/gpu/command_buffer/service/vertex_attrib_manager_unittest.cc +++ b/gpu/command_buffer/service/vertex_attrib_manager_unittest.cc @@ -174,6 +174,40 @@ TEST_F(VertexAttribManagerTest, CanAccess) { buffer_manager.Destroy(false); } +TEST_F(VertexAttribManagerTest, Unbind) { + BufferManager buffer_manager; + buffer_manager.CreateBufferInfo(1, 2); + buffer_manager.CreateBufferInfo(3, 4); + BufferManager::BufferInfo* buffer1 = buffer_manager.GetBufferInfo(1); + BufferManager::BufferInfo* buffer2 = buffer_manager.GetBufferInfo(3); + ASSERT_TRUE(buffer1 != NULL); + ASSERT_TRUE(buffer2 != NULL); + + VertexAttribManager::VertexAttribInfo* info1 = + manager_.GetVertexAttribInfo(1); + VertexAttribManager::VertexAttribInfo* info3 = + manager_.GetVertexAttribInfo(3); + + // Attach to 2 buffers. + manager_.SetAttribInfo(1, buffer1, 3, GL_SHORT, GL_TRUE, 32, 32, 4); + manager_.SetAttribInfo(3, buffer1, 3, GL_SHORT, GL_TRUE, 32, 32, 4); + // Check they were attached. + EXPECT_EQ(buffer1, info1->buffer()); + EXPECT_EQ(buffer1, info3->buffer()); + // Unbind unattached buffer. + manager_.Unbind(buffer2); + // Should be no-op. + EXPECT_EQ(buffer1, info1->buffer()); + EXPECT_EQ(buffer1, info3->buffer()); + // Unbind buffer. + manager_.Unbind(buffer1); + // Check they were detached + EXPECT_TRUE(NULL == info1->buffer()); + EXPECT_TRUE(NULL == info3->buffer()); + + buffer_manager.Destroy(false); +} + } // namespace gles2 } // namespace gpu |