Branch: krita-canvasrotation-silvioheinrich

Dmitry Kazakov dimula73 at gmail.com
Thu Feb 3 13:37:07 CET 2011


Ok. That's a really cool patch, Silvio! You are awesome! =) But there are a
few things to fix.

1) You didn't compile the tests. There are tests for KisCoordinatesConverter
and KisPrescaledProjection. They test public interface of the converter.
They all should pass before we can say the patch is safe for the master. To
activate the tests you need to set KDE4_BUILD_TESTS=TRUE in cmake.

2) I don't really approve the way you use m_postprocessingTransform. You
aggregated all the image transformations inside this structure, that is not
really good, because this transform does not represent any real-world
coordinate system (c.s.) anymore (in your patch). In the previous version
every QTransform represented a conversion between two distinct coordinate
systems (out of 6 we have in Krita). And m_postprocessingTransform was only
in charge of canvas effects like rotation and mirroring. You can read about
them in [0] and see a picture in attachment. Now you are forced to use such
a code to change relative position of one c.s. against another one:

    m_d->postprocessingTransform *=
QTransform::fromTranslate(-center.x(),-center.y());
    m_d->postprocessingTransform *= m_d->rotation.inverted();
    m_d->postprocessingTransform *= mirror;
    m_d->postprocessingTransform *= m_d->rotation;
    m_d->postprocessingTransform *=
QTransform::fromTranslate(center.x(),center.y());

I would really like you split these transformation into the distinct ones
(representing distinct c.s.'s). Or, maybe, you had some good reason to merge
them?

3) In kis_qpainter_canvas.cpp:122 you use .inverted() method instead of
using corresponding method directly. I guess, this is a misprint =)

4) There is a bug:
- open image
- rotate the canvas a bit
- try zooming in/out using the mouse wheel -- you'll see the insane dancing
of the canvas on a screen ;)


[0] -
http://old.nabble.com/Re%3A-Canvas-rotation-preliminary-patch-p29430465.html

-- 
Dmitry Kazakov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kimageshop/attachments/20110203/555c27aa/attachment-0001.htm 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: krita_axes.png
Type: image/png
Size: 1662264 bytes
Desc: not available
Url : http://mail.kde.org/pipermail/kimageshop/attachments/20110203/555c27aa/attachment-0001.png 


More information about the kimageshop mailing list