Log Message
Partially loaded JPEGs should have alpha channel https://bugs.webkit.org/show_bug.cgi?id=78239
Patch by Sami Kyostila <skyos...@chromium.org> on 2012-03-05 Reviewed by Kenneth Russell. Source/WebCore: While a JPEG image is loading, the area outside the decoded region should be fully transparent. Since currently all JPEG frames are marked as opaque, a renderer respecting this flag will draw the partially loaded image with garbage outside the valid image region. Hence, a partially loaded JPEG image should be marked as having an alpha channel while decoding is in progress. For performance reasons we mark the image opaque after decoding has finished. Graphics corruption caused by this bug was recently observed on Chromium (http://code.google.com/p/chromium/issues/detail?id=113171). A recent Skia change (r3036) changed SkBitmap::extractSubset() to produce a bitmap with the same opaqueness flag as the parent. This meant that the renderer was now seeing an opaque image from the JPEG decoder, and drawing it appropriately resulted in garbage outside the decoded region. Test: http/tests/incremental/partial-jpeg.html * platform/image-decoders/jpeg/JPEGImageDecoder.cpp: (WebCore::JPEGImageDecoder::outputScanlines): (WebCore::JPEGImageDecoder::jpegComplete): LayoutTests: While a JPEG image is being loaded, the parts which have not been decoded yet should show whatever is behind the image. This tests verifies this by displaying a JPEG which never fully completes loading. This is achieved by serving the JPEG from a PHP script that strips the end of image marker (ff d9) from the data. The test image is 32x32 pixels, so compresses to 4x4 JPEG MCU blocks. The expected result is that the final row of MCU blocks (32x4 pixels) never finishes loading due to the missing end of image marker and the indicator div is shown through this area. * http/tests/incremental/partial-jpeg-expected.png: Added. * http/tests/incremental/partial-jpeg-expected.txt: Added. * http/tests/incremental/partial-jpeg.html: Added. * http/tests/incremental/resources/checkerboard.jpg: Added. * http/tests/incremental/resources/partial-jpeg.php: Added.
Modified Paths
- trunk/LayoutTests/ChangeLog
- trunk/Source/WebCore/ChangeLog
- trunk/Source/WebCore/platform/image-decoders/jpeg/JPEGImageDecoder.cpp
Added Paths
- trunk/LayoutTests/http/tests/incremental/partial-jpeg-expected.png
- trunk/LayoutTests/http/tests/incremental/partial-jpeg-expected.txt
- trunk/LayoutTests/http/tests/incremental/partial-jpeg.html
- trunk/LayoutTests/http/tests/incremental/resources/checkerboard.jpg
- trunk/LayoutTests/http/tests/incremental/resources/partial-jpeg.php
Diff
Modified: trunk/LayoutTests/ChangeLog (109778 => 109779)
--- trunk/LayoutTests/ChangeLog 2012-03-05 19:50:05 UTC (rev 109778)
+++ trunk/LayoutTests/ChangeLog 2012-03-05 19:54:58 UTC (rev 109779)
@@ -1,3 +1,27 @@
+2012-03-05 Sami Kyostila <skyos...@chromium.org>
+
+ Partially loaded JPEGs should have alpha channel
+ https://bugs.webkit.org/show_bug.cgi?id=78239
+
+ Reviewed by Kenneth Russell.
+
+ While a JPEG image is being loaded, the parts which have not been
+ decoded yet should show whatever is behind the image. This tests
+ verifies this by displaying a JPEG which never fully completes
+ loading. This is achieved by serving the JPEG from a PHP script that
+ strips the end of image marker (ff d9) from the data.
+
+ The test image is 32x32 pixels, so compresses to 4x4 JPEG MCU blocks.
+ The expected result is that the final row of MCU blocks (32x4 pixels)
+ never finishes loading due to the missing end of image marker and the
+ indicator div is shown through this area.
+
+ * http/tests/incremental/partial-jpeg-expected.png: Added.
+ * http/tests/incremental/partial-jpeg-expected.txt: Added.
+ * http/tests/incremental/partial-jpeg.html: Added.
+ * http/tests/incremental/resources/checkerboard.jpg: Added.
+ * http/tests/incremental/resources/partial-jpeg.php: Added.
+
2012-03-05 Adam Klein <ad...@chromium.org>
Unreviewed test expectations update: widen slowness of jquery/offset.html.
Added: trunk/LayoutTests/http/tests/incremental/partial-jpeg-expected.png (0 => 109779)
--- trunk/LayoutTests/http/tests/incremental/partial-jpeg-expected.png (rev 0)
+++ trunk/LayoutTests/http/tests/incremental/partial-jpeg-expected.png 2012-03-05 19:54:58 UTC (rev 109779)
@@ -0,0 +1,6 @@
+\x89PNG
+
+
+IHDR X ' )tEXtchecksum d44cebe6673df4032a998cb607740338\\x95\x8F\xED IDATx\x9C\xED\xDC\xDDi\xC30@ѨdG\xD1m\xDAm\xBCQ\xC6Q\xB0()\x87\x94s\xF0\x93%\xFC\xF3v\xF94\xE6\x9C7 :\xAF\xFE \x80\xFFF` \xC4 @L` \xC4 @L` \xC4 @\xEC\xBE[8\x8E\xE3\xF4\xFE\xEB\xF4\xFEZ\xE3\xA9\xFDs~\xFE\xF6m o\xC9 &\xB0 b &\xB0 b &\xB0 b \xB6
+\xAC1\xD6\xE9\xB5\xD68\xBD\x9E\xDD\xE5O \\xC9 &\xB0 b &\xB0 b &\xB0 b \xB6
+\xAC꼫\xDD\xFE+ \xE0J&X 1\x81 X 1\x81 X 1\x81 X \xB1\xFBna|\x8F\xF3\x85\xAF\xDD\xFE̓6\xFBo\x8F\xB9{5 \xC0[3\xC1 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88m\xCF\xC1\x9AΩ \xF8, \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\x98\xC0 \x88 , \x80\xD8 \x8Eqsw[\xD1 IEND\xAEB`\x82
\ No newline at end of file
Added: trunk/LayoutTests/http/tests/incremental/partial-jpeg-expected.txt (0 => 109779)
--- trunk/LayoutTests/http/tests/incremental/partial-jpeg-expected.txt (rev 0)
+++ trunk/LayoutTests/http/tests/incremental/partial-jpeg-expected.txt 2012-03-05 19:54:58 UTC (rev 109779)
@@ -0,0 +1,9 @@
+layer at (0,0) size 800x600
+ RenderView at (0,0) size 800x600
+layer at (0,0) size 800x8
+ RenderBlock {HTML} at (0,0) size 800x8
+ RenderBody {BODY} at (8,8) size 784x0
+layer at (8,8) size 32x32
+ RenderBlock (positioned) {DIV} at (8,8) size 32x32 [bgcolor=#008000]
+layer at (8,8) size 32x32
+ RenderImage {IMG} at (8,8) size 32x32
Added: trunk/LayoutTests/http/tests/incremental/partial-jpeg.html (0 => 109779)
--- trunk/LayoutTests/http/tests/incremental/partial-jpeg.html (rev 0)
+++ trunk/LayoutTests/http/tests/incremental/partial-jpeg.html 2012-03-05 19:54:58 UTC (rev 109779)
@@ -0,0 +1,39 @@
+<!DOCTYPE html>
+<html>
+<head>
+ <script type="text/_javascript_">
+ function forceDisplay() {
+ window.layoutTestController.display();
+ setTimeout(stopLoading, 0);
+ }
+
+ function stopLoading() {
+ window.stop();
+ window.layoutTestController.notifyDone();
+ }
+
+ if (window.layoutTestController) {
+ window.layoutTestController.waitUntilDone();
+ setTimeout(forceDisplay, 0);
+ }
+ </script>
+
+ <style type="text/css">
+ .box {
+ width: 32px;
+ height: 32px;
+ position: absolute;
+ }
+
+ .indicator {
+ background: green;
+ }
+ </style>
+</head>
+
+<body>
+ <!-- The indicator box should be visible through unloaded parts of the image. -->
+ <div class="indicator box"></div>
+ <img class="box" src=""
+</body>
+</html>
Added: trunk/LayoutTests/http/tests/incremental/resources/checkerboard.jpg (0 => 109779)
--- trunk/LayoutTests/http/tests/incremental/resources/checkerboard.jpg (rev 0)
+++ trunk/LayoutTests/http/tests/incremental/resources/checkerboard.jpg 2012-03-05 19:54:58 UTC (rev 109779)
@@ -0,0 +1,3 @@
+\xFF\xD8\xFF\xE0 JFIF H H \xFF\xDB C \xFF\xDB C\xFF\xC0 \xFF\xC4
+\xFF\xC4 \xFF\xC4
+ \xFF\xC4 \xFF\xDA ? \xDF\xC30;\xD7\xF0+\x9CA \xE5@;\xD7\xF0+\xA6c\x88 \xA8z\xFEs\x88 \xA8L\xC0\xEF_\xC0\xAEq\x95 \xEF_\xC0\xAE\x99\x8E \x80r\xA0\xEB\xF8\xCE \x80r\xA1\xFF\xD9
\ No newline at end of file
Added: trunk/LayoutTests/http/tests/incremental/resources/partial-jpeg.php (0 => 109779)
--- trunk/LayoutTests/http/tests/incremental/resources/partial-jpeg.php (rev 0)
+++ trunk/LayoutTests/http/tests/incremental/resources/partial-jpeg.php 2012-03-05 19:54:58 UTC (rev 109779)
@@ -0,0 +1,10 @@
+<?
+$file = "checkerboard.jpg";
+$size = filesize($file);
+$end_marker_size = 2; // Strip the end marker (ff d9)
+$contents = file_get_contents($file, false, NULL, 0, $size - $end_marker_size);
+header("Content-Type: image/jpeg");
+header("Content-Length: " . $size);
+flush();
+echo $contents;
+?>
Modified: trunk/Source/WebCore/ChangeLog (109778 => 109779)
--- trunk/Source/WebCore/ChangeLog 2012-03-05 19:50:05 UTC (rev 109778)
+++ trunk/Source/WebCore/ChangeLog 2012-03-05 19:54:58 UTC (rev 109779)
@@ -1,3 +1,32 @@
+2012-03-05 Sami Kyostila <skyos...@chromium.org>
+
+ Partially loaded JPEGs should have alpha channel
+ https://bugs.webkit.org/show_bug.cgi?id=78239
+
+ Reviewed by Kenneth Russell.
+
+ While a JPEG image is loading, the area outside the decoded region
+ should be fully transparent. Since currently all JPEG frames are marked
+ as opaque, a renderer respecting this flag will draw the partially
+ loaded image with garbage outside the valid image region.
+
+ Hence, a partially loaded JPEG image should be marked as having an alpha
+ channel while decoding is in progress. For performance reasons we mark
+ the image opaque after decoding has finished.
+
+ Graphics corruption caused by this bug was recently observed on
+ Chromium (http://code.google.com/p/chromium/issues/detail?id=113171). A
+ recent Skia change (r3036) changed SkBitmap::extractSubset() to produce
+ a bitmap with the same opaqueness flag as the parent. This meant that
+ the renderer was now seeing an opaque image from the JPEG decoder, and
+ drawing it appropriately resulted in garbage outside the decoded region.
+
+ Test: http/tests/incremental/partial-jpeg.html
+
+ * platform/image-decoders/jpeg/JPEGImageDecoder.cpp:
+ (WebCore::JPEGImageDecoder::outputScanlines):
+ (WebCore::JPEGImageDecoder::jpegComplete):
+
2012-03-05 James Robinson <jam...@chromium.org>
[chromium] Initialize CCOverdrawCounts struct to zero
Modified: trunk/Source/WebCore/platform/image-decoders/jpeg/JPEGImageDecoder.cpp (109778 => 109779)
--- trunk/Source/WebCore/platform/image-decoders/jpeg/JPEGImageDecoder.cpp 2012-03-05 19:50:05 UTC (rev 109778)
+++ trunk/Source/WebCore/platform/image-decoders/jpeg/JPEGImageDecoder.cpp 2012-03-05 19:54:58 UTC (rev 109779)
@@ -501,7 +501,9 @@
if (!buffer.setSize(scaledSize().width(), scaledSize().height()))
return setFailed();
buffer.setStatus(ImageFrame::FramePartial);
- buffer.setHasAlpha(false);
+ // The buffer is transparent outside the decoded area while the image is
+ // loading. The completed image will be marked fully opaque in jpegComplete().
+ buffer.setHasAlpha(true);
buffer.setColorProfile(m_colorProfile);
// For JPEGs, the frame always fills the entire image.
@@ -569,7 +571,9 @@
// Hand back an appropriately sized buffer, even if the image ended up being
// empty.
- m_frameBufferCache[0].setStatus(ImageFrame::FrameComplete);
+ ImageFrame& buffer = m_frameBufferCache[0];
+ buffer.setStatus(ImageFrame::FrameComplete);
+ buffer.setHasAlpha(false);
}
void JPEGImageDecoder::decode(bool onlySize)
_______________________________________________ webkit-changes mailing list webkit-changes@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes