449655 - nsThebesImage::GetIsImageComplete returns true with interlaced PNG images while only first decompression pass is completed

Status: NEW

(Core :: Graphics: ImageLib, defect)

Description

[Francis Robichaud]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.0.1) Gecko/2008072820 Firefox/3.0.1
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.0.1) Gecko/2008073015 Minefield/3.0.1

While developing a patch for #395260, I had to rely on the GetIsImageComplete() method to optimize the drawing of image surfaces. I encountered the problem that GetIsImageComplete() would return true for an interlaced image (see url) while it was still not completely decompressed. This image is a PNG image using Adam7 compression as an interlacing algorithm.

The GetIsImageComplete() verifies if the size of the decoded data matches the size of the actual nsThebesImage. In the case of an interlaced image, the first pass will set the mImageComplete to true. This variable should not be set to true and there should be a way to know this image has only been partially decoded since it uses interlacing.

Reproducible: Always

Steps to Reproduce:
1. Patch a Firefox build with #395260's latest attachment
2 [review]. Browse to the url specified above
Actual Results:  
If the image renders perfectly, you might be lucky to have a very fast connection with the web server and only the full decompressed image is shown.(Try hard-refreshing the page)
Else, rendering of the image will stop before the decoding is complete and the image will be blurry.

Expected Results:  
Image should render more than once, probably two to three times depending of how fast your connection is with the web server. The image quality should gets better and better after each paint until it shows perfectly.

Comment 1

[Francis Robichaud]

Attachment: nsIImage.h additionnal flag for interlaced image

Only affected interlaced PNG images.

Comment 2

[Francis Robichaud]

Attachment: nsIImage.h additionnal flag for interlaced image - previous patch missing spaces

Comment 3

[Joe Drew (not getting mail)]

Francis, this patch was bitrotted by my changes to bug 753. Can you update it? With the changes to current review requirements, we can get this in without a superreview (which Stuart is unlikely to do as he's busy).

Comment 4

[Francis Robichaud]

To reproduce this issue, we first need to update the patch for 395260, which may be harder to adapt.

You haven't worked on 395260 since last year, right ?

Comment 5

[Joe Drew (not getting mail)]

Correct - it has not been a priority of mine.