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