Commit 19e92b54 authored by danakj's avatar danakj Committed by Commit Bot

Use immutable textures in VideoResourceUpdater.

The upload uses CopySubTextureCHROMIUM which is compatible with
immutable textures, as it does not change the texture size. It used
to use CopyTextureCHROMIUM, which is not compatible, and had a unit
test describing this. But in reality this has changed, so always make
the resource immutible, and stop checking for that when recycling.

R=piman@chromium.org

Bug: 768976
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel
Change-Id: Ia9a31769fcc6c513291b6e67be4dea07d1c02168
Reviewed-on: https://chromium-review.googlesource.com/685742Reviewed-by: default avatarAntoine Labour <piman@chromium.org>
Commit-Queue: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#504481}
parent 1695ba44
...@@ -210,7 +210,6 @@ VideoResourceUpdater::RecycleOrAllocateResource( ...@@ -210,7 +210,6 @@ VideoResourceUpdater::RecycleOrAllocateResource(
viz::ResourceFormat resource_format, viz::ResourceFormat resource_format,
const gfx::ColorSpace& color_space, const gfx::ColorSpace& color_space,
bool software_resource, bool software_resource,
bool immutable_hint,
int unique_id, int unique_id,
int plane_index) { int plane_index) {
ResourceList::iterator recyclable_resource = all_resources_.end(); ResourceList::iterator recyclable_resource = all_resources_.end();
...@@ -240,8 +239,7 @@ VideoResourceUpdater::RecycleOrAllocateResource( ...@@ -240,8 +239,7 @@ VideoResourceUpdater::RecycleOrAllocateResource(
if (!in_use && it->resource_size() == resource_size && if (!in_use && it->resource_size() == resource_size &&
it->resource_format() == resource_format && it->resource_format() == resource_format &&
it->mailbox().IsZero() == software_resource && it->mailbox().IsZero() == software_resource) {
resource_provider_->IsImmutable(it->resource_id()) == immutable_hint) {
recyclable_resource = it; recyclable_resource = it;
} }
} }
...@@ -251,22 +249,19 @@ VideoResourceUpdater::RecycleOrAllocateResource( ...@@ -251,22 +249,19 @@ VideoResourceUpdater::RecycleOrAllocateResource(
// There was nothing available to reuse or recycle. Allocate a new resource. // There was nothing available to reuse or recycle. Allocate a new resource.
return AllocateResource(resource_size, resource_format, color_space, return AllocateResource(resource_size, resource_format, color_space,
!software_resource, immutable_hint); !software_resource);
} }
VideoResourceUpdater::ResourceList::iterator VideoResourceUpdater::ResourceList::iterator
VideoResourceUpdater::AllocateResource(const gfx::Size& plane_size, VideoResourceUpdater::AllocateResource(const gfx::Size& plane_size,
viz::ResourceFormat format, viz::ResourceFormat format,
const gfx::ColorSpace& color_space, const gfx::ColorSpace& color_space,
bool has_mailbox, bool has_mailbox) {
bool immutable_hint) {
// TODO(danakj): Abstract out hw/sw resource create/delete from // TODO(danakj): Abstract out hw/sw resource create/delete from
// ResourceProvider and stop using ResourceProvider in this class. // ResourceProvider and stop using ResourceProvider in this class.
const viz::ResourceId resource_id = resource_provider_->CreateResource( const viz::ResourceId resource_id = resource_provider_->CreateResource(
plane_size, plane_size, ResourceProvider::TEXTURE_HINT_IMMUTABLE, format,
immutable_hint ? ResourceProvider::TEXTURE_HINT_IMMUTABLE color_space);
: ResourceProvider::TEXTURE_HINT_DEFAULT,
format, color_space);
DCHECK_NE(resource_id, 0u); DCHECK_NE(resource_id, 0u);
gpu::Mailbox mailbox; gpu::Mailbox mailbox;
...@@ -389,10 +384,9 @@ VideoFrameExternalResources VideoResourceUpdater::CreateForSoftwarePlanes( ...@@ -389,10 +384,9 @@ VideoFrameExternalResources VideoResourceUpdater::CreateForSoftwarePlanes(
return VideoFrameExternalResources(); return VideoFrameExternalResources();
} }
const bool is_immutable = true;
ResourceList::iterator resource_it = RecycleOrAllocateResource( ResourceList::iterator resource_it = RecycleOrAllocateResource(
output_plane_resource_size, output_resource_format, output_color_space, output_plane_resource_size, output_resource_format, output_color_space,
software_compositor, is_immutable, video_frame->unique_id(), i); software_compositor, video_frame->unique_id(), i);
resource_it->add_ref(); resource_it->add_ref();
plane_resources.push_back(resource_it); plane_resources.push_back(resource_it);
...@@ -623,13 +617,12 @@ void VideoResourceUpdater::CopyPlaneTexture( ...@@ -623,13 +617,12 @@ void VideoResourceUpdater::CopyPlaneTexture(
// target to avoid loss of precision or dropping any alpha component. // target to avoid loss of precision or dropping any alpha component.
const viz::ResourceFormat copy_target_format = viz::ResourceFormat::RGBA_8888; const viz::ResourceFormat copy_target_format = viz::ResourceFormat::RGBA_8888;
const bool is_immutable = false;
const int no_unique_id = 0; const int no_unique_id = 0;
const int no_plane_index = -1; // Do not recycle referenced textures. const int no_plane_index = -1; // Do not recycle referenced textures.
VideoResourceUpdater::ResourceList::iterator resource = VideoResourceUpdater::ResourceList::iterator resource =
RecycleOrAllocateResource(output_plane_resource_size, copy_target_format, RecycleOrAllocateResource(output_plane_resource_size, copy_target_format,
resource_color_space, false, is_immutable, resource_color_space, false, no_unique_id,
no_unique_id, no_plane_index); no_plane_index);
resource->add_ref(); resource->add_ref();
ResourceProvider::ScopedWriteLockGL lock(resource_provider_, ResourceProvider::ScopedWriteLockGL lock(resource_provider_,
......
...@@ -154,14 +154,12 @@ class CC_EXPORT VideoResourceUpdater { ...@@ -154,14 +154,12 @@ class CC_EXPORT VideoResourceUpdater {
viz::ResourceFormat resource_format, viz::ResourceFormat resource_format,
const gfx::ColorSpace& color_space, const gfx::ColorSpace& color_space,
bool software_resource, bool software_resource,
bool immutable_hint,
int unique_id, int unique_id,
int plane_index); int plane_index);
ResourceList::iterator AllocateResource(const gfx::Size& plane_size, ResourceList::iterator AllocateResource(const gfx::Size& plane_size,
viz::ResourceFormat format, viz::ResourceFormat format,
const gfx::ColorSpace& color_space, const gfx::ColorSpace& color_space,
bool has_mailbox, bool has_mailbox);
bool immutable_hint);
void DeleteResource(ResourceList::iterator resource_it); void DeleteResource(ResourceList::iterator resource_it);
void CopyPlaneTexture(media::VideoFrame* video_frame, void CopyPlaneTexture(media::VideoFrame* video_frame,
const gfx::ColorSpace& resource_color_space, const gfx::ColorSpace& resource_color_space,
......
...@@ -41,7 +41,6 @@ class WebGraphicsContext3DUploadCounter : public TestWebGraphicsContext3D { ...@@ -41,7 +41,6 @@ class WebGraphicsContext3DUploadCounter : public TestWebGraphicsContext3D {
GLuint internalformat, GLuint internalformat,
GLint width, GLint width,
GLint height) override { GLint height) override {
immutable_texture_created_ = true;
} }
GLuint createTexture() override { GLuint createTexture() override {
...@@ -65,13 +64,9 @@ class WebGraphicsContext3DUploadCounter : public TestWebGraphicsContext3D { ...@@ -65,13 +64,9 @@ class WebGraphicsContext3DUploadCounter : public TestWebGraphicsContext3D {
int TextureCreationCount() { return created_texture_count_; } int TextureCreationCount() { return created_texture_count_; }
void ResetTextureCreationCount() { created_texture_count_ = 0; } void ResetTextureCreationCount() { created_texture_count_ = 0; }
bool WasImmutableTextureCreated() { return immutable_texture_created_; }
void ResetImmutableTextureCreated() { immutable_texture_created_ = false; }
private: private:
int upload_count_; int upload_count_;
int created_texture_count_; int created_texture_count_;
bool immutable_texture_created_;
}; };
class SharedBitmapManagerAllocationCounter : public TestSharedBitmapManager { class SharedBitmapManagerAllocationCounter : public TestSharedBitmapManager {
...@@ -575,7 +570,6 @@ TEST_F(VideoResourceUpdaterTest, CreateForHardwarePlanes_StreamTexture) { ...@@ -575,7 +570,6 @@ TEST_F(VideoResourceUpdaterTest, CreateForHardwarePlanes_StreamTexture) {
// GL_TEXTURE_2D texture. // GL_TEXTURE_2D texture.
context3d_->ResetTextureCreationCount(); context3d_->ResetTextureCreationCount();
video_frame = CreateTestStreamTextureHardwareVideoFrame(true); video_frame = CreateTestStreamTextureHardwareVideoFrame(true);
context3d_->ResetImmutableTextureCreated();
resources = updater.CreateExternalResourcesFromVideoFrame(video_frame); resources = updater.CreateExternalResourcesFromVideoFrame(video_frame);
EXPECT_EQ(VideoFrameExternalResources::RGBA_PREMULTIPLIED_RESOURCE, EXPECT_EQ(VideoFrameExternalResources::RGBA_PREMULTIPLIED_RESOURCE,
resources.type); resources.type);
...@@ -584,13 +578,6 @@ TEST_F(VideoResourceUpdaterTest, CreateForHardwarePlanes_StreamTexture) { ...@@ -584,13 +578,6 @@ TEST_F(VideoResourceUpdaterTest, CreateForHardwarePlanes_StreamTexture) {
EXPECT_EQ(1u, resources.release_callbacks.size()); EXPECT_EQ(1u, resources.release_callbacks.size());
EXPECT_EQ(0u, resources.software_resources.size()); EXPECT_EQ(0u, resources.software_resources.size());
EXPECT_EQ(1, context3d_->TextureCreationCount()); EXPECT_EQ(1, context3d_->TextureCreationCount());
// The texture copy path requires the use of CopyTextureCHROMIUM, which
// enforces that the target texture not be immutable, as it may need
// to alter the storage of the texture. Therefore, this test asserts
// that an immutable texture wasn't created by glTexStorage2DEXT, when
// that extension is supported.
EXPECT_FALSE(context3d_->WasImmutableTextureCreated());
} }
TEST_F(VideoResourceUpdaterTest, CreateForHardwarePlanes_TextureQuad) { TEST_F(VideoResourceUpdaterTest, CreateForHardwarePlanes_TextureQuad) {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment