Re: [boost] [gil::io] Feedback for scanline_read_iterator

Subject: Re: [boost] [gil::io] Feedback for scanline_read_iterator
From: Mateusz Loskot (mateusz_at_[hidden])
Date: 2013-02-14 07:04:27

On 12 February 2013 01:13, Christian Henning <chhenning_at_[hidden]> wrote:
>
> I'm getting close to finally commit the new io extension for
> boost::gil. As it was required from the boost review I have
> implemented a basic image_read_iterator to read large images.

Hi Christian,

It's great to see it rolling forward.

> I would like to get some feedback on my interface and maybe on the actual
> implementation, as well. I must admit this is my first iterator that I
> have designed and implemented from scratch with some help by other people.

I have update my clone of GIL IOv2 and briefly played with your
example a bit [1]
using VS 2012 on Windows 8. FYI, your example compiles and runs well.

[1] https://github.com/mloskot/boost-gil-workshop

A few questions from me:

1) Are we speaking of image_read_iterator or scanline_read_iterator?

2) What is the difference between two templates in two different headers:
/boost/gil/extension/io_new/scanline_read_iterator.hpp
/boost/gil/extension/io_new/detail/image_read_iterator.hpp

3) scanline_read_iterator is an input iterator, do you also plan output one too?

4) The meat of your example:

reader_t reader = make_scanline_reader( file_name, bmp_tag() );
byte_t* buffer = new byte_t[ reader._scanline_length ];
scanline_read_iterator<reader_t> it  =
scanline_read_iterator<reader_t>(reader, buffer);
scanline_read_iterator<reader_t> end = scanline_read_iterator<reader_t>();
int row = 0;
while( it != end )
{
    *it; // ?

    copy_pixels(
        interleaved_view(reader._info._width, 1,
            (image_t::view_t::x_iterator) buffer, reader._scanline_length)
            , subimage_view(view(dst), 0, row, reader._info._width, 1));

    ++row;
}

a) Initially, I was wondering why not to hide the buffer behind the reader.
    But, I think the proposed composition of iterator from reader and buffer
    given separately gives better flexibility.

b) reader_t::begin(buffer) could be added, for convenience

c) Any particular reason you don't use result of the iterator
deference directly?

(image_t::view_t::x_iterator) *it

d) By the way, have you tested writing the dst into a file?

Finally, question not related directly to iterators, but to GIL
interface in general.
GIL functions require to cast various objects to pointers or iterators,
like in the example above:

 (image_t::view_t::x_iterator) buffer

IMO, it's weak link and it's the point where users mostly get into
trouble with GIL.
Would it be possible to at least document rules for such conversions?
What are the basic steps to decide on particular conversion of image
buffer or view to
certain type of pointer/iterator/location.
Obviously, it generally depends on image layout and configuration.
The trouble for me so far was that to figure out valid and required conversions,
one has to look at concrete types generated, frequently dig deep into GIL code.
Shortly, would it be possible to describe systematic way of deciding
about these conversions in GIL?

Thanks for the good work you do for GIL!

Best regards,
--
Mateusz Loskot, http://mateusz.loskot.net