Passing alpha to DeferredImageDecoder once decoding completes

https://bugs.webkit.org/show_bug.cgi?id=108892

Patch by Min Qin <qinmin@chromium.org> on 2013-02-14
Reviewed by Stephen White.

Source/WebCore:

We should pass hasAlpha value back to the DeferredImageDecoder once decoding is completed
Added unit tests in ImageFrameGeneratorTest.

* platform/graphics/chromium/DeferredImageDecoder.cpp:
(WebCore::DeferredImageDecoder::frameHasAlphaAtIndex):
* platform/graphics/chromium/ImageFrameGenerator.cpp:
(WebCore::ImageFrameGenerator::tryToScale):
(WebCore::ImageFrameGenerator::decode):
* platform/graphics/chromium/LazyDecodingPixelRef.cpp:
(WebCore::LazyDecodingPixelRef::LazyDecodingPixelRef):
(WebCore::LazyDecodingPixelRef::onUnlockPixels):
* platform/graphics/chromium/LazyDecodingPixelRef.h:
(WebCore::LazyDecodingPixelRef::hasAlpha):
(LazyDecodingPixelRef):
* platform/graphics/chromium/ScaledImageFragment.cpp:
(WebCore::ScaledImageFragment::ScaledImageFragment):
* platform/graphics/chromium/ScaledImageFragment.h:
(WebCore::ScaledImageFragment::create):
(ScaledImageFragment):
(WebCore::ScaledImageFragment::hasAlpha):

Source/WebKit/chromium:

Add test to check that alpha value is passed from the decoder to ImageFrameGenerator.

* tests/ImageFrameGeneratorTest.cpp:
(WebCore::MockImageDecoderFactory::create):
(WebCore::TEST_F):
* tests/MockImageDecoder.h:
(WebCore::MockImageDecoder::MockImageDecoder):
(WebCore::MockImageDecoder::setFrameHasAlpha):
(MockImageDecoder):
(WebCore::MockImageDecoder::frameHasAlphaAtIndex):

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

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index e9db4851b8a108b1585ccf2438addef3b0644edc..3e8bcbf926ef5365452b56f773105b55a7e10809 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,31 @@
+2013-02-14  Min Qin  <qinmin@chromium.org>
+
+        Passing alpha to DeferredImageDecoder once decoding completes
+        https://bugs.webkit.org/show_bug.cgi?id=108892
+
+        Reviewed by Stephen White.
+
+        We should pass hasAlpha value back to the DeferredImageDecoder once decoding is completed
+        Added unit tests in ImageFrameGeneratorTest.
+
+        * platform/graphics/chromium/DeferredImageDecoder.cpp:
+        (WebCore::DeferredImageDecoder::frameHasAlphaAtIndex):
+        * platform/graphics/chromium/ImageFrameGenerator.cpp:
+        (WebCore::ImageFrameGenerator::tryToScale):
+        (WebCore::ImageFrameGenerator::decode):
+        * platform/graphics/chromium/LazyDecodingPixelRef.cpp:
+        (WebCore::LazyDecodingPixelRef::LazyDecodingPixelRef):
+        (WebCore::LazyDecodingPixelRef::onUnlockPixels):
+        * platform/graphics/chromium/LazyDecodingPixelRef.h:
+        (WebCore::LazyDecodingPixelRef::hasAlpha):
+        (LazyDecodingPixelRef):
+        * platform/graphics/chromium/ScaledImageFragment.cpp:
+        (WebCore::ScaledImageFragment::ScaledImageFragment):
+        * platform/graphics/chromium/ScaledImageFragment.h:
+        (WebCore::ScaledImageFragment::create):
+        (ScaledImageFragment):
+        (WebCore::ScaledImageFragment::hasAlpha):
+
 2013-02-14  David Grogan  <dgrogan@chromium.org>
 
         IndexedDB: Add a few more histogram calls

diff --git a/Source/WebCore/platform/graphics/chromium/DeferredImageDecoder.cpp b/Source/WebCore/platform/graphics/chromium/DeferredImageDecoder.cpp
index 050f439e596cb9553ba1ed715d89f7b9383b92e9..bf03f991b62988e7cf8516733de40a566a212a22 100644 (file)
--- a/Source/WebCore/platform/graphics/chromium/DeferredImageDecoder.cpp
+++ b/Source/WebCore/platform/graphics/chromium/DeferredImageDecoder.cpp
@@ -181,10 +181,7 @@ void DeferredImageDecoder::clearFrameBufferCache(size_t clearBeforeFrame)
 
 bool DeferredImageDecoder::frameHasAlphaAtIndex(size_t index) const
 {
-    // FIXME: Synchronize this state with ImageDecodingStore when image is
-    // actually decoded. Return true here is correct in terms of rendering but
-    // may not go through some optimized rendering code path.
-    return m_actualDecoder ? m_actualDecoder->frameHasAlphaAtIndex(index) : true;
+    return m_actualDecoder ? m_actualDecoder->frameHasAlphaAtIndex(index) : m_frameGenerator->hasAlpha();
 }
 
 unsigned DeferredImageDecoder::frameBytesAtIndex(size_t index) const

