[krita/kazakov/animation-cache-swapping] libs/ui: Fix openGL mipmaps syncing problem on OSX

Dmitry Kazakov null at kde.org
Fri Jun 1 08:51:33 UTC 2018


Git commit e13e8f4ec2ab7a20d0eaba335241e1c9f23d3a4e by Dmitry Kazakov.
Committed on 01/06/2018 at 08:51.
Pushed by dkazakov into branch 'kazakov/animation-cache-swapping'.

Fix openGL mipmaps syncing problem on OSX

The mipmaps problem on OSX happened because we used different
openGL context for uploading textures data and rendering. With
this patch, uploading will happen in the same context, so
explicit syncing with glFinish is not needed anymore.

WARNING: this patch changes the way how we render the frames.
I have streamlined rendering of new uploaded frames and added
a FPS-based compressor before them (to avoid too much context
switches). In my opinion, the rendering became slightly faster,
but the painters might not agree :) So it needs a bit of
testing.

CC:kimageshop at kde.org

M  +1    -0    libs/ui/canvas/kis_abstract_canvas_widget.h
M  +41   -13   libs/ui/canvas/kis_canvas2.cpp
M  +13   -0    libs/ui/canvas/kis_canvas_widget_base.cpp
M  +3    -0    libs/ui/canvas/kis_canvas_widget_base.h
M  +1    -0    libs/ui/canvas/kis_qpainter_canvas.h
M  +23   -6    libs/ui/opengl/kis_opengl_canvas2.cpp
M  +1    -0    libs/ui/opengl/kis_opengl_canvas2.h

https://commits.kde.org/krita/e13e8f4ec2ab7a20d0eaba335241e1c9f23d3a4e

diff --git a/libs/ui/canvas/kis_abstract_canvas_widget.h b/libs/ui/canvas/kis_abstract_canvas_widget.h
index 46e2cb44018..c769f3cbb60 100644
--- a/libs/ui/canvas/kis_abstract_canvas_widget.h
+++ b/libs/ui/canvas/kis_abstract_canvas_widget.h
@@ -79,6 +79,7 @@ public:
 
     // Called from KisCanvas2::updateCanvasProjection
     virtual QRect updateCanvasProjection(KisUpdateInfoSP info) = 0;
