Hello.

I am new to D and I must admit I really like the language. In my
opinion it takes the best from C++ and, say, Python and combines
it really elegantly. Great work!

I am currently working on my first library in D - related to
TARGA image format.

Here's the link to the repo: http://bit.ly/1mIuGhv

It's a work-in-progress case: at the moment the library does what
I need for my other projects, but there is a couple of things
that I want to add/fix soon.

Perhaps someone could have a look at the code and point out some
obvious traps that I fell into etc.

Any feedback would be great!

Best regards,
Mike

On Wednesday, 11 June 2014 at 18:29:27 UTC, Mike wrote:
> Hello.
> Here's the link to the repo: http://bit.ly/1mIuGhv

Hi, sorry didn't read through your code yet, but while ago I wrote some encoders/decoders for jpeg and png (https://github.com/callumenator/imaged, haven't compiled it in a while). Might it be worth stitching things together into a proper image processing package?

Cheers,
cal

On Thursday, 12 June 2014 at 00:20:28 UTC, cal wrote:
> Might it be worth stitching things together into a proper image processing package?

Well I started working on TGA because I was disappointed that no image abstraction is present in Phobos. Go has some imaging APIs and I think D would benefit from having one too (out of the box).

Would I work on std.image? Sure.

Best,
Mike

On Thursday, 12 June 2014 at 15:46:12 UTC, Mike wrote:
> On Thursday, 12 June 2014 at 00:20:28 UTC, cal wrote:
>> Might it be worth stitching things together into a proper image processing package?
>
> Well I started working on TGA because I was disappointed that no image abstraction is present in Phobos. Go has some imaging APIs and I think D would benefit from having one too (out of the box).
>
> Would I work on std.image? Sure.
>
> Best,
> Mike

I'm looking over your code ATM but I'd like to reply to this first.

IMO it's not a good idea to create something like std.image. The std lib should ideally never have breaking changes, so it's easy to get stuck with a sub optimal API or become increasingly hard to maintain.

We have Dub. Better keep the std lib lean and maintainable. If you're looking to create an awesome idiomatic D image library you're probably better of building it on top of derelict-devil or derelict-freeimage, then publish it on code.dlang.org

The discoverability of good code.dlang.org projects is still limited. Some kind of rating or like system would be useful, but that's another discussion.

Le 12/06/2014 20:09, Rene Zwanenburg a écrit :
> On Thursday, 12 June 2014 at 15:46:12 UTC, Mike wrote:
>> On Thursday, 12 June 2014 at 00:20:28 UTC, cal wrote:
>>> Might it be worth stitching things together into a proper image
>>> processing package?
>>
>> Well I started working on TGA because I was disappointed that no image
>> abstraction is present in Phobos. Go has some imaging APIs and I think
>> D would benefit from having one too (out of the box).
>>
>> Would I work on std.image? Sure.
>>
>> Best,
>> Mike
>
> I'm looking over your code ATM but I'd like to reply to this first.
>
> IMO it's not a good idea to create something like std.image. The std lib
> should ideally never have breaking changes, so it's easy to get stuck
> with a sub optimal API or become increasingly hard to maintain.
>
> We have Dub. Better keep the std lib lean and maintainable. If you're
> looking to create an awesome idiomatic D image library you're probably
> better of building it on top of derelict-devil or derelict-freeimage,
> then publish it on code.dlang.org
>
> The discoverability of good code.dlang.org projects is still limited.
> Some kind of rating or like system would be useful, but that's another
> discussion.

I think it can be a great advantage to have some things like image management in phobos, cause often dub projects are big and users don't want necessary a complete multimedia library but just small pieces that are standard.
For example a GUI library, will allow image manipulations, but not only, and extracting only the image modules can be hard. That the case of our DQuick project.

For a project like DQuick, I would be happy to find things like images, geometric algebra,environment analysis,... in phobos. This will allow use to be focused on other things making an UI framework (Windows, events, rendering, resource management,...).

Having such minimalistic APIs would be a great benefit IMO, maybe in this case some extending libraries would appear in dub.

Le 12/06/2014 20:35, Xavier Bigand a écrit :
> Le 12/06/2014 20:09, Rene Zwanenburg a écrit :
>> On Thursday, 12 June 2014 at 15:46:12 UTC, Mike wrote:
>>> On Thursday, 12 June 2014 at 00:20:28 UTC, cal wrote:
>>>> Might it be worth stitching things together into a proper image
>>>> processing package?
>>>
>>> Well I started working on TGA because I was disappointed that no image
>>> abstraction is present in Phobos. Go has some imaging APIs and I think
>>> D would benefit from having one too (out of the box).
>>>
>>> Would I work on std.image? Sure.
>>>
>>> Best,
>>> Mike
>>
>> I'm looking over your code ATM but I'd like to reply to this first.
>>
>> IMO it's not a good idea to create something like std.image. The std lib
>> should ideally never have breaking changes, so it's easy to get stuck
>> with a sub optimal API or become increasingly hard to maintain.
>>
>> We have Dub. Better keep the std lib lean and maintainable. If you're
>> looking to create an awesome idiomatic D image library you're probably
>> better of building it on top of derelict-devil or derelict-freeimage,
>> then publish it on code.dlang.org
>>
>> The discoverability of good code.dlang.org projects is still limited.
>> Some kind of rating or like system would be useful, but that's another
>> discussion.
>
> I think it can be a great advantage to have some things like image
> management in phobos, cause often dub projects are big and users don't
> want necessary a complete multimedia library but just small pieces that
> are standard.
> For example a GUI library, will allow image manipulations, but not only,
> and extracting only the image modules can be hard. That the case of our
> DQuick project.
>
> For a project like DQuick, I would be happy to find things like images,
> geometric algebra,environment analysis,... in phobos. This will allow
> use to be focused on other things making an UI framework (Windows,
> events, rendering, resource management,...).
>
> Having such minimalistic APIs would be a great benefit IMO, maybe in
> this case some extending libraries would appear in dub.

