Coordinated Graphics: Directly composited animated GIFs only render the first image.
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 13 Nov 2012 08:11:18 +0000 (08:11 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 13 Nov 2012 08:11:18 +0000 (08:11 +0000)
https://bugs.webkit.org/show_bug.cgi?id=102043

Patch by Huang Dongsung <luxtella@company100.net> on 2012-11-13
Reviewed by Noam Rosenthal.

.:

Add a test to check that a gif animation can run on a compositing layer.

* ManualTests/animated-gif-on-compositing-layer.html: Added.

Source/WebKit2:

CoordinatedGraphicsLayer::setContentsToImage() checks the pointer to the image,
not nativeImagePtr, so Coordinated Graphics currently draws only the first frame
of gif animations. This patch makes Coordinated Graphics draw gif animations.

In addition, this patch modifies the style of direct image compositing
code to match other parts of CoordinatedGraphicsLayer.

Test: ManualTests/animated-gif-on-compositing-layer.html

* WebProcess/WebPage/CoordinatedGraphics/CoordinatedGraphicsLayer.cpp:
(WebCore::CoordinatedGraphicsLayer::didChangeImageBacking):
(WebCore):
(WebCore::CoordinatedGraphicsLayer::CoordinatedGraphicsLayer):
(WebCore::CoordinatedGraphicsLayer::setContentsNeedsDisplay):
(WebCore::CoordinatedGraphicsLayer::setContentsToImage):
(WebCore::CoordinatedGraphicsLayer::syncImageBacking):
(WebCore::CoordinatedGraphicsLayer::flushCompositingStateForThisLayerOnly):
(WebCore::CoordinatedGraphicsLayer::releaseImageBackingIfNeeded):
(WebCore::CoordinatedGraphicsLayer::purgeBackingStores):
(WebCore::CoordinatedGraphicsLayer::hasPendingVisibleChanges):
* WebProcess/WebPage/CoordinatedGraphics/CoordinatedGraphicsLayer.h:
(CoordinatedGraphicsLayer):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@134376 268f45cc-cd09-0410-ab3c-d52691b4dbfc

ChangeLog
ManualTests/animated-gif-on-compositing-layer.html [new file with mode: 0644]
Source/WebKit2/ChangeLog
Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/CoordinatedGraphicsLayer.cpp
Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/CoordinatedGraphicsLayer.h

index 61843ee..174c3a3 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,14 @@
+2012-11-13  Huang Dongsung  <luxtella@company100.net>
+
+        Coordinated Graphics: Directly composited animated GIFs only render the first image.
+        https://bugs.webkit.org/show_bug.cgi?id=102043
+
+        Reviewed by Noam Rosenthal.
+
+        Add a test to check that a gif animation can run on a compositing layer.
+
+        * ManualTests/animated-gif-on-compositing-layer.html: Added.
+
 2012-11-12  KyungTae Kim  <ktf.kim@samsung.com>
 
         [EFL] Turn on errors on warnings for WebKit1 and WebKit2 libraries
diff --git a/ManualTests/animated-gif-on-compositing-layer.html b/ManualTests/animated-gif-on-compositing-layer.html
new file mode 100644 (file)
index 0000000..4dc2d0b
--- /dev/null
@@ -0,0 +1,13 @@
+<html><head>
+<title>Animated GIF on a compositing layer</title>
+<style type="text/css" media="screen">
+.compositing {
+ top: 0px;
+ left: 0px;
+ -webkit-transform:translateZ(0);
+}
+</style>
+</head><body>
+<p>This test checks that a gif animation can run on a compositing layer. The test passes if the gif animation runs.</p>
+<img class="compositing" src="./resources/animated-infinite.gif" />
+</body></html>
index 7b7e6dd..6c84141 100644 (file)
@@ -1,3 +1,33 @@
+2012-11-13  Huang Dongsung  <luxtella@company100.net>
+
+        Coordinated Graphics: Directly composited animated GIFs only render the first image.
+        https://bugs.webkit.org/show_bug.cgi?id=102043
+
+        Reviewed by Noam Rosenthal.
+
+        CoordinatedGraphicsLayer::setContentsToImage() checks the pointer to the image,
+        not nativeImagePtr, so Coordinated Graphics currently draws only the first frame
+        of gif animations. This patch makes Coordinated Graphics draw gif animations.
+
+        In addition, this patch modifies the style of direct image compositing
+        code to match other parts of CoordinatedGraphicsLayer.
+
+        Test: ManualTests/animated-gif-on-compositing-layer.html
+
+        * WebProcess/WebPage/CoordinatedGraphics/CoordinatedGraphicsLayer.cpp:
+        (WebCore::CoordinatedGraphicsLayer::didChangeImageBacking):
+        (WebCore):
+        (WebCore::CoordinatedGraphicsLayer::CoordinatedGraphicsLayer):
+        (WebCore::CoordinatedGraphicsLayer::setContentsNeedsDisplay):
+        (WebCore::CoordinatedGraphicsLayer::setContentsToImage):
+        (WebCore::CoordinatedGraphicsLayer::syncImageBacking):
+        (WebCore::CoordinatedGraphicsLayer::flushCompositingStateForThisLayerOnly):
+        (WebCore::CoordinatedGraphicsLayer::releaseImageBackingIfNeeded):
+        (WebCore::CoordinatedGraphicsLayer::purgeBackingStores):
+        (WebCore::CoordinatedGraphicsLayer::hasPendingVisibleChanges):
+        * WebProcess/WebPage/CoordinatedGraphics/CoordinatedGraphicsLayer.h:
+        (CoordinatedGraphicsLayer):
+
 2012-11-12  Huang Dongsung  <luxtella@company100.net>
 
         [Qt] REGRESSION(134142): overscaled tiles in pixel test results and MiniBrowser
index 75923e4..6eb2fa4 100644 (file)
@@ -82,6 +82,13 @@ void CoordinatedGraphicsLayer::didChangeFilters()
 }
 #endif
 
