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

List:       kde-kimageshop
Subject:    [krita/kazakov/animation-cache-swapping] libs/ui: Fix openGL mipmaps syncing problem on OSX
From:       Dmitry Kazakov <null () kde ! org>
Date:       2018-06-01 8:51:33
Message-ID: E1fOfmX-0000M0-C4 () code ! kde ! org
[Download RAW message or body]

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@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;


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