rhi: gl: Avoid magic adjustments to the context/window format

...by removing the entire adjustedFormat() helper.

Qt Quick has never used this, which indicates it is not that
useful. Same goes for Qt Multimedia or Qt 3D. Ensuring depth and
stencil is requested is already solved by using
QSurfaceFormat::setDefaultFormat() or by adjusting the formats
everywhere as appropriate.

The helper function's usages are in the manual tests that use it as a
shortcut, and in the GL backend itself. Remove it and leave it up the
client to set the depth or stencil buffer size, typically in the
global default surface format. (which in fact many of the mentioned
manual tests already did, so some of calls to
window->setFormat(adjustedFormat()) were completely unnecessary)

By not having the built-in magic that tries to always force depth and
stencil, we avoid problems that arise then the helper cannot be easily
invoked (thinking of widgets and backingstores), and so one ends up
with unexpected stencil (or depth) in the context (where the GL
backend auto-adjusts), but not in the window (which is not under
QRhi's control).

It was in practice possible to trigger EGL_BAD_MATCH failures with the
new rhi-based widget composition on EGL-based systems. For example, if
an application with a QOpenGLWidget did not set both depth and stencil
(but only one, or none), it ended up failing due to the context -
surface EGLConfig mismatches. On other platforms this matters less due
to less strict config/pixelformat management.

Pick-to: 6.4
Change-Id: I28ae2de163de63ee91bee3ceae08b58e106e1380
Fixes: QTBUG-104951
Reviewed-by: Andy Nichols <andy.nichols@qt.io>

8 files changed, 30 insertions, 46 deletions

diff --git a/src/gui/rhi/qrhigles2.cpp b/src/gui/rhi/qrhigles2.cpp
index 0912ed077f..4c079e0ace 100644
--- a/src/gui/rhi/qrhigles2.cpp
+++ b/src/gui/rhi/qrhigles2.cpp
@@ -54,14 +54,14 @@ QT_BEGIN_NAMESPACE
 
     The QSurfaceFormat for the context is specified in \l format. The
     constructor sets this to QSurfaceFormat::defaultFormat() so applications
-    that use QSurfaceFormat::setDefaultFormat() do not need to set the format
-    again.
+    that call QSurfaceFormat::setDefaultFormat() with the appropriate settings
+    before the constructor runs will not need to change value of \l format.
 
-    \note The depth and stencil buffer sizes are set automatically to 24 and 8
-    when no size was explicitly set for these buffers in \l format. As there
-    are possible adjustments to \l format, applications can use
-    adjustedFormat() to query the effective format that is passed to
-    QOpenGLContext::setFormat() internally.
+    \note Remember to set the depth and stencil buffer sizes to 24 and 8 when
+    the renderer relies on depth or stencil testing, either in the global
+    default QSurfaceFormat, or, alternatively, separately in all the involved
+    QSurfaceFormat instances: in \l format, the format argument passed to
+    newFallbackSurface(), and on any QWindow that is used with the QRhi.
 
     A QSurface has to be specified in \l fallbackSurface. In order to prevent
     mistakes in threaded situations, this is never created automatically by the
@@ -447,28 +447,12 @@ QRhiGles2InitParams::QRhiGles2InitParams()
 }
 
 /*!
-    \return the QSurfaceFormat that will be set on the QOpenGLContext before
-    calling QOpenGLContext::create(). This format is based on \a format, but
-    may be adjusted. Applicable only when QRhi creates the context.
-    Applications are advised to set this format on their QWindow in order to
-    avoid potential BAD_MATCH failures.
- */
-QSurfaceFormat QRhiGles2InitParams::adjustedFormat(const QSurfaceFormat &format)
-{
-    QSurfaceFormat fmt = format;
-
-    if (fmt.depthBufferSize() == -1)
-        fmt.setDepthBufferSize(24);
-    if (fmt.stencilBufferSize() == -1)
-        fmt.setStencilBufferSize(8);
-
-    return fmt;
-}
-
-/*!
     \return a new QOffscreenSurface that can be used with a QRhi by passing it
     via a QRhiGles2InitParams.
 
+    When \a format is not specified, its default value is the global default
+    format settable via QSurfaceFormat::setDefaultFormat().
+
     \a format is adjusted as appropriate in order to avoid having problems
     afterwards due to an incompatible context and surface.
 
@@ -480,7 +464,7 @@ QSurfaceFormat QRhiGles2InitParams::adjustedFormat(const QSurfaceFormat &format)
  */
 QOffscreenSurface *QRhiGles2InitParams::newFallbackSurface(const QSurfaceFormat &format)
 {
-    QSurfaceFormat fmt = adjustedFormat(format);
+    QSurfaceFormat fmt = format;
 
     // To resolve all fields in the format as much as possible, create a context.
     // This may be heavy, but allows avoiding BAD_MATCH on some systems.
@@ -501,7 +485,7 @@ QOffscreenSurface *QRhiGles2InitParams::newFallbackSurface(const QSurfaceFormat
 QRhiGles2::QRhiGles2(QRhiGles2InitParams *params, QRhiGles2NativeHandles *importDevice)
     : ofr(this)
 {
-    requestedFormat = QRhiGles2InitParams::adjustedFormat(params->format);
+    requestedFormat = params->format;
     fallbackSurface = params->fallbackSurface;
     maybeWindow = params->window; // may be null
     maybeShareContext = params->shareContext; // may be null
diff --git a/src/gui/rhi/qrhigles2_p.h b/src/gui/rhi/qrhigles2_p.h
index f7570ae01c..e3f4fbe7d7 100644
--- a/src/gui/rhi/qrhigles2_p.h
+++ b/src/gui/rhi/qrhigles2_p.h
@@ -35,7 +35,6 @@ struct Q_GUI_EXPORT QRhiGles2InitParams : public QRhiInitParams
     QOpenGLContext *shareContext = nullptr;
 
     static QOffscreenSurface *newFallbackSurface(const QSurfaceFormat &format = QSurfaceFormat::defaultFormat());
-    static QSurfaceFormat adjustedFormat(const QSurfaceFormat &format = QSurfaceFormat::defaultFormat());
 };
 
 struct Q_GUI_EXPORT QRhiGles2NativeHandles : public QRhiNativeHandles
diff --git a/tests/auto/gui/rhi/qrhi/tst_qrhi.cpp b/tests/auto/gui/rhi/qrhi/tst_qrhi.cpp
index 2b4bf4a971..76527f83ed 100644
--- a/tests/auto/gui/rhi/qrhi/tst_qrhi.cpp
+++ b/tests/auto/gui/rhi/qrhi/tst_qrhi.cpp
@@ -165,6 +165,12 @@ private:
 void tst_QRhi::initTestCase()
 {
 #ifdef TST_GL
+    QSurfaceFormat fmt;
+    fmt.setDepthBufferSize(24);
+    fmt.setStencilBufferSize(8);
+    QSurfaceFormat::setDefaultFormat(fmt);
+
+    initParams.gl.format = QSurfaceFormat::defaultFormat();
     fallbackSurface = QRhiGles2InitParams::newFallbackSurface();
     initParams.gl.fallbackSurface = fallbackSurface;
 #endif
@@ -710,7 +716,6 @@ void tst_QRhi::nativeHandlesImportOpenGL()
 #ifdef TST_GL
     QRhiGles2NativeHandles h;
     QScopedPointer<QOpenGLContext> ctx(new QOpenGLContext);
-    ctx->setFormat(QRhiGles2InitParams::adjustedFormat());
     if (!ctx->create())
         QSKIP("No OpenGL context, skipping OpenGL-specific test");
     h.context = ctx.data();
@@ -3269,7 +3274,6 @@ void tst_QRhi::setWindowType(QWindow *window, QRhi::Implementation impl)
     switch (impl) {
 #ifdef TST_GL
     case QRhi::OpenGLES2:
-        window->setFormat(QRhiGles2InitParams::adjustedFormat());
         window->setSurfaceType(QSurface::OpenGLSurface);
         break;
 #endif
diff --git a/tests/manual/rhi/hellominimalcrossgfxtriangle/window.cpp b/tests/manual/rhi/hellominimalcrossgfxtriangle/window.cpp
index 2a3475538d..c75dd73908 100644
--- a/tests/manual/rhi/hellominimalcrossgfxtriangle/window.cpp
+++ b/tests/manual/rhi/hellominimalcrossgfxtriangle/window.cpp
@@ -11,9 +11,6 @@ Window::Window(QRhi::Implementation graphicsApi)
     switch (graphicsApi) {
     case QRhi::OpenGLES2:
         setSurfaceType(OpenGLSurface);
-#if QT_CONFIG(opengl)
-        setFormat(QRhiGles2InitParams::adjustedFormat());
-#endif
         break;
     case QRhi::Vulkan:
         setSurfaceType(VulkanSurface);
diff --git a/tests/manual/rhi/multiwindow/multiwindow.cpp b/tests/manual/rhi/multiwindow/multiwindow.cpp
index 0240c21a2f..f25a68c621 100644
--- a/tests/manual/rhi/multiwindow/multiwindow.cpp
+++ b/tests/manual/rhi/multiwindow/multiwindow.cpp
@@ -268,17 +268,15 @@ Window::Window(const QString &title, const QColor &bgColor, int axis, bool noVSy
     switch (graphicsApi) {
     case OpenGL:
     {
-#if QT_CONFIG(opengl)
         setSurfaceType(OpenGLSurface);
-        QSurfaceFormat fmt = QRhiGles2InitParams::adjustedFormat(); // default + depth/stencil
-        fmt.setSwapInterval(noVSync ? 0 : 1); // + swap interval
+        QSurfaceFormat fmt = QSurfaceFormat::defaultFormat();
+        fmt.setSwapInterval(noVSync ? 0 : 1);
         setFormat(fmt);
-#endif
     }
         break;
     case Vulkan:
-#if QT_CONFIG(vulkan)
         setSurfaceType(VulkanSurface);
+#if QT_CONFIG(vulkan)
         setVulkanInstance(r.instance);
 #endif
         break;
@@ -511,6 +509,10 @@ int main(int argc, char **argv)
     qDebug("Selected graphics API is %s", qPrintable(graphicsApiName()));
     qDebug("This is a multi-api example, use command line arguments to override:\n%s", qPrintable(cmdLineParser.helpText()));
 
+    QSurfaceFormat fmt;
+    fmt.setDepthBufferSize(24);
+    QSurfaceFormat::setDefaultFormat(fmt);
+
 #if QT_CONFIG(vulkan)
     r.instance = new QVulkanInstance;
     if (graphicsApi == Vulkan) {
diff --git a/tests/manual/rhi/multiwindow_threaded/multiwindow_threaded.cpp b/tests/manual/rhi/multiwindow_threaded/multiwindow_threaded.cpp
index 6546d122d2..ef690ca828 100644
--- a/tests/manual/rhi/multiwindow_threaded/multiwindow_threaded.cpp
+++ b/tests/manual/rhi/multiwindow_threaded/multiwindow_threaded.cpp
@@ -692,6 +692,10 @@ int main(int argc, char **argv)
     qDebug("Selected graphics API is %s", qPrintable(graphicsApiName()));
     qDebug("This is a multi-api example, use command line arguments to override:\n%s", qPrintable(cmdLineParser.helpText()));
 
+    QSurfaceFormat fmt;
+    fmt.setDepthBufferSize(24);
+    QSurfaceFormat::setDefaultFormat(fmt);
+
 #if QT_CONFIG(vulkan)
     instance = new QVulkanInstance;
     if (graphicsApi == Vulkan) {
diff --git a/tests/manual/rhi/multiwindow_threaded/window.cpp b/tests/manual/rhi/multiwindow_threaded/window.cpp
index 9c0d5bfdf9..ea098b7f37 100644
--- a/tests/manual/rhi/multiwindow_threaded/window.cpp
+++ b/tests/manual/rhi/multiwindow_threaded/window.cpp
@@ -16,14 +16,11 @@ Window::Window(const QString &title, GraphicsApi api)
 {
     switch (api) {
     case OpenGL:
-#if QT_CONFIG(opengl)
         setSurfaceType(OpenGLSurface);
-        setFormat(QRhiGles2InitParams::adjustedFormat());
-#endif
         break;
     case Vulkan:
-#if QT_CONFIG(vulkan)
         setSurfaceType(VulkanSurface);
+#if QT_CONFIG(vulkan)
         setVulkanInstance(instance);
 #endif
         break;
diff --git a/tests/manual/rhi/shared/examplefw.h b/tests/manual/rhi/shared/examplefw.h
index 33a7e4ea48..4f49084537 100644
--- a/tests/manual/rhi/shared/examplefw.h
+++ b/tests/manual/rhi/shared/examplefw.h
@@ -135,10 +135,7 @@ Window::Window()
     // Tell the platform plugin what we want.
     switch (graphicsApi) {
     case OpenGL:
-#if QT_CONFIG(opengl)
         setSurfaceType(OpenGLSurface);
-        setFormat(QRhiGles2InitParams::adjustedFormat());
-#endif
         break;
     case Vulkan:
         setSurfaceType(VulkanSurface);