Initial feedback for std.experimental.image

[Vladimir Panteleev via Digitalmars-d]

On Friday, 10 July 2015 at 04:35:51 UTC, rikki cattermole wrote:
> write("output.png", image
> .flipHorizontalRange
> .flipVerticalRange
> .rotate90Range
> .copyInto(new TheImage!RGB8(2, 2))
>
> .asPNG().toBytes);

Why the "Range" suffix for functions? Aren't most functions going 
to be lazy, and thus have a "Range" suffix?

Does copyInto do resizing / colorspace conversion? Such 
operations should be explicit. If it doesn't, then most of that 
sub-expression is redundant...

Have you looked at ae.utils.graphics.image.copy? If a second 
argument is not provided, an appropriate image is allocated 
automatically (on the heap though).

> I have an idea similar to copyInto except:
> .copyIntoTemporary!TheImage(allocator) // return 
> SwappableImage!(ImageColor!TheImage)(allocator.make!TheImage, 
> allocator); // auto ref return type

It took me a while to understand what SwappableImage does. The 
documentation could be improved.

I think you should not be adding runtime polymorphism implicitly 
anywhere. There are virtual range interfaces in Phobos, yes, but 
wrapping is only done explicitly, as it should be here.

> Basically SwappableImage can already deallocate the original 
> storage instance when it's destructor gets called. If provided 
> with the allocator. Same with RangeOf.

I suggest to avoid conflating Phobos range terminology with 2D 
image views. The current naming scheme makes me think that it 
returns a range of rows, or some other range which is directly 
pluggable into std.algorith. What's wrong with "View"?

I'd also like to ask why you decided to write a library from 
scratch instead of reusing existing ones. I've spent days solving 
certain design issues of ae.utils.graphics, it seems wasteful to 
me to discard that and duplicate the effort. If the license is 
the issue, I'd be happy to relicence it.