Re: [boost] GIL io_new review

Subject: Re: [boost] GIL io_new review
From: Christian Henning (chhenning_at_[hidden])
Date: 2010-12-11 12:36:40

Hi Domagoj,

>> On the issue of using classes vs free functions, I don't think there is a
>> huge difference either way. The the new_io code borrows the style from
>> GIL, which uses free functions as algorithms (if you call I/O
>> "algorithms").
>> GIL in turn borrows this style from the STL. Using functions one can
>> extend
>> functionality with overloading (but there are ways to do this with classes
>> too)
>> and it is nicer to be able to read/write a file with one line, without
>> having to
>> first construct a reader/writer and then call it. But these are minor
>> quibbles.
>
> I understand you had limited time to go through all the, now really numerous
> posts, but I think I've shown (in the pre-review discussion) that these are
> not minor quibbles...The free function interface has issues both in terms of
> performance and in terms flexibility/power of access to the backend that it
> gives...So far shown attempts to fix those where all shifts towards public
> classes/objects representing the image to be read/written (and/or further
> increase in the number of parameters)...
>
> Furthermore, the classes approach can also provide a oneliner interface as
> io2 already does (as shown in the pre-review discussion) using static member
> functions...it is provided for all backends in a single place by the CRTP
> base class...so you can for example simply write libpng_image::read( ... )
> or libtiff_image::write( ... ) which will construct the local reader/writer
> object for you and do the rest of the required work....

I'm still crunching on the better design that your are proposing. I
did some reading up on CRTP and as I as I understand it now it's used
for compile time polymorphism.

Though you have  libpng_image::read( ... ) and libtiff_image::write(
... ) but would you have a lodepng::read( ... ) as well or is that png
backend somehow plugged into the reader class differently?

>
> It is no wonder that 'we live in the age' of several-megabyte-single-dialog
> software if 'every' library author thinks it OK to add a few tens or even a
> couple of hundreds of unnecessary kilobytes of code because it supposedly
> will not be noticeable because of disk access or some similar 'by-the-book'
> excuse (that is rarely actually tested)...

One of the good things we have with io_new is that the header files
let you define what to include quite well. Only reading jpeg then just
include jpeg_read.hpp. No writing part of jpeg or any other backend is
included.

>
> ps. Ideally, in most cases there should be no 'inner-loop' in GIL.IO (other
> than those within the backend library itself)...

??? How do you read progressive png then? Meaning you have to read an
image a couple of times to actual get the final result. I rather
relieve the user from doing that.

> That iostreams are evil is a simple _fact_ demonstrated countless times by
> numerous people...And it seems there is no longer any point in arguing this
> fact as it is simply ignored by people that seem to have some sort of a
> priori trust in the STL (and you can't rationally argue with 'blind faith')
> as if it is some 'holy artefact' of the C++ language when in fact it is
> sometimes its stumbling block...

Are all iostreams bad? Or do you only speak for STL's ones? To me the
std::streams only represent an interface. It has been demonstrated in
some other email thread that you can use boost::iostreams with io_new.

In case you don't like all streams what would be a better way of
supporting in-memory images. Please remember a common interface is
needed for flexibility.

>
> The 'massiveness' of the thing in our hands is further pointed out by the
> fact that we have so far mostly or even exclusively seen reviews of only the
> IO extension which comprises only a part of Christian's work (which I
> suspect might also be 'frustrating' for Christian...as even a negative
> review is often better than no review at all)...

If you speak of the Toolbox than I don't really care much. Introducing
the Toolbox is just a way for me and hopefully others to add more and
more small tools over time. Right now it only consists of a couple of
color spaces and some metafunctions that io_new needs.

>
> For example adding another set of tags could further complicate
> things/implementation (even maybe slowing compilation) with another required
> tag->implementation mapping...Directly using the backend wrapper (e.g.
> libjpeg_image::... ) seems IMHO simpler with no adverse effects...

Each tag comes with a set of headers, xxx_read.hpp, xxx_write.hpp,
xxx_all.hpp, and xxx_io_old.hpp. If you don't include such headers no
compile time slowing down should occur.

I understand that having an xxx_image class to wrap a backend makes
sense to me. At least it gives you the access to the backend when
needed.

>
>
> ps. There is also one important aspect (that has AFAICT only been briefly
> mentioned so far) and that is of a header only library...having dealt with a
> good number of different backends (and their various 'quirks') and being
> able to extract/produce a fair amount of non-template code I'd also vote for
> a change to a non-header-only library for various reasons...

Mhmm, we are just wrapping some backend. ASIO is header only too, correct?

I think I read somewhere you're becoming a father soon. Congratulations!

Regards,
Christian