+    virtual QVector<QRect> updateCanvasProjection(const QVector<KisUpdateInfoSP> &infoObjects) = 0;
 
     /**
      * Returns true if the asynchromous engine of the canvas
diff --git a/libs/ui/canvas/kis_canvas2.cpp b/libs/ui/canvas/kis_canvas2.cpp
index 416d73ef65d..77bed8a2d30 100644
--- a/libs/ui/canvas/kis_canvas2.cpp
+++ b/libs/ui/canvas/kis_canvas2.cpp
@@ -21,6 +21,9 @@
 
 #include "kis_canvas2.h"
 
+#include <functional>
+#include <numeric>
+
 #include <QApplication>
 #include <QWidget>
 #include <QVBoxLayout>
@@ -120,7 +123,7 @@ public:
     KisPrescaledProjectionSP prescaledProjection;
     bool vastScrolling;
 
-    KisSignalCompressor updateSignalCompressor;
+    KisSignalCompressor canvasUpdateCompressor;
     QRect savedUpdateRect;
 
     QBitArray channelFlags;
@@ -140,6 +143,8 @@ public:
     QPointer<KoShapeManager> currentlyActiveShapeManager;
     KisInputActionGroupsMask inputActionGroupsMask = AllActionGroup;
 
+    KisSignalCompressor frameRenderStartCompressor;
+
     KisSignalCompressor regionOfInterestUpdateCompressor;
     QRect regionOfInterest;
 
@@ -193,8 +198,11 @@ KisCanvas2::KisCanvas2(KisCoordinatesConverter *coordConverter, KoCanvasResource
 
     KisImageConfig config;
 
-    m_d->updateSignalCompressor.setDelay(1000 / config.fpsLimit());
-    m_d->updateSignalCompressor.setMode(KisSignalCompressor::FIRST_ACTIVE);
+    m_d->canvasUpdateCompressor.setDelay(1000 / config.fpsLimit());
+    m_d->canvasUpdateCompressor.setMode(KisSignalCompressor::FIRST_ACTIVE);
+
+    m_d->frameRenderStartCompressor.setDelay(1000 / config.fpsLimit());
+    m_d->frameRenderStartCompressor.setMode(KisSignalCompressor::FIRST_ACTIVE);
 }
 
 void KisCanvas2::setup()
@@ -234,7 +242,13 @@ void KisCanvas2::setup()
     connect(kritaShapeController, SIGNAL(currentLayerChanged(const KoShapeLayer*)),
             globalShapeManager()->selection(), SIGNAL(currentLayerChanged(const KoShapeLayer*)));
 
-    connect(&m_d->updateSignalCompressor, SIGNAL(timeout()), SLOT(slotDoCanvasUpdate()));
+    connect(&m_d->canvasUpdateCompressor, SIGNAL(timeout()), SLOT(slotDoCanvasUpdate()));
+
+    connect(this, SIGNAL(sigCanvasCacheUpdated()), &m_d->frameRenderStartCompressor, SLOT(start()));
+    connect(&m_d->frameRenderStartCompressor, SIGNAL(timeout()), SLOT(updateCanvasProjection()));
+
+    connect(this, SIGNAL(sigContinueResizeImage(qint32,qint32)), SLOT(finishResizingImage(qint32,qint32)));
+
     connect(&m_d->regionOfInterestUpdateCompressor, SIGNAL(timeout()), SLOT(slotUpdateRegionOfInterest()));
 
     initializeFpsDecoration();
@@ -542,10 +556,8 @@ void KisCanvas2::initializeImage()
     m_d->toolProxy.initializeImage(image);
 
     connect(image, SIGNAL(sigImageUpdated(QRect)), SLOT(startUpdateCanvasProjection(QRect)), Qt::DirectConnection);
-    connect(this, SIGNAL(sigCanvasCacheUpdated()), SLOT(updateCanvasProjection()));
     connect(image, SIGNAL(sigProofingConfigChanged()), SLOT(slotChangeProofingConfig()));
     connect(image, SIGNAL(sigSizeChanged(const QPointF&, const QPointF&)), SLOT(startResizingImage()), Qt::DirectConnection);
-    connect(this, SIGNAL(sigContinueResizeImage(qint32,qint32)), SLOT(finishResizingImage(qint32,qint32)));
     connect(image->undoAdapter(), SIGNAL(selectionChanged()), SLOT(slotTrySwitchShapeManager()));
 
     connectCurrentCanvas();
@@ -736,21 +748,37 @@ void KisCanvas2::startUpdateCanvasProjection(const QRect & rc)
 
 void KisCanvas2::updateCanvasProjection()
 {
+    QVector<KisUpdateInfoSP> infoObjects;
     while (KisUpdateInfoSP info = m_d->projectionUpdatesCompressor.takeUpdateInfo()) {
-        QRect vRect = m_d->canvasWidget->updateCanvasProjection(info);
-        if (!vRect.isEmpty()) {
-            updateCanvasWidgetImpl(m_d->coordinatesConverter->viewportToWidget(vRect).toAlignedRect());
-        }
+        infoObjects << info;
     }
 
+    QVector<QRect> viewportRects = m_d->canvasWidget->updateCanvasProjection(infoObjects);
+
+    const QRect vRect = std::accumulate(viewportRects.constBegin(), viewportRects.constEnd(),
+                                        QRect(), std::bit_or<QRect>());
+
     // TODO: Implement info->dirtyViewportRect() for KisOpenGLCanvas2 to avoid updating whole canvas
     if (m_d->currentCanvasIsOpenGL) {
-        updateCanvasWidgetImpl();
+        m_d->savedUpdateRect = QRect();
+
+        // we already had a compression in frameRenderStartCompressor, so force the update directly
+        slotDoCanvasUpdate();
+    } else if (/* !m_d->currentCanvasIsOpenGL && */ !vRect.isEmpty()) {
+        m_d->savedUpdateRect = m_d->coordinatesConverter->viewportToWidget(vRect).toAlignedRect();
+
+        // we already had a compression in frameRenderStartCompressor, so force the update directly
+        slotDoCanvasUpdate();
     }
 }
 
 void KisCanvas2::slotDoCanvasUpdate()
 {
+    /**
+     * WARNING: in isBusy() we access openGL functions without making the painting
+     * context current. We hope that currently active context will be Qt's one,
+     * which is shared with our own.
+     */
     if (m_d->canvasWidget->isBusy()) {
         // just restarting the timer
         updateCanvasWidgetImpl(m_d->savedUpdateRect);
@@ -770,11 +798,11 @@ void KisCanvas2::slotDoCanvasUpdate()
 
 void KisCanvas2::updateCanvasWidgetImpl(const QRect &rc)
 {
-    if (!m_d->updateSignalCompressor.isActive() ||
+    if (!m_d->canvasUpdateCompressor.isActive() ||
         !m_d->savedUpdateRect.isEmpty()) {
         m_d->savedUpdateRect |= rc;
     }
-    m_d->updateSignalCompressor.start();
+    m_d->canvasUpdateCompressor.start();
 }
 
 void KisCanvas2::updateCanvas()
diff --git a/libs/ui/canvas/kis_canvas_widget_base.cpp b/libs/ui/canvas/kis_canvas_widget_base.cpp
index 3f48fdbda2c..c6a85410448 100644
--- a/libs/ui/canvas/kis_canvas_widget_base.cpp
+++ b/libs/ui/canvas/kis_canvas_widget_base.cpp
@@ -39,6 +39,8 @@
 #include "KisViewManager.h"
 #include "kis_selection_manager.h"
 #include "KisDocument.h"
+#include "kis_update_info.h"
+
 
 struct KisCanvasWidgetBase::Private
 {
@@ -240,6 +242,17 @@ KisCoordinatesConverter* KisCanvasWidgetBase::coordinatesConverter() const
     return m_d->coordinatesConverter;
 }
 
+QVector<QRect> KisCanvasWidgetBase::updateCanvasProjection(const QVector<KisUpdateInfoSP> &infoObjects)
+{
+    QVector<QRect> dirtyViewRects;
+
+    Q_FOREACH (KisUpdateInfoSP info, infoObjects) {
+        dirtyViewRects << this->updateCanvasProjection(info);
+    }
+
+    return dirtyViewRects;
+}
+
 KoToolProxy *KisCanvasWidgetBase::toolProxy() const
 {
     return m_d->toolProxy;
diff --git a/libs/ui/canvas/kis_canvas_widget_base.h b/libs/ui/canvas/kis_canvas_widget_base.h
index 011e65e1be9..06d55f3db1e 100644
--- a/libs/ui/canvas/kis_canvas_widget_base.h
+++ b/libs/ui/canvas/kis_canvas_widget_base.h
@@ -79,6 +79,9 @@ public: // KisAbstractCanvasWidget
 
     KisCoordinatesConverter* coordinatesConverter() const;
 
+    QVector<QRect> updateCanvasProjection(const QVector<KisUpdateInfoSP> &infoObjects) override;
+    using KisAbstractCanvasWidget::updateCanvasProjection;
+
 protected:
     KisCanvas2 *canvas() const;
 
diff --git a/libs/ui/canvas/kis_qpainter_canvas.h b/libs/ui/canvas/kis_qpainter_canvas.h
index f4812de554a..70ef94996ef 100644
--- a/libs/ui/canvas/kis_qpainter_canvas.h
+++ b/libs/ui/canvas/kis_qpainter_canvas.h
@@ -68,6 +68,7 @@ public: // Implement kis_abstract_canvas_widget interface
     void finishResizingImage(qint32 w, qint32 h) override;
     KisUpdateInfoSP startUpdateCanvasProjection(const QRect & rc, const QBitArray &channelFlags) override;
     QRect updateCanvasProjection(KisUpdateInfoSP info) override;
+    using KisCanvasWidgetBase::updateCanvasProjection;
 
     QWidget * widget() override {
         return this;
diff --git a/libs/ui/opengl/kis_opengl_canvas2.cpp b/libs/ui/opengl/kis_opengl_canvas2.cpp
index eba77f33a22..678cfe5972f 100644
--- a/libs/ui/opengl/kis_opengl_canvas2.cpp
+++ b/libs/ui/opengl/kis_opengl_canvas2.cpp
@@ -909,19 +909,36 @@ QRect KisOpenGLCanvas2::updateCanvasProjection(KisUpdateInfoSP info)
     if (isOpenGLUpdateInfo) {
         d->openGLImageTextures->recalculateCache(info);
     }
+    return QRect(); // FIXME: Implement dirty rect for OpenGL
+}
 
+QVector<QRect> KisOpenGLCanvas2::updateCanvasProjection(const QVector<KisUpdateInfoSP> &infoObjects)
+{
 #ifdef Q_OS_OSX
     /**
-     * There is a bug on OSX: if we issue frame redraw before the tiles finished
-     * uploading, the tiles will become corrupted. Depending on the GPU/driver
-     * version either the tile itself, or its mipmaps will become totally
-     * transparent.
+     * On OSX openGL defferent (shared) contexts have different execution queues.
+     * It means that the textures uploading and their painting can be easily reordered.
+     * To overcome the issue, we should ensure that the textures are uploaded in the
+     * same openGL context as the painting is done.
      */
 
-    glFinish();
+    QOpenGLContext *oldContext = QOpenGLContext::currentContext();
+    QSurface *oldSurface = oldContext ? oldContext->surface() : 0;
+
+    this->makeCurrent();
 #endif
 
-    return QRect(); // FIXME: Implement dirty rect for OpenGL
+    QVector<QRect> result = KisCanvasWidgetBase::updateCanvasProjection(infoObjects);
+
+#ifdef Q_OS_OSX
+    if (oldContext) {
+        oldContext->makeCurrent(oldSurface);
+    } else {
+        this->doneCurrent();
+    }
+#endif
+
+    return result;
 }
 
 bool KisOpenGLCanvas2::callFocusNextPrevChild(bool next)
diff --git a/libs/ui/opengl/kis_opengl_canvas2.h b/libs/ui/opengl/kis_opengl_canvas2.h
index 9622e5e7b0b..7e177efecc3 100644
--- a/libs/ui/opengl/kis_opengl_canvas2.h
+++ b/libs/ui/opengl/kis_opengl_canvas2.h
@@ -89,6 +89,7 @@ public: // Implement kis_abstract_canvas_widget interface
     void finishResizingImage(qint32 w, qint32 h) override;
     KisUpdateInfoSP startUpdateCanvasProjection(const QRect & rc, const QBitArray &channelFlags) override;
     QRect updateCanvasProjection(KisUpdateInfoSP info) override;
+    QVector<QRect> updateCanvasProjection(const QVector<KisUpdateInfoSP> &infoObjects) override;
 
     QWidget *widget() override {
         return this;


More information about the kimageshop mailing list