cc: Don't leave resources in a changed filter state.

When returning resources from a parent compositor, we want to return
them with the filter in the same state which they arrived with.

When we go to return the resource, if the filter has been changed on
the resource, reset it to the way the ResourceProvider received the
resource from the child.

Tests:
ResourceProviderTest.TextureFilters_ChildNearestParentLinear
ResourceProviderTest.TextureFilters_ChildLinearParentNearest

R=piman
BUG=263069

Review URL: https://chromiumcodereview.appspot.com/23710030

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@222891 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 202 insertions, 107 deletions

diff --git a/cc/resources/resource_provider.cc b/cc/resources/resource_provider.cc
index 787757c..5c8d39c 100644
--- a/cc/resources/resource_provider.cc
+++ b/cc/resources/resource_provider.cc
@@ -96,7 +96,9 @@ ResourceProvider::Resource::Resource()
       read_lock_fence(NULL),
       size(),
       format(0),
+      original_filter(0),
       filter(0),
+      target(0),
       image_id(0),
       texture_pool(0),
       wrap_mode(0),
@@ -131,7 +133,9 @@ ResourceProvider::Resource::Resource(
       read_lock_fence(NULL),
       size(size),
       format(format),
+      original_filter(filter),
       filter(filter),
+      target(0),
       image_id(0),
       texture_pool(texture_pool),
       wrap_mode(wrap_mode),
@@ -164,7 +168,9 @@ ResourceProvider::Resource::Resource(
       read_lock_fence(NULL),
       size(size),
       format(format),
+      original_filter(filter),
       filter(filter),
+      target(0),
       image_id(0),
       texture_pool(0),
       wrap_mode(wrap_mode),
@@ -854,9 +860,21 @@ void ResourceProvider::PrepareSendReturnsToChild(
     DCHECK(child_info.parent_to_child_map.find(*it) !=
            child_info.parent_to_child_map.end());
 
+    if (resource->filter != resource->original_filter) {
+      DCHECK(resource->target);
+      DCHECK(resource->gl_id);
+
+      GLC(context3d, context3d->bindTexture(resource->target, resource->gl_id));
+      GLC(context3d, context3d->texParameteri(resource->target,
+                                              GL_TEXTURE_MIN_FILTER,
+                                              resource->original_filter));
+      GLC(context3d, context3d->texParameteri(resource->target,
+                                              GL_TEXTURE_MAG_FILTER,
+                                              resource->original_filter));
+    }
+
     ReturnedResource returned;
     returned.id = child_info.parent_to_child_map[*it];
-    returned.filter = resource->filter;
     returned.sync_point = resource->mailbox.sync_point();
     if (!returned.sync_point)
       need_sync_point = true;
@@ -941,7 +959,6 @@ void ResourceProvider::ReceiveReturnsFromParent(
     resource->exported_count -= it->count;
     if (resource->exported_count)
       continue;
-    resource->filter = it->filter;
     if (resource->gl_id) {
       if (it->sync_point)
         GLC(context3d, context3d->waitSyncPoint(it->sync_point));
@@ -1129,6 +1146,10 @@ void ResourceProvider::BindForSampling(ResourceProvider::ResourceId resource_id,
                                             GL_TEXTURE_MAG_FILTER,
                                             filter));
     resource->filter = filter;
+    if (resource->target == 0)
+      resource->target = target;
+    else
+      DCHECK_EQ(resource->target, target);
   }
 
   if (resource->image_id)
diff --git a/cc/resources/resource_provider.h b/cc/resources/resource_provider.h
index e2d7878..8b4b01b 100644
--- a/cc/resources/resource_provider.h
+++ b/cc/resources/resource_provider.h
@@ -370,7 +370,9 @@ class CC_EXPORT ResourceProvider {
     gfx::Size size;
     GLenum format;
     // TODO(skyostil): Use a separate sampler object for filter state.
+    GLenum original_filter;
     GLenum filter;
+    GLenum target;
     unsigned image_id;
     GLenum texture_pool;
     GLint wrap_mode;
diff --git a/cc/resources/resource_provider_unittest.cc b/cc/resources/resource_provider_unittest.cc
index 3f786ef..afb2330 100644
--- a/cc/resources/resource_provider_unittest.cc
+++ b/cc/resources/resource_provider_unittest.cc
@@ -47,6 +47,22 @@ size_t TextureSize(gfx::Size size, WGC3Denum format) {
       bytes_per_component;
 }
 
+class TextureStateTrackingContext : public TestWebGraphicsContext3D {
+ public:
+  MOCK_METHOD2(bindTexture, void(WGC3Denum target, WebGLId texture));
+  MOCK_METHOD3(texParameteri,
+               void(WGC3Denum target, WGC3Denum pname, WGC3Dint param));
+  MOCK_METHOD1(waitSyncPoint, void(unsigned sync_point));
+  MOCK_METHOD0(insertSyncPoint, unsigned(void));
+  MOCK_METHOD2(produceTextureCHROMIUM, void(WGC3Denum target,
+                                            const WGC3Dbyte* mailbox));
+  MOCK_METHOD2(consumeTextureCHROMIUM, void(WGC3Denum target,
+                                            const WGC3Dbyte* mailbox));
+
+  // Force all textures to be "1" so we can test for them.
+  virtual WebKit::WebGLId NextTextureId() OVERRIDE { return 1; }
+};
+
 struct Texture : public base::RefCounted<Texture> {
   Texture() : format(0), filter(GL_NEAREST_MIPMAP_LINEAR) {}
 
@@ -394,23 +410,13 @@ class ResourceProviderTest
     resource_provider_ = ResourceProvider::Create(output_surface_.get(), 0);
   }
 
-  void SetResourceFilter(ResourceProvider* resource_provider,
+  static void SetResourceFilter(ResourceProvider* resource_provider,
                          ResourceProvider::ResourceId id,
                          WGC3Denum filter) {
     ResourceProvider::ScopedSamplerGL sampler(
         resource_provider, id, GL_TEXTURE_2D, filter);
   }
 
-  WGC3Denum GetResourceFilter(ResourceProvider* resource_provider,
-                              ResourceProviderContext* context,
-                              ResourceProvider::ResourceId id) {
-    DCHECK_EQ(GetParam(), ResourceProvider::GLTexture);
-    ResourceProvider::ScopedReadLockGL lock_gl(resource_provider, id);
-    EXPECT_NE(0u, lock_gl.texture_id());
-    context->bindTexture(GL_TEXTURE_2D, lock_gl.texture_id());
-    return context->GetTextureFilter();
-  }
-
   ResourceProviderContext* context() { return context3d_; }
 
  protected:
@@ -724,84 +730,165 @@ TEST_P(ResourceProviderTest, DeleteTransferredResources) {
   EXPECT_EQ(0u, child_resource_provider->num_resources());
 }
 
-TEST_P(ResourceProviderTest, TextureFilters) {
-  // Resource transfer is only supported with GL textures for now.
-  if (GetParam() != ResourceProvider::GLTexture)
-    return;
+class ResourceProviderTestTextureFilters : public ResourceProviderTest {
+ public:
+  static void RunTest(GLenum child_filter, GLenum parent_filter) {
+    scoped_ptr<TextureStateTrackingContext> child_context_owned(
+        new TextureStateTrackingContext);
+    TextureStateTrackingContext* child_context = child_context_owned.get();
 
-  scoped_ptr<ResourceProviderContext> child_context_owned(
-      ResourceProviderContext::Create(shared_data_.get()));
-  ResourceProviderContext* child_context = child_context_owned.get();
+    FakeOutputSurfaceClient child_output_surface_client;
+    scoped_ptr<OutputSurface> child_output_surface(FakeOutputSurface::Create3d(
+        child_context_owned.PassAs<TestWebGraphicsContext3D>()));
+    CHECK(child_output_surface->BindToClient(&child_output_surface_client));
 
-  FakeOutputSurfaceClient child_output_surface_client;
-  scoped_ptr<OutputSurface> child_output_surface(FakeOutputSurface::Create3d(
-      child_context_owned.PassAs<TestWebGraphicsContext3D>()));
-  CHECK(child_output_surface->BindToClient(&child_output_surface_client));
+    scoped_ptr<ResourceProvider> child_resource_provider(
+        ResourceProvider::Create(child_output_surface.get(), 0));
 
-  scoped_ptr<ResourceProvider> child_resource_provider(
-      ResourceProvider::Create(child_output_surface.get(), 0));
+    scoped_ptr<TextureStateTrackingContext> parent_context_owned(
+        new TextureStateTrackingContext);
+    TextureStateTrackingContext* parent_context = parent_context_owned.get();
 
-  gfx::Size size(1, 1);
-  WGC3Denum format = GL_RGBA;
-  size_t pixel_size = TextureSize(size, format);
-  ASSERT_EQ(4U, pixel_size);
+    FakeOutputSurfaceClient parent_output_surface_client;
+    scoped_ptr<OutputSurface> parent_output_surface(FakeOutputSurface::Create3d(
+        parent_context_owned.PassAs<TestWebGraphicsContext3D>()));
+    CHECK(parent_output_surface->BindToClient(&parent_output_surface_client));
 
-  ResourceProvider::ResourceId id = child_resource_provider->CreateResource(
-      size, format, GL_CLAMP_TO_EDGE, ResourceProvider::TextureUsageAny);
-  uint8_t data[4] = { 1, 2, 3, 4 };
-  gfx::Rect rect(size);
-  child_resource_provider->SetPixels(id, data, rect, rect, gfx::Vector2d());
-  EXPECT_EQ(static_cast<unsigned>(GL_LINEAR),
-            GetResourceFilter(child_resource_provider.get(),
-                              child_context,
-                              id));
-  SetResourceFilter(child_resource_provider.get(), id, GL_NEAREST);
-  EXPECT_EQ(static_cast<unsigned>(GL_NEAREST),
-            GetResourceFilter(child_resource_provider.get(),
-                              child_context,
-                              id));
+    scoped_ptr<ResourceProvider> parent_resource_provider(
+        ResourceProvider::Create(parent_output_surface.get(), 0));
 
-  int child_id = resource_provider_->CreateChild();
-  {
-    // Transfer some resource to the parent.
-    ResourceProvider::ResourceIdArray resource_ids_to_transfer;
-    resource_ids_to_transfer.push_back(id);
-    TransferableResourceArray list;
-    child_resource_provider->PrepareSendToParent(resource_ids_to_transfer,
-                                                 &list);
-    ASSERT_EQ(1u, list.size());
-    EXPECT_EQ(static_cast<unsigned>(GL_NEAREST), list[0].filter);
-    resource_provider_->ReceiveFromChild(child_id, list);
-  }
-  ResourceProvider::ResourceIdMap resource_map =
-      resource_provider_->GetChildToParentMap(child_id);
-  ResourceProvider::ResourceId mapped_id = resource_map[id];
-  EXPECT_NE(0u, mapped_id);
-  EXPECT_EQ(static_cast<unsigned>(GL_NEAREST),
-            GetResourceFilter(resource_provider_.get(), context(), mapped_id));
-  SetResourceFilter(resource_provider_.get(), mapped_id, GL_LINEAR);
-  EXPECT_EQ(static_cast<unsigned>(GL_LINEAR),
-            GetResourceFilter(resource_provider_.get(), context(), mapped_id));
-  {
-    // Transfer resources back from the parent to the child.
-    ResourceProvider::ResourceIdArray resource_ids_to_transfer;
-    resource_ids_to_transfer.push_back(mapped_id);
-    ReturnedResourceArray list;
-    resource_provider_->PrepareSendReturnsToChild(
-        child_id, resource_ids_to_transfer, &list);
-    ASSERT_EQ(1u, list.size());
-    EXPECT_EQ(static_cast<unsigned>(GL_LINEAR), list[0].filter);
-    child_resource_provider->ReceiveReturnsFromParent(list);
+    gfx::Size size(1, 1);
+    WGC3Denum format = GL_RGBA;
+    int texture_id = 1;
+
+    size_t pixel_size = TextureSize(size, format);
+    ASSERT_EQ(4U, pixel_size);
+
+    ResourceProvider::ResourceId id = child_resource_provider->CreateResource(
+        size, format, GL_CLAMP_TO_EDGE, ResourceProvider::TextureUsageAny);
+
+    // The new texture is created with GL_LINEAR.
+    EXPECT_CALL(*child_context, bindTexture(GL_TEXTURE_2D, texture_id))
+        .Times(2);  // Once to create and once to allocate.
+    EXPECT_CALL(*child_context,
+                texParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR));
+    EXPECT_CALL(*child_context,
+                texParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR));
+    EXPECT_CALL(
+        *child_context,
+        texParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE));
+    EXPECT_CALL(
+        *child_context,
+        texParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE));
+    EXPECT_CALL(*child_context,
+                texParameteri(GL_TEXTURE_2D,
+                              GL_TEXTURE_POOL_CHROMIUM,
+                              GL_TEXTURE_POOL_UNMANAGED_CHROMIUM));
+    child_resource_provider->AllocateForTesting(id);
+    Mock::VerifyAndClearExpectations(child_context);
+
+    uint8_t data[4] = { 1, 2, 3, 4 };
+    gfx::Rect rect(size);
+
+    EXPECT_CALL(*child_context, bindTexture(GL_TEXTURE_2D, texture_id));
+    child_resource_provider->SetPixels(id, data, rect, rect, gfx::Vector2d());
+    Mock::VerifyAndClearExpectations(child_context);
+
+    // The texture is set to |child_filter| in the child.
+    EXPECT_CALL(*child_context, bindTexture(GL_TEXTURE_2D, texture_id));
+    if (child_filter != GL_LINEAR) {
+      EXPECT_CALL(
+          *child_context,
+          texParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, child_filter));
+      EXPECT_CALL(
+          *child_context,
+          texParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, child_filter));
+    }
+    SetResourceFilter(child_resource_provider.get(), id, child_filter);
+    Mock::VerifyAndClearExpectations(child_context);
+
+    int child_id = parent_resource_provider->CreateChild();
+    {
+      // Transfer some resource to the parent.
+      ResourceProvider::ResourceIdArray resource_ids_to_transfer;
+      resource_ids_to_transfer.push_back(id);
+      TransferableResourceArray list;
+
+      EXPECT_CALL(*child_context, bindTexture(GL_TEXTURE_2D, texture_id));
+      EXPECT_CALL(*child_context,
+                  produceTextureCHROMIUM(GL_TEXTURE_2D, _));
+      EXPECT_CALL(*child_context, insertSyncPoint());
+      child_resource_provider->PrepareSendToParent(resource_ids_to_transfer,
+                                                   &list);
+      Mock::VerifyAndClearExpectations(child_context);
+
+      ASSERT_EQ(1u, list.size());
+      EXPECT_EQ(static_cast<unsigned>(child_filter), list[0].filter);
+
+      EXPECT_CALL(*parent_context, bindTexture(GL_TEXTURE_2D, texture_id));
+      EXPECT_CALL(*parent_context,
+                  consumeTextureCHROMIUM(GL_TEXTURE_2D, _));
+      parent_resource_provider->ReceiveFromChild(child_id, list);
+      Mock::VerifyAndClearExpectations(parent_context);
+    }
+    ResourceProvider::ResourceIdMap resource_map =
+        parent_resource_provider->GetChildToParentMap(child_id);
+    ResourceProvider::ResourceId mapped_id = resource_map[id];
+    EXPECT_NE(0u, mapped_id);
+
+    // The texture is set to |parent_filter| in the parent.
+    EXPECT_CALL(*parent_context, bindTexture(GL_TEXTURE_2D, texture_id));
+    EXPECT_CALL(
+        *parent_context,
+        texParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, parent_filter));
+    EXPECT_CALL(
+        *parent_context,
+        texParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, parent_filter));
+    SetResourceFilter(parent_resource_provider.get(), mapped_id, parent_filter);
+    Mock::VerifyAndClearExpectations(parent_context);
+
+    // The texture should be reset to |child_filter| in the parent when it is
+    // returned, since that is how it was received.
+    EXPECT_CALL(*parent_context, bindTexture(GL_TEXTURE_2D, texture_id));
+    EXPECT_CALL(
+        *parent_context,
+        texParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, child_filter));
+    EXPECT_CALL(
+        *parent_context,
+        texParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, child_filter));
+
+    {
+      // Transfer resources back from the parent to the child.
+      ResourceProvider::ResourceIdArray resource_ids_to_transfer;
+      resource_ids_to_transfer.push_back(mapped_id);
+      ReturnedResourceArray list;
+
+      EXPECT_CALL(*parent_context, insertSyncPoint());
+
+      parent_resource_provider->PrepareSendReturnsToChild(
+          child_id, resource_ids_to_transfer, &list);
+      ASSERT_EQ(1u, list.size());
+      child_resource_provider->ReceiveReturnsFromParent(list);
+    }
+    Mock::VerifyAndClearExpectations(parent_context);
+
+    // The child remembers the texture filter is set to |child_filter|.
+    EXPECT_CALL(*child_context, bindTexture(GL_TEXTURE_2D, texture_id));
+    SetResourceFilter(child_resource_provider.get(), id, child_filter);
+    Mock::VerifyAndClearExpectations(child_context);
   }