diff --git a/Source/WebCore/platform/graphics/chromium/ImageFrameGenerator.cpp b/Source/WebCore/platform/graphics/chromium/ImageFrameGenerator.cpp
index 93f95ef441733ae3bf9da0ae92886149f4abd40a..d34f546c1f794007ba23f114302ee3eb6d13913b 100644 (file)
--- a/Source/WebCore/platform/graphics/chromium/ImageFrameGenerator.cpp
+++ b/Source/WebCore/platform/graphics/chromium/ImageFrameGenerator.cpp
@@ -48,6 +48,7 @@ skia::ImageOperations::ResizeMethod resizeMethod()
 ImageFrameGenerator::ImageFrameGenerator(const SkISize& fullSize, PassRefPtr<SharedBuffer> data, bool allDataReceived)
     : m_fullSize(fullSize)
     , m_decodeFailedAndEmpty(false)
+    , m_hasAlpha(true)
 {
     setData(data.get(), allDataReceived);
 }
@@ -211,9 +212,19 @@ PassOwnPtr<ScaledImageFragment> ImageFrameGenerator::decode(ImageDecoder** decod
 
     bool isComplete = frame->status() == ImageFrame::FrameComplete;
     SkBitmap fullSizeBitmap = frame->getSkBitmap();
+    {
+        MutexLocker lock(m_alphaMutex);
+        m_hasAlpha = !fullSizeBitmap.isOpaque();
+    }
     ASSERT(fullSizeBitmap.width() == m_fullSize.width() && fullSizeBitmap.height() == m_fullSize.height());
 
     return ScaledImageFragment::create(m_fullSize, fullSizeBitmap, isComplete);
 }
 
+bool ImageFrameGenerator::hasAlpha()
+{
+    MutexLocker lock(m_alphaMutex);
+    return m_hasAlpha;
+}
+
 } // namespace WebCore

diff --git a/Source/WebCore/platform/graphics/chromium/ImageFrameGenerator.h b/Source/WebCore/platform/graphics/chromium/ImageFrameGenerator.h
index 550a07025fb0333828f8e39c102751e2e78eb6d0..343e7b51ec6d40b0c7a0a1586c68966dbb410b73 100644 (file)
--- a/Source/WebCore/platform/graphics/chromium/ImageFrameGenerator.h
+++ b/Source/WebCore/platform/graphics/chromium/ImageFrameGenerator.h
@@ -67,6 +67,8 @@ public:
 
     void setImageDecoderFactoryForTesting(PassOwnPtr<ImageDecoderFactory> factory) { m_imageDecoderFactory = factory; }
 
+    bool hasAlpha();
+
 private:
     // These methods are called while m_decodeMutex is locked.
     const ScaledImageFragment* tryToLockCompleteCache(const SkISize& scaledSize);
@@ -80,12 +82,16 @@ private:
     SkISize m_fullSize;
     ThreadSafeDataTransport m_data;
     bool m_decodeFailedAndEmpty;
+    bool m_hasAlpha;
     DiscardablePixelRefAllocator m_allocator;
 
     OwnPtr<ImageDecoderFactory> m_imageDecoderFactory;
 
     // Prevents multiple decode operations on the same data.
     Mutex m_decodeMutex;
+
+    // Protect concurrent access to m_hasAlpha.
+    Mutex m_alphaMutex;
 };
 
 } // namespace WebCore

diff --git a/Source/WebKit/chromium/ChangeLog b/Source/WebKit/chromium/ChangeLog
index 3537987a3221921ca9eee44f6536e7b837096d4d..d9f656a62224aa9a5646eaadf681edfaeb6b8e7e 100644 (file)
--- a/Source/WebKit/chromium/ChangeLog
+++ b/Source/WebKit/chromium/ChangeLog
@@ -1,3 +1,21 @@
+2013-02-14  Min Qin  <qinmin@chromium.org>
+
+        Passing alpha to DeferredImageDecoder once decoding completes
+        https://bugs.webkit.org/show_bug.cgi?id=108892
+
+        Reviewed by Stephen White.
+
+        Add test to check that alpha value is passed from the decoder to ImageFrameGenerator.
+
+        * tests/ImageFrameGeneratorTest.cpp:
+        (WebCore::MockImageDecoderFactory::create):
+        (WebCore::TEST_F):
+        * tests/MockImageDecoder.h:
+        (WebCore::MockImageDecoder::MockImageDecoder):
+        (WebCore::MockImageDecoder::setFrameHasAlpha):
+        (MockImageDecoder):
+        (WebCore::MockImageDecoder::frameHasAlphaAtIndex):
+
 2013-02-08  Andrey Kosyakov  <caseq@chromium.org>
 
         Web Inspector: expose did{Begin,Cancel}Frame() and {will,did}Composite() on WebDebToolsAgent

diff --git a/Source/WebKit/chromium/tests/ImageFrameGeneratorTest.cpp b/Source/WebKit/chromium/tests/ImageFrameGeneratorTest.cpp
index b4169a139bae3b63997c14a0ce84484b5e9b5116..113d78816ed1d912eb51368a7a8ff81882215f39 100644 (file)
--- a/Source/WebKit/chromium/tests/ImageFrameGeneratorTest.cpp
+++ b/Source/WebKit/chromium/tests/ImageFrameGeneratorTest.cpp
@@ -124,6 +124,7 @@ PassOwnPtr<ImageDecoder> MockImageDecoderFactory::create()
 {
     OwnPtr<MockImageDecoder> decoder = MockImageDecoder::create(m_test);
     decoder->setSize(fullSize().width(), fullSize().height());
+    decoder->setFrameHasAlpha(false);
     return decoder.release();
 }
 
@@ -137,6 +138,7 @@ TEST_F(ImageFrameGeneratorTest, cacheHit)
     const ScaledImageFragment* tempImage = m_generator->decodeAndScale(fullSize());
     EXPECT_EQ(fullImage, tempImage);
     EXPECT_EQ(fullSize(), tempImage->scaledSize());
+    EXPECT_TRUE(m_generator->hasAlpha());
     ImageDecodingStore::instance()->unlockCache(m_generator.get(), tempImage);
     EXPECT_EQ(0, m_frameBufferRequestCount);
 }
@@ -152,12 +154,14 @@ TEST_F(ImageFrameGeneratorTest, cacheMissWithScale)
     const ScaledImageFragment* scaledImage = m_generator->decodeAndScale(scaledSize());
     EXPECT_NE(fullImage, scaledImage);
     EXPECT_EQ(scaledSize(), scaledImage->scaledSize());
+    EXPECT_TRUE(m_generator->hasAlpha());
     ImageDecodingStore::instance()->unlockCache(m_generator.get(), scaledImage);
 
     // Cache hit.
     const ScaledImageFragment* tempImage = m_generator->decodeAndScale(scaledSize());
     EXPECT_EQ(scaledImage, tempImage);
     EXPECT_EQ(scaledSize(), tempImage->scaledSize());
+    EXPECT_TRUE(m_generator->hasAlpha());
     ImageDecodingStore::instance()->unlockCache(m_generator.get(), tempImage);
     EXPECT_EQ(0, m_frameBufferRequestCount);
 }
@@ -170,6 +174,7 @@ TEST_F(ImageFrameGeneratorTest, cacheMissWithDecodeAndScale)
     const ScaledImageFragment* scaledImage = m_generator->decodeAndScale(scaledSize());
     EXPECT_EQ(1, m_frameBufferRequestCount);
     EXPECT_EQ(scaledSize(), scaledImage->scaledSize());
+    EXPECT_FALSE(m_generator->hasAlpha());
     ImageDecodingStore::instance()->unlockCache(m_generator.get(), scaledImage);
     EXPECT_EQ(1, m_decodersDestroyed);
 
@@ -177,12 +182,14 @@ TEST_F(ImageFrameGeneratorTest, cacheMissWithDecodeAndScale)
     const ScaledImageFragment* fullImage = m_generator->decodeAndScale(fullSize());
     EXPECT_NE(scaledImage, fullImage);
     EXPECT_EQ(fullSize(), fullImage->scaledSize());
+    EXPECT_FALSE(m_generator->hasAlpha());
     ImageDecodingStore::instance()->unlockCache(m_generator.get(), fullImage);
 
     // Cache hit.
     const ScaledImageFragment* tempImage = m_generator->decodeAndScale(scaledSize());
     EXPECT_EQ(scaledImage, tempImage);
     EXPECT_EQ(scaledSize(), tempImage->scaledSize());
+    EXPECT_FALSE(m_generator->hasAlpha());
     ImageDecodingStore::instance()->unlockCache(m_generator.get(), tempImage);
     EXPECT_EQ(1, m_frameBufferRequestCount);
 }

diff --git a/Source/WebKit/chromium/tests/MockImageDecoder.h b/Source/WebKit/chromium/tests/MockImageDecoder.h
index 5c9a6237b94f85ba3fd624bbb55a7029ad89f841..ba342178ab309fb9f2dae649988df6021e2f6202 100644 (file)
--- a/Source/WebKit/chromium/tests/MockImageDecoder.h
+++ b/Source/WebKit/chromium/tests/MockImageDecoder.h
@@ -59,6 +59,8 @@ public:
         return true;
     }
 
+    virtual void setFrameHasAlpha(bool hasAlpha) { m_frameBufferCache[0].setHasAlpha(hasAlpha); }
+
     virtual String filenameExtension() const
     {
         return "mock";