Subject: [boost] Fwd: Formal Review of IO and Toolbox extensions to Boost.GIL starts TOMORROW
From: Christian Henning (chhenning_at_[hidden])
Date: 2010-12-09 14:51:40
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