inheritance in Pigment

Cyrille Berger cberger at cberger.net
Mon Sep 4 23:20:48 CEST 2006


You have raised the issue just slightly before me :) but I have been giving a 
lot of thought about this. I think we need to use templates. Before anyone 
start flaming they don't want templates in their application, of course, I 
keep the KoColorSpace as it is. That means, applications will never see those 
templates.

The level of copy/paste code in pigment and in the colorspaces is simply 
outstanding, using PMD/CPD (http://pmd.sourceforge.net/cpd.html), I have 
around 3500, you can find the output here : 
http://cyrille.diwi.org/tmp/krita/duplicatecodecolorspaces.txt (I have 
excluded, the wetcolorspaces from the statistic), and the library + the 
colorspaces have around 15213 lines of code, which means that around 23% of 
the code is a duplicate. That's a lot, and PMD/CPD missed a lot of the 
duplicate. I said that to demonstrate the need for a big refactor of the 
internal of the library.

Furthermore, some functions are implemented in KisColorSpaceAbstract using a 
fallback mechanism to QColor (for instance the convolve function) destroying 
the whole point of the library. The main reason of that is the lazyness of 
the developers. But seriously, how can we blame them (including me) ? This 
week-end, to solve a problem, I thought I had to add a new function to 
KoColorSpace, all the colorspaces would have had exactly the same mathematic. 
So I wrote it as a templates, but still I had to wrote the call to that 
function for the 14 colorspaces, with a small adjustement to template 
parameters for each, it tooks me at least an hour of copy/paste, adjusting 
parameters, and then checking them, make it compile... and how many bugs I 
might have add to pigment if I hadn't choosen to remove the code because in 
the end it was useless ?

Our experience with kritacolor has shown, that a lot of the mathematics of 
each colorspace is identical. What Boudjewin propose would fix a little bit 
of that problem, but still, we will need four identical implementations : one 
for integer 8bit and 16bits, float 16bits, and 32bits.

That's where templates have their role.

So here is what I propose:

KoColorSpace (the actual pure virtual used by applications)
   |
KoColorSpaceAbstract< class T, int numberofchannels>
(contains all the generic maths, and if those maths can be generic 
(brightnesscoloradjustement for instance) then offers a fallback mechanism to 
lab, using the from/toLAB )
  |                                             \
KoColorSpaceLCMS            KoColorSpaceLMS32F,YCbCr8U, YCbCr16U...
< class T, int numberofchannels>
 |
RGB8,16... CMYK, LAB...

If there is some maths, that are specific to RGBFLOAT (hdr stuff), then create 
a KisColorSpaceRGBFLOAT<T> : public KoColorSpaceLCMS<T,3> with the specific 
code.

That's said I am ready to do the hard work for this.

And bonus question, I wonder if the fallback to LAB shouldn't be 32bits, I see 
no point in limiting ourself to 16bits.

-- 
--- Cyrille Berger ---


More information about the kimageshop mailing list