Re: [boost] GIL - late review

From: Hans Meine (meine_at_[hidden])
Date: 2006-11-14 09:47:42

Hi Lubomir!

On Tuesday, 14. November 2006 00:40, Lubomir Bourdev wrote:
> When searching for appropriate names, we thought about Traverser too,
> and we like it. It emphasizes more the navigation aspect, whereas
> Locator emphasizes the access aspect. Both Locator and Traverser have
> precedence. We ended up choosing Locator because it is a bit shorter.
Thanks for the explanation.

> Another term we considered is Position.
To me, a position is the difference_type of a traverser and a specific 'origin 
traverser'.

> Someone also brought up the term Cursor.
I am not sure if there are strong precendents within the image analysis 
domain, but that term probably already has too many associations.

> I don't have a strong opinion about sticking with Locator; other than
> the work to change the name everywhere. If people overwhelmingly prefer
> one term over another we can change it.
I prefer Traverser, but I can understand that you don't want to change all 
code just because of me. ;-)  (OTOH, that's a killer argument - it would be 
sad if good design changes would not be made just because it's a lot of 
work.)

> > > Similarly,
> > > I prefer names like resize_and_clobber(), resize_and_invalidate(),
> > > create_with_new_dimensions(), recreate(), or reset() than resize().
> > > What do other people think?
> >
> > What about reshape()?
>
> Reshape is used in Matlab to change the dimensions of an array, but it
> does not invalidate the elements of the array. So I wouldn't recommend
> using it for the same reason I don't like resize() - because there are
> precedents with the same name and different semantics.
Still, I would say that there's a big difference between a Matlab predecessor 
and an STL one.  But I agree that Matlab is well-known in the image analysis 
domain, so..

> How about image::recreate(width,height)?
..I very much prefer this one! :-)  (Ulli seems to like it, too.)

> For non-compatible channels you must use copy_and_convert_pixels, not
> copy_pixels, to emphasize that the copy may be lossy. And your channel
> types must be convertible. That is, there must be a function:
>
> DstChannel channel_convert<DstChannel>(SrcChannel c);
>
> It will define how to convert one channel type to another (scale, shift
> by 8 bits, etc.). The default simply transforms the range of one to the
> other and rounds to the nearest element.
If I may mention the principle of least surprise again - the default should be 
copy-and-round, not scale IMO, but..

> If you don't like this transformation, you may choose to create a custom
> channel type, you may provide custom conversion function object,
..it should be easy to do so, to provide a custom converter, and the current 
default converters (changing the range) should be available as such functors.
I would not like to create custom "channel type"s just for this I think, but 
maybe I am not thinking GILish enough yet.

> or you 
> may choose to use transform_pixels instead of copy_and_convert_pixels,
> depending on your design.
In VIGRA, copyImage is simply copying the image data (using traits to do an 
explicit type conversion to make the compiler silent if necessary), and you 
use transformImage with e.g. a LinearIntensityTransform functor to change the 
range while doing so:

// transform 0..255 byte image to 0.f..1.f float image:
transformImage(srcImageRange(byteImage), destImage(floatImage),
                          linearIntensityTransform(1.0/255));

This is very close to the STL, isn't it?  Now that I think of it, what's the 
difference between transform_pixels and copy_and_convert_pixels (when using 
function objects)?

> > When discussing the stacking of views, your argument was:
> > > I am in favor of letting the programmer choose what is best rather
> > > than imposing one strategy, which in my opinion is the style of C++
> > > and STL as compared to other languages.
> >
> > Does this argument not hold for the above case?
>
> The two contexts are not the same.
> I was arguing that we shouldn't prevent creating color_converted_view
> with certain color spaces simply because color conversion with them is
> expensive. We should let the programmer decide whether it makes sense to
> use color converted view or explicitly color convert.
>
> But the semantics of color_converted_view with LAB is very clear - it is
> the same as with any other color space, whereas how to copy between two
> incompatible images is not always obvious.

That's a good argument.  However, how clear are the semantics of 
copy_and_convert_pixels with the default converters?  To me, "convert" was 
simply "change type" (which already has clear semantics in C++, even if they 
may not always be what you need), not "change type, scale, and round".

> It comes down to some basic principles:
>
> 1. We want equality comparison to be an equivalence relation.
> 2. We also want equality comparison and copying to be defined on the
> same set of types.
> 3. The two operations should work as expected. In particular, after a=b,
> you can assert that a==b.
>
> I hope you will agree that these are fundamental rules that hold in
> mathematics and violating them can lead to unintuitive and bug-prone
> systems.

Yes, I agree to that and the consequences, and I thus agree that it's good 
design to forbid copy_pixels on non-compatible types.  However, that's 
inconvenient, as you will probably agree.  That's why you introduced the 
default channel conversions: for convenience.

> Regardless of what we do, there is always a notion of the operating
> range of a channel, whether it is implicit or explicit.
> You are essentially suggesting that the range of a channel be defined at
> run time, rather than at compile time, right?

You are right, this seems to be the main difference between the VIGRA and GIL 
approaches in this respect: In VIGRA, short is simply a (pixel/channel) type, 
but does not imply any operating range (that's up to the application), 
whereas in GIL, it has an operating range associated with it (BTW: -2^7..2^7 
or starting from zero?) and you need to define new channel types in order to 
change that operating range, right?

-- 
Ciao, /  /
     /--/
    /  / ANS