-  EXPECT_EQ(static_cast<unsigned>(GL_LINEAR),
-            GetResourceFilter(child_resource_provider.get(),
-                              child_context,
-                              id));
-  SetResourceFilter(child_resource_provider.get(), id, GL_NEAREST);
-  EXPECT_EQ(static_cast<unsigned>(GL_NEAREST),
-            GetResourceFilter(child_resource_provider.get(),
-                              child_context,
-                              id));
+};
+
+TEST_P(ResourceProviderTest, TextureFilters_ChildNearestParentLinear) {
+  if (GetParam() != ResourceProvider::GLTexture)
+    return;
+  ResourceProviderTestTextureFilters::RunTest(GL_NEAREST, GL_LINEAR);
+}
+
+TEST_P(ResourceProviderTest, TextureFilters_ChildLinearParentNearest) {
+  if (GetParam() != ResourceProvider::GLTexture)
+    return;
+  ResourceProviderTestTextureFilters::RunTest(GL_LINEAR, GL_NEAREST);
 }
 
 void ReleaseTextureMailbox(unsigned* release_sync_point,
@@ -1059,22 +1146,6 @@ TEST_P(ResourceProviderTest, LostContext) {
   EXPECT_TRUE(lost_resource);
 }
 
-class TextureStateTrackingContext : public TestWebGraphicsContext3D {
- public:
-  MOCK_METHOD2(bindTexture, void(WGC3Denum target, WebGLId texture));
-  MOCK_METHOD3(texParameteri,
-               void(WGC3Denum target, WGC3Denum pname, WGC3Dint param));
-  MOCK_METHOD1(waitSyncPoint, void(unsigned sync_point));
-  MOCK_METHOD0(insertSyncPoint, unsigned(void));
-  MOCK_METHOD2(produceTextureCHROMIUM, void(WGC3Denum target,
-                                            const WGC3Dbyte* mailbox));
-  MOCK_METHOD2(consumeTextureCHROMIUM, void(WGC3Denum target,
-                                            const WGC3Dbyte* mailbox));
-
-  // Force all textures to be "1" so we can test for them.
-  virtual WebKit::WebGLId NextTextureId() OVERRIDE { return 1; }
-};
-
 TEST_P(ResourceProviderTest, ScopedSampler) {
   // Sampling is only supported for GL textures.
   if (GetParam() != ResourceProvider::GLTexture)
@@ -1096,6 +1167,9 @@ TEST_P(ResourceProviderTest, ScopedSampler) {
   WGC3Denum format = GL_RGBA;
   int texture_id = 1;
 
+  ResourceProvider::ResourceId id = resource_provider->CreateResource(
+      size, format, GL_CLAMP_TO_EDGE, ResourceProvider::TextureUsageAny);
+
   // Check that the texture gets created with the right sampler settings.
   EXPECT_CALL(*context, bindTexture(GL_TEXTURE_2D, texture_id))
       .Times(2);  // Once to create and once to allocate.
@@ -1113,9 +1187,8 @@ TEST_P(ResourceProviderTest, ScopedSampler) {
               texParameteri(GL_TEXTURE_2D,
                             GL_TEXTURE_POOL_CHROMIUM,
                             GL_TEXTURE_POOL_UNMANAGED_CHROMIUM));
-  ResourceProvider::ResourceId id = resource_provider->CreateResource(
-      size, format, GL_CLAMP_TO_EDGE, ResourceProvider::TextureUsageAny);
   resource_provider->AllocateForTesting(id);
+  Mock::VerifyAndClearExpectations(context);
 
   // Creating a sampler with the default filter should not change any texture
   // parameters.
@@ -1123,6 +1196,7 @@ TEST_P(ResourceProviderTest, ScopedSampler) {
     EXPECT_CALL(*context, bindTexture(GL_TEXTURE_2D, texture_id));
     ResourceProvider::ScopedSamplerGL sampler(
         resource_provider.get(), id, GL_TEXTURE_2D, GL_LINEAR);
+    Mock::VerifyAndClearExpectations(context);
   }
 
   // Using a different filter should be reflected in the texture parameters.
@@ -1136,6 +1210,7 @@ TEST_P(ResourceProviderTest, ScopedSampler) {
         texParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST));
     ResourceProvider::ScopedSamplerGL sampler(
         resource_provider.get(), id, GL_TEXTURE_2D, GL_NEAREST);
+    Mock::VerifyAndClearExpectations(context);
   }
 
   // Test resetting to the default filter.
@@ -1147,9 +1222,8 @@ TEST_P(ResourceProviderTest, ScopedSampler) {
                 texParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR));
     ResourceProvider::ScopedSamplerGL sampler(
         resource_provider.get(), id, GL_TEXTURE_2D, GL_LINEAR);
