cc: Defer texture allocation (to allow async allocations).

For textures, delay allocating until the first setPixels
or beginSetPixels. This allows us to do a combined async
allocation/upload in beginSetPixels.

BUG=161337


Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=173875

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=174035

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

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

cc/heads_up_display_layer_impl.cc

@@ -77,8 +77,14 @@ void HeadsUpDisplayLayerImpl::willDraw(ResourceProvider* resourceProvider)
     if (m_hudTexture->size() != bounds() || resourceProvider->inUseByConsumer(m_hudTexture->id()))
         m_hudTexture->Free();
 
-    if (!m_hudTexture->id())
+    if (!m_hudTexture->id()) {
         m_hudTexture->Allocate(bounds(), GL_RGBA, ResourceProvider::TextureUsageAny);
+        // TODO(epenner): This texture was being used before setPixels was called,
+        // which is now not allowed (it's an uninitialized read). This should be fixed
+        // and this allocateForTesting() removed.
+        // http://crbug.com/166784
+        resourceProvider->allocateForTesting(m_hudTexture->id());
+    }
 }
 
 void HeadsUpDisplayLayerImpl::appendQuads(QuadSink& quadSink, AppendQuadsData& appendQuadsData)

cc/layer_tree_host_impl_unittest.cc

@@ -1130,6 +1130,7 @@ private:
         setSkipsDraw(skipsDraw);
         if (!tileMissing) {
             ResourceProvider::ResourceId resource = resourceProvider->createResource(gfx::Size(), GL_RGBA, ResourceProvider::TextureUsageAny);
+            resourceProvider->allocateForTesting(resource);
             pushTileProperties(0, 0, resource, gfx::Rect(), false);
         }
         if (animating)
@@ -1755,6 +1756,7 @@ private:
         , m_quadVisibleRect(5, 5, 5, 5)
         , m_resourceId(resourceProvider->createResource(gfx::Size(1, 1), GL_RGBA, ResourceProvider::TextureUsageAny))
     {
+        resourceProvider->allocateForTesting(m_resourceId);
         setAnchorPoint(gfx::PointF(0, 0));
         setBounds(gfx::Size(10, 10));
         setContentBounds(gfx::Size(10, 10));

cc/resource_provider.cc

@@ -60,6 +60,7 @@ ResourceProvider::Resource::Resource()
     , exported(false)
     , markedForDeletion(false)
     , pendingSetPixels(false)
+    , allocated(false)
     , size()
     , format(0)
     , filter(0)
@@ -83,6 +84,7 @@ ResourceProvider::Resource::Resource(unsigned textureId, const gfx::Size& size,
     , exported(false)
     , markedForDeletion(false)
     , pendingSetPixels(false)
+    , allocated(false)
     , size(size)
     , format(format)
     , filter(filter)
@@ -102,6 +104,7 @@ ResourceProvider::Resource::Resource(uint8_t* pixels, const gfx::Size& size, GLe
     , exported(false)
     , markedForDeletion(false)
     , pendingSetPixels(false)
+    , allocated(false)
     , size(size)
     , format(format)
     , filter(filter)
@@ -188,22 +191,19 @@ ResourceProvider::ResourceId ResourceProvider::createGLTexture(const gfx::Size&
     DCHECK(context3d);
     GLC(context3d, textureId = context3d->createTexture());
     GLC(context3d, context3d->bindTexture(GL_TEXTURE_2D, textureId));
+
+    // Set texture properties. Allocation is delayed until needed.
     GLC(context3d, context3d->texParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR));
     GLC(context3d, context3d->texParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR));
     GLC(context3d, context3d->texParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE));
     GLC(context3d, context3d->texParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE));
     GLC(context3d, context3d->texParameteri(GL_TEXTURE_2D, GL_TEXTURE_POOL_CHROMIUM, texturePool));
-
     if (m_useTextureUsageHint && hint == TextureUsageFramebuffer)
         GLC(context3d, context3d->texParameteri(GL_TEXTURE_2D, GL_TEXTURE_USAGE_ANGLE, GL_FRAMEBUFFER_ATTACHMENT_ANGLE));
-    if (m_useTextureStorageExt && isTextureFormatSupportedForStorage(format)) {
-        GLenum storageFormat = textureToStorageFormat(format);
-        GLC(context3d, context3d->texStorage2DEXT(GL_TEXTURE_2D, 1, storageFormat, size.width(), size.height()));
-    } else
-        GLC(context3d, context3d->texImage2D(GL_TEXTURE_2D, 0, format, size.width(), size.height(), 0, format, GL_UNSIGNED_BYTE, 0));
 
     ResourceId id = m_nextId++;
     Resource resource(textureId, size, format, GL_LINEAR);
