Re: [boost] Review request: gil::io

Subject: Re: [boost] Review request: gil::io
From: Zach Laine (whatwasthataddress_at_[hidden])
Date: 2009-05-14 19:49:57

On Thu, May 14, 2009 at 10:28 AM, Christian Henning <chhenning_at_[hidden]> wrote:
>> - There is no reference section.  It took me a good long while
>> grepping about in the code to find the things I was looking for (in
>> particular, image_read_info<>, so I could see what the available
>> parameters where for PNGs in particular).
>
> A reference section is missing, I agree. Did you start with the tutorial?

Yes, it was just a little brief.  To use the library to do default
loads, I don't need anything but what's in the tutorial.  If I want to
specify a gamma when loading a PNG, I immediately need to look at
code, since there is no reference.

>> - There are several formatting problems with the QuickBook docs, like
>> _Supported_Formats_ on the tutorial page, which looks wrong
>> formatting-wise, and points to the root of the GIL documentation tree,
>> instead of where ever it should point.
>
> I define [link _Supported_Formats_ Supported Image Formats] and I
> thought it would link to the [h2 Supported Image Formats] more down in
> the documentation. But honestly, I'm a novice when it comes to
> Quickbooks. I once compiled the doc to a XML file but never could
> never figure out how to create html or pdf.
>
> Did you try to compile the doc? If so, can you show me how to do that?

Yes, I did, but on Windows, I have no idea.  Maybe a post to the docs
list would help.

>> Implementation:
>> - Support for direct reads into any_image<>s appears to have been
>> dropped.  This is a pretty big regression, considering that this usage
>> pattern was presented as a key selling point in the original GIL docs.
>>  Is the new recommendation that I use read_image_info to find out the
>> image properties I care about, and then have N cases for the different
>> formats I want to support?  This is a lot klunkier than the
>> direct-read-into-any_image<> method.  It requires me to know what the
>> different spellings of _num_channels are for all the file types I want
>> to support; it requires me to know which of my desired image types are
>> supported for which file formats; and it requires me to write a *lot*
>> of code that GIL formerly generated for me.
>
> Support for dynamic images has been added to the trunk and will be
> part in the next release candidate.

Good to know.

>> - Mixed LF and CR/LF line endings (a nit, I know).
>
> Any idea how to get rid of those? I'm on Windows and don't know of a
> good tool to do that.

Cygwin has the common UNIX program dos2unix.  I'd download Cygwin and use that.

>> Changes include:
>> - Implemented is_allowed<>() for the PNG format.
>> - Placed the TIFF and PNG is_allowed<>() implementations in different
>> namespaces as a hack to prevent collisions.  This should of course be
>> refactored.
>> - Added a user gamma value to image_read_settings<png_tag>.
>> - Added code to reader<..., png_tag, ...>::get_info() to get the
>> number of color channels and the gamma factor set in the file.
>> - Fixed interlaced PNG reading.
>>
>> Background colors and nonsquare pixels are still not handled properly,
>> but like I said before, no one will probably care.
>
> I care. Any ideas on how to do that. For instance I could imagine to
> create an image type with both a background image and the actual
> image. But what do to with non-square pixels? Gil doesn't support this
> kind of thing yet.

For the nonsquare pixels, you need to use the values in the pHYs chunk
to determine the aspect ratio, and then scale the image.  This is
probably best done as a post-loading step -- after you load the image
into for instance a 4x32 image, scale it up to 32x32 before returning
it.

The background color can be found in the bKGD chunk.  Simply alpha
blend the loaded image onto the background color.  Note that the
result needs to be full-alpha.

The transformation from a 4x32 image to a 32x32 image, and the
transformation from transparency mask / alpha channel plus background
color to a flattened, full-alpha image raise some interesting
questions.  Should the results in image_read_info<> be

1) just the properties directly read out of the file -- in our
examples, that would be a 4x32 image, or an image with transparency /
alpha;
2) just the derived properties, as they would exist after the entire
loading process is complete -- a 32x32 image, or an image with no
transparency;
3) or both?

Further, in the case of 2, should we advertise the final image as
having no alpha channel, or should it have one that is set to 100% in
every pixel?

I don't have good answers for these questions, which is why I skipped
the implementation of these infrequently used features in the PNG
loader.

Zach