From 6eb29cab2a7b750eacd9a81a2d08877f699e47e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tinja=20Paavosepp=C3=A4?= Date: Tue, 9 Apr 2024 16:31:24 +0300 Subject: [PATCH] Android: clean up Surface resources when it is destroyed Clean up any resources using the Android Surface when it has been destroyed. Previously the resource clean up was done only after Qt app state changed to Hidden or below and we initiate the removal of the Surface. However, in the case of QtSurface which is a SurfaceView, it will destroy its Surface before we ever get to that part, leading to the resources holding a reference to a destroyed Surface. Task-number: QTBUG-118231 Change-Id: I282ddcc2813bf0a4e19cbb906376258dd2b4004f Reviewed-by: Assam Boudjelthia (cherry picked from commit 9343d1ab6e1a5df3166d211809f2eb0e5a3cd878) Reviewed-by: Qt Cherry-pick Bot --- .../src/org/qtproject/qt/android/QtSurface.java | 2 ++ .../android/qandroidplatformopenglcontext.cpp | 6 ++++++ .../android/qandroidplatformopenglwindow.cpp | 16 +++++++++------- .../android/qandroidplatformopenglwindow.h | 2 +- .../android/qandroidplatformvulkanwindow.h | 4 +++- .../platforms/android/qandroidplatformwindow.cpp | 6 +++++- .../platforms/android/qandroidplatformwindow.h | 1 + 7 files changed, 27 insertions(+), 10 deletions(-) diff --git a/src/android/jar/src/org/qtproject/qt/android/QtSurface.java b/src/android/jar/src/org/qtproject/qt/android/QtSurface.java index f4ca3947d44..01d1a360691 100644 --- a/src/android/jar/src/org/qtproject/qt/android/QtSurface.java +++ b/src/android/jar/src/org/qtproject/qt/android/QtSurface.java @@ -47,6 +47,8 @@ public void surfaceChanged(SurfaceHolder holder, int format, int width, int heig @Override public void surfaceDestroyed(SurfaceHolder holder) { + // Once we return from this function, the Surface will be destroyed, + // so everything holding a reference to it needs to clean it up before we do that if (m_surfaceCallback != null) m_surfaceCallback.onSurfaceChanged(null); } diff --git a/src/plugins/platforms/android/qandroidplatformopenglcontext.cpp b/src/plugins/platforms/android/qandroidplatformopenglcontext.cpp index fbfbb625dd2..00b2af18b3a 100644 --- a/src/plugins/platforms/android/qandroidplatformopenglcontext.cpp +++ b/src/plugins/platforms/android/qandroidplatformopenglcontext.cpp @@ -50,6 +50,12 @@ bool QAndroidPlatformOpenGLContext::makeCurrent(QPlatformSurface *surface) QAndroidPlatformOpenGLWindow *window = static_cast(surface); window->lockSurface(); + // Has the Surface been destroyed? + if (window->eglSurface(eglConfig()) == EGL_NO_SURFACE) { + qWarning() << "makeCurrent(): no EGLSurface, likely Surface destroyed by Android."; + window->unlockSurface(); + return false; + } const bool ok = QEGLPlatformContext::makeCurrent(surface); window->unlockSurface(); return ok; diff --git a/src/plugins/platforms/android/qandroidplatformopenglwindow.cpp b/src/plugins/platforms/android/qandroidplatformopenglwindow.cpp index 77b486cce88..05a9ea1b342 100644 --- a/src/plugins/platforms/android/qandroidplatformopenglwindow.cpp +++ b/src/plugins/platforms/android/qandroidplatformopenglwindow.cpp @@ -33,7 +33,7 @@ QAndroidPlatformOpenGLWindow::~QAndroidPlatformOpenGLWindow() m_surfaceWaitCondition.wakeOne(); lockSurface(); destroySurface(); - clearEgl(); + clearSurface(); unlockSurface(); } @@ -58,13 +58,15 @@ EGLSurface QAndroidPlatformOpenGLWindow::eglSurface(EGLConfig config) QGuiApplication::applicationState() == Qt::ApplicationSuspended) { return m_eglSurface; } - + // If we haven't called createSurface() yet, call it and wait until Android has created + // the Surface if (!m_surfaceCreated) { AndroidDeadlockProtector protector; if (!protector.acquire()) return m_eglSurface; createSurface(); + qCDebug(lcQpaWindow) << "called createSurface(), waiting for Surface to be ready..."; m_surfaceWaitCondition.wait(&m_surfaceMutex); } @@ -79,7 +81,7 @@ EGLSurface QAndroidPlatformOpenGLWindow::eglSurface(EGLConfig config) bool QAndroidPlatformOpenGLWindow::checkNativeSurface(EGLConfig config) { // Either no surface created, or the m_eglSurface already wraps the active Surface - // -> makeCurrent is NOT needed. + // -> makeCurrent is NOT needed, and we should not create a new EGL surface if (!m_surfaceCreated || !m_androidSurfaceObject.isValid()) return false; @@ -96,14 +98,14 @@ void QAndroidPlatformOpenGLWindow::applicationStateChanged(Qt::ApplicationState if (state <= Qt::ApplicationHidden) { lockSurface(); destroySurface(); - clearEgl(); + clearSurface(); unlockSurface(); } } void QAndroidPlatformOpenGLWindow::createEgl(EGLConfig config) { - clearEgl(); + clearSurface(); QJniEnvironment env; m_nativeWindow = ANativeWindow_fromSurface(env.jniEnv(), m_androidSurfaceObject.object()); m_androidSurfaceObject = QJniObject(); @@ -124,7 +126,7 @@ QSurfaceFormat QAndroidPlatformOpenGLWindow::format() const return m_format; } -void QAndroidPlatformOpenGLWindow::clearEgl() +void QAndroidPlatformOpenGLWindow::clearSurface() { if (m_eglSurface != EGL_NO_SURFACE) { eglMakeCurrent(m_eglDisplay, EGL_NO_SURFACE, EGL_NO_SURFACE, EGL_NO_CONTEXT); @@ -134,7 +136,7 @@ void QAndroidPlatformOpenGLWindow::clearEgl() if (m_nativeWindow) { ANativeWindow_release(m_nativeWindow); - m_nativeWindow = 0; + m_nativeWindow = nullptr; } } diff --git a/src/plugins/platforms/android/qandroidplatformopenglwindow.h b/src/plugins/platforms/android/qandroidplatformopenglwindow.h index b78eddc2e75..6e31bf68fd6 100644 --- a/src/plugins/platforms/android/qandroidplatformopenglwindow.h +++ b/src/plugins/platforms/android/qandroidplatformopenglwindow.h @@ -31,7 +31,7 @@ class QAndroidPlatformOpenGLWindow : public QAndroidPlatformWindow protected: void createEgl(EGLConfig config); - void clearEgl(); + void clearSurface() override; private: EGLDisplay m_eglDisplay = EGL_NO_DISPLAY; diff --git a/src/plugins/platforms/android/qandroidplatformvulkanwindow.h b/src/plugins/platforms/android/qandroidplatformvulkanwindow.h index 29986e64a9c..23a06c5ecbb 100644 --- a/src/plugins/platforms/android/qandroidplatformvulkanwindow.h +++ b/src/plugins/platforms/android/qandroidplatformvulkanwindow.h @@ -31,8 +31,10 @@ class QAndroidPlatformVulkanWindow : public QAndroidPlatformWindow VkSurfaceKHR *vkSurface(); +protected: + void clearSurface() override; + private: - void clearSurface(); void destroyAndClearSurface(); ANativeWindow *m_nativeWindow; diff --git a/src/plugins/platforms/android/qandroidplatformwindow.cpp b/src/plugins/platforms/android/qandroidplatformwindow.cpp index 7f43f175079..4c76bfb3d48 100644 --- a/src/plugins/platforms/android/qandroidplatformwindow.cpp +++ b/src/plugins/platforms/android/qandroidplatformwindow.cpp @@ -303,10 +303,14 @@ void QAndroidPlatformWindow::destroySurface() void QAndroidPlatformWindow::onSurfaceChanged(QtJniTypes::Surface surface) { lockSurface(); + const bool surfaceIsValid = surface.isValid(); + qCDebug(lcQpaWindow) << "onSurfaceChanged():, valid Surface received" << surfaceIsValid; m_androidSurfaceObject = surface; - if (m_androidSurfaceObject.isValid()) { + if (surfaceIsValid) { // wait until we have a valid surface to draw into m_surfaceWaitCondition.wakeOne(); + } else { + clearSurface(); } unlockSurface(); diff --git a/src/plugins/platforms/android/qandroidplatformwindow.h b/src/plugins/platforms/android/qandroidplatformwindow.h index e493875beef..d3822e2d96e 100644 --- a/src/plugins/platforms/android/qandroidplatformwindow.h +++ b/src/plugins/platforms/android/qandroidplatformwindow.h @@ -77,6 +77,7 @@ class QAndroidPlatformWindow: public QPlatformWindow void sendExpose() const; bool blockedByModal() const; bool isEmbeddingContainer() const; + virtual void clearSurface() {} Qt::WindowFlags m_windowFlags; Qt::WindowStates m_windowState;