From 655687d84d6a5914227f2580889e20b7e1b31095 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tor=20Arne=20Vestb=C3=B8?= Date: Wed, 1 Jun 2016 13:29:24 +0200 Subject: [PATCH] UIKit: Don't reallocate renderbuffer on QIOSContext::swapBuffers() During device rotation, the backing CEAGLLayer of our custom UIView is resized by the system. Normally this is the time where we would then reconfigure the corresponding renderbuffer that we render to, which shares memory with the CEAGLLayer, but we chose a lazy approach where we'd defer the reconfigure until client code actually called makeCurrent. This caused problems because not only did we implement the lazy reconfig in makeCurrent, but in every QIOSContext function that operated on the default FBO, including swapBuffers(). When using threaded rendering, such as in Qt Quick, the render thread may be half way in rendering a new frame when the system resizes the CEAGLLayer, and we pick up that resize on the swapBuffer call and allocate a new renderbuffer, before flushing the queued up GL commands that were operating on another renderbuffer of a different size. This resulted in the following crash: 0 - gpus_ReturnObjectErrorKillClient() 1 - gpusSubmitDataBuffers() 2 - glrFlushContextToken() 3 - flush(__GLIContextRec*)() 4 - QIOSContext::swapBuffers(QPlatformSurface*) ... We solve this by still being lazy in how we reconfigure, but limit the reconfigure to makeCurrent(). If the CEAGLLayer is resized in between two frames, we skip the half-drawn frame. The old frame will then be scaled to match the new size by the system, but this is preferable to flushing a new frame that may have been drawn with two conflicting window geometries. Task-number: QTBUG-50017 Change-Id: Ie229f26d156dfbfc7ed8d9efd0eb5e992eee73f1 Reviewed-by: Richard Moe Gustavsen --- src/plugins/platforms/ios/qioscontext.h | 5 +- src/plugins/platforms/ios/qioscontext.mm | 154 +++++++++++++---------- src/plugins/platforms/ios/qioswindow.h | 2 + src/plugins/platforms/ios/qioswindow.mm | 6 + 4 files changed, 100 insertions(+), 67 deletions(-) diff --git a/src/plugins/platforms/ios/qioscontext.h b/src/plugins/platforms/ios/qioscontext.h index ec678fee44..a95856aee1 100644 --- a/src/plugins/platforms/ios/qioscontext.h +++ b/src/plugins/platforms/ios/qioscontext.h @@ -34,6 +34,7 @@ #ifndef QIOSCONTEXT_H #define QIOSCONTEXT_H +#include #include @class EAGLContext; @@ -83,7 +84,9 @@ private: static void deleteBuffers(const FramebufferObject &framebufferObject); FramebufferObject &backingFramebufferObjectFor(QPlatformSurface *) const; - mutable QHash m_framebufferObjects; + mutable QHash m_framebufferObjects; + + bool needsRenderbufferResize(QPlatformSurface *) const; }; QT_END_NAMESPACE diff --git a/src/plugins/platforms/ios/qioscontext.mm b/src/plugins/platforms/ios/qioscontext.mm index 8bdb4bcdb3..e76a2b9902 100644 --- a/src/plugins/platforms/ios/qioscontext.mm +++ b/src/plugins/platforms/ios/qioscontext.mm @@ -42,6 +42,8 @@ #import #import +Q_LOGGING_CATEGORY(lcQpaGLContext, "qt.qpa.glcontext"); + QIOSContext::QIOSContext(QOpenGLContext *context) : QPlatformOpenGLContext() , m_sharedContext(static_cast(context->shareHandle())) @@ -75,6 +77,8 @@ QIOSContext::QIOSContext(QOpenGLContext *context) // could take advantage of the unchanged buffer, but this means clients (and Qt) // will also assume that swapBufferes() is not needed, which is _not_ the case. m_format.setSwapBehavior(QSurfaceFormat::DoubleBuffer); + + qCDebug(lcQpaGLContext) << "created context with format" << m_format << "shared with" << m_sharedContext; } QIOSContext::~QIOSContext() @@ -117,10 +121,12 @@ static QString fboStatusString(GLenum status) } } +#define Q_ASSERT_IS_GL_SURFACE(surface) \ + Q_ASSERT(surface && (surface->surface()->surfaceType() & (QSurface::OpenGLSurface | QSurface::RasterGLSurface))) + bool QIOSContext::makeCurrent(QPlatformSurface *surface) { - Q_ASSERT(surface && (surface->surface()->surfaceType() == QSurface::OpenGLSurface - || surface->surface()->surfaceType() == QSurface::RasterGLSurface)); + Q_ASSERT_IS_GL_SURFACE(surface); [EAGLContext setCurrentContext:m_eaglContext]; @@ -128,54 +134,11 @@ bool QIOSContext::makeCurrent(QPlatformSurface *surface) if (surface->surface()->surfaceClass() == QSurface::Offscreen) return true; + Q_ASSERT(surface->surface()->surfaceClass() == QSurface::Window); FramebufferObject &framebufferObject = backingFramebufferObjectFor(surface); - // We bind the default FBO even if it's incomplete, so that clients who - // call glCheckFramebufferStatus as a result of this function returning - // false will get a matching error code. - glBindFramebuffer(GL_FRAMEBUFFER, framebufferObject.handle); - - return framebufferObject.isComplete; -} - -void QIOSContext::doneCurrent() -{ - [EAGLContext setCurrentContext:nil]; -} - -void QIOSContext::swapBuffers(QPlatformSurface *surface) -{ - Q_ASSERT(surface && (surface->surface()->surfaceType() == QSurface::OpenGLSurface - || surface->surface()->surfaceType() == QSurface::RasterGLSurface)); - - if (surface->surface()->surfaceClass() == QSurface::Offscreen) - return; // Nothing to do - - FramebufferObject &framebufferObject = backingFramebufferObjectFor(surface); - - [EAGLContext setCurrentContext:m_eaglContext]; - glBindRenderbuffer(GL_RENDERBUFFER, framebufferObject.colorRenderbuffer); - [m_eaglContext presentRenderbuffer:GL_RENDERBUFFER]; -} - -QIOSContext::FramebufferObject &QIOSContext::backingFramebufferObjectFor(QPlatformSurface *surface) const -{ - // We keep track of default-FBOs in the root context of a share-group. This assumes - // that the contexts form a tree, where leaf nodes are always destroyed before their - // parents. If that assumption (based on the current implementation) doesn't hold we - // should probably use QOpenGLMultiGroupSharedResource to track the shared default-FBOs. - if (m_sharedContext) - return m_sharedContext->backingFramebufferObjectFor(surface); - - Q_ASSERT(surface && surface->surface()->surfaceClass() == QSurface::Window); - QIOSWindow *window = static_cast(surface); - - FramebufferObject &framebufferObject = m_framebufferObjects[window]; - - // Set up an FBO for the window if it hasn't been created yet if (!framebufferObject.handle) { - [EAGLContext setCurrentContext:m_eaglContext]; - + // Set up an FBO for the window if it hasn't been created yet glGenFramebuffers(1, &framebufferObject.handle); glBindFramebuffer(GL_FRAMEBUFFER, framebufferObject.handle); @@ -195,17 +158,17 @@ QIOSContext::FramebufferObject &QIOSContext::backingFramebufferObjectFor(QPlatfo framebufferObject.depthRenderbuffer); } - connect(window, SIGNAL(destroyed(QObject*)), this, SLOT(windowDestroyed(QObject*))); + connect(static_cast(surface), SIGNAL(destroyed(QObject*)), this, SLOT(windowDestroyed(QObject*))); + } else { + glBindFramebuffer(GL_FRAMEBUFFER, framebufferObject.handle); } - // Ensure that the FBO's buffers match the size of the layer - UIView *view = reinterpret_cast(window->winId()); - CAEAGLLayer *layer = static_cast(view.layer); - if (framebufferObject.renderbufferWidth != (layer.frame.size.width * layer.contentsScale) || - framebufferObject.renderbufferHeight != (layer.frame.size.height * layer.contentsScale)) { - - [EAGLContext setCurrentContext:m_eaglContext]; - glBindFramebuffer(GL_FRAMEBUFFER, framebufferObject.handle); + if (needsRenderbufferResize(surface)) { + // Ensure that the FBO's buffers match the size of the layer + CAEAGLLayer *layer = static_cast(surface)->eaglLayer(); + qCDebug(lcQpaGLContext, "Reallocating renderbuffer storage - current: %dx%d, layer: %gx%g", + framebufferObject.renderbufferWidth, framebufferObject.renderbufferHeight, + layer.frame.size.width * layer.contentsScale, layer.frame.size.height * layer.contentsScale); glBindRenderbuffer(GL_RENDERBUFFER, framebufferObject.colorRenderbuffer); [m_eaglContext renderbufferStorage:GL_RENDERBUFFER fromDrawable:layer]; @@ -228,12 +191,49 @@ QIOSContext::FramebufferObject &QIOSContext::backingFramebufferObjectFor(QPlatfo framebufferObject.isComplete = glCheckFramebufferStatus(GL_FRAMEBUFFER) == GL_FRAMEBUFFER_COMPLETE; if (!framebufferObject.isComplete) { - qWarning("QIOSContext failed to make complete framebuffer object (%s)", + qCWarning(lcQpaGLContext, "QIOSContext failed to make complete framebuffer object (%s)", qPrintable(fboStatusString(glCheckFramebufferStatus(GL_FRAMEBUFFER)))); } } - return framebufferObject; + return framebufferObject.isComplete; +} + +void QIOSContext::doneCurrent() +{ + [EAGLContext setCurrentContext:nil]; +} + +void QIOSContext::swapBuffers(QPlatformSurface *surface) +{ + Q_ASSERT_IS_GL_SURFACE(surface); + + if (surface->surface()->surfaceClass() == QSurface::Offscreen) + return; // Nothing to do + + FramebufferObject &framebufferObject = backingFramebufferObjectFor(surface); + Q_ASSERT_X(framebufferObject.isComplete, "QIOSContext", "swapBuffers on incomplete FBO"); + + if (needsRenderbufferResize(surface)) { + qCWarning(lcQpaGLContext, "CAEAGLLayer was resized between makeCurrent and swapBuffers, skipping flush"); + return; + } + + [EAGLContext setCurrentContext:m_eaglContext]; + glBindRenderbuffer(GL_RENDERBUFFER, framebufferObject.colorRenderbuffer); + [m_eaglContext presentRenderbuffer:GL_RENDERBUFFER]; +} + +QIOSContext::FramebufferObject &QIOSContext::backingFramebufferObjectFor(QPlatformSurface *surface) const +{ + // We keep track of default-FBOs in the root context of a share-group. This assumes + // that the contexts form a tree, where leaf nodes are always destroyed before their + // parents. If that assumption (based on the current implementation) doesn't hold we + // should probably use QOpenGLMultiGroupSharedResource to track the shared default-FBOs. + if (m_sharedContext) + return m_sharedContext->backingFramebufferObjectFor(surface); + else + return m_framebufferObjects[surface]; } GLuint QIOSContext::defaultFramebufferObject(QPlatformSurface *surface) const @@ -245,19 +245,41 @@ GLuint QIOSContext::defaultFramebufferObject(QPlatformSurface *surface) const return 0; } - return backingFramebufferObjectFor(surface).handle; + FramebufferObject &framebufferObject = backingFramebufferObjectFor(surface); + Q_ASSERT_X(framebufferObject.handle, "QIOSContext", "can't resolve default FBO before makeCurrent"); + + return framebufferObject.handle; +} + +bool QIOSContext::needsRenderbufferResize(QPlatformSurface *surface) const +{ + Q_ASSERT(surface->surface()->surfaceClass() == QSurface::Window); + + FramebufferObject &framebufferObject = backingFramebufferObjectFor(surface); + CAEAGLLayer *layer = static_cast(surface)->eaglLayer(); + + if (framebufferObject.renderbufferWidth != (layer.frame.size.width * layer.contentsScale)) + return true; + + if (framebufferObject.renderbufferHeight != (layer.frame.size.height * layer.contentsScale)) + return true; + + return false; } void QIOSContext::windowDestroyed(QObject *object) { QIOSWindow *window = static_cast(object); - if (m_framebufferObjects.contains(window)) { - EAGLContext *originalContext = [EAGLContext currentContext]; - [EAGLContext setCurrentContext:m_eaglContext]; - deleteBuffers(m_framebufferObjects[window]); - m_framebufferObjects.remove(window); - [EAGLContext setCurrentContext:originalContext]; - } + if (!m_framebufferObjects.contains(window)) + return; + + qCDebug(lcQpaGLContext) << object << "destroyed, deleting corresponding FBO"; + + EAGLContext *originalContext = [EAGLContext currentContext]; + [EAGLContext setCurrentContext:m_eaglContext]; + deleteBuffers(m_framebufferObjects[window]); + m_framebufferObjects.remove(window); + [EAGLContext setCurrentContext:originalContext]; } QFunctionPointer QIOSContext::getProcAddress(const QByteArray& functionName) diff --git a/src/plugins/platforms/ios/qioswindow.h b/src/plugins/platforms/ios/qioswindow.h index c1c4cad01a..8d8a1fd0f7 100644 --- a/src/plugins/platforms/ios/qioswindow.h +++ b/src/plugins/platforms/ios/qioswindow.h @@ -84,6 +84,8 @@ public: void requestUpdate() Q_DECL_OVERRIDE; + CAEAGLLayer *eaglLayer() const; + private: void applicationStateChanged(Qt::ApplicationState state); void applyGeometry(const QRect &rect); diff --git a/src/plugins/platforms/ios/qioswindow.mm b/src/plugins/platforms/ios/qioswindow.mm index 42609fd983..6723181c34 100644 --- a/src/plugins/platforms/ios/qioswindow.mm +++ b/src/plugins/platforms/ios/qioswindow.mm @@ -376,6 +376,12 @@ void QIOSWindow::requestUpdate() static_cast(screen())->setUpdatesPaused(false); } +CAEAGLLayer *QIOSWindow::eaglLayer() const +{ + Q_ASSERT([m_view.layer isKindOfClass:[CAEAGLLayer class]]); + return static_cast(m_view.layer); +} + #include "moc_qioswindow.cpp" QT_END_NAMESPACE