[prev in list] [next in list] [prev in thread] [next in thread] 

List:       koffice-devel
Subject:    Re: Review Request: The patch for KCurve and KisPerChannelFilter
From:       "Dmitry Kazakov" <dimula73 () gmail ! com>
Date:       2009-02-20 19:17:41
Message-ID: 20090220191741.14729.15811 () localhost
[Download RAW message or body]



> On 2009-02-20 02:02:29, Boudewijn Rempt wrote:
> > About the change of curve type, I'm prepared to take anybodies word for it. We \
> > have had complaints about the curve type before, although I cannot find the \
> > comments right now. 
> > Note that we also use the curve widget in other places, for instance the brush \
> > engine tablet response curve. We need to check that it's doing the right thing \
> > there, too. 
> > Nits: why this change:
> > virtual ~KCurve();  279     /*virtual*/ ~KCurve();
> > 
> > Also, generally speaking, we try to define vars as close to the place they are \
> > used as possible, so preferably (for int i; instead of int i; for (i;, e.g. \
> > around line 393. 
> > KTridiagonalSystem -> KTriDiagonalSystem
> > 
> > But these are just nits. 
> > 
> > I would like to do the following at the same time:
> > 
> > * rename the classes and files to KisXXX and kis_xxx, instead of KXXX and kxxx
> > * move the template classes KTriDiagonalSystem and KCubicSpline system to a \
> > private header, kis_curve_widget_p.h, since they are not used outside the curve \
> >                 widget (as far as I can see))
> > * move the m_ variables in to a d-pointer class (because it's public api)
> > 
> > I will be testing this patch (and the other one) as soon as they are done \
> > compiling, but in general, given that there are no new strings, I'd like this in \
> > for 2.0. I also think that Dmitry should start applying for an svn account :-)

Curve type.
The first issue is Compatibility. Photoshop, Gimp, and many-many-others use splines. \
The previous curve worked quite strange! It even had broken first derivative, so it \
was more like freehand curve painting.

The only thing i can say for the previous curve is back compatibility with curves \
used for calibrating pens. Waiting for the messages from pen users.

> > /*virtual*/ ~KCurve();
=( simply a bug. Should be uncommented.

> > int i;
ok

> > KTridiagonalSystem -> KTriDiagonalSystem
tridiagonal system is a mathematical term =) And it's written in one word.

> > KisXXX - changes throughout the tree?
ok

> > kis_curve_widget_p.h
ok. didn't know how to call.

> > d-pointer class
where can i read about d-pointers? I've heard about them, but don't know anything... \
=(


- Dmitry


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/128/#review191
-----------------------------------------------------------


On 2009-02-19 13:30:40, Dmitry Kazakov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/128/
> -----------------------------------------------------------
> 
> (Updated 2009-02-19 13:30:40)
> 
> 
> Review request for KOffice.
> 
> 
> Summary
> -------
> 
> This patch goes together with the next one!
> http://reviewboard.kde.org/r/129/
> 
> What is done:
> KCurve:
> - implemented cubic splines in KCurve widget. Now all the calculations are made \
>                 inside KCubicSpline template.
> - faster calculations of the spline because of caching of the coefficients inside \
>                 the template
> - faster (*much faster*) redrawing of the curve because of caching of the _scaled_ \
>                 @m_pix histogram image in @m_pixmapCache object
> - fixed 'out_of_range' bug when you tried to delete the last point of the curve
> 
> KCurve+KisPerChannelWidget
> - added In/Out spinboxes like in photoshop to allow user to modify the curve with \
>                 keyboard
> - with spinboxes you can move the point to any place of the curve, so you can \
>                 change the order of the points
> - added handy function that creates new point in the middle of the curve and \
>                 focuses on spinboxes to set it
> - and a few small fixes
> 
> 
> The fist patch changes KCurve class. The second one makes KisPerChannelFilter use \
> new features. 
> 
> PS:
> What is planned to be done:
> 1) Look through preview mechanism, because it still slows everything down much.
> 2) Let user unselect the point of the curve by clicking on empty area
> 2,5) Hide In/Out controls when nothing selected
> 3) Far-far-future: color sampler mode for the curve (clicking on the canvas shows \
> the point on the curve) 
> 
> Diffs
> -----
> 
> trunk/koffice/krita/ui/widgets/kcurve.h 928664 
> trunk/koffice/krita/ui/widgets/kcurve.cc 928664 
> 
> Diff: http://reviewboard.kde.org/r/128/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Dmitry
> 
> 

_______________________________________________
koffice-devel mailing list
koffice-devel@kde.org
https://mail.kde.org/mailman/listinfo/koffice-devel


[prev in list] [next in list] [prev in thread] [next in thread]