From 2decbe04170842c23119dc752d45f51021773836 Mon Sep 17 00:00:00 2001
From: Laszlo Agocs <laszlo.agocs@qt.io>
Date: Wed, 19 Oct 2022 14:18:34 +0200
Subject: [PATCH] rhi: gl: Apply a workaround for macOS

We do not yet have a solution on the platform plugin level. All we can
tell so far is calling clearDrawable when making an Offscreen surface
current has unintended effects in certain situations. As well behaved
clients pass in a window when creating the QRhi, we can try to prefer
that in place of our QOffscreenSurface while the window is still
valid. However, to not potentially deoptimize on other platforms
(e.g. where surfaceless contexts are a thing), do this only for
macOS for now.

Fixes: QTBUG-107666
Change-Id: I23c7340a769f474712f7f6d7bb191c70aeec3924
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Andy Nichols <andy.nichols@qt.io>
---
 src/gui/rhi/qrhigles2.cpp   | 22 +++++++++++++++++++---
 src/gui/rhi/qrhigles2_p_p.h |  7 ++++---
 2 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/src/gui/rhi/qrhigles2.cpp b/src/gui/rhi/qrhigles2.cpp
index a8b7919647..e03bdb7d04 100644
--- a/src/gui/rhi/qrhigles2.cpp
+++ b/src/gui/rhi/qrhigles2.cpp
@@ -2,7 +2,6 @@
 // SPDX-License-Identifier: LicenseRef-Qt-Commercial OR LGPL-3.0-only OR GPL-2.0-only OR GPL-3.0-only
 
 #include "qrhigles2_p_p.h"
-#include <QWindow>
 #include <QOffscreenSurface>
 #include <QOpenGLContext>
 #include <QtCore/qmap.h>
@@ -515,15 +514,32 @@ static inline QSurface *currentSurfaceForCurrentContext(QOpenGLContext *ctx)
     return currentSurface;
 }
 
+QSurface *QRhiGles2::evaluateFallbackSurface() const
+{
+    // With Apple's deprecated OpenGL support we need to minimize the usage of
+    // QOffscreenSurface since delicate problems can pop up with
+    // NSOpenGLContext and drawables.
+#if defined(Q_OS_MACOS)
+    return maybeWindow && maybeWindow->handle() ? static_cast<QSurface *>(maybeWindow) : fallbackSurface;
+#else
+    return fallbackSurface;
+#endif
+}
+
 bool QRhiGles2::ensureContext(QSurface *surface) const
 {
     if (!surface) {
+        // null means any surface is good because not going to render
         if (currentSurfaceForCurrentContext(ctx))
             return true;
-        surface = fallbackSurface;
+        // if the context is not already current with a valid surface, use our
+        // fallback surface, but platform specific quirks may apply
+        surface = evaluateFallbackSurface();
     } else if (surface->surfaceClass() == QSurface::Window && !surface->surfaceHandle()) {
-        surface = fallbackSurface;
+        // the window is not usable anymore (no native window underneath), behave as if offscreen
+        surface = evaluateFallbackSurface();
     } else if (!needsMakeCurrentDueToSwap && currentSurfaceForCurrentContext(ctx) == surface) {
+        // bail out if the makeCurrent is not necessary
         return true;
     }
     needsMakeCurrentDueToSwap = false;
diff --git a/src/gui/rhi/qrhigles2_p_p.h b/src/gui/rhi/qrhigles2_p_p.h
index 9efa659863..69164e5ebf 100644
--- a/src/gui/rhi/qrhigles2_p_p.h
+++ b/src/gui/rhi/qrhigles2_p_p.h
@@ -20,8 +20,8 @@
 #include "qshaderdescription_p.h"
 #include <qopengl.h>
 #include <QByteArray>
-#include <QSurface>
-
+#include <QWindow>
+#include <QPointer>
 #include <QtCore/private/qduplicatetracker_p.h>
 
 QT_BEGIN_NAMESPACE
@@ -816,6 +816,7 @@ public:
     void setPipelineCacheData(const QByteArray &data) override;
 
     bool ensureContext(QSurface *surface = nullptr) const;
+    QSurface *evaluateFallbackSurface() const;
     void executeDeferredReleases();
     void trackedBufferBarrier(QGles2CommandBuffer *cbD, QGles2Buffer *bufD, QGles2Buffer::Access access);
     void trackedImageBarrier(QGles2CommandBuffer *cbD, QGles2Texture *texD, QGles2Texture::Access access);
@@ -879,7 +880,7 @@ public:
     bool importedContext = false;
     QSurfaceFormat requestedFormat;
     QSurface *fallbackSurface = nullptr;
-    QWindow *maybeWindow = nullptr;
+    QPointer<QWindow> maybeWindow = nullptr;
     QOpenGLContext *maybeShareContext = nullptr;
     mutable bool needsMakeCurrentDueToSwap = false;
     QOpenGLExtensions *f = nullptr;