Hi Michael,
I'm not sure exactly what _reader->clean_up(); does but the destructor seems likely wrong. If you make a copy of the iterator and the original or the copy gets destructed it calls clean_up while the other copy may still be in use. Shouldn't clean_up just be part of the scanline_reader destructor and not the iterator?
I have removed clean_up. It was not used by any image format implementation.
The set_reader and set_buffer member functions seem unnecessary and confuse things.
Done.
I think the checks at the top of the dereference operator are unnecessary. You're reporting programming errors with exceptions.
if( _reader == NULL ) { io_error( "Reader cannot be null for this operation." ); } if( _buffer == NULL ) { io_error( "Buffer cannot be null for this operation." ); }
Same in increase_pos if( this->_reader == NULL ) { io_error("Reader cannot be null for this operation."); }
Done.
The process of generating the buffer and supplying it to the iterator feels strange to me but I could be wrong.
I moved the buffer creation internally.
There is an equal member function that has nothing to do with operator==. Seems like equal should just go away.
OK.
I don't see why end has to be constructed in that manner. It would seem to me that an end iterator constructed with the reader makes the implementation a whole lot simpler. I would prefer an interface where you just call reader.begin() and reader.end() to get your iterators. You'll no longer need a sentinel value for past-the-end, _pos could be initialized to reader->_info._height.
The readers now have a begin() and end() members.
Speaking of which is _info._height part of the Reader concept? Is the Reader concept documented anywhere I couldn't find it but I only did a cursory search.
No there is no Reader concept. What do you have in mind?
I don't really understand the second parameter to _reader->skip and _reader->read. It seems to be ignored for everything except the TIFF reader. Which leads me to believe that if TIFF reader needs to keep track of its current row it should do so internally as all the other readers track their internally (seemingly implicitly). Also I'm not sure but given the interface it seems that TIFF skip could maybe be a noop if the row is passed to read still or just a simple increment of an internal row counter.
Skipping can be different for different formats. jpeg for instance still reads the scanline when skipping. For now, I think, we are good.
Note also that I think the requirement on InputIterator concept that Postincrement and dereference (*i++) be valid forces lazy evaluation. If you did eager reading/skipping somehow in increment you'd break that expression.
Yes, the postincrement operator is missing. I'll have to look up how it's usually implemented.
Here is a sketch of a possible implementation. http://codepad.org/9hjn3SpQ
I see you're using iterator_facade. I was a little afraid of using it since I have never used it before. What do you think of the updated implementation. I think we are getting closer to what we want! If you have the time, would you mind and have another look? Thank you very much for your insight. It's been very helpful! Regards, Christian