From 8e70241dccaf5a9e5c79c8d6da5665b881c5914d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tor=20Arne=20Vestb=C3=B8?= Date: Tue, 3 Oct 2017 19:24:50 +0200 Subject: [PATCH] Let QPlatformBackingStore handle its own QOpenGLContext MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The resources allocated by QPlatformBackingStore are owned by the class, and should be allocated in a context the class also owns. This removes the asymmetry of having to pass in a context to composeAndFlush, while having to make the same context current before destroying the platform backingstore. The context owned by QPlatformBackingStore is shared with the associated window though a new QWindowPrivate::shareContext() API. The result is that on e.g. iOS, the backingstore does not need to tie the resource allocation of QPlatformBackingStore to the global share context, but can instead tie them to the per-window context, and hence clean them up after each window is closed. Task-number: QTBUG-56653 Change-Id: Ic1bcae50dafeeafaa8d16a7febd83b840ec6367a Reviewed-by: Morten Johan Sørvig Reviewed-by: Laszlo Agocs --- src/gui/kernel/qwindow.cpp | 6 ++++ src/gui/kernel/qwindow_p.h | 2 ++ src/gui/painting/qplatformbackingstore.cpp | 32 +++++++++++++------ src/gui/painting/qplatformbackingstore.h | 2 +- .../qopenglcompositorbackingstore.cpp | 5 ++- .../qopenglcompositorbackingstore_p.h | 2 +- src/plugins/platforms/ios/qiosbackingstore.h | 3 -- src/plugins/platforms/ios/qiosbackingstore.mm | 15 +-------- .../platforms/xcb/qxcbbackingstore.cpp | 4 +-- src/plugins/platforms/xcb/qxcbbackingstore.h | 2 +- src/widgets/kernel/qwidget.cpp | 26 +-------------- src/widgets/kernel/qwidgetbackingstore.cpp | 6 ++-- src/widgets/kernel/qwidgetwindow.cpp | 8 +++++ 13 files changed, 50 insertions(+), 63 deletions(-) diff --git a/src/gui/kernel/qwindow.cpp b/src/gui/kernel/qwindow.cpp index b4e2b47c03..42b54bf98a 100644 --- a/src/gui/kernel/qwindow.cpp +++ b/src/gui/kernel/qwindow.cpp @@ -45,6 +45,7 @@ #ifndef QT_NO_OPENGL #include #include "qopenglcontext.h" +#include "qopenglcontext_p.h" #endif #include "qscreen.h" @@ -2633,6 +2634,11 @@ QWindow *QWindowPrivate::topLevelWindow() const return window; } +QOpenGLContext *QWindowPrivate::shareContext() const +{ + return qt_gl_global_share_context(); +}; + /*! Creates a local representation of a window created by another process or by using native libraries below Qt. diff --git a/src/gui/kernel/qwindow_p.h b/src/gui/kernel/qwindow_p.h index 9b8e2c47e4..bf6be1bb98 100644 --- a/src/gui/kernel/qwindow_p.h +++ b/src/gui/kernel/qwindow_p.h @@ -130,6 +130,8 @@ public: QWindow *topLevelWindow() const; + virtual QOpenGLContext *shareContext() const; + virtual QWindow *eventReceiver() { Q_Q(QWindow); return q; } virtual void setVisible(bool visible); diff --git a/src/gui/painting/qplatformbackingstore.cpp b/src/gui/painting/qplatformbackingstore.cpp index 6cb115afba..8ab22beb31 100644 --- a/src/gui/painting/qplatformbackingstore.cpp +++ b/src/gui/painting/qplatformbackingstore.cpp @@ -50,6 +50,7 @@ #include #ifndef QT_NO_OPENGL #include +#include #endif #include #include @@ -95,14 +96,15 @@ public: ~QPlatformBackingStorePrivate() { #ifndef QT_NO_OPENGL - QOpenGLContext *ctx = QOpenGLContext::currentContext(); - if (ctx) { + if (context) { + QOffscreenSurface offscreenSurface; + offscreenSurface.setFormat(context->format()); + offscreenSurface.create(); + context->makeCurrent(&offscreenSurface); if (textureId) - ctx->functions()->glDeleteTextures(1, &textureId); + context->functions()->glDeleteTextures(1, &textureId); if (blitter) blitter->destroy(); - } else if (textureId || blitter) { - qWarning("No context current during QPlatformBackingStore destruction, OpenGL resources not released"); } delete blitter; #endif @@ -110,6 +112,7 @@ public: QWindow *window; QBackingStore *backingStore; #ifndef QT_NO_OPENGL + QScopedPointer context; mutable GLuint textureId; mutable QSize textureSize; mutable bool needsSwizzle; @@ -316,20 +319,31 @@ static void blitTextureForWidget(const QPlatformTextureList *textures, int idx, void QPlatformBackingStore::composeAndFlush(QWindow *window, const QRegion ®ion, const QPoint &offset, - QPlatformTextureList *textures, QOpenGLContext *context, + QPlatformTextureList *textures, bool translucentBackground) { if (!qt_window_private(window)->receivedExpose) return; - if (!context->makeCurrent(window)) { + if (!d_ptr->context) { + d_ptr->context.reset(new QOpenGLContext); + d_ptr->context->setFormat(d_ptr->window->requestedFormat()); + d_ptr->context->setScreen(d_ptr->window->screen()); + d_ptr->context->setShareContext(qt_window_private(d_ptr->window)->shareContext()); + if (!d_ptr->context->create()) { + qWarning("composeAndFlush: QOpenGLContext creation failed"); + return; + } + } + + if (!d_ptr->context->makeCurrent(window)) { qWarning("composeAndFlush: makeCurrent() failed"); return; } QWindowPrivate::get(window)->lastComposeTime.start(); - QOpenGLFunctions *funcs = context->functions(); + QOpenGLFunctions *funcs = d_ptr->context->functions(); funcs->glViewport(0, 0, window->width() * window->devicePixelRatio(), window->height() * window->devicePixelRatio()); funcs->glClearColor(0, 0, 0, translucentBackground ? 0 : 1); funcs->glClear(GL_COLOR_BUFFER_BIT); @@ -435,7 +449,7 @@ void QPlatformBackingStore::composeAndFlush(QWindow *window, const QRegion ®i funcs->glDisable(GL_BLEND); d_ptr->blitter->release(); - context->swapBuffers(window); + d_ptr->context->swapBuffers(window); } #endif /*! diff --git a/src/gui/painting/qplatformbackingstore.h b/src/gui/painting/qplatformbackingstore.h index 9956c032a9..381c564079 100644 --- a/src/gui/painting/qplatformbackingstore.h +++ b/src/gui/painting/qplatformbackingstore.h @@ -120,7 +120,7 @@ public: virtual void flush(QWindow *window, const QRegion ®ion, const QPoint &offset) = 0; #ifndef QT_NO_OPENGL virtual void composeAndFlush(QWindow *window, const QRegion ®ion, const QPoint &offset, - QPlatformTextureList *textures, QOpenGLContext *context, + QPlatformTextureList *textures, bool translucentBackground); #endif virtual QImage toImage() const; diff --git a/src/platformsupport/platformcompositor/qopenglcompositorbackingstore.cpp b/src/platformsupport/platformcompositor/qopenglcompositorbackingstore.cpp index b24491b187..e938020437 100644 --- a/src/platformsupport/platformcompositor/qopenglcompositorbackingstore.cpp +++ b/src/platformsupport/platformcompositor/qopenglcompositorbackingstore.cpp @@ -202,14 +202,13 @@ void QOpenGLCompositorBackingStore::flush(QWindow *window, const QRegion ®ion } void QOpenGLCompositorBackingStore::composeAndFlush(QWindow *window, const QRegion ®ion, const QPoint &offset, - QPlatformTextureList *textures, QOpenGLContext *context, + QPlatformTextureList *textures, bool translucentBackground) { // QOpenGLWidget/QQuickWidget content provided as textures. The raster content goes on top. Q_UNUSED(region); Q_UNUSED(offset); - Q_UNUSED(context); Q_UNUSED(translucentBackground); QOpenGLCompositor *compositor = QOpenGLCompositor::instance(); @@ -218,7 +217,7 @@ void QOpenGLCompositorBackingStore::composeAndFlush(QWindow *window, const QRegi // The compositor's context and the context to which QOpenGLWidget/QQuickWidget // textures belong are not the same. They share resources, though. - Q_ASSERT(context->shareGroup() == dstCtx->shareGroup()); + Q_ASSERT(qt_window_private(window)->shareContext()->shareGroup() == dstCtx->shareGroup()); QWindow *dstWin = compositor->targetWindow(); if (!dstWin) diff --git a/src/platformsupport/platformcompositor/qopenglcompositorbackingstore_p.h b/src/platformsupport/platformcompositor/qopenglcompositorbackingstore_p.h index 0b025e4304..da68b90e92 100644 --- a/src/platformsupport/platformcompositor/qopenglcompositorbackingstore_p.h +++ b/src/platformsupport/platformcompositor/qopenglcompositorbackingstore_p.h @@ -75,7 +75,7 @@ public: QImage toImage() const Q_DECL_OVERRIDE; void composeAndFlush(QWindow *window, const QRegion ®ion, const QPoint &offset, - QPlatformTextureList *textures, QOpenGLContext *context, + QPlatformTextureList *textures, bool translucentBackground) Q_DECL_OVERRIDE; const QPlatformTextureList *textures() const { return m_textures; } diff --git a/src/plugins/platforms/ios/qiosbackingstore.h b/src/plugins/platforms/ios/qiosbackingstore.h index 3954347471..e6b890251a 100644 --- a/src/plugins/platforms/ios/qiosbackingstore.h +++ b/src/plugins/platforms/ios/qiosbackingstore.h @@ -55,9 +55,6 @@ public: ~QIOSBackingStore(); void flush(QWindow *window, const QRegion ®ion, const QPoint &offset) Q_DECL_OVERRIDE; - -private: - QOpenGLContext *m_context; }; QT_END_NAMESPACE diff --git a/src/plugins/platforms/ios/qiosbackingstore.mm b/src/plugins/platforms/ios/qiosbackingstore.mm index 74229684e3..db4dd81b2e 100644 --- a/src/plugins/platforms/ios/qiosbackingstore.mm +++ b/src/plugins/platforms/ios/qiosbackingstore.mm @@ -55,7 +55,6 @@ QT_BEGIN_NAMESPACE */ QIOSBackingStore::QIOSBackingStore(QWindow *window) : QRasterBackingStore(window) - , m_context(new QOpenGLContext) { // We use the surface both for raster operations and for GL drawing (when // we blit the raster image), so the type needs to cover both use cases. @@ -64,22 +63,10 @@ QIOSBackingStore::QIOSBackingStore(QWindow *window) Q_ASSERT_X(window->surfaceType() != QSurface::OpenGLSurface, "QIOSBackingStore", "QBackingStore on iOS can only be used with raster-enabled surfaces."); - - m_context->setFormat(window->requestedFormat()); - m_context->setScreen(window->screen()); - Q_ASSERT(QOpenGLContext::globalShareContext()); - m_context->setShareContext(QOpenGLContext::globalShareContext()); - m_context->create(); } QIOSBackingStore::~QIOSBackingStore() { - // We're using composeAndFlush from QPlatformBackingStore, which - // need to clean up any textures in its destructor, so make the - // context current and keep it alive until QPlatformBackingStore - // has cleaned up everything. - m_context->makeCurrent(window()); - m_context->deleteLater(); } void QIOSBackingStore::flush(QWindow *window, const QRegion ®ion, const QPoint &offset) @@ -98,7 +85,7 @@ void QIOSBackingStore::flush(QWindow *window, const QRegion ®ion, const QPoin } static QPlatformTextureList emptyTextureList; - composeAndFlush(window, region, offset, &emptyTextureList, m_context, false); + composeAndFlush(window, region, offset, &emptyTextureList, false); } QT_END_NAMESPACE diff --git a/src/plugins/platforms/xcb/qxcbbackingstore.cpp b/src/plugins/platforms/xcb/qxcbbackingstore.cpp index 17927af3e3..420d1ac7c5 100644 --- a/src/plugins/platforms/xcb/qxcbbackingstore.cpp +++ b/src/plugins/platforms/xcb/qxcbbackingstore.cpp @@ -600,10 +600,10 @@ void QXcbBackingStore::flush(QWindow *window, const QRegion ®ion, const QPoin #ifndef QT_NO_OPENGL void QXcbBackingStore::composeAndFlush(QWindow *window, const QRegion ®ion, const QPoint &offset, - QPlatformTextureList *textures, QOpenGLContext *context, + QPlatformTextureList *textures, bool translucentBackground) { - QPlatformBackingStore::composeAndFlush(window, region, offset, textures, context, translucentBackground); + QPlatformBackingStore::composeAndFlush(window, region, offset, textures, translucentBackground); QXcbWindow *platformWindow = static_cast(window->handle()); if (platformWindow->needsSync()) { diff --git a/src/plugins/platforms/xcb/qxcbbackingstore.h b/src/plugins/platforms/xcb/qxcbbackingstore.h index 94b5994004..2e8fbfb7fa 100644 --- a/src/plugins/platforms/xcb/qxcbbackingstore.h +++ b/src/plugins/platforms/xcb/qxcbbackingstore.h @@ -61,7 +61,7 @@ public: void flush(QWindow *window, const QRegion ®ion, const QPoint &offset) override; #ifndef QT_NO_OPENGL void composeAndFlush(QWindow *window, const QRegion ®ion, const QPoint &offset, - QPlatformTextureList *textures, QOpenGLContext *context, + QPlatformTextureList *textures, bool translucentBackground) override; #endif QImage toImage() const override; diff --git a/src/widgets/kernel/qwidget.cpp b/src/widgets/kernel/qwidget.cpp index be9287d39b..a7258d8f7e 100644 --- a/src/widgets/kernel/qwidget.cpp +++ b/src/widgets/kernel/qwidget.cpp @@ -1876,39 +1876,15 @@ static void deleteBackingStore(QWidgetPrivate *d) { QTLWExtra *topData = d->topData(); - // The context must be current when destroying the backing store as it may attempt to - // release resources like textures and shader programs. The window may not be suitable - // anymore as there will often not be a platform window underneath at this stage. Fall - // back to a QOffscreenSurface in this case. - QScopedPointer tempSurface; -#ifndef QT_NO_OPENGL - if (d->textureChildSeen && topData->shareContext) { - if (topData->window->handle()) { - topData->shareContext->makeCurrent(topData->window); - } else { - tempSurface.reset(new QOffscreenSurface); - tempSurface->setFormat(topData->shareContext->format()); - tempSurface->create(); - topData->shareContext->makeCurrent(tempSurface.data()); - } - } -#endif - delete topData->backingStore; topData->backingStore = 0; - -#ifndef QT_NO_OPENGL - if (d->textureChildSeen && topData->shareContext) - topData->shareContext->doneCurrent(); -#endif } void QWidgetPrivate::deleteTLSysExtra() { if (extra && extra->topextra) { //the qplatformbackingstore may hold a reference to the window, so the backingstore - //needs to be deleted first. If the backingstore holds GL resources, we need to - // make the context current here. This is taken care of by deleteBackingStore(). + //needs to be deleted first. extra->topextra->backingStoreTracker.destroy(); deleteBackingStore(this); diff --git a/src/widgets/kernel/qwidgetbackingstore.cpp b/src/widgets/kernel/qwidgetbackingstore.cpp index bb421927db..52080fe05f 100644 --- a/src/widgets/kernel/qwidgetbackingstore.cpp +++ b/src/widgets/kernel/qwidgetbackingstore.cpp @@ -143,10 +143,8 @@ void QWidgetBackingStore::qt_flush(QWidget *widget, const QRegion ®ion, QBack // WA_TranslucentBackground. Therefore the compositor needs to know whether the app intends // to rely on translucency, in order to decide if it should clear to transparent or opaque. const bool translucentBackground = widget->testAttribute(Qt::WA_TranslucentBackground); - // Use the tlw's context, not widget's. The difference is important with native child - // widgets where tlw != widget. - backingStore->handle()->composeAndFlush(widget->windowHandle(), effectiveRegion, offset, widgetTextures, - tlw->d_func()->shareContext(), translucentBackground); + backingStore->handle()->composeAndFlush(widget->windowHandle(), effectiveRegion, offset, + widgetTextures, translucentBackground); widget->window()->d_func()->sendComposeStatus(widget->window(), true); } else #endif diff --git a/src/widgets/kernel/qwidgetwindow.cpp b/src/widgets/kernel/qwidgetwindow.cpp index d30154410c..319eccf223 100644 --- a/src/widgets/kernel/qwidgetwindow.cpp +++ b/src/widgets/kernel/qwidgetwindow.cpp @@ -96,6 +96,7 @@ public: } QRectF closestAcceptableGeometry(const QRectF &rect) const Q_DECL_OVERRIDE; + QOpenGLContext *shareContext() const override; }; QRectF QWidgetWindowPrivate::closestAcceptableGeometry(const QRectF &rect) const @@ -127,6 +128,13 @@ QRectF QWidgetWindowPrivate::closestAcceptableGeometry(const QRectF &rect) const return result; } +QOpenGLContext *QWidgetWindowPrivate::shareContext() const +{ + Q_Q(const QWidgetWindow); + const QWidgetPrivate *widgetPrivate = QWidgetPrivate::get(q->widget()); + return widgetPrivate->shareContext(); +} + QWidgetWindow::QWidgetWindow(QWidget *widget) : QWindow(*new QWidgetWindowPrivate(), 0) , m_widget(widget)