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-17 17:46:46

On 16 February 2013 23:30, Christian Henning <chhenning_at_[hidden]> wrote:
> Hi Mateusz,
>
>>>> b) reader_t::begin(buffer) could be added, for convenience
>>>
>>> Mhmm, you think so?
>>
>> With this suggestion, I hoped to provoke some wider discussion about
>> such interface. Not very common prototype of begin() function, is it?
>> It seems convenient as it hides the "boring" boilerplate, analogously to
>> std::make_pair and other such functions.
>
> Could you give me some prototype code?

>From top of my head, using reader template not necessarily exactly
defined in GIL:

template<template Device, template Tag>
struct scanline_reader
{
   typedef typename scanline_reader<Device, Tag> reader_type:

   // deduce accurate type of buffer from reader's Device
   scanline_read_iterator<reader_type> begin(const char* buffer)
   {
       return scanline_read_iterator<reader_type>(*this, buffer);
   }
};

>>>> c) Any particular reason you don't use result of the iterator
>>>> deference directly?
>>>>
>>>> (image_t::view_t::x_iterator) *it
>>>
>>> I don't need to in my example. The dereference operator returns the
>>> buffer which we already have access to.
>>
>> Yes, I understand that, but IMHO it makes the purpose of this iterator obscure.
>> You know the internal details of the iterator, so you use the fact
>> that the iterator simply advances the buffer pointer.
>> You simply use the iteration side effect here.
>> But, I think most of  users who don't dig how this iterator is implemented, will
>> wonder why the result of derefence is not used.
>>
>> I think, discoverability is important and most users will try to apply
>> the knowledge about iterators they already have got.
>> Is there similar analogy to this in std:: or other commonly used
>> iterators that I've missed?
>
> I don't wanna create an "obscure" iterator and try my best to make
> sure it's easy to use and also to behave as expected. I know I'm not
> an expert in the field but I'm here to learn.

In fact, I have tried to provoke some opinions on my "judgement of
obscurity" above,
if my impression is isolated or others would confirm it.

> Anyway, there are two thing I need to accomplish when reading image
> and the common iterator interface doesn't seem to support it
> naturally:
>
> 1) read a scanline, via dereference operator
> 2) don't read the scanline but still move the internal scanline
> counter, via ++operator

Sure, it is not uncommon, that advance operation is very 'lightweight',
but the real work on data (fetch, copy) occurs during dereference operation.

(continuing my other question, not related to the scanline iterator)

>> How do I decide in particular case between casting to x_iterator or
>> rgb8_pixel_t*?
>
>
> For rgb8_image_t the x_iterator is rgb8_pixel_t*. x_iterator you use
> when you don't know the image_t or view_t during compile time.

In other words, for every type of image or view, x_iterator is a valid
pixel iterator.
It follows, that for every type of image, this form of cast is always valid:

boost::gil::interleaved_view(my_width, my_height,
(my_view::x_iterator)pixels, my_rowsize_in_bytes)

Then, I see no point in "teaching" users to use hard-wired
rgb8_pixel_t* as pixel iterator
when the x_iterator is the valid yet generic type of conversion here.

And, it does not matter that in scope of example we know that the x_iterator is
in fact rgb8_pixel_t*, this knowledge should not be used as we have a
generic type
available for that purpose..

My point is this:
if examples in GIL use explicit type like rgb8_pixel_t*, then in consequence
this knowledge is useless for users who work with different type of images.

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