[pngpp-devel] Re: png++

[Alexander Shulgin]

On Feb 6, 2008 10:16 PM,  <address@hidden> wrote:
> Hello
>
> First of all, I think your wrapper is very very nice, great job with 
> templates! I am so relieved that I've found it on the web. Man I so hate to 
> deal with all that C stuff when programming in C++.

Hello,

Thanks for your feedback! :-)

> I have a few suggestions though:
>
> 1) It would be nice if the user could be able to change rows for colums 
> during template instantiation using possibly a template parameter. My 
> rationale is that although indeed in computer graphics you put the column 
> first and the row second, in C++ (and generally in math when we speak about 
> matrices) it is the other way around. Sure you can easily manipulate images 
> as it is now, but it would be just _nice_ if the user had a choice how to 
> deal with coordinates from the very instantiation. You could for example 
> leave it as it is as the default behaviour and add a default non-type 
> paremeter.

So this should only affect image::operator[], right?

I don't think adding a template parameter will do any good in this
case--consider some generic algorithm which takes png::image<T>.  You
either need two versions of the same function to work with both types
of image orientations or you cannot pass image<pixel,true> to a
function taking image<pixel,false>...

However, some adapter class might help here, it even could be derived
from image, like this:

template<...>
class ximage : public image<...>
{
...
  column& operator[](size_t index);

  class column { ... };
};

Objects of class ximage<T> could be passed to functions taking
image<T>&.  Do you like this approach?

However, there might be obvious flaws I just cannot spot right now. ;)

> 2) This one is more important: I've found out that it would be really, really 
> great if the user could just iterate over every pixel (or a range) using 
> iterators, pretty much like you would do with std::vector.
> You could then use very easily std::algorithms, for example 
> std::fill(image.begin(), image.end(), 255); That would immensely simplify and 
> just make more convenient very much stuff, because on many occasions the user 
> would be able to get rid of all those double for loops. Not to mention it 
> would look even more like a nice modern C++ powerful stuff :) Please add the 
> support for the iterators :)

I like the idea, but could you please clarify a few things?

Right now, you may use iterators for a row of (non-packed) pixels
because it's old good std::vector:

png::image< png::gray_pixel > image;
...
png::image< png::gray_pixel >::row_type row = image[row_index];
std::fill(row.begin(), row.end(), 255);

Are you talking about adding begin()/end() for image to iterate over
rows?  Or may be image.begin() should be the left-top corner and
image.end()--right-bottom?

> 3) Another a bit related thing. It would be nice if the user could pass a 
> std::vector with the size of rows and columns to the constructor. Thus one 
> could do all the manipulations in the std::vector (when convenient) and then 
> just do for example png::image<png::gray_pixel> image(myvector, rows, cols).

I like this too.  Here, I can see a bit more of improvement: we can
pass a range of rows to image constructor:

png::image<png::gray_pixel> myimage(vec.begin(), vec.end());

Isn't it great?

Waiting for you comments. :-)

--
Cheers,
Alex
PS: lets keep pngpp-devel@ in Cc: for the benefit of the project, if
you don't mind.

http://www.shulgin.org.ua/~shulz/
Tip: use google's cache if the page does not load.