We need GTests for downscale-during-decode. To make these tests useful, we need to add an image comparison function to Common.h - single-color images aren't good enough to test downscaling. I'll add regressions that we weren't able to add tests for yet as "See Also" bugs on this bug.

OK, let's get these tests in; they're long overdue. This patch is pretty trivial; it just lets us create an anonymous decoder (i.e. a decoder without an associated RasterImage, the kind we use in GTests) that does downscaling during decode. The additional parameter to CreateAnonymousDecoder() and the code to implement it are identical to the similar code for CreateDecoder(); simple enough.

This patch adds the bulk of the tests. There's some refactoring to allow sharing code between the regular decoder tests and the downscale-during-decode tests. The main difference between the two cases is that we pass an output size (called a 'target size' in older code; I need to go through and clean that up) to the Decoder in the DDD case, and the expected output is different between the two cases. The DDD testcases use the same alternating green-and-red images that we use in TestDownscalingFilter. There are tests for every image format. I've verified that bug 1261964 would've been caught by these tests, as well.

It'd be really nice to have better test coverage of ICOs with AND mask transparency. This patch adds a DDD test for that situation. The testcase comes from bug 1206836. Unfortunately it's a bit difficult to work with such images (I can't figure out how to make ImageMagick produce them) so for now I'm forced to use an existing image which isn't easily checked by the existing output testing functions. As a result, this new test only verifies that the decoder produced a surface of the correct size and so forth and doesn't crash (or, implicitly, trigger an ASAN violation). I've filed a followup bug for adding an image comparison function which would allow us to verify the output, too. I think it's worth having this test in the tree even without that, though.

Comment on attachment 8756728 [details] [diff] [review] (Part 2) - Add downscale-during-decode GTests for all image formats. Review of attachment 8756728 [details] [diff] [review]: ----------------------------------------------------------------- Hooray for tests! Especially tests that provide coverage to places where real bugs were found. Why does the log message have all those "New Binary File:" lines? ::: image/test/gtest/TestDecoders.cpp @@ +202,5 @@ > + WithSingleChunkDecode(aTestCase, Some(outputSize), [&](Decoder* aDecoder) { > + RefPtr<SourceSurface> surface = CheckDecoderState(aTestCase, aDecoder); > + if (!surface) { > + return; > + } This surprised me. I think CheckDecoderState will only return null if the TEST_CASE_HAS_ERROR flag is set, right? Do these tests have that flag? If not, using EXPECT_TRUE (or similar) would seem to be more appropriate.

Comment on attachment 8756729 [details] [diff] [review] (Part 3) - Add a test that downscaling ICOs with AND mask transparency doesn't cause disaster. Review of attachment 8756729 [details] [diff] [review]: ----------------------------------------------------------------- Again, why the "New binary File" line in the commit message?

(In reply to Nicholas Nethercote [:njn] from comment #6) > This surprised me. I think CheckDecoderState will only return null if the > TEST_CASE_HAS_ERROR flag is set, right? Do these tests have that flag? If > not, using EXPECT_TRUE (or similar) would seem to be more appropriate. Yep, I concur. I'll make that change (and add a comment as to why EXPECT_TRUE is preferable).

In this updated version of part 2 I implement the change requested above and increase the fuzz by 1 on the first row.

https://hg.mozilla.org/integration/mozilla-inbound/rev/1bc7102718a99bc960054cbad9f178df932234c8 Bug 1207830 (Part 1) - Make it possible to create an anonymous decoder that downscales. r=njn https://hg.mozilla.org/integration/mozilla-inbound/rev/acbbb2038fdd02faf8228587b5d3d6cf7a9fb0d6 Bug 1207830 (Part 2) - Add downscale-during-decode GTests for all image formats. r=njn https://hg.mozilla.org/integration/mozilla-inbound/rev/f10f4b5ce594dd6004b5de023b4e53782ed9c0d5 Bug 1207830 (Part 3) - Add a test that downscaling ICOs with AND mask transparency doesn't cause disaster. r=njn

this made gtests unhappy and had to back this out (sorry seth) for test failures like https://treeherder.mozilla.org/logviewer.html#?job_id=28884712&repo=mozilla-inbound

Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/100c964f5e75 https://hg.mozilla.org/integration/mozilla-inbound/rev/40d54fc24c41 https://hg.mozilla.org/integration/mozilla-inbound/rev/6b521813510a

(In reply to Carsten Book [:Tomcat] from comment #11) > this made gtests unhappy and had to back this out (sorry seth) for test > failures like > https://treeherder.mozilla.org/logviewer.html#?job_id=28884712&repo=mozilla- > inbound It looks like somewhat more fuzz is necessary for the JPEG test on Windows. I noticed already that the JPEG test behaves differently between OS X and Linux. This is a bit bizarre to me; is libjpeg-turbo doing something platform-specific? It's also possible that it's a color management thing; if so, that should become clear pretty quickly when I add tests for color management, which is on my todo list.

https://hg.mozilla.org/integration/mozilla-inbound/rev/3b02a890b94e4b1879f0bd8e2361444e17de5dc5 Bug 1207830 (Part 1) - Make it possible to create an anonymous decoder that downscales. r=njn https://hg.mozilla.org/integration/mozilla-inbound/rev/3a61f536f0e6b992ddc587a2d4620e58da3a2fe6 Bug 1207830 (Part 2) - Add downscale-during-decode GTests for all image formats. r=njn https://hg.mozilla.org/integration/mozilla-inbound/rev/7856fc2284735fbca3b33f76eb045c9ad3e8b8c9 Bug 1207830 (Part 3) - Add a test that downscaling ICOs with AND mask transparency doesn't cause disaster. r=njn

Increased the fuzz by one more notch. Looks like this unbusts Windows, per the try job above.

https://hg.mozilla.org/mozilla-central/rev/3b02a890b94e https://hg.mozilla.org/mozilla-central/rev/3a61f536f0e6 https://hg.mozilla.org/mozilla-central/rev/7856fc228473