I am thinking an other benefit is what you see as a default, the assurance of D standard conformances (portability, safety, quality, support,...).

I can shed some light on my, beginners, point of view and the rationale behind this tiny library.

I want to create a bar/matrix-code generator (QR, DataMatrix etc.). I really like graphics-related subjects and I thought this would be a good start in D.

Coming from Java island and having experience in Go I expected to find some basic imaging functionalities in the standard library: not necessary support for all image formats, but at least some bitmap i/o and data model (Pixel, Image, Filter ...).

I found none of that :-(

IMHO (one of the) pain(s) of C++ is that the stdlib is far behind what a modern developer would consider "elementary".

But I expected something from D, because Phobos is already more than C++'s stdlib is: Phobos has e.g. digests. IMO digests are not (as) necessary for a standard library as, say, a qsort is. So if there are digests, why not imaging?

Either way, D is really nice and if I can help, I will :-) So far - the TGA lib.

Best,
Mike

On Wednesday, 11 June 2014 at 18:29:27 UTC, Mike wrote:
> Here's the link to the repo: http://bit.ly/1mIuGhv

I usually don't trust shortened URL's. Can you please post full URL's when not constrained by a character limit?

> Any feedback would be great!

First of all, I like your coding style. It's nice and readable :)

https://github.com/emesx/TGA/blob/develop/source/tga/model/utils.d

All those functions can be marked pure. Pure is D is a bit different than in other languages. I can recommend the following article as a good explanation:

http://klickverbot.at/blog/2012/05/purity-in-d/

Also, depending on your preference, you may want to put those functions in a

pure nothrow
{

}

block, or put

pure nothrow:

at the top of the source file. I'm not sure if all those functions can be nothrow though.


https://github.com/emesx/TGA/blob/develop/source/tga/model/utils.d#L8

These array literals will be allocated every time the function is called. 'only' can be used instead:

only(16, 32).canFind // etc..

http://dlang.org/library/std/range/only.html

I remember a function which does something like only only + canFind on one go. It would look something like

header.colorMapDepth.among(16, 32);

but I can't find it right now.. Maybe it was only proposed but never added.


https://github.com/emesx/TGA/blob/develop/source/tga/model/utils.d#L101

This has a pretty bad time complexity. In the worst case this is O(n^2), I think. You could use a hashmap to build the table, then convert to an array. Or even only use hashmaps as color tables internally to improve performance across the board.


https://github.com/emesx/TGA/blob/develop/source/tga/model/types.d#L11

Depending on your taste, a union + anonymous struct could be used:

struct Pixel
{
	union
	{
		ubyte[4] bytes;
		
		struct
		{
			ubyte b, g, r, a;
		}
	}
}


https://github.com/emesx/TGA/blob/develop/source/tga/model/validation.d#L47

to!string is so common it has a special alias: text. It's placed in std.conv so you still need to import that.

"Invalid color map pixel depth: " ~ header.colorMapDepth.text


https://github.com/emesx/TGA/blob/develop/source/tga/io/readers.d#L24

Yeah, we need something to read an entire struct from a file and convert it to the correct endianness..


https://github.com/emesx/TGA/blob/develop/source/tga/io/readers.d#L24
https://github.com/emesx/TGA/blob/develop/source/tga/io/readers.d#L91

Unnecessary GC allocation in a hot loop == :(
Make a buffer outside the loop and call the normal rawRead:

auto buffer = new ubyte[colorMapByteDepth];
foreach(i; 0 .. (header.colorMapLength - header.colorMapOffset))
{
	file.rawRead(buffer);
	colorMap[i] = pixelReader(buffer);
}

Even better, use a static array with the max byte depth, then slice to the correct length:

ubyte[MaxByteDepth] buffer;
foreach(i; 0 .. (header.colorMapLength - header.colorMapOffset))
{
	colorMap[i] = file.rawRead(buffer[0 .. colorMapByteDepth]).pixelReader;
}

Do the same thing here:
https://github.com/emesx/TGA/blob/develop/source/tga/io/utils.d#L5

T read(T)(File file) if(isNumeric!T){
    ubyte[T.sizeof] bytes;
    file.rawRead(s[]);
    return littleEndianToNative!T(bytes);
}


https://github.com/emesx/TGA/blob/develop/source/tga/io/readers.d#L144

Depending on your taste. The first is built in, the other needs std.algorithm.

pixel.bytes[] = chunk[];
chunk.copy(pixel.bytes);


https://github.com/emesx/TGA/blob/develop/source/tga/io/utils.d#L40

There's a version in std.algorithm:
http://dlang.org/phobos/std_algorithm.html#min


I need to go. Please don't mind any typo's and untested code ;). Didn't take a look at writers yet, readers and util need some more scrutiny, but the main theme is: eliminate unnecessary temporary GC allocations.

On Thursday, 12 June 2014 at 19:30:06 UTC, Rene Zwanenburg wrote:
> I need to go. Please don't mind any typo's and untested code ;). Didn't take a look at writers yet, readers and util need some more scrutiny, but the main theme is: eliminate unnecessary temporary GC allocations.

Thank you for feedback, hoping for more! I will apply the suggestions tonight.

On 06/12/2014 09:30 PM, Rene Zwanenburg wrote:
> I remember a function which does something like only only + canFind on
> one go. It would look something like
>
> header.colorMapDepth.among(16, 32);
>
> but I can't find it right now.. Maybe it was only proposed but never added.

http://dlang.org/library/std/algorithm/among.html

-- 
Mike Wey