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.

Reply via email to