+    resource.allocated = false;
     m_resources[id] = resource;
     return id;
 }
@@ -216,6 +216,7 @@ ResourceProvider::ResourceId ResourceProvider::createBitmap(const gfx::Size& siz
 
     ResourceId id = m_nextId++;
     Resource resource(pixels, size, GL_RGBA, GL_LINEAR);
+    resource.allocated = true;
     m_resources[id] = resource;
     return id;
 }
@@ -235,6 +236,7 @@ ResourceProvider::ResourceId ResourceProvider::createResourceFromExternalTexture
     ResourceId id = m_nextId++;
     Resource resource(textureId, gfx::Size(), 0, GL_LINEAR);
     resource.external = true;
+    resource.allocated = true;
     m_resources[id] = resource;
     return id;
 }
@@ -267,9 +269,9 @@ void ResourceProvider::deleteResource(ResourceId id)
     ResourceMap::iterator it = m_resources.find(id);
     CHECK(it != m_resources.end());
     Resource* resource = &it->second;
-    DCHECK(!resource->lockedForWrite);
     DCHECK(!resource->lockForReadCount);
     DCHECK(!resource->markedForDeletion);
+    DCHECK(resource->pendingSetPixels || !resource->lockedForWrite);
 
     if (resource->exported) {
         resource->markedForDeletion = true;
@@ -333,8 +335,10 @@ void ResourceProvider::setPixels(ResourceId id, const uint8_t* image, const gfx:
     DCHECK(!resource->lockForReadCount);
     DCHECK(!resource->external);
     DCHECK(!resource->exported);
+    lazyAllocate(resource);
 
     if (resource->glId) {
+        DCHECK(!resource->pendingSetPixels);
         WebGraphicsContext3D* context3d = m_outputSurface->Context3D();
         DCHECK(context3d);
         DCHECK(m_textureUploader.get());
@@ -348,6 +352,7 @@ void ResourceProvider::setPixels(ResourceId id, const uint8_t* image, const gfx:
     }
 
     if (resource->pixels) {
+        DCHECK(resource->allocated);
         DCHECK(resource->format == GL_RGBA);
         SkBitmap srcFull;
         srcFull.setConfig(SkBitmap::kARGB_8888_Config, imageRect.width(), imageRect.height());
@@ -422,6 +427,8 @@ const ResourceProvider::Resource* ResourceProvider::lockForRead(ResourceId id)
     Resource* resource = &it->second;
     DCHECK(!resource->lockedForWrite);
     DCHECK(!resource->exported);
+    DCHECK(resource->allocated); // Uninitialized! Call setPixels or lockForWrite first.
+
     resource->lockForReadCount++;
     return resource;
 }
@@ -447,6 +454,8 @@ const ResourceProvider::Resource* ResourceProvider::lockForWrite(ResourceId id)
     DCHECK(!resource->lockForReadCount);
     DCHECK(!resource->exported);
     DCHECK(!resource->external);
+    lazyAllocate(resource);
+
     resource->lockedForWrite = true;
     return resource;
 }
@@ -683,6 +692,8 @@ void ResourceProvider::receiveFromChild(int child, const TransferableResourceLis
         ResourceId id = m_nextId++;
         Resource resource(textureId, it->size, it->format, it->filter);
         resource.mailbox.setName(it->mailbox.name);
+        // Don't allocate a texture for a child.
+        resource.allocated = true;
         m_resources[id] = resource;
         childInfo.parentToChildMap[id] = it->id;
         childInfo.childToParentMap[it->id] = id;
@@ -723,6 +734,7 @@ bool ResourceProvider::transferResource(WebGraphicsContext3D* context, ResourceI
     DCHECK(!source->lockedForWrite);
     DCHECK(!source->lockForReadCount);
     DCHECK(!source->external || (source->external && !source->mailbox.isZero()));
+    DCHECK(source->allocated);
     if (source->exported)
         return false;
     resource->id = id;
@@ -870,6 +882,7 @@ void ResourceProvider::setPixelsFromBuffer(ResourceId id)
     DCHECK(!resource->lockForReadCount);
     DCHECK(!resource->external);
     DCHECK(!resource->exported);
+    lazyAllocate(resource);
 
     if (resource->glId) {
         WebGraphicsContext3D* context3d = m_outputSurface->Context3D();
@@ -931,7 +944,10 @@ void ResourceProvider::beginSetPixels(ResourceId id)
     CHECK(it != m_resources.end());
     Resource* resource = &it->second;
     DCHECK(!resource->pendingSetPixels);
+    DCHECK(resource->glId || resource->allocated);
 
+    bool allocate = !resource->allocated;
+    resource->allocated = true;
     lockForWrite(id);
 
     if (resource->glId) {
@@ -947,6 +963,17 @@ void ResourceProvider::beginSetPixels(ResourceId id)
         context3d->beginQueryEXT(
             GL_ASYNC_PIXEL_TRANSFERS_COMPLETED_CHROMIUM,
             resource->glUploadQueryId);
+        if (allocate) {
+          context3d->asyncTexImage2DCHROMIUM(GL_TEXTURE_2D,
+                                             0, /* level */
+                                             resource->format,
+                                             resource->size.width(),
+                                             resource->size.height(),
+                                             0, /* border */
+                                             resource->format,
+                                             GL_UNSIGNED_BYTE,
+                                             NULL);
+        } else {
           context3d->asyncTexSubImage2DCHROMIUM(GL_TEXTURE_2D,
                                                 0, /* level */
                                                 0, /* x */
@@ -956,6 +983,7 @@ void ResourceProvider::beginSetPixels(ResourceId id)
                                                 resource->format,
                                                 GL_UNSIGNED_BYTE,
                                                 NULL);
+        }
         context3d->endQueryEXT(GL_ASYNC_PIXEL_TRANSFERS_COMPLETED_CHROMIUM);
         context3d->bindBuffer(GL_PIXEL_UNPACK_TRANSFER_BUFFER_CHROMIUM, 0);
     }
@@ -993,4 +1021,31 @@ bool ResourceProvider::didSetPixelsComplete(ResourceId id) {
     return true;
 }
 
+void ResourceProvider::allocateForTesting(ResourceId id) {
+    ResourceMap::iterator it = m_resources.find(id);
+    CHECK(it != m_resources.end());
+    Resource* resource = &it->second;
+    lazyAllocate(resource);
+}
+
+void ResourceProvider::lazyAllocate(Resource* resource) {
+    DCHECK(resource);
+    DCHECK(resource->glId || resource->allocated);
+
+    if (resource->allocated || !resource->glId)
+        return;
+
+    resource->allocated = true;
+    WebGraphicsContext3D* context3d = m_outputSurface->Context3D();
+    gfx::Size& size = resource->size;
+    GLenum format = resource->format;
+    GLC(context3d, context3d->bindTexture(GL_TEXTURE_2D, resource->glId));
+    if (m_useTextureStorageExt && isTextureFormatSupportedForStorage(format)) {
+        GLenum storageFormat = textureToStorageFormat(format);
+        GLC(context3d, context3d->texStorage2DEXT(GL_TEXTURE_2D, 1, storageFormat, size.width(), size.height()));
+    } else
+        GLC(context3d, context3d->texImage2D(GL_TEXTURE_2D, 0, format, size.width(), size.height(), 0, format, GL_UNSIGNED_BYTE, 0));
+}
+
+
 }  // namespace cc

cc/resource_provider.h

@@ -236,6 +236,10 @@ public:
     void beginSetPixels(ResourceId id);
     bool didSetPixelsComplete(ResourceId id);
 
+    // For tests only! This prevents detecting uninitialized reads.
+    // Use setPixels or lockForWrite to allocate implicitly.
+    void allocateForTesting(ResourceId id);
+
 private:
     struct Resource {
         Resource();
@@ -258,6 +262,7 @@ private:
         bool exported;
         bool markedForDeletion;
         bool pendingSetPixels;
+        bool allocated;
         gfx::Size size;
         GLenum format;
         // TODO(skyostil): Use a separate sampler object for filter state.
@@ -285,6 +290,7 @@ private:
 
     bool transferResource(WebKit::WebGraphicsContext3D*, ResourceId, TransferableResource*);
     void deleteResourceInternal(ResourceMap::iterator it);
+    void lazyAllocate(Resource*);
 
     OutputSurface* m_outputSurface;
     ResourceId m_nextId;

cc/resource_provider_unittest.cc

@@ -21,6 +21,10 @@
 using namespace WebKit;
 
 using testing::Mock;
+using testing::NiceMock;
+using testing::Return;
+using testing::StrictMock;
+using testing::_;
 
 namespace cc {
 namespace {
@@ -584,13 +588,15 @@ TEST_P(ResourceProviderTest, ScopedSampler)
     int textureId = 1;
 
     // Check that the texture gets created with the right sampler settings.
-    EXPECT_CALL(*context, bindTexture(GL_TEXTURE_2D, textureId));
+    EXPECT_CALL(*context, bindTexture(GL_TEXTURE_2D, textureId))
+        .Times(2); // Once to create and once to allocate.
     EXPECT_CALL(*context, texParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR));
     EXPECT_CALL(*context, texParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR));
     EXPECT_CALL(*context, texParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE));
     EXPECT_CALL(*context, texParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE));
     EXPECT_CALL(*context, texParameteri(GL_TEXTURE_2D, GL_TEXTURE_POOL_CHROMIUM, GL_TEXTURE_POOL_UNMANAGED_CHROMIUM));
     ResourceProvider::ResourceId id = resourceProvider->createResource(size, format, ResourceProvider::TextureUsageAny);
+    resourceProvider->allocateForTesting(id);
 
     // Creating a sampler with the default filter should not change any texture
     // parameters.
@@ -644,6 +650,92 @@ TEST_P(ResourceProviderTest, ManagedResource)
     Mock::VerifyAndClearExpectations(context);
 }
 
+class AllocationTrackingContext3D : public FakeWebGraphicsContext3D {
+public:
+    MOCK_METHOD0(createTexture, WebGLId());
+    MOCK_METHOD1(deleteTexture, void(WebGLId texture_id));
+    MOCK_METHOD2(bindTexture, void(WGC3Denum target, WebGLId texture));
+    MOCK_METHOD9(texImage2D, void(WGC3Denum target, WGC3Dint level, WGC3Denum internalformat,
+                                  WGC3Dsizei width, WGC3Dsizei height, WGC3Dint border, WGC3Denum format,
+                                  WGC3Denum type, const void* pixels));
+    MOCK_METHOD9(texSubImage2D, void(WGC3Denum target, WGC3Dint level, WGC3Dint xoffset, WGC3Dint yoffset,
+                                     WGC3Dsizei width, WGC3Dsizei height, WGC3Denum format,
+                                     WGC3Denum type, const void* pixels));
+    MOCK_METHOD9(asyncTexImage2DCHROMIUM, void(WGC3Denum target, WGC3Dint level, WGC3Denum internalformat,
+                                              WGC3Dsizei width, WGC3Dsizei height, WGC3Dint border, WGC3Denum format,
+                                              WGC3Denum type, const void* pixels));
+    MOCK_METHOD9(asyncTexSubImage2DCHROMIUM, void(WGC3Denum target, WGC3Dint level, WGC3Dint xoffset, WGC3Dint yoffset,
+                                                  WGC3Dsizei width, WGC3Dsizei height, WGC3Denum format,
+                                                  WGC3Denum type, const void* pixels));
+};
+
+TEST_P(ResourceProviderTest, TextureAllocation)
+{
+    // Only for GL textures.
+    if (GetParam() != ResourceProvider::GLTexture)
+        return;
+    scoped_ptr<WebKit::WebGraphicsContext3D> mock_context(
+        static_cast<WebKit::WebGraphicsContext3D*>(new NiceMock<AllocationTrackingContext3D>));
+    scoped_ptr<OutputSurface> outputSurface(FakeOutputSurface::Create3d(mock_context.Pass()));
+
+    gfx::Size size(2, 2);
+    gfx::Vector2d offset(0, 0);
+    gfx::Rect rect(0, 0, 2, 2);
+    WGC3Denum format = GL_RGBA;
+    ResourceProvider::ResourceId id = 0;
+    uint8_t pixels[16] = {0};
+    int textureId = 123;
+
+    AllocationTrackingContext3D* context = static_cast<AllocationTrackingContext3D*>(outputSurface->Context3D());
+    scoped_ptr<ResourceProvider> resourceProvider(ResourceProvider::create(outputSurface.get()));
+
+    // Lazy allocation. Don't allocate when creating the resource.
+    EXPECT_CALL(*context, createTexture()).WillOnce(Return(textureId));
+    EXPECT_CALL(*context, deleteTexture(textureId)).Times(1);
+    EXPECT_CALL(*context, bindTexture(GL_TEXTURE_2D, textureId)).Times(1);
+    EXPECT_CALL(*context, texImage2D(_,_,_,_,_,_,_,_,_)).Times(0);
+    EXPECT_CALL(*context, asyncTexImage2DCHROMIUM(_,_,_,_,_,_,_,_,_)).Times(0);
+    id = resourceProvider->createResource(size, format, ResourceProvider::TextureUsageAny);
+    resourceProvider->deleteResource(id);
+    Mock::VerifyAndClearExpectations(context);
+
+    // Do allocate when we set the pixels.
+    EXPECT_CALL(*context, createTexture()).WillOnce(Return(textureId));
+    EXPECT_CALL(*context, deleteTexture(textureId)).Times(1);
+    EXPECT_CALL(*context, bindTexture(GL_TEXTURE_2D, textureId)).Times(3);
+    EXPECT_CALL(*context, texImage2D(_,_,_,2,2,_,_,_,_)).Times(1);
+    EXPECT_CALL(*context, texSubImage2D(_,_,_,_,2,2,_,_,_)).Times(1);
+    id = resourceProvider->createResource(size, format, ResourceProvider::TextureUsageAny);
+    resourceProvider->setPixels(id, pixels, rect, rect, offset);
+    resourceProvider->deleteResource(id);
+    Mock::VerifyAndClearExpectations(context);
+
+    // Same for setPixelsFromBuffer
+    EXPECT_CALL(*context, createTexture()).WillOnce(Return(textureId));
+    EXPECT_CALL(*context, deleteTexture(textureId)).Times(1);
+    EXPECT_CALL(*context, bindTexture(GL_TEXTURE_2D, textureId)).Times(3);
+    EXPECT_CALL(*context, texImage2D(_,_,_,2,2,_,_,_,_)).Times(1);
+    EXPECT_CALL(*context, texSubImage2D(_,_,_,_,2,2,_,_,_)).Times(1);
+    id = resourceProvider->createResource(size, format, ResourceProvider::TextureUsageAny);
+    resourceProvider->acquirePixelBuffer(id);
+    resourceProvider->setPixelsFromBuffer(id);
+    resourceProvider->releasePixelBuffer(id);
+    resourceProvider->deleteResource(id);
+    Mock::VerifyAndClearExpectations(context);
+
+    // Same for async version.
+    EXPECT_CALL(*context, createTexture()).WillOnce(Return(textureId));
+    EXPECT_CALL(*context, deleteTexture(textureId)).Times(1);
+    EXPECT_CALL(*context, bindTexture(GL_TEXTURE_2D, textureId)).Times(2);
+    EXPECT_CALL(*context, asyncTexImage2DCHROMIUM(_,_,_,2,2,_,_,_,_)).Times(1);
+    id = resourceProvider->createResource(size, format, ResourceProvider::TextureUsageAny);
+    resourceProvider->acquirePixelBuffer(id);
+    resourceProvider->beginSetPixels(id);
+    resourceProvider->releasePixelBuffer(id);
+    resourceProvider->deleteResource(id);
+    Mock::VerifyAndClearExpectations(context);
+}
+
 INSTANTIATE_TEST_CASE_P(ResourceProviderTests,
                         ResourceProviderTest,
                         ::testing::Values(ResourceProvider::GLTexture,

cc/resource_update_controller.cc

@@ -141,9 +141,9 @@ void ResourceUpdateController::updateTexture(ResourceUpdate update)
         // is available in other shared contexts. It is important to do here
         // because the backing texture is created in one context while it is
         // being written to in another.
-        m_resourceProvider->flush();
         ResourceProvider::ScopedWriteLockGL lock(
             m_resourceProvider, texture->resourceId());
+        m_resourceProvider->flush();
 
         // Make sure ganesh uses the correct GL context.
         paintContext->makeContextCurrent();

cc/test/render_pass_test_common.cc

@@ -29,6 +29,7 @@ void TestRenderPass::AppendOneOfEveryQuadType(
           gfx::Size(20, 12),
           resourceProvider->bestTextureFormat(),
           ResourceProvider::TextureUsageAny);
+  resourceProvider->allocateForTesting(texture_resource);
   unsigned texture_id = ResourceProvider::ScopedReadLockGL(
       resourceProvider, texture_resource).textureId();
 
@@ -163,6 +164,7 @@ void TestRenderPass::AppendOneOfEveryQuadType(
             gfx::Size(20, 12),
             resourceProvider->bestTextureFormat(),
             ResourceProvider::TextureUsageAny);
+    resourceProvider->allocateForTesting(planes[i].resourceId);
     planes[i].size = gfx::Size(100, 100);
     planes[i].format = GL_LUMINANCE;
   }

cc/texture_layer_unittest.cc

@@ -384,6 +384,7 @@ TEST_F(TextureLayerImplWithMailboxTest, testCallbackOnInUseResource)
         provider->createResourceFromTextureMailbox(
            m_testData.m_mailbox1,
            m_testData.m_releaseMailbox1);
+    provider->allocateForTesting(id);
 
     // Transfer some resources to the parent.
     ResourceProvider::ResourceIdArray resourceIdsToTransfer;