+    Mock::VerifyAndClearExpectations(context);
   }
-
-  Mock::VerifyAndClearExpectations(context);
 }
 
 TEST_P(ResourceProviderTest, ManagedResource) {
diff --git a/cc/resources/returned_resource.h b/cc/resources/returned_resource.h
index aa6b99a..eeb4ba2 100644
--- a/cc/resources/returned_resource.h
+++ b/cc/resources/returned_resource.h
@@ -13,10 +13,9 @@
 namespace cc {
 
 struct CC_EXPORT ReturnedResource {
-  ReturnedResource() : id(0), sync_point(0), filter(0), count(0) {}
+  ReturnedResource() : id(0), sync_point(0), count(0) {}
   unsigned id;
   unsigned sync_point;
-  uint32 filter;
   int count;
 };
 
diff --git a/cc/resources/transferable_resource.cc b/cc/resources/transferable_resource.cc
index 9e90696..8a200c3 100644
--- a/cc/resources/transferable_resource.cc
+++ b/cc/resources/transferable_resource.cc
@@ -22,7 +22,6 @@ ReturnedResource TransferableResource::ToReturnedResource() const {
   ReturnedResource returned;
   returned.id = id;
   returned.sync_point = sync_point;
-  returned.filter = filter;
   returned.count = 1;
   return returned;
 }