Christian Henning <chhenning <at> gmail.com> writes:
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.
I think this might be even worse unfortunately, if the iterator is copied, such as with (*i++) it's going to make a full copy of the buffer. Now that readers have a begin and end maybe the reader could own the buffer?
Skipping can be different for different formats. jpeg for instance still reads the scanline when skipping. For now, I think, we are good.
Sure that makes sense, but the interface lies IMO. If I have jpeg and I call skip( buffer, 0 ) it's not going to be skipping over scanline 0 (unless it happens to be the first call to read/skip), it's just skipping forward one scanline, the second parameter is unused. Similarly the TIFF reader seems to read skipped scanlines unnecessarily as skip forwards to read but the TIFF interface supports reading any scanline you want directly with TIFFReadScanline.
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.
iterator_facade is super duper simple. You just implement a handful of private member functions, specify the appropriate iterator tag and it generates the rest of the iterator for you with no hassles. I highly recommend trying it.
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?
I'm not confident your increment operator is correct. I don't think a correct increment can call skip/read for an input_iterator and yours can still call skip. I think this is a better implementation (recreated here with more comments and without the iterator_facade so as not to confuse anything) scanline_read_iterator( Reader& reader ) : _pos( 0 ) , _last_pos( -1 ) { } scanline_read_iterator< Reader >& operator++() { // simply advance _pos, that's all we need to do ++_pos; return (*this); } reference operator*() { // if this is the first dereference, or we've advanced // pos since the last dereference we have work to do // otherwise buffer already contains the scanline data for _pos if ( _last_pos != _pos ) { // advance last_pos skipping any lines until we reach _pos // if we reach _pos after one increment we don't need to do // any skipping and we can just read as it's the next scanline while ( ++_last_pos != _pos ) { _reader->skip( _buffer, _last_pos ); } _reader->read( _buffer, _pos ); } return _buffer; }