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