Source/WebCore: Remove image decoding in some BitmapImage metadata functions
authorhclam@chromium.org <hclam@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 9 Aug 2012 06:23:42 +0000 (06:23 +0000)
committerhclam@chromium.org <hclam@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 9 Aug 2012 06:23:42 +0000 (06:23 +0000)
https://bugs.webkit.org/show_bug.cgi?id=93171

Reviewed by Simon Fraser.

These two metadata functions were decoding an entire frame:
- frameOrientationAtIndex
- frameHasAlphaAtIndex

This change removes image decoding from these two methods. This is for
preparation of having asynchronous image decoding, intending to reduce
code location that trigger image decoding.

frameOrientationAtIndex() doesn't require decoding a frame. This method
is only implemented in CG port in ImageSourceCG.cpp which doesn't do
image decoding.

frameHasAlphaAtIndex() is used to optimize certain drawing operations
and accelerated compositing. This change uses a heuristic for non-CG
port to determine if an image has alpha. If an image is not yet
decoded the function answers having alpha. Only if a frame is decoded
and cached that the alpha state of the frame is returned. This is an
admissible heuristic that postpone answering the question until a frame
is decoded.

Tested this change with a fully loaded image and partially loaded image
with background color.

Test: http/tests/images/jpg-img-partial-load.html

* platform/graphics/BitmapImage.cpp:
(WebCore::BitmapImage::frameHasAlphaAtIndex):
(WebCore::BitmapImage::frameOrientationAtIndex):
* platform/graphics/ImageSource.cpp:
(WebCore::ImageSource::frameHasAlphaAtIndex):
* platform/image-decoders/ImageDecoder.cpp:
(WebCore::ImageDecoder::frameHasAlphaAtIndex):
(WebCore):
* platform/image-decoders/ImageDecoder.h:
(ImageDecoder):

LayoutTests: Test rendering JPEG <img> with CSS background
https://bugs.webkit.org/show_bug.cgi?id=93171

Reviewed by Simon Fraser.

Tests defer answering the alpha state for BitmapImage not yet
decoded doesn't affect rendering results.

Test: http/tests/images/jpg-img-partial-load.html

Tests the rendering results of a partially loaded JPEG image.
The image should be green in the top region and blue in the
lower region. The blue is the background color revealed by
full alpha of the undecoded region.

* fast/images/resources/green-256x256.jpg: Added.
* http/tests/images/jpg-img-partial-load-expected.png: Added.
* http/tests/images/jpg-img-partial-load-expected.txt: Added.
* http/tests/images/jpg-img-partial-load.html: Added.
* http/tests/resources/load-and-stall.php: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/images/resources/green-256x256.jpg [new file with mode: 0644]
LayoutTests/http/tests/images/jpg-img-partial-load-expected.png [new file with mode: 0644]
LayoutTests/http/tests/images/jpg-img-partial-load-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/images/jpg-img-partial-load.html [new file with mode: 0644]
LayoutTests/http/tests/resources/load-and-stall.php [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/BitmapImage.cpp
Source/WebCore/platform/graphics/ImageSource.cpp
Source/WebCore/platform/image-decoders/ImageDecoder.cpp
Source/WebCore/platform/image-decoders/ImageDecoder.h

index 6c77067..27f5d88 100644 (file)
@@ -1,3 +1,26 @@
+2012-08-08  Alpha Lam  <hclam@chromium.org>
+
+        Test rendering JPEG <img> with CSS background
+        https://bugs.webkit.org/show_bug.cgi?id=93171
+
+        Reviewed by Simon Fraser.
+
+        Tests defer answering the alpha state for BitmapImage not yet
+        decoded doesn't affect rendering results.
+
+        Test: http/tests/images/jpg-img-partial-load.html
+
+        Tests the rendering results of a partially loaded JPEG image.
+        The image should be green in the top region and blue in the
+        lower region. The blue is the background color revealed by
+        full alpha of the undecoded region.
+
+        * fast/images/resources/green-256x256.jpg: Added.
+        * http/tests/images/jpg-img-partial-load-expected.png: Added.
+        * http/tests/images/jpg-img-partial-load-expected.txt: Added.
+        * http/tests/images/jpg-img-partial-load.html: Added.
+        * http/tests/resources/load-and-stall.php: Added.
+
 2012-08-08  Xingnan Wang  <xingnan.wang@intel.com>
 
         Skip CoreAnimation plugin model tests on android
diff --git a/LayoutTests/fast/images/resources/green-256x256.jpg b/LayoutTests/fast/images/resources/green-256x256.jpg
new file mode 100644 (file)
index 0000000..497ed77
Binary files /dev/null and b/LayoutTests/fast/images/resources/green-256x256.jpg differ
diff --git a/LayoutTests/http/tests/images/jpg-img-partial-load-expected.png b/LayoutTests/http/tests/images/jpg-img-partial-load-expected.png
new file mode 100644 (file)
index 0000000..8dd1d20
Binary files /dev/null and b/LayoutTests/http/tests/images/jpg-img-partial-load-expected.png differ
diff --git a/LayoutTests/http/tests/images/jpg-img-partial-load-expected.txt b/LayoutTests/http/tests/images/jpg-img-partial-load-expected.txt
new file mode 100644 (file)
index 0000000..8b13789
--- /dev/null
@@ -0,0 +1 @@
+
diff --git a/LayoutTests/http/tests/images/jpg-img-partial-load.html b/LayoutTests/http/tests/images/jpg-img-partial-load.html
new file mode 100644 (file)
index 0000000..239c197
--- /dev/null
@@ -0,0 +1,30 @@
+<html>
+<head>
+<script>
+if (window.testRunner) {
+    testRunner.dumpAsText(true);
+    testRunner.waitUntilDone();
+}
+
+function checkWidth()
+{
+    var image = document.getElementById("image");
+    if (image.width == 256 && window.testRunner)
+        testRunner.notifyDone();
+
+    setTimeout(checkWidth, 1);
+}
+
+function runTest()
+{
+    var image = document.getElementById("image");
+    image.src = "http://127.0.0.1:8000/resources/load-and-stall.php?name=../../../fast/images/resources/green-256x256.jpg&mimeType=image%2Fjpeg&stallAt=1025&stallFor=60";
+
+    setTimeout(checkWidth, 1);
+}
+</script>
+</head>
+<body onload="setTimeout('runTest()', 1);" style="background-color: yellow;">
+<img id="image" style="background-color: blue;">
+</body>
+</html>
diff --git a/LayoutTests/http/tests/resources/load-and-stall.php b/LayoutTests/http/tests/resources/load-and-stall.php
new file mode 100644 (file)
index 0000000..4530481
--- /dev/null
@@ -0,0 +1,37 @@
+<?php
+$name = $_GET['name'];
+$stallAt = $_GET['stallAt'];
+$stallFor = $_GET['stallFor'];
+$mimeType = $_GET['mimeType'];
+
+$file = fopen($name, "rb");
+if (!$file)
+    die("Cannot open file.");
+
+header("Content-Type: " . $mimeType);
+header("Content-Length: " . filesize($name));
+
+if (isset($stallAt) && isset($stallFor)) {
+    $stallAt = (int)$stallAt;
+    $stallFor = (int)$stallFor;
+    if ($stallAt > filesize($name))
+        die("Incorrect value for stallAt.");
+
+    while ($written < $stallAt) {
+        $write = 1024;
+        if ($write > $stallAt - $written)
+            $write = $stallAt - $written;
+
+        echo(fread($file, $write));
+        $written += $write;
+        flush();
+        ob_flush();
+    }
+    sleep($stallFor);
+    echo(fread($file, filesize($name) - $stallAt));
+} else {
+    echo(fread($file, filesize($name)));
+}
+
+fclose($file);
+?>
index 865429a..9e591da 100644 (file)
@@ -1,3 +1,46 @@
+2012-08-08  Alpha Lam  <hclam@chromium.org>
+
+        Remove image decoding in some BitmapImage metadata functions
+        https://bugs.webkit.org/show_bug.cgi?id=93171
+
+        Reviewed by Simon Fraser.
+
+        These two metadata functions were decoding an entire frame:
+        - frameOrientationAtIndex
+        - frameHasAlphaAtIndex
+
+        This change removes image decoding from these two methods. This is for
+        preparation of having asynchronous image decoding, intending to reduce
+        code location that trigger image decoding.
+
+        frameOrientationAtIndex() doesn't require decoding a frame. This method
+        is only implemented in CG port in ImageSourceCG.cpp which doesn't do
+        image decoding.
+
+        frameHasAlphaAtIndex() is used to optimize certain drawing operations
+        and accelerated compositing. This change uses a heuristic for non-CG
+        port to determine if an image has alpha. If an image is not yet
+        decoded the function answers having alpha. Only if a frame is decoded
+        and cached that the alpha state of the frame is returned. This is an
+        admissible heuristic that postpone answering the question until a frame
+        is decoded.
+
+        Tested this change with a fully loaded image and partially loaded image
+        with background color.
+
+        Test: http/tests/images/jpg-img-partial-load.html
+
+        * platform/graphics/BitmapImage.cpp:
+        (WebCore::BitmapImage::frameHasAlphaAtIndex):
+        (WebCore::BitmapImage::frameOrientationAtIndex):
+        * platform/graphics/ImageSource.cpp:
+        (WebCore::ImageSource::frameHasAlphaAtIndex):
+        * platform/image-decoders/ImageDecoder.cpp:
+        (WebCore::ImageDecoder::frameHasAlphaAtIndex):
+        (WebCore):
+        * platform/image-decoders/ImageDecoder.h:
+        (ImageDecoder):
+
 2012-08-08  Sheriff Bot  <webkit.review.bot@gmail.com>
 
         Unreviewed, rolling out r125146.
index 45460f8..e286a7c 100644 (file)
@@ -322,11 +322,13 @@ NativeImagePtr BitmapImage::nativeImageForCurrentFrame()
 
 bool BitmapImage::frameHasAlphaAtIndex(size_t index)
 {
-    // When a frame has not finished decoding, always mark it as having alpha.
-    // See ImageSource::framehasAlphaAtIndex for explanation of why incomplete images claim to have alpha.
-    if (!ensureFrameIsCached(index))
+    if (m_frames.size() <= index)
         return true;
-    return m_frames[index].m_hasAlpha;
+
+    if (m_frames[index].m_haveMetadata)
+        return m_frames[index].m_hasAlpha;
+
+    return m_source.frameHasAlphaAtIndex(index);
 }
 
 bool BitmapImage::currentFrameHasAlpha()
@@ -341,9 +343,13 @@ ImageOrientation BitmapImage::currentFrameOrientation()
 
 ImageOrientation BitmapImage::frameOrientationAtIndex(size_t index)
 {
-    if (!ensureFrameIsCached(index))
+    if (m_frames.size() <= index)
         return DefaultImageOrientation;
-    return m_frames[index].m_orientation;
+
+    if (m_frames[index].m_haveMetadata)
+        return m_frames[index].m_orientation;
+
+    return m_source.orientationAtIndex(index);
 }
 
 #if !ASSERT_DISABLED
index 75ed5a1..c433de4 100644 (file)
@@ -184,14 +184,9 @@ ImageOrientation ImageSource::orientationAtIndex(size_t index) const
 
 bool ImageSource::frameHasAlphaAtIndex(size_t index)
 {
-    // When a frame has not finished decoding, always mark it as having alpha.
-    // Ports that check the result of this function to determine their
-    // compositing op need this in order to not draw the undecoded portion as
-    // black.
-    // TODO: Perhaps we should ensure that each individual decoder returns true
-    // in this case.
-    return !frameIsCompleteAtIndex(index)
-        || m_decoder->frameBufferAtIndex(index)->hasAlpha();
+    if (!m_decoder)
+        return true;
+    return m_decoder->frameHasAlphaAtIndex(index);
 }
 
 bool ImageSource::frameIsCompleteAtIndex(size_t index)
index 72b5ddc..9951108 100644 (file)
@@ -277,6 +277,15 @@ template <MatchType type> int getScaledValue(const Vector<int>& scaledValues, in
 
 }
 
+bool ImageDecoder::frameHasAlphaAtIndex(size_t index) const
+{
+    if (m_frameBufferCache.size() <= index)
+        return true;
+    if (m_frameBufferCache[index].status() == ImageFrame::FrameComplete)
+        return m_frameBufferCache[index].hasAlpha();
+    return true;
+}
+
 void ImageDecoder::prepareScaleDataIfNecessary()
 {
     m_scaled = false;
index 911878e..f2aaecc 100644 (file)
@@ -276,6 +276,9 @@ namespace WebCore {
         // ImageDecoder-owned pointer.
         virtual ImageFrame* frameBufferAtIndex(size_t) = 0;
 
+        // Make the best effort guess to check if the requested frame has alpha channel.
+        virtual bool frameHasAlphaAtIndex(size_t) const;
+
         void setIgnoreGammaAndColorProfile(bool flag) { m_ignoreGammaAndColorProfile = flag; }
         bool ignoresGammaAndColorProfile() const { return m_ignoreGammaAndColorProfile; }