Hi Zach,
There are some specific problems with the tests, though. For instance, png_read_test.cpp takes an image file, reads it, then writes it to a temporary file, then re-reads it from the temp file, and compares the two read-in images for equality. This strikes me as a bad test. It is quite possible to read an image, get the wrong results, then write those wrong results to disk, re-read them, and then have the re-read results compare equal to the original read results. There may be other tests with a similar organization; I did not check.
This would assume that both png reader and writer have some bugs in common. I consider that less probable.
No.
Consider an image I that is solid red on the left half, and 0% alpha on the right half, with a blue PNG background chunk. When loaded, it should produce an image that is red on the left half, and blue on the right half.
Your PNG reading code ignores the background-color chunk (not a big deal, as so does all other png reading code I've ever seen).
I'll add this as an action item to my todo list. Although, with a lower priority.
This means that when you read I, you get an image that is red on the left, and transparent on the right. If you write that image to disk, you have written an image that contains the wrong data (i.e. that is different from the source image file). If you re-read it, it will compare as equal to the initial read of the original image. You have a correct writer, but an incorrect reader, and your test cannot detect this.
Yes, I agree not ideal. I have to think about what I can to better.
- Did you try to use the extensions? With what compiler? Did you have any problems?
I only evaluated the PNG loading code, in an existing library that already uses (a modified) GIL for image file loading. My library's modified copy of GIL has numerous fixes to the PNG loading code. When I substituted gil.io_new, I found that several files were again unreadable.
I just did a quick test with space_elevator.png you mention below. May it be that you forgot to set the BOOST_GIL_IO_ENABLE_GRAY_ALPHA symbol? The following code works just fine:
#define BOOST_GIL_IO_ENABLE_GRAY_ALPHA 1
Yes, that was what I was missing. If only I had had the docs built, I could have figured that out for myself. All the images in the application now load correctly.
What is the rationale for needing to define this? Why can't it be always-on, or opt-out, instead of opt-in?
gray_alpha lives in the toolbox and though it's not part of gil, yet. I'm just covering the case when a user only downloads io_new and not the toolbox. This compiler symbol should go away when io_new and toolbox are added to boost. Would you mind changing your vote? Assuming you changed your opinion. Regards, Christian