BenjaminSchaaf / daffodil

A image processing library for D
MIT License
12 stars 3 forks source link

Redesign color conversion to use template checking instead of inheritance #35

Open ljubobratovicrelja opened 8 years ago

ljubobratovicrelja commented 8 years ago

As suggested on forums, I too strongly advise to use template arguments with static checking for color space definition and conversion instead of inheritance: https://github.com/BenjaminSchaaf/daffodil/blob/master/source/daffodil/color.d#L18-L24

@BenjaminSchaaf what do you think of that?

BenjaminSchaaf commented 8 years ago

The problem is, the color space needs to be determined by the image loader. So it can't be a template argument. However that should most likely be a struct of function pointers instead. I still need to determine a proper way of abstracting that into a proper extendable API.

BenjaminSchaaf commented 8 years ago

I've implemented the new color space API. A color space is defined by the ColorSpace struct. The color space is then passed along with the image range from image loaders to an Image object. Sadly, unless D implements compile time mutation, I can't template the type used in those functions as the color space needs to be passed from a dynamically dispatched loader function from the primary API. This means that any internal storage type other than real will have a performance overhead. To make up for this I've made that a default, such that using a different internal storage type is purely an optimization on memory.

Regrettably there is no other way I see of having difference color spaces as well as an extensible API.

ljubobratovicrelja commented 8 years ago

Looks cool, nice trick! Sorry if asking something obvious, how would color conversion fit in this API? Say you have RGB and HSV defined as dedicated structs, how would the conversion be defined between the two?

BenjaminSchaaf commented 8 years ago

As I haven't gotten as far as implementing a different color space I haven't implemented a conversion API (its not worth it until its properly testable, even if I could implement it), but I have given it some thought.

Ideally I'd want a system where conversions between common color spaces is directly implemented, but for implementing a new color space doesn't have to implement conversion to all others. Having an API where color spaces are registered, alongside functions specifically for conversion, its possible to find the shorted path of conversion (or possibly the one introducing the least errors) for converting between two color spaces.

An alternative would be a more manual conversion, ie. with std.algorithm.map and conversion functions defined in daffodil.colorspace

ljubobratovicrelja commented 8 years ago

Ideally I'd want a system where conversions between common color spaces is directly implemented, but for implementing a new color space doesn't have to implement conversion to all others. Having an API where color spaces are registered, alongside functions specifically for conversion, its possible to find the shorted path of conversion (or possibly the one introducing the least errors) for converting between two color spaces.

Sounds nice. But to be honest, it also sounds a bit too overcomplicated in design - there's a limited amount of commonly used and known color spaces, it's not like people come up with new ones that often. I think you need not force the extensibility in design that much. But, please don't get me wrong - It definitely sounds interesting, and If you implement a prototype, I'd gladly take a look! :)

BenjaminSchaaf commented 8 years ago

Sounds nice. But to be honest, it also sounds a bit too overcomplicated in design

My thoughts exactly. I'll implement a basic conversion API once I get to other color spaces