[boost] Fwd: Formal Review of IO and Toolbox extensions to Boost.GIL starts TOMORROW

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