+void CoordinatedGraphicsLayer::didChangeImageBacking()
+{
+    m_shouldSyncImageBacking = true;
+    if (client())
+        client()->notifyFlushRequired(this);
+}
+
 void CoordinatedGraphicsLayer::setShouldUpdateVisibleRect()
 {
     m_shouldUpdateVisibleRect = true;
@@ -105,11 +112,13 @@ CoordinatedGraphicsLayer::CoordinatedGraphicsLayer(GraphicsLayerClient* client)
     , m_shouldSyncLayerState(true)
     , m_shouldSyncChildren(true)
     , m_shouldSyncFilters(true)
+    , m_shouldSyncImageBacking(true)
     , m_shouldSyncAnimations(true)
     , m_fixedToViewport(false)
     , m_canvasNeedsDisplay(false)
     , m_CoordinatedGraphicsLayerClient(0)
     , m_contentsScale(1)
+    , m_compositedNativeImagePtr(0)
     , m_canvasPlatformLayer(0)
     , m_animationStartedTimer(this, &CoordinatedGraphicsLayer::animationStartedTimerFired)
 {
@@ -318,9 +327,6 @@ void CoordinatedGraphicsLayer::setContentsRect(const IntRect& r)
 
 void CoordinatedGraphicsLayer::setContentsNeedsDisplay()
 {
-    RefPtr<Image> image = m_image;
-    setContentsToImage(0);
-    setContentsToImage(image.get());
     m_canvasNeedsDisplay = true;
     if (client())
         client()->notifyFlushRequired(this);
@@ -347,25 +353,25 @@ bool CoordinatedGraphicsLayer::setFilters(const FilterOperations& newFilters)
 
 void CoordinatedGraphicsLayer::setContentsToImage(Image* image)
 {
-    if (image == m_image)
-        return;
-    int64_t newID = 0;
-    if (m_CoordinatedGraphicsLayerClient) {
-        // We adopt first, in case this is the same frame - that way we avoid destroying and recreating the image.
-        newID = m_CoordinatedGraphicsLayerClient->adoptImageBackingStore(image);
-        m_CoordinatedGraphicsLayerClient->releaseImageBackingStore(m_layerInfo.imageBackingStoreID);
-        didChangeLayerState();
-        if (m_layerInfo.imageBackingStoreID && newID == m_layerInfo.imageBackingStoreID)
+    if (image) {
+        // This code makes the assumption that pointer equality on a NativeImagePtr is a valid way to tell if the image is changed.
+        // This assumption is true in Qt, GTK and EFL.
+        NativeImagePtr newNativeImagePtr = image->nativeImageForCurrentFrame();
+        if (!newNativeImagePtr)
+            return;
+
+        if (newNativeImagePtr == m_compositedNativeImagePtr)
             return;
+
+        m_compositedImage = image;
+        m_compositedNativeImagePtr = newNativeImagePtr;
     } else {
-        // If m_CoordinatedGraphicsLayerClient is not set yet there should be no backing store ID.
-        ASSERT(!m_layerInfo.imageBackingStoreID);
-        didChangeLayerState();
+        m_compositedImage = 0;
+        m_compositedNativeImagePtr = 0;
     }
 
-    m_layerInfo.imageBackingStoreID = newID;
-    m_image = image;
     GraphicsLayer::setContentsToImage(image);
+    didChangeImageBacking();
 }
 
 void CoordinatedGraphicsLayer::setMaskLayer(GraphicsLayer* layer)
@@ -397,8 +403,6 @@ bool CoordinatedGraphicsLayer::shouldDirectlyCompositeImage(Image* image) const
         return false;
 
     return true;
-
-    // TODO: don't directly composite images with more than one frame.
 }
 
 void CoordinatedGraphicsLayer::setReplicatedByLayer(GraphicsLayer* layer)
@@ -473,6 +477,25 @@ void CoordinatedGraphicsLayer::syncFilters()
 }
 #endif
 
+void CoordinatedGraphicsLayer::syncImageBacking()
+{
+    if (!m_shouldSyncImageBacking)
+        return;
+    m_shouldSyncImageBacking = false;
+
+    releaseImageBackingIfNeeded();
+
+    if (m_compositedNativeImagePtr) {
+        ASSERT(!m_mainBackingStore);
+        ASSERT(m_compositedImage);
+
+        m_layerInfo.imageBackingStoreID = m_CoordinatedGraphicsLayerClient->adoptImageBackingStore(m_compositedImage.get());
+    }
+
+    // This method changes m_layerInfo.imageBackingStoreID.
+    didChangeLayerState();
+}
+
 void CoordinatedGraphicsLayer::syncLayerState()
 {
     if (!m_shouldSyncLayerState)
@@ -525,18 +548,9 @@ void CoordinatedGraphicsLayer::syncCanvas()
     m_canvasNeedsDisplay = false;
 }
 
-void CoordinatedGraphicsLayer::ensureImageBackingStore()
-{
-    if (!m_image)
-        return;
-    if (!m_layerInfo.imageBackingStoreID)
-        m_layerInfo.imageBackingStoreID = m_CoordinatedGraphicsLayerClient->adoptImageBackingStore(m_image.get());
-}
-
 void CoordinatedGraphicsLayer::flushCompositingStateForThisLayerOnly()
 {
-    // The remote image might have been released by purgeBackingStores.
-    ensureImageBackingStore();
+    syncImageBacking();
     syncLayerState();
     syncAnimations();
     computeTransformedVisibleRect();
@@ -548,6 +562,16 @@ void CoordinatedGraphicsLayer::flushCompositingStateForThisLayerOnly()
     syncCanvas();
 }
 
+void CoordinatedGraphicsLayer::releaseImageBackingIfNeeded()
+{
+    if (!m_layerInfo.imageBackingStoreID)
+        return;
+
+    ASSERT(m_CoordinatedGraphicsLayerClient);
+    m_CoordinatedGraphicsLayerClient->releaseImageBackingStore(m_layerInfo.imageBackingStoreID);
+    m_layerInfo.imageBackingStoreID = 0;
+}
+
 void CoordinatedGraphicsLayer::tiledBackingStorePaintBegin()
 {
 }
@@ -698,10 +722,7 @@ void CoordinatedGraphicsLayer::purgeBackingStores()
     m_mainBackingStore.clear();
     m_previousBackingStore.clear();
 
-    if (m_layerInfo.imageBackingStoreID) {
-        m_CoordinatedGraphicsLayerClient->releaseImageBackingStore(m_layerInfo.imageBackingStoreID);
-        m_layerInfo.imageBackingStoreID = 0;
-    }
+    releaseImageBackingIfNeeded();
 
     didChangeLayerState();
     didChangeChildren();
@@ -747,7 +768,7 @@ bool CoordinatedGraphicsLayer::hasPendingVisibleChanges()
             return true;
     }
 
-    if (!m_shouldSyncLayerState && !m_shouldSyncChildren && !m_shouldSyncFilters && !m_shouldSyncAnimations && !m_canvasNeedsDisplay)
+    if (!m_shouldSyncLayerState && !m_shouldSyncChildren && !m_shouldSyncFilters && !m_shouldSyncImageBacking && !m_shouldSyncAnimations && !m_canvasNeedsDisplay)
         return false;
 
     return selfOrAncestorHaveNonAffineTransforms() || !tiledBackingStoreVisibleRect().isEmpty();
index 04668aa..a46f277 100644 (file)
@@ -169,6 +169,7 @@ private:
 #if ENABLE(CSS_FILTERS)
     void didChangeFilters();
 #endif
+    void didChangeImageBacking();
 
     void syncLayerState();
     void syncAnimations();
@@ -176,12 +177,14 @@ private:
 #if ENABLE(CSS_FILTERS)
     void syncFilters();
 #endif
+    void syncImageBacking();
     void syncCanvas();
     void ensureImageBackingStore();
     void computeTransformedVisibleRect();
     void updateContentBuffers();
 
     void createBackingStore();
+    void releaseImageBackingIfNeeded();
 
     bool selfOrAncestorHaveNonAffineTransforms();
     bool shouldUseTiledBackingStore();
@@ -194,7 +197,6 @@ private:
 
     WebKit::WebLayerID m_id;
     WebKit::WebLayerInfo m_layerInfo;
-    RefPtr<Image> m_image;
     GraphicsLayer* m_maskTarget;
     FloatRect m_needsDisplayRect;
     GraphicsLayerTransform m_layerTransform;
@@ -203,6 +205,7 @@ private:
     bool m_shouldSyncLayerState: 1;
     bool m_shouldSyncChildren: 1;
     bool m_shouldSyncFilters: 1;
+    bool m_shouldSyncImageBacking: 1;
     bool m_shouldSyncAnimations: 1;
     bool m_fixedToViewport : 1;
     bool m_canvasNeedsDisplay : 1;
@@ -214,6 +217,10 @@ private:
     OwnPtr<TiledBackingStore> m_mainBackingStore;
     OwnPtr<TiledBackingStore> m_previousBackingStore;
     float m_contentsScale;
+
+    RefPtr<Image> m_compositedImage;
+    NativeImagePtr m_compositedNativeImagePtr;
+
     PlatformLayer* m_canvasPlatformLayer;
     Timer<CoordinatedGraphicsLayer> m_animationStartedTimer;
     GraphicsLayerAnimations m_animations;