Skip to content

Commit

Permalink
Android: clean up Surface resources when it is destroyed
Browse files Browse the repository at this point in the history
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 <assam.boudjelthia@qt.io>
(cherry picked from commit 9343d1a)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
  • Loading branch information
Tinja Paavoseppä authored and Qt Cherry-pick Bot committed Oct 2, 2024
1 parent 868324d commit 6eb29ca
Show file tree
Hide file tree
Showing 7 changed files with 27 additions and 10 deletions.
2 changes: 2 additions & 0 deletions src/android/jar/src/org/qtproject/qt/android/QtSurface.java
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,12 @@ bool QAndroidPlatformOpenGLContext::makeCurrent(QPlatformSurface *surface)

QAndroidPlatformOpenGLWindow *window = static_cast<QAndroidPlatformOpenGLWindow *>(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;
Expand Down
16 changes: 9 additions & 7 deletions src/plugins/platforms/android/qandroidplatformopenglwindow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ QAndroidPlatformOpenGLWindow::~QAndroidPlatformOpenGLWindow()
m_surfaceWaitCondition.wakeOne();
lockSurface();
destroySurface();
clearEgl();
clearSurface();
unlockSurface();
}

Expand All @@ -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);
}

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

Expand All @@ -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();
Expand All @@ -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);
Expand All @@ -134,7 +136,7 @@ void QAndroidPlatformOpenGLWindow::clearEgl()

if (m_nativeWindow) {
ANativeWindow_release(m_nativeWindow);
m_nativeWindow = 0;
m_nativeWindow = nullptr;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 3 additions & 1 deletion src/plugins/platforms/android/qandroidplatformvulkanwindow.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,10 @@ class QAndroidPlatformVulkanWindow : public QAndroidPlatformWindow

VkSurfaceKHR *vkSurface();

protected:
void clearSurface() override;

private:
void clearSurface();
void destroyAndClearSurface();

ANativeWindow *m_nativeWindow;
Expand Down
6 changes: 5 additions & 1 deletion src/plugins/platforms/android/qandroidplatformwindow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
1 change: 1 addition & 0 deletions src/plugins/platforms/android/qandroidplatformwindow.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit 6eb29ca

Please sign in to comment.