From 868324d1e458113ae6bf2c1c3a71d1df13e092a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tinja=20Paavosepp=C3=A4?= Date: Tue, 9 Apr 2024 16:03:15 +0300 Subject: [PATCH] Android: Guard EGLSurface with a mutex QAndroidPlatformOpenGlWindow creates an EGLSurface to wrap the native Android Surface. Once this EGLSurface is returned to QPlatformEglContext, nothing is guarding it anymore, and Android can freely destroy the underlying Surface. This leads to failed EGL operations when leaving from the app or returning to it, as that is when Android destroys/recreates the Surface. Use the mutex from QAndroidPlatformWindow to guard setting the Surface, and during the makeCurrent() and swapBuffers() calls. Locking until returning from these functions ensures Android cannot destroy the Surface in the middle of them. Task-number: QTBUG-118231 Change-Id: I614575c42f7f0c2c17022994d3bc542726ebf039 Reviewed-by: Assam Boudjelthia (cherry picked from commit e5513a9447771dc722ca27e553fd3e966ee7d44a) Reviewed-by: Qt Cherry-pick Bot --- .../android/qandroidplatformopenglcontext.cpp | 30 ++++++++++++++++--- .../android/qandroidplatformopenglwindow.cpp | 19 ++++++------ .../android/qandroidplatformwindow.cpp | 10 +++---- .../android/qandroidplatformwindow.h | 11 +++++-- 4 files changed, 47 insertions(+), 23 deletions(-) diff --git a/src/plugins/platforms/android/qandroidplatformopenglcontext.cpp b/src/plugins/platforms/android/qandroidplatformopenglcontext.cpp index faccf8b1173..fbfbb625dd2 100644 --- a/src/plugins/platforms/android/qandroidplatformopenglcontext.cpp +++ b/src/plugins/platforms/android/qandroidplatformopenglcontext.cpp @@ -22,19 +22,41 @@ QAndroidPlatformOpenGLContext::QAndroidPlatformOpenGLContext(const QSurfaceForma void QAndroidPlatformOpenGLContext::swapBuffers(QPlatformSurface *surface) { - if (surface->surface()->surfaceClass() == QSurface::Window && - static_cast(surface)->checkNativeSurface(eglConfig())) { - QEGLPlatformContext::makeCurrent(surface); + if (surface->surface()->surfaceClass() != QSurface::Window) { + QEGLPlatformContext::swapBuffers(surface); + return; } + QAndroidPlatformOpenGLWindow *window = static_cast(surface); + // Since QEGLPlatformContext::makeCurrent() and QEGLPlatformContext::swapBuffers() + // will be using the eglSurface of the window, which wraps the Android Surface, we + // need to lock here to make sure we don't end up using a Surface already destroyed + // by Android + window->lockSurface(); + + if (window->checkNativeSurface(eglConfig())) { + // Call base class implementation directly since we are already locked + QEGLPlatformContext::makeCurrent(surface); + } QEGLPlatformContext::swapBuffers(surface); + + window->unlockSurface(); } bool QAndroidPlatformOpenGLContext::makeCurrent(QPlatformSurface *surface) { - return QEGLPlatformContext::makeCurrent(surface); + if (surface->surface()->surfaceClass() != QSurface::Window) + return QEGLPlatformContext::makeCurrent(surface); + + QAndroidPlatformOpenGLWindow *window = static_cast(surface); + window->lockSurface(); + const bool ok = QEGLPlatformContext::makeCurrent(surface); + window->unlockSurface(); + return ok; } +// Called from inside QEGLPlatformContext::swapBuffers() and QEGLPlatformContext::makeCurrent(), +// already locked EGLSurface QAndroidPlatformOpenGLContext::eglSurfaceForPlatformSurface(QPlatformSurface *surface) { if (surface->surface()->surfaceClass() == QSurface::Window) { diff --git a/src/plugins/platforms/android/qandroidplatformopenglwindow.cpp b/src/plugins/platforms/android/qandroidplatformopenglwindow.cpp index c052b004131..77b486cce88 100644 --- a/src/plugins/platforms/android/qandroidplatformopenglwindow.cpp +++ b/src/plugins/platforms/android/qandroidplatformopenglwindow.cpp @@ -50,6 +50,8 @@ void QAndroidPlatformOpenGLWindow::setGeometry(const QRect &rect) } } +// Called by QAndroidPlatformOpenGLContext::eglSurfaceForPlatformSurface(), +// surface is already locked when calling this EGLSurface QAndroidPlatformOpenGLWindow::eglSurface(EGLConfig config) { if (QAndroidEventDispatcherStopper::stopped() || @@ -57,8 +59,6 @@ EGLSurface QAndroidPlatformOpenGLWindow::eglSurface(EGLConfig config) return m_eglSurface; } - QMutexLocker lock(&m_surfaceMutex); - if (!m_surfaceCreated) { AndroidDeadlockProtector protector; if (!protector.acquire()) @@ -69,25 +69,24 @@ EGLSurface QAndroidPlatformOpenGLWindow::eglSurface(EGLConfig config) } if (m_eglSurface == EGL_NO_SURFACE) { - m_surfaceMutex.unlock(); checkNativeSurface(config); - m_surfaceMutex.lock(); } return m_eglSurface; } +// Only called by eglSurface() and QAndroidPlatformOpenGLContext::swapBuffers(), +// m_surfaceMutex already locked bool QAndroidPlatformOpenGLWindow::checkNativeSurface(EGLConfig config) { - QMutexLocker lock(&m_surfaceMutex); + // Either no surface created, or the m_eglSurface already wraps the active Surface + // -> makeCurrent is NOT needed. if (!m_surfaceCreated || !m_androidSurfaceObject.isValid()) - return false; // makeCurrent is NOT needed. + return false; createEgl(config); - // we've create another surface, the window should be repainted - QRect availableGeometry = screen()->availableGeometry(); - if (geometry().width() > 0 && geometry().height() > 0 && availableGeometry.width() > 0 && availableGeometry.height() > 0) - QWindowSystemInterface::handleExposeEvent(window(), QRegion(QRect(QPoint(), geometry().size()))); + // we've created another Surface, the window should be repainted + sendExpose(); return true; // makeCurrent is needed! } diff --git a/src/plugins/platforms/android/qandroidplatformwindow.cpp b/src/plugins/platforms/android/qandroidplatformwindow.cpp index e86e617ca3e..7f43f175079 100644 --- a/src/plugins/platforms/android/qandroidplatformwindow.cpp +++ b/src/plugins/platforms/android/qandroidplatformwindow.cpp @@ -304,14 +304,12 @@ void QAndroidPlatformWindow::onSurfaceChanged(QtJniTypes::Surface surface) { lockSurface(); m_androidSurfaceObject = surface; - if (m_androidSurfaceObject.isValid()) // wait until we have a valid surface to draw into - m_surfaceWaitCondition.wakeOne(); - unlockSurface(); - if (m_androidSurfaceObject.isValid()) { - // repaint the window, when we have a valid surface - sendExpose(); + // wait until we have a valid surface to draw into + m_surfaceWaitCondition.wakeOne(); } + + unlockSurface(); } void QAndroidPlatformWindow::sendExpose() const diff --git a/src/plugins/platforms/android/qandroidplatformwindow.h b/src/plugins/platforms/android/qandroidplatformwindow.h index 221b98fce55..e493875beef 100644 --- a/src/plugins/platforms/android/qandroidplatformwindow.h +++ b/src/plugins/platforms/android/qandroidplatformwindow.h @@ -67,10 +67,11 @@ public: static bool registerNatives(QJniEnvironment &env); void onSurfaceChanged(QtJniTypes::Surface surface); -protected: - void setGeometry(const QRect &rect) override; void lockSurface() { m_surfaceMutex.lock(); } void unlockSurface() { m_surfaceMutex.unlock(); } + +protected: + void setGeometry(const QRect &rect) override; void createSurface(); void destroySurface(); void sendExpose() const; @@ -85,7 +86,11 @@ protected: QtJniTypes::QtWindow m_nativeQtWindow; SurfaceContainer m_surfaceContainerType = SurfaceContainer::SurfaceView; QtJniTypes::QtWindow m_nativeParentQtWindow; - // The Android Surface, accessed from multiple threads, guarded by m_surfaceMutex + // The Android Surface, accessed from multiple threads, guarded by m_surfaceMutex. + // If the window is using QtSurface, which is a SurfaceView subclass, this Surface will be + // automatically created by Android when QtSurface is in a layout and visible. If the + // QtSurface is detached or hidden (app goes to background), Android will automatically + // destroy the Surface. QtJniTypes::Surface m_androidSurfaceObject; QWaitCondition m_surfaceWaitCondition; bool m_surfaceCreated = false;