Subject: [boost] GIL IO extension review
From: Kenny Riddile (kfriddile_at_[hidden])
Date: 2010-12-06 12:52:47
Sorry if this review seems short, but most of my time is spoken for.
Still, I think this extension is useful enough that I found time to
review it. I've been using the new version of the GIL IO extension for
over a year now and am glad to finally see it up for review. I've
primarily used it for loading various image formats for use as OpenGL
textures. I recently gained some experience with extending the library
to support an additional image format (TARGA) as well.
- What is your evaluation of the design?
My use case for the library is conceptually simple, so I would expect
the library to make it simple to accomplish. The provided read_image()
and write_view() functions are about as simple an interface as one could
expect, but without sacrificing flexibility and extensibility due to the
usage of tag dispatching and standard iostreams. As a user, I have no
complaints about the design and how it integrates with GIL.
- What is your evaluation of the implementation?
I hadn't looked at the implementation much prior to recently adding
support for reading and writing TARGA images, and my evaluation of the
implementation is from that standpoint. The fact that I (of all people)
was able to easily add support for a new format without much
documentation on the subject suggests that the relevant parts of the
implementation are easily understandable. I mostly used the BMP
implementation as a reference, and didn't notice anything egregious or
suspicious. I have some concerns about the usage of setjmp/longjmp in
the PNG and JPEG implementations (honestly I get suspicious any time I
see those). I think there was discussion elsewhere about adding
additional test cases to exercise the error cases that result in a
longjmp so that all of the supported platforms can be verified. I think
this is a good idea. Another nitpick that came up when adding TARGA
support are the read_int8, read_int16, read_int32, write_int8, etc.
member functions of the Device models in io_device.hpp. To me, these
names suggest that these functions return and accept signed integers
when in reality they return and accept unsigned integers.
- What is your evaluation of the documentation?
The documentation was sufficient enough for me to quickly figure out how
to read and write images from all of the supported formats. The section
on extending the library to support new formats was enough to get me
started, but I ended up just relying on the existing BMP code as a
reference. This suggests that maybe that section of the documentation
should be fleshed out a little more.
- What is your evaluation of the potential usefulness of the extensions?
This new IO extension is extremely useful. It nicely fills huge gaps in
the existing IO extension, such as loading from streams instead of only
being able to supply a file name. The interface is simple enough to
integrate into just about any design, while still providing both
flexibility and extensibility.
- Did you try to use the extensions? With what compiler? Did you have
I've used the proposed IO extension to read and write BMP, PNG, JPEG,
and TARGA files with both MSVC9 and GCC in combination with the last
several Boost releases. I had no problems.
- Do you think the extensions should be accepted as a part of Boost.GIL
Yes, provided additional tests are added to exercise currently untested
error cases, and they pass on all platforms officially supported by
Boost. This extension is just as simple to use as the existing GIL IO
extension, yet is better in every way. Therefore, I see no reason not
to accept it as a replacement.
Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk