From a0ee95ed8a530ebd2ee428162fa80ba47e72802c Mon Sep 17 00:00:00 2001 From: zmo Date: Fri, 10 Jul 2015 18:33:11 -0700 Subject: [PATCH] Fix framebuffer completeness ES3 behavior. In ES3, dimensions of images are no longer required to match. BUG=429053 TEST=gpu_unittests, webgl_conformance2 R=piman@chromium.org Review URL: https://codereview.chromium.org/1231273002 Cr-Commit-Position: refs/heads/master@{#338420} --- gpu/command_buffer/service/context_group.cc | 4 +- gpu/command_buffer/service/framebuffer_manager.cc | 10 ++- gpu/command_buffer/service/framebuffer_manager.h | 10 ++- .../service/framebuffer_manager_unittest.cc | 71 +++++++++++++++++++--- .../service/texture_manager_unittest.cc | 6 +- 5 files changed, 85 insertions(+), 16 deletions(-) diff --git a/gpu/command_buffer/service/context_group.cc b/gpu/command_buffer/service/context_group.cc index c975347ea682..4a82c561b3a6 100644 --- a/gpu/command_buffer/service/context_group.cc +++ b/gpu/command_buffer/service/context_group.cc @@ -154,8 +154,8 @@ bool ContextGroup::Initialize( buffer_manager_.reset( new BufferManager(memory_tracker_.get(), feature_info_.get())); - framebuffer_manager_.reset( - new FramebufferManager(max_draw_buffers_, max_color_attachments_)); + framebuffer_manager_.reset(new FramebufferManager( + max_draw_buffers_, max_color_attachments_, context_type)); renderbuffer_manager_.reset(new RenderbufferManager( memory_tracker_.get(), max_renderbuffer_size, max_samples, feature_info_.get())); diff --git a/gpu/command_buffer/service/framebuffer_manager.cc b/gpu/command_buffer/service/framebuffer_manager.cc index 173c2868ed0f..2798a5e2afd3 100644 --- a/gpu/command_buffer/service/framebuffer_manager.cc +++ b/gpu/command_buffer/service/framebuffer_manager.cc @@ -260,12 +260,14 @@ FramebufferManager::TextureDetachObserver::TextureDetachObserver() {} FramebufferManager::TextureDetachObserver::~TextureDetachObserver() {} FramebufferManager::FramebufferManager( - uint32 max_draw_buffers, uint32 max_color_attachments) + uint32 max_draw_buffers, uint32 max_color_attachments, + ContextGroup::ContextType context_type) : framebuffer_state_change_count_(1), framebuffer_count_(0), have_context_(true), max_draw_buffers_(max_draw_buffers), - max_color_attachments_(max_color_attachments) { + max_color_attachments_(max_color_attachments), + context_type_(context_type) { DCHECK_GT(max_draw_buffers_, 0u); DCHECK_GT(max_color_attachments_, 0u); } @@ -476,7 +478,9 @@ GLenum Framebuffer::IsPossiblyComplete() const { if (width == 0 || height == 0) { return GL_FRAMEBUFFER_INCOMPLETE_ATTACHMENT; } - } else { + } else if (manager_->context_type() != ContextGroup::CONTEXT_TYPE_WEBGL2) { + // TODO(zmo): revisit this if we create ES3 contexts for clients other + // than WebGL 2. if (attachment->width() != width || attachment->height() != height) { return GL_FRAMEBUFFER_INCOMPLETE_DIMENSIONS_EXT; } diff --git a/gpu/command_buffer/service/framebuffer_manager.h b/gpu/command_buffer/service/framebuffer_manager.h index 75f7e2a7ec60..172125403d64 100644 --- a/gpu/command_buffer/service/framebuffer_manager.h +++ b/gpu/command_buffer/service/framebuffer_manager.h @@ -11,6 +11,7 @@ #include "base/containers/hash_tables.h" #include "base/memory/ref_counted.h" #include "base/memory/scoped_ptr.h" +#include "gpu/command_buffer/service/context_group.h" #include "gpu/command_buffer/service/gl_utils.h" #include "gpu/gpu_export.h" @@ -240,7 +241,8 @@ class GPU_EXPORT FramebufferManager { DISALLOW_COPY_AND_ASSIGN(TextureDetachObserver); }; - FramebufferManager(uint32 max_draw_buffers, uint32 max_color_attachments); + FramebufferManager(uint32 max_draw_buffers, uint32 max_color_attachments, + ContextGroup::ContextType context_type); ~FramebufferManager(); // Must call before destruction. @@ -285,6 +287,10 @@ class GPU_EXPORT FramebufferManager { texture_detach_observers_.end()); } + ContextGroup::ContextType context_type() const { + return context_type_; + } + private: friend class Framebuffer; @@ -311,6 +317,8 @@ class GPU_EXPORT FramebufferManager { uint32 max_draw_buffers_; uint32 max_color_attachments_; + ContextGroup::ContextType context_type_; + typedef std::vector TextureDetachObserverVector; TextureDetachObserverVector texture_detach_observers_; diff --git a/gpu/command_buffer/service/framebuffer_manager_unittest.cc b/gpu/command_buffer/service/framebuffer_manager_unittest.cc index 5f42390f62d5..36b87f6c9d5a 100644 --- a/gpu/command_buffer/service/framebuffer_manager_unittest.cc +++ b/gpu/command_buffer/service/framebuffer_manager_unittest.cc @@ -34,7 +34,7 @@ const bool kUseDefaultTextures = false; class FramebufferManagerTest : public GpuServiceTest { public: FramebufferManagerTest() - : manager_(1, 1), + : manager_(1, 1, ContextGroup::CONTEXT_TYPE_UNDEFINED), feature_info_(new FeatureInfo()) { texture_manager_.reset(new TextureManager(NULL, feature_info_.get(), @@ -55,7 +55,6 @@ class FramebufferManagerTest : public GpuServiceTest { } protected: - FramebufferManager manager_; scoped_refptr feature_info_; scoped_ptr texture_manager_; @@ -106,13 +105,13 @@ TEST_F(FramebufferManagerTest, Destroy) { ASSERT_TRUE(framebuffer1 == NULL); } -class FramebufferInfoTest : public GpuServiceTest { +class FramebufferInfoTestBase : public GpuServiceTest { public: static const GLuint kClient1Id = 1; static const GLuint kService1Id = 11; - FramebufferInfoTest() - : manager_(kMaxDrawBuffers, kMaxColorAttachments), + explicit FramebufferInfoTestBase(ContextGroup::ContextType context_type) + : manager_(kMaxDrawBuffers, kMaxColorAttachments, context_type), feature_info_(new FeatureInfo()) { texture_manager_.reset(new TextureManager(NULL, feature_info_.get(), @@ -126,7 +125,7 @@ class FramebufferInfoTest : public GpuServiceTest { kMaxSamples, feature_info_.get())); } - ~FramebufferInfoTest() override { + ~FramebufferInfoTestBase() override { manager_.Destroy(false); texture_manager_->Destroy(false); renderbuffer_manager_->Destroy(false); @@ -156,10 +155,17 @@ class FramebufferInfoTest : public GpuServiceTest { scoped_ptr error_state_; }; +class FramebufferInfoTest : public FramebufferInfoTestBase { + public: + FramebufferInfoTest() + : FramebufferInfoTestBase(ContextGroup::CONTEXT_TYPE_UNDEFINED) { + } +}; + // GCC requires these declarations, but MSVC requires they not be present #ifndef COMPILER_MSVC -const GLuint FramebufferInfoTest::kClient1Id; -const GLuint FramebufferInfoTest::kService1Id; +const GLuint FramebufferInfoTestBase::kClient1Id; +const GLuint FramebufferInfoTestBase::kService1Id; #endif TEST_F(FramebufferInfoTest, Basic) { @@ -920,6 +926,55 @@ TEST_F(FramebufferInfoTest, GetStatus) { framebuffer_->GetStatus(texture_manager_.get(), GL_READ_FRAMEBUFFER); } +class FramebufferInfoES3Test : public FramebufferInfoTestBase { + public: + FramebufferInfoES3Test() + : FramebufferInfoTestBase(ContextGroup::CONTEXT_TYPE_WEBGL2) { + } +}; + +TEST_F(FramebufferInfoES3Test, DifferentDimensions) { + const GLuint kRenderbufferClient1Id = 33; + const GLuint kRenderbufferService1Id = 333; + const GLuint kRenderbufferClient2Id = 34; + const GLuint kRenderbufferService2Id = 334; + const GLsizei kWidth1 = 16; + const GLsizei kHeight1 = 32; + const GLenum kFormat1 = GL_RGBA4; + const GLsizei kSamples1 = 0; + const GLsizei kWidth2 = 32; // Different from kWidth1 + const GLsizei kHeight2 = 32; + const GLenum kFormat2 = GL_DEPTH_COMPONENT16; + const GLsizei kSamples2 = 0; + + EXPECT_FALSE(framebuffer_->HasUnclearedAttachment(GL_COLOR_ATTACHMENT0)); + EXPECT_FALSE(framebuffer_->HasUnclearedAttachment(GL_DEPTH_ATTACHMENT)); + EXPECT_FALSE(framebuffer_->HasUnclearedAttachment(GL_STENCIL_ATTACHMENT)); + EXPECT_FALSE( + framebuffer_->HasUnclearedAttachment(GL_DEPTH_STENCIL_ATTACHMENT)); + + renderbuffer_manager_->CreateRenderbuffer( + kRenderbufferClient1Id, kRenderbufferService1Id); + Renderbuffer* renderbuffer1 = + renderbuffer_manager_->GetRenderbuffer(kRenderbufferClient1Id); + ASSERT_TRUE(renderbuffer1 != NULL); + renderbuffer_manager_->SetInfo( + renderbuffer1, kSamples1, kFormat1, kWidth1, kHeight1); + framebuffer_->AttachRenderbuffer(GL_COLOR_ATTACHMENT0, renderbuffer1); + + renderbuffer_manager_->CreateRenderbuffer( + kRenderbufferClient2Id, kRenderbufferService2Id); + Renderbuffer* renderbuffer2 = + renderbuffer_manager_->GetRenderbuffer(kRenderbufferClient2Id); + ASSERT_TRUE(renderbuffer2 != NULL); + renderbuffer_manager_->SetInfo( + renderbuffer2, kSamples2, kFormat2, kWidth2, kHeight2); + framebuffer_->AttachRenderbuffer(GL_DEPTH_ATTACHMENT, renderbuffer2); + + EXPECT_EQ(static_cast(GL_FRAMEBUFFER_COMPLETE), + framebuffer_->IsPossiblyComplete()); +} + } // namespace gles2 } // namespace gpu diff --git a/gpu/command_buffer/service/texture_manager_unittest.cc b/gpu/command_buffer/service/texture_manager_unittest.cc index ed25a7d45203..7568648baf10 100644 --- a/gpu/command_buffer/service/texture_manager_unittest.cc +++ b/gpu/command_buffer/service/texture_manager_unittest.cc @@ -1848,9 +1848,11 @@ TEST_F(SharedTextureTest, TextureSafetyAccounting) { TEST_F(SharedTextureTest, FBOCompletenessCheck) { const GLenum kCompleteValue = GL_FRAMEBUFFER_COMPLETE; - FramebufferManager framebuffer_manager1(1, 1); + FramebufferManager framebuffer_manager1( + 1, 1, ContextGroup::CONTEXT_TYPE_UNDEFINED); texture_manager1_->set_framebuffer_manager(&framebuffer_manager1); - FramebufferManager framebuffer_manager2(1, 1); + FramebufferManager framebuffer_manager2( + 1, 1, ContextGroup::CONTEXT_TYPE_UNDEFINED); texture_manager2_->set_framebuffer_manager(&framebuffer_manager2); scoped_refptr ref1 = texture_manager1_->CreateTexture(10, 10); -- 2.11.4.GIT