Closed Bug 386651 Opened 14 years ago Closed 13 years ago

Need regression tests for PNG image decoding

Categories

(Core :: ImageLib, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: Dolske, Assigned: Dolske)

References

(Depends on 1 open bug)

Details

Attachments

(6 files, 3 obsolete files)

Bug 196295 caused a few regressions in displaying animated GIFs; this hilights the need for a set of regression tests for image decoding.

I'm not sure what the history has been in regards to such regressions, but I believe the GIF format has historically been an ugly and ill-defined format, such that future changes will always have a risk of breaking some edge case [or, worse, re-introducing some security bug].

Testing animations might be a little tricky, but the tests could probably at least check to see if the frames were rendered properly, and perhaps use Litmus tests for things that are hard to automate (frame timing?).

JPEG and PNG should probably have some tests too, although being based on standard, stable 3rd party libraries they probably have a lower risk of problems.

A good collection of images for testing can probably be obtained by mining Bugzilla for past bugs with attachments.

Bug 196295 comment #44 mentions http://www.thinkingphp.org/2006/09/10/test-driven-development-in-real-world-apps/ as something to look at. I previously added some APNG encoding tests for bug 375733 and bug 375741, which are somewhat similar.
Flags: blocking1.9?
Bug 274391 has some test files.
And I can deliver much more, if you need them.
I'll go ahead and sign up for this.

Alfred, I'd be interested in whatever extra test images you might have. Can you attach them here?
Assignee: nobody → dolske
How are you planning on testing this? There is one thing that might help. I have figured out how to use reftest, with scripts around the firefox invocation, to do exact image comparison.

I think there will be an issue here as the reftest snapshot takes place at the final load event. There has been some reason change in this, as reftest was not waiting for background images, but that fix is in the trunk.

See my blog post on planet.mozilla.org if you are interested, and I can, of course, answer any questions.
Attached patch Draft patch, v.1 (obsolete) — Splinter Review
Proof of concept, using reftest framework.

This was surprisingly easy. :-) The general idea here is to just use the existing reftest framework, and compare a test image (eg, a GIF) to a reference image (a PNG). This has the unfortunate side effect or requiring two image files for each reftest, but I don't see a good way around that (data: URLs would be unwieldy).

The first two tests compare some images directly, the last two tests use an HTML wrapper to display the image against some different backgrounds (so that transparency bugs can be caught).
Attached file Draft patch, v.1 (images.zip) (obsolete) —
Flags: blocking1.9? → blocking1.9-
Actually, I think a better way to do this would be to:

a) use small images (like 10x10 or so)
b) use reference images that are built out of a 10x10 array of pixel-sized divs

That way there's no dependency on image decoding for the reference image...
My thinking was that many bugs originate as "hey, this image doesn't display right", and reducing that down to a tiny image for a test suite can be a lot of work (especially for cases where the problem is due to quirky things in the image format, which would be hard to recreate). I suppose, though, that if HTMLifying an image result doesn't in a grotesquely large file, then there's not much advantage to using an image for the reference.

I guess I assumed, but didn't state, that we could do something like what you suggest based on a PNG test suite (eg, http://www.libpng.org/pub/png/pngsuite.html), and then assume that if PNG decoding checks out, it's safe to use that as a reference format.
I emailed Willem van Schaik yesterday, who made the images in the PNG test suite at http://www.libpng.org/pub/png/pngsuite.html / http://www.schaik.com/pngsuite/. There was no copyright/licensing mentioned, so I wanted to confirm we could use the images in our test suite.

-----
Hi Justin,

You're right, there is no mentioning of copyright, copyleft :-) or any other
licensing rules, because PngSuite is free stuff that can be used as you
like.

So, please go ahead!!

See you,
Willem
-----

Patch coming soon!
Depends on: 399668
Attached patch Patch, work in progress v.1 (obsolete) — Splinter Review
This is the beginning of using pngsuite as a set of reftests... It will be a little tedious to convert all the images to HTML format (ala comment #6), so I just did one to begin with. If this looks ok, I can do the whole set for a "real" review.

Note that because of bug 399668 the HTML test I generated here doesn't actually, uhm, work. The color values output by the img2html.html tool are slightly wrong.
Attachment #272449 - Attachment is obsolete: true
Attachment #272450 - Attachment is obsolete: true
Attachment #284713 - Flags: review?(pavlov)
Comment on attachment 284713 [details] [diff] [review]
Patch, work in progress v.1

rubberstamp=me
Attachment #284713 - Flags: review?(pavlov) → review+
Yet more fodder: http://pmt.sourceforge.net/iccp/index.html

Also, I checked with Stuart that I can start checking these in. Bug 399668 doesn't block this since it only manifests when color management is enabled. (Having bug 389366 fixed also makes life easier.)
No longer depends on: 399668
This is the initial framework and a few images. I'll add more later assuming nothing blows up.
Attachment #284713 - Attachment is obsolete: true
Attachment #288596 - Attachment description: Patch for checkin, Part 1 (framework + pngsuite-zlib/) → [checked in] Part 1 (framework + pngsuite-zlib/)
So, after checking in Part 1, all the pngsuite reftests failed on the Windows boxes (qm-win2k3-01, qm-winxp01). I checked MD5s of the logged images, and both boxes failed in an identical manner on the first test, which I'll examine here... The reftests pass on my OS X and Solaris boxes. Manually the test html/png under Vista seems ok to me.

Attached is a composite of the results for z00n2c08.png. The test image is just a boring RGB gradient, no transparency, alpha, gamma, ICC, etc (verified with pngcheck). The docs say these particular test images were made with different zlib compression levels specified, which I wouldn't expect to be relevant to the displayed result.

The image on the left is from the reftest log for the "reference" image (the HTML table). The middle image is from the "test" image (the PNG loaded in the reftest browser window). The rightmost image is the PNG as loaded in GIMP when I created this composite.

Note that the left and right image appear identical (although I didn't test this exhaustively). The middle image has a subtle but noticeable horizontal interruption in the center of the image. You'll need to zoom in. :-) Flipping between them, it looks like most of the image differs subtly but more noticeably towards the middle. Some quick GIMP filtering seems to show that most of the image has a slightly-off green channel, with a handful of pixels having red/blue differences.

I measured the blueish pixel marked with a black pixel between images... The pixels in the left and right images are rgb(0,123,132). The value specified in the HTML is rgb(0,123,132). The value in the middle image is rgb(0,121,132) -- WRONG! [All pixels were fully opaque]

This implies that PNG is being painted incorrectly in the test environment [or perhaps that drawWindow() is mangling it?].
Depends on: 405384
Checking in a few more bits of pngsuite, to see if it might shed some light on why the initial tests from the first part were failing. [HTML references not included in this attachment to save space.]
Depends on: 405398
Depends on: 408622
Depends on: 408625
Changing this bug to be specific to PNG decoding. I've spun off bugs for the other formats...

JPEG - bug 411626
GIF  - bug 411627
APNG - bug 411628
Summary: Need regression tests for image decoding → Need regression tests for PNG image decoding
All of PngSuite is now checked in, resolving FIXED.
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Flags: in-testsuite+
Depends on: 411636
Depends on: 820319
You need to log in before you can comment on or make changes to this bug.