diff --git a/gpu/command_buffer/service/gles2_cmd_decoder.cc b/gpu/command_buffer/service/gles2_cmd_decoder.cc index fd122713639d0..aa6e9d0ab70b4 100644 --- a/gpu/command_buffer/service/gles2_cmd_decoder.cc +++ b/gpu/command_buffer/service/gles2_cmd_decoder.cc @@ -1128,10 +1128,6 @@ class GLES2DecoderImpl : public GLES2Decoder, public ErrorStateClient { void DoProduceTextureDirectCHROMIUM(GLuint texture, const volatile GLbyte* key); - void ProduceTextureRef(const char* func_name, - bool clear, - TextureRef* texture_ref, - const volatile GLbyte* data); void DoCreateAndConsumeTextureINTERNAL(GLuint client_id, const volatile GLbyte* key); @@ -5274,8 +5270,8 @@ void GLES2DecoderImpl::TakeFrontBuffer(const Mailbox& mailbox) { } void GLES2DecoderImpl::ReturnFrontBuffer(const Mailbox& mailbox, bool is_lost) { - Texture* texture = - static_cast<Texture*>(group_->mailbox_manager()->ConsumeTexture(mailbox)); + TextureBase* texture = mailbox_manager()->ConsumeTexture(mailbox); + mailbox_manager()->TextureDeleted(texture); if (offscreen_single_buffer_) return; @@ -18108,40 +18104,19 @@ void GLES2DecoderImpl::DoProduceTextureDirectCHROMIUM( TRACE_EVENT2("gpu", "GLES2DecoderImpl::DoProduceTextureDirectCHROMIUM", "context", logger_.GetLogPrefix(), "mailbox[0]", static_cast<unsigned char>(data[0])); - - ProduceTextureRef("glProduceTextureDirectCHROMIUM", !client_id, - GetTexture(client_id), data); -} - -void GLES2DecoderImpl::ProduceTextureRef(const char* func_name, - bool clear, - TextureRef* texture_ref, - const volatile GLbyte* data) { Mailbox mailbox = Mailbox::FromVolatile(*reinterpret_cast<const volatile Mailbox*>(data)); DLOG_IF(ERROR, !mailbox.Verify()) << "ProduceTextureDirectCHROMIUM was not passed a crypto-random mailbox."; - if (clear) { - DCHECK(!texture_ref); - - group_->mailbox_manager()->ProduceTexture(mailbox, nullptr); - return; - } - + TextureRef* texture_ref = GetTexture(client_id); if (!texture_ref) { - LOCAL_SET_GL_ERROR(GL_INVALID_OPERATION, func_name, "unknown texture"); + LOCAL_SET_GL_ERROR(GL_INVALID_OPERATION, "glProduceTextureDirectCHROMIUM", + "unknown texture"); return; } - Texture* produced = texture_manager()->Produce(texture_ref); - if (!produced) { - LOCAL_SET_GL_ERROR( - GL_INVALID_OPERATION, func_name, "invalid texture"); - return; - } - - group_->mailbox_manager()->ProduceTexture(mailbox, produced); + group_->mailbox_manager()->ProduceTexture(mailbox, texture_ref->texture()); } void GLES2DecoderImpl::DoCreateAndConsumeTextureINTERNAL( diff --git a/gpu/command_buffer/service/gles2_cmd_decoder_passthrough.cc b/gpu/command_buffer/service/gles2_cmd_decoder_passthrough.cc index f8ae65a3a71e3..bdd7f29512129 100644 --- a/gpu/command_buffer/service/gles2_cmd_decoder_passthrough.cc +++ b/gpu/command_buffer/service/gles2_cmd_decoder_passthrough.cc @@ -1009,8 +1009,8 @@ void GLES2DecoderPassthroughImpl::TakeFrontBuffer(const Mailbox& mailbox) { void GLES2DecoderPassthroughImpl::ReturnFrontBuffer(const Mailbox& mailbox, bool is_lost) { - TexturePassthrough* texture = static_cast<TexturePassthrough*>( - mailbox_manager_->ConsumeTexture(mailbox)); + TextureBase* texture = mailbox_manager_->ConsumeTexture(mailbox); + mailbox_manager_->TextureDeleted(texture); if (offscreen_single_buffer_) { return; diff --git a/gpu/command_buffer/service/gles2_cmd_decoder_unittest_textures.cc b/gpu/command_buffer/service/gles2_cmd_decoder_unittest_textures.cc index d372b9ad32ca9..e7d4bffdc936a 100644 --- a/gpu/command_buffer/service/gles2_cmd_decoder_unittest_textures.cc +++ b/gpu/command_buffer/service/gles2_cmd_decoder_unittest_textures.cc @@ -3098,81 +3098,6 @@ TEST_P(GLES2DecoderTest, TextureUsageAngleExtNotEnabledByDefault) { EXPECT_EQ(GL_INVALID_ENUM, GetGLError()); } -TEST_P(GLES2DecoderTest, ProduceAndConsumeTextureCHROMIUM) { - Mailbox mailbox = Mailbox::Generate(); - GLuint new_texture_id = kNewClientId; - - DoBindTexture(GL_TEXTURE_2D, client_texture_id_, kServiceTextureId); - DoTexImage2D( - GL_TEXTURE_2D, 0, GL_RGBA, 3, 1, 0, GL_RGBA, GL_UNSIGNED_BYTE, 0, 0); - DoTexImage2D( - GL_TEXTURE_2D, 1, GL_RGBA, 2, 4, 0, GL_RGBA, GL_UNSIGNED_BYTE, 0, 0); - TextureRef* texture_ref = - group().texture_manager()->GetTexture(client_texture_id_); - ASSERT_TRUE(texture_ref != nullptr); - Texture* texture = texture_ref->texture(); - EXPECT_EQ(kServiceTextureId, texture->service_id()); - - ProduceTextureDirectCHROMIUMImmediate& produce_cmd = - *GetImmediateAs<ProduceTextureDirectCHROMIUMImmediate>(); - produce_cmd.Init(client_texture_id_, mailbox.name); - EXPECT_EQ(error::kNoError, - ExecuteImmediateCmd(produce_cmd, sizeof(mailbox.name))); - EXPECT_EQ(GL_NO_ERROR, GetGLError()); - - // Texture didn't change. - GLsizei width; - GLsizei height; - GLenum type; - GLenum internal_format; - - EXPECT_TRUE( - texture->GetLevelSize(GL_TEXTURE_2D, 0, &width, &height, nullptr)); - EXPECT_EQ(3, width); - EXPECT_EQ(1, height); - EXPECT_TRUE(texture->GetLevelType(GL_TEXTURE_2D, 0, &type, &internal_format)); - EXPECT_EQ(static_cast<GLenum>(GL_RGBA), internal_format); - EXPECT_EQ(static_cast<GLenum>(GL_UNSIGNED_BYTE), type); - - EXPECT_TRUE( - texture->GetLevelSize(GL_TEXTURE_2D, 1, &width, &height, nullptr)); - EXPECT_EQ(2, width); - EXPECT_EQ(4, height); - EXPECT_TRUE(texture->GetLevelType(GL_TEXTURE_2D, 1, &type, &internal_format)); - EXPECT_EQ(static_cast<GLenum>(GL_RGBA), internal_format); - EXPECT_EQ(static_cast<GLenum>(GL_UNSIGNED_BYTE), type); - - // Service ID has not changed. - EXPECT_EQ(kServiceTextureId, texture->service_id()); - - CreateAndConsumeTextureINTERNALImmediate& consume_cmd = - *GetImmediateAs<CreateAndConsumeTextureINTERNALImmediate>(); - consume_cmd.Init(new_texture_id, mailbox.name); - EXPECT_EQ(error::kNoError, - ExecuteImmediateCmd(consume_cmd, sizeof(mailbox.name))); - EXPECT_EQ(GL_NO_ERROR, GetGLError()); - - // Texture is redefined. - EXPECT_TRUE( - texture->GetLevelSize(GL_TEXTURE_2D, 0, &width, &height, nullptr)); - EXPECT_EQ(3, width); - EXPECT_EQ(1, height); - EXPECT_TRUE(texture->GetLevelType(GL_TEXTURE_2D, 0, &type, &internal_format)); - EXPECT_EQ(static_cast<GLenum>(GL_RGBA), internal_format); - EXPECT_EQ(static_cast<GLenum>(GL_UNSIGNED_BYTE), type); - - EXPECT_TRUE( - texture->GetLevelSize(GL_TEXTURE_2D, 1, &width, &height, nullptr)); - EXPECT_EQ(2, width); - EXPECT_EQ(4, height); - EXPECT_TRUE(texture->GetLevelType(GL_TEXTURE_2D, 1, &type, &internal_format)); - EXPECT_EQ(static_cast<GLenum>(GL_RGBA), internal_format); - EXPECT_EQ(static_cast<GLenum>(GL_UNSIGNED_BYTE), type); - - // Service ID is restored. - EXPECT_EQ(kServiceTextureId, texture->service_id()); -} - TEST_P(GLES2DecoderTest, ProduceAndConsumeDirectTextureCHROMIUM) { Mailbox mailbox = Mailbox::Generate(); diff --git a/gpu/command_buffer/service/mailbox_manager_impl.cc b/gpu/command_buffer/service/mailbox_manager_impl.cc index 2cd186adce438..f66f9d3b56b49 100644 --- a/gpu/command_buffer/service/mailbox_manager_impl.cc +++ b/gpu/command_buffer/service/mailbox_manager_impl.cc @@ -30,25 +30,17 @@ TextureBase* MailboxManagerImpl::ConsumeTexture(const Mailbox& mailbox) { if (it != mailbox_to_textures_.end()) return it->second->first; - return NULL; + return nullptr; } void MailboxManagerImpl::ProduceTexture(const Mailbox& mailbox, TextureBase* texture) { + DCHECK(texture); MailboxToTextureMap::iterator it = mailbox_to_textures_.find(mailbox); if (it != mailbox_to_textures_.end()) { - if (it->second->first == texture) - return; - TextureToMailboxMap::iterator texture_it = it->second; - mailbox_to_textures_.erase(it); - textures_to_mailboxes_.erase(texture_it); + DLOG(ERROR) << "Ignored attempt to reassign a mailbox"; + return; } - if (texture) - InsertTexture(mailbox, texture); -} - -void MailboxManagerImpl::InsertTexture(const Mailbox& mailbox, - TextureBase* texture) { texture->SetMailboxManager(this); TextureToMailboxMap::iterator texture_it = textures_to_mailboxes_.insert(std::make_pair(texture, mailbox)); diff --git a/gpu/command_buffer/service/mailbox_manager_impl.h b/gpu/command_buffer/service/mailbox_manager_impl.h index ec66830216466..5651fcd6f9790 100644 --- a/gpu/command_buffer/service/mailbox_manager_impl.h +++ b/gpu/command_buffer/service/mailbox_manager_impl.h @@ -33,8 +33,6 @@ class GPU_GLES2_EXPORT MailboxManagerImpl : public MailboxManager { void TextureDeleted(TextureBase* texture) override; private: - void InsertTexture(const Mailbox& mailbox, TextureBase* texture); - // This is a bidirectional map between mailbox and textures. We can have // multiple mailboxes per texture, but one texture per mailbox. We keep an // iterator in the MailboxToTextureMap to be able to manage changes to diff --git a/gpu/command_buffer/service/mailbox_manager_sync.cc b/gpu/command_buffer/service/mailbox_manager_sync.cc index 1ffcdcaf2c71d..5924465338b57 100644 --- a/gpu/command_buffer/service/mailbox_manager_sync.cc +++ b/gpu/command_buffer/service/mailbox_manager_sync.cc @@ -222,37 +222,25 @@ Texture* MailboxManagerSync::ConsumeTexture(const Mailbox& mailbox) { void MailboxManagerSync::ProduceTexture(const Mailbox& mailbox, TextureBase* texture_base) { + DCHECK(texture_base); base::AutoLock lock(g_lock.Get()); // Relax the cross-thread access restriction to non-thread-safe RefCount. // The lock above protects non-thread-safe RefCount in TextureGroup. base::ScopedAllowCrossThreadRefCountAccess scoped_allow_cross_thread_ref_count_access; + if (TextureGroup::FromName(mailbox)) { + DLOG(ERROR) << "Ignored attempt to reassign a mailbox"; + return; + } Texture* texture = static_cast<Texture*>(texture_base); TextureToGroupMap::iterator tex_it = texture_to_group_.find(texture); - TextureGroup* group_for_mailbox = TextureGroup::FromName(mailbox); - TextureGroup* group_for_texture = NULL; + TextureGroup* group_for_texture = nullptr; if (tex_it != texture_to_group_.end()) { group_for_texture = tex_it->second.group.get(); DCHECK(group_for_texture); - if (group_for_mailbox == group_for_texture) { - // The texture is already known under this name. - return; - } - } - - if (group_for_mailbox) { - // Unlink the mailbox from its current group. - group_for_mailbox->RemoveName(mailbox); - } - - if (!texture) - return; - - if (group_for_texture) { - group_for_texture->AddName(mailbox); } else { // This is a new texture, so create a new group. texture->SetMailboxManager(this); @@ -263,10 +251,10 @@ void MailboxManagerSync::ProduceTexture(const Mailbox& mailbox, } group_for_texture = new TextureGroup(definition); group_for_texture->AddTexture(this, texture); - group_for_texture->AddName(mailbox); texture_to_group_.insert(std::make_pair( texture, TextureGroupRef(kNewTextureVersion, group_for_texture))); } + group_for_texture->AddName(mailbox); DCHECK(texture->mailbox_manager() == this); } diff --git a/gpu/command_buffer/service/mailbox_manager_unittest.cc b/gpu/command_buffer/service/mailbox_manager_unittest.cc index 1f1f201e52d48..1efa4f0cf1796 100644 --- a/gpu/command_buffer/service/mailbox_manager_unittest.cc +++ b/gpu/command_buffer/service/mailbox_manager_unittest.cc @@ -143,43 +143,17 @@ TEST_F(MailboxManagerTest, ProduceMultipleTexture) { manager_->ProduceTexture(name, texture1); EXPECT_EQ(texture1, manager_->ConsumeTexture(name)); - // Can produce a second time with the same mailbox, but different texture. + // Producing a second time is ignored. manager_->ProduceTexture(name, texture2); - EXPECT_EQ(texture2, manager_->ConsumeTexture(name)); + EXPECT_EQ(texture1, manager_->ConsumeTexture(name)); // Destroying the texture that's under no mailbox shouldn't have an effect. - DestroyTexture(texture1); - EXPECT_EQ(texture2, manager_->ConsumeTexture(name)); + DestroyTexture(texture2); + EXPECT_EQ(texture1, manager_->ConsumeTexture(name)); // Destroying the texture that's bound should clean up. - DestroyTexture(texture2); - EXPECT_EQ(NULL, manager_->ConsumeTexture(name)); -} - -TEST_F(MailboxManagerTest, ProduceMultipleTextureMailbox) { - Texture* texture1 = CreateTexture(); - Texture* texture2 = CreateTexture(); - Mailbox name1 = Mailbox::Generate(); - Mailbox name2 = Mailbox::Generate(); - - // Put texture1 on name1 and name2. - manager_->ProduceTexture(name1, texture1); - manager_->ProduceTexture(name2, texture1); - EXPECT_EQ(texture1, manager_->ConsumeTexture(name1)); - EXPECT_EQ(texture1, manager_->ConsumeTexture(name2)); - - // Put texture2 on name2. - manager_->ProduceTexture(name2, texture2); - EXPECT_EQ(texture1, manager_->ConsumeTexture(name1)); - EXPECT_EQ(texture2, manager_->ConsumeTexture(name2)); - - // Destroy texture1, shouldn't affect name2. DestroyTexture(texture1); - EXPECT_EQ(NULL, manager_->ConsumeTexture(name1)); - EXPECT_EQ(texture2, manager_->ConsumeTexture(name2)); - - DestroyTexture(texture2); - EXPECT_EQ(NULL, manager_->ConsumeTexture(name2)); + EXPECT_EQ(NULL, manager_->ConsumeTexture(name)); } const GLsizei kMaxTextureWidth = 64; @@ -288,7 +262,7 @@ TEST_F(MailboxManagerSyncTest, ProduceSyncDestroy) { EXPECT_EQ(NULL, manager2_->ConsumeTexture(name)); } -TEST_F(MailboxManagerSyncTest, ProduceSyncClobberDestroy) { +TEST_F(MailboxManagerSyncTest, ProduceSyncMultipleMailbox) { InSequence sequence; Texture* texture = DefineTexture(); @@ -297,10 +271,11 @@ TEST_F(MailboxManagerSyncTest, ProduceSyncClobberDestroy) { manager_->ProduceTexture(name, texture); manager_->PushTextureUpdates(g_sync_token); - // Clobber + // Producing a second time with the same mailbox is ignored. Texture* old_texture = texture; texture = DefineTexture(); manager_->ProduceTexture(name, texture); + EXPECT_EQ(old_texture, manager_->ConsumeTexture(name)); DestroyTexture(old_texture); DestroyTexture(texture); @@ -447,73 +422,6 @@ TEST_F(MailboxManagerSyncTest, ProduceConsumeBidirectional) { DestroyTexture(new_texture2); } -// If a texture is shared with another manager instance, but the mailbox -// is then clobbered with a different texture in the source context, this should -// disconnect the earlier texture from updates. -TEST_F(MailboxManagerSyncTest, ProduceAndClobber) { - const GLuint kNewTextureId = 1234; - InSequence sequence; - - Texture* texture = DefineTexture(); - Mailbox name = Mailbox::Generate(); - - manager_->ProduceTexture(name, texture); - EXPECT_EQ(texture, manager_->ConsumeTexture(name)); - - // Synchronize - manager_->PushTextureUpdates(g_sync_token); - manager2_->PullTextureUpdates(g_sync_token); - - EXPECT_CALL(*gl_, GenTextures(1, _)) - .WillOnce(SetArgPointee<1>(kNewTextureId)); - SetupUpdateTexParamExpectations( - kNewTextureId, GL_LINEAR, GL_LINEAR, GL_REPEAT, GL_REPEAT); - Texture* new_texture = static_cast<Texture*>(manager2_->ConsumeTexture(name)); - EXPECT_NE(nullptr, new_texture); - EXPECT_NE(texture, new_texture); - EXPECT_EQ(kNewTextureId, new_texture->service_id()); - - Texture* old_texture = texture; - texture = DefineTexture(); - manager_->ProduceTexture(name, texture); - - // Make a change to the new texture - DCHECK_EQ(static_cast<GLuint>(GL_LINEAR), texture->min_filter()); - EXPECT_EQ(static_cast<GLenum>(GL_NO_ERROR), - SetParameter(texture, GL_TEXTURE_MIN_FILTER, GL_NEAREST)); - - // Synchronize in both directions - no changes, since it's not shared - manager_->PushTextureUpdates(g_sync_token); - manager2_->PullTextureUpdates(g_sync_token); - EXPECT_EQ(static_cast<GLuint>(GL_LINEAR), new_texture->min_filter()); - - // Make a change to the previously shared texture - DCHECK_EQ(static_cast<GLuint>(GL_LINEAR), old_texture->mag_filter()); - EXPECT_EQ(static_cast<GLenum>(GL_NO_ERROR), - SetParameter(old_texture, GL_TEXTURE_MAG_FILTER, GL_NEAREST)); - - // Synchronize and expect update - manager_->PushTextureUpdates(g_sync_token); - SetupUpdateTexParamExpectations( - new_texture->service_id(), GL_LINEAR, GL_NEAREST, GL_REPEAT, GL_REPEAT); - manager2_->PullTextureUpdates(g_sync_token); - - EXPECT_CALL(*gl_, GenTextures(1, _)) - .WillOnce(SetArgPointee<1>(kNewTextureId)); - SetupUpdateTexParamExpectations( - kNewTextureId, GL_NEAREST, GL_LINEAR, GL_REPEAT, GL_REPEAT); - TextureBase* tmp_texture = manager2_->ConsumeTexture(name); - EXPECT_NE(new_texture, tmp_texture); - DestroyTexture(tmp_texture); - - DestroyTexture(old_texture); - DestroyTexture(texture); - DestroyTexture(new_texture); - - EXPECT_EQ(NULL, manager_->ConsumeTexture(name)); - EXPECT_EQ(NULL, manager2_->ConsumeTexture(name)); -} - TEST_F(MailboxManagerSyncTest, ClearedStateSynced) { const GLuint kNewTextureId = 1234; diff --git a/gpu/command_buffer/service/raster_decoder.cc b/gpu/command_buffer/service/raster_decoder.cc index b6f0e2962b279..a39bbeec5190a 100644 --- a/gpu/command_buffer/service/raster_decoder.cc +++ b/gpu/command_buffer/service/raster_decoder.cc @@ -2284,34 +2284,17 @@ void RasterDecoderImpl::DoProduceTextureDirect(GLuint client_id, Mailbox mailbox = Mailbox::FromVolatile(*reinterpret_cast<const volatile Mailbox*>(key)); - DLOG_IF(ERROR, !mailbox.Verify()) << "ProduceTextureDirect was passed a " - "mailbox that was not generated by " - "GenMailboxCHROMIUM."; + DLOG_IF(ERROR, !mailbox.Verify()) + << "ProduceTextureDirect was not passed a crypto-random mailbox."; TextureRef* texture_ref = GetTexture(client_id); - - bool clear = !client_id; - if (clear) { - DCHECK(!texture_ref); - - group_->mailbox_manager()->ProduceTexture(mailbox, nullptr); - return; - } - if (!texture_ref) { LOCAL_SET_GL_ERROR(GL_INVALID_OPERATION, "ProduceTextureDirect", "unknown texture"); return; } - Texture* produced = texture_manager()->Produce(texture_ref); - if (!produced) { - LOCAL_SET_GL_ERROR(GL_INVALID_OPERATION, "ProduceTextureDirect", - "invalid texture"); - return; - } - - group_->mailbox_manager()->ProduceTexture(mailbox, produced); + group_->mailbox_manager()->ProduceTexture(mailbox, texture_ref->texture()); } bool RasterDecoderImpl::TexStorage2DImage( diff --git a/gpu/command_buffer/service/texture_manager.cc b/gpu/command_buffer/service/texture_manager.cc index 77c37b8af266b..4a65ec64bd1ba 100644 --- a/gpu/command_buffer/service/texture_manager.cc +++ b/gpu/command_buffer/service/texture_manager.cc @@ -2186,11 +2186,6 @@ void TextureManager::SetLevelInfo(TextureRef* ref, texture->estimated_size()); } -Texture* TextureManager::Produce(TextureRef* ref) { - DCHECK(ref); - return ref->texture(); -} - TextureRef* TextureManager::Consume( GLuint client_id, Texture* texture) { diff --git a/gpu/command_buffer/service/texture_manager.h b/gpu/command_buffer/service/texture_manager.h index b534f909ff642..0c5548d5a6bdc 100644 --- a/gpu/command_buffer/service/texture_manager.h +++ b/gpu/command_buffer/service/texture_manager.h @@ -844,8 +844,6 @@ class GPU_GLES2_EXPORT TextureManager GLenum type, const gfx::Rect& cleared_rect); - Texture* Produce(TextureRef* ref); - // Maps an existing texture into the texture manager, at a given client ID. TextureRef* Consume(GLuint client_id, Texture* texture); diff --git a/gpu/command_buffer/service/texture_manager_unittest.cc b/gpu/command_buffer/service/texture_manager_unittest.cc index 34cabe75cd38f..57e60f2330f31 100644 --- a/gpu/command_buffer/service/texture_manager_unittest.cc +++ b/gpu/command_buffer/service/texture_manager_unittest.cc @@ -1966,11 +1966,7 @@ class ProduceConsumeTextureTest : public TextureTest, return info; } - Texture* Produce(TextureRef* texture_ref) { - Texture* texture = manager_->Produce(texture_ref); - EXPECT_TRUE(texture != NULL); - return texture; - } + Texture* Produce(TextureRef* texture_ref) { return texture_ref->texture(); } void Consume(GLuint client_id, Texture* texture) { EXPECT_TRUE(manager_->Consume(client_id, texture));