Re: [boost] Half-baked GIL IO extension review

Subject: Re: [boost] Half-baked GIL IO extension review
From: Christian Henning (chhenning_at_[hidden])
Date: 2010-12-07 10:49:01

Hi Edd, thanks for taking the time.

>
>
> - What is your evaluation of the design?
>
> I think it's fine on the whole. But I'm left with some questions, some
> arising from real-world uses:
>
> 1. is there explicit support for reading images from files with Unicode file
> names? Or do I have to prepare a stream myself? I ask because there seem to
> be a lot of overloads for reading from files whose paths are specified by
> char*s and std::strings. IMO, in anything other than a toy application,
> these would be useless for writing portable code. And for toy code I'd
> probably use the PIL[3] (Python) instead. So for *me*, these functions are
> redundant. I'm aware of this redundancy because it exists in my code too :)

You can use string, wstring, char*, and filesystem::basic_path ( when
compiling with BOOST_GIL_IO_ADD_FS_PATH_SUPPORT ) to define a
filename. But the wide character strings are then converted back
std::string. Though, I have to admit that reading a unicode filename
isn't supported yet. I'll make this a top priority.

Basically I have to add some overloads that use wfopen. Or is there more to do?

>
> 2. is it wise to drag the stuff from e.g. the IJG JPEG library's public
> headers in to client scope? I ask because I've had clashes with the
> "boolean" macro before and this would outright prevent me from using the
> library. The implication of addressing this would probably mean that the GIL
> would no longer be header-only. That's fine by me, but perhaps it's a design
> goal of the GIL for some reason?

Would a namespace do the trick?

>
> 3. Is there support for things like performing lossless JPEG rotations? Not
> a big deal to me right now, but potentially I would like to use such a
> facility in future.

No support yet. I think I was looking for a code example at one point
but couldn't find one. This feature is already in my todo list.

>
> 4. Is there support for obtaining the real-world dimensions of a loaded
> raster in millimetres, say? I work on CAD/graphics applications which need
> this functionality in order to generate reasonable texture coordinates.

Is that part of the header information? If not, I don't think this is
the right library to do unit conversion. This library should provide
ways to read and write all header information and ways to read and
write pixels.

>
>
> - What is your evaluation of the implementation?
>
> Looking again at the JPEG code here. As far as I can tell setjmp/longjmp
> have been used with appropriate care, but I worry what will happen if a
> method on a Device throws an exception, for example. I've had both Visual
> C++ 2008 and Apple's g++ 4.0.1 crash on me in this kind of situation.

Do you mean when an std::fstream throws an exception or when FILE
issues an error?

>
> In my code, I attempt to store an exception by doing something akin to
> boost::current_exception(), then longjmp out to a safe point before
> re-throwing it.

Mhmm, I have to admit I'm unaware of boost::current_exception. Can you
point where you use it in your code?

>
> The IJG JPEG library can be compiled as C++ code and there's some new stuff
> in Version 7 to control whether or not extern "C" blocks are used (look for
> DONT_USE_EXTERN_C in the IJG source). It would be desirable in my opinion to
> allow the user of the library the ability to use a build of libjpeg that was
> compiled as C++. If compiled in such a manner, with a little care it is
> possible to entirely replace setjmp/longjmp with exceptions. The same is
> true of libpng and possibly of libtiff.

Interesting. I'll have a look.

>
>
> - What is your evaluation of the documentation?
>
> Couldn't find a lot. Either my eyes are poor, or the documentation is :)
>

Did you see the .qbk file in the /doc folder?
>
>
> - What is your evaluation of the potential usefulness of the extensions?
>
> Potentially very useful.
>
>
> - Are you knowledgeable about the problem domain?
>
> I know little about the inner workings of the file formats and compression
> schemes involved, but have written my own C++ wrappers for libpng and the
> IJG JPEG library ('libjpeg').
>
>
>
> - Do you think the extensions should be accepted as a part of Boost.GIL
> library?
>
> I personally couldn't use it until some of my concerns/requests were
> addressed. But I certainly think it could be of value to others as-is.

I think most of your concerns will be taken care once the review has
finished. I'll let you know.

>
> However, I would say that there has to be more documentation, assuming I
> haven't simply missed it... For this reason, my answer at the moment is
> regrettably 'no'.
>
>
> I hope this was useful and my lack of general GIL knowledge doesn't
> invalidate too much of my evaluation.

In no ways. ;-) Thanks for taking your time.

Regards,
Christian