[krita/krita/4.0] libs/ui: Fix openGL mipmaps syncing problem on OSX

Dmitry Kazakov null at kde.org
Mon Jun 4 08:00:35 UTC 2018


Git commit f38a5013e8677cb7bbab952de4edd7e652e98104 by Dmitry Kazakov.
Committed on 04/06/2018 at 07:59.
Pushed by dkazakov into branch 'krita/4.0'.

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  +40   -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/f38a5013e8677cb7bbab952de4edd7e652e98104

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 06b5e53c4f3..3d9d0bf185b 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>
@@ -117,7 +120,7 @@ public:
     KisPrescaledProjectionSP prescaledProjection;
     bool vastScrolling;
 
-    KisSignalCompressor updateSignalCompressor;
+    KisSignalCompressor canvasUpdateCompressor;
     QRect savedUpdateRect;
 
     QBitArray channelFlags;
@@ -137,6 +140,8 @@ public:
     QPointer<KoShapeManager> currentlyActiveShapeManager;
     KisInputActionGroupsMask inputActionGroupsMask = AllActionGroup;
 
+    KisSignalCompressor frameRenderStartCompressor;
+
     bool effectiveLodAllowedInCanvas() {
         return lodAllowedInCanvas && !bootstrapLodBlocked;
     }
@@ -185,8 +190,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()
@@ -226,7 +234,12 @@ 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)));
 
     initializeFpsDecoration();
 }
@@ -533,10 +546,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();
@@ -727,21 +738,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);
@@ -761,11 +788,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 6d2fac96463..c6643dec056 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
 {
@@ -238,6 +240,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 0de410bafb8..97cccb1072e 100644
--- a/libs/ui/opengl/kis_opengl_canvas2.cpp
+++ b/libs/ui/opengl/kis_opengl_canvas2.cpp
@@ -888,19 +888,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