Initial feedback for std.experimental.image

[rikki cattermole via Digitalmars-d]

On Friday, 10 July 2015 at 04:52:54 UTC, Vladimir Panteleev wrote:
> 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?

There are three kinds of mutation functions per functionality.

1) Modifies an image in place and returns it
2) image.rangeOf.function
3) Takes in a PixelPoint input range

Range suffix seemed like a good way to separate out which return 
a range and which don't.

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

Nope. It just copies a PixelPoint input range into an image 
storage type.

> 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.

In this case the only reason SwappableImage would be used is to 
use its lifetime management facilities. But hey you can manage it 
yourself if you want too...

>> 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"?

As far as I am aware the range capabilities could be used with 
std.algorithm. There is nothing stopping you. It just uses 
PixelPoint!Color to represent a single pixel instead an instance 
of the color.
http://cattermole.co.nz/phobosImage/docs/html/std_experimental_image_interfaces.html#.PixelPoint

> 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.

Licensing isn't an issue. I choose to write a new one to take 
advantage of std.experimental.color. It originally was meant to 
just test it. In fact I've been taking a lot of queues from 
ae.utils.graphics for this very reason. It is a very good piece 
of code and it already has solved may of the issues already.
Although now it is also pretty much testing e.g. 
std.experimental.allocators too.

I chose against reusing ae.utils.graphics as a base (same as e.g. 
dlib's and Devisualization.Image) because it would take a lot of 
work to get working with the newer Phobos to be modules. Almost 
as much as a new library would take.

You were the only person I was really interested in opinion wise. 
Manu's is great, but you have already done this. You know what 
works and doesn't.