Need review and testing for 'zoom-pan-testing-kazakov' branch

Friedrich W. H. Kossebau kossebau at kde.org
Sun Sep 9 15:12:56 UTC 2012


Hi Dmitry & all,

Am Sonntag, 2. September 2012, 23:34:33 schrieb Dmitry Kazakov:
> Hi!
> 
> I've just finished the branch which fixes various zoom/pan bugs in Krita
> and adds comprehensive testing for it. It would be really cool if someone
> could test it and review before I merge it to master. I send the mail to
> calligra list as well, because it touches a couple of classes in
> ./libs/flake/ as well.

I am currently seeing into fixing the offsets in the rulers if the displayed 
document is smaller then the available screensize (see yourself with low 
zooming numbers), happens in Flow, Stage and Krita.

But when seeing to fix it for Krita like done for Flow and Stage, which is 
simply subtracting the d->canvas->documentOrigin() offset like in done in 
Karbon, I came across this change in your zooming/panning fixes, as for Krita 
that does not work:

--- 8<---
 QPoint KisCanvas2::documentOrigin() const
 {
-    return m_d->coordinatesConverter->documentOrigin();
-}
-
-
-void KisCanvas2::adjustOrigin()
-{
-    QPoint newOrigin;
-
-    QSize documentSize = m_d->coordinatesConverter-
>imageRectInWidgetPixels().toAlignedRect().size();
-    QSize widgetSize = m_d->canvasWidget->widget()->size();
-
-    if(!m_d->vastScrolling) {
-        int widthDiff = widgetSize.width() - documentSize.width();
-        int heightDiff = widgetSize.height() - documentSize.height();
-
-        if (widthDiff > 0)
-            newOrigin.rx() = qRound(0.5 * widthDiff);
-        if (heightDiff > 0)
-            newOrigin.ry() = qRound(0.5 * heightDiff);
-    }
-
-    m_d->coordinatesConverter->setDocumentOrigin(newOrigin);
-
-    emit documentOriginChanged();
+    qWarning() << "Krita does not use documentOrigin() anymore. Please fix 
the code.";
+    return QPoint();
 }

 QPoint KisCanvas2::documentOrigin() const
 {
-    return m_d->coordinatesConverter->documentOrigin();
-}
-
-
-void KisCanvas2::adjustOrigin()
-{
-    QPoint newOrigin;
-
-    QSize documentSize = m_d->coordinatesConverter-
>imageRectInWidgetPixels().toAlignedRect().size();
-    QSize widgetSize = m_d->canvasWidget->widget()->size();
-
-    if(!m_d->vastScrolling) {
-        int widthDiff = widgetSize.width() - documentSize.width();
-        int heightDiff = widgetSize.height() - documentSize.height();
-
-        if (widthDiff > 0)
-            newOrigin.rx() = qRound(0.5 * widthDiff);
-        if (heightDiff > 0)
-            newOrigin.ry() = qRound(0.5 * heightDiff);
-    }
-
-    m_d->coordinatesConverter->setDocumentOrigin(newOrigin);
-
-    emit documentOriginChanged();
+    qWarning() << "Krita does not use documentOrigin() anymore. Please fix 
the code.";
+    return QPoint();
 }
--- 8<---

I am not sure this is a perfect change, as it breaks the API contract 
defclared in KoCanvasBase::documentOrigin():
--- 8<---
    /**
     * Return the position of the document origin inside the canvas widget, in 
pixels.
     * By default the origin of the canvas widget and the position of the
     * document origin are coincident, thus an empty point is returned.
     */
    virtual QPoint documentOrigin() const {
--- 8<---

So what is the policy in Calligra for this? While Krita may not use 
documentOrigin() anymore, other parts/plugins of Calligra do, but will now 
fail if used with Krita. Not perfect. Would anyone agree with me that 
KisCanvas2::documentOrigin() needs a proper implementation again?

If not, what would be the proper way now to fix 
KisZoomManager::mousePositionChanged(), where does one get the offset of the 
document in the canvas?

Cheers
Friedrich


More information about the kimageshop mailing list