Skip to content

Commit

Permalink
macOS: Improve handling of wantsBestResolutionOpenGLSurface
Browse files Browse the repository at this point in the history
We were disabling wantsBestResolutionOpenGLSurface whenever we detected
the Apple software renderer, but this isn't needed when layer-backed,
and did in fact result in the exact same visual result as the bug the
code was working around -- only rendering to a quarter of the viewport.

We now apply the workaround only when software rendering is combined
with surface-backed views.

The logic has also been improved to not rely on string comparison to
look for the software renderer, but instead uses the renderer ID that
the context provides.

Since tweaking the wantsBestResolutionOpenGLSurface is only relevant
when using a window for GL rendering the logic has been moved into
QCocoaGLContext.

Change-Id: I021aaefbb7a9782bc8ee3c9703da246510326d50
Reviewed-by: Timur Pocheptsov <[email protected]>
  • Loading branch information
torarnv committed Oct 24, 2019
1 parent 86a0f1c commit 444e947
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 30 deletions.
4 changes: 3 additions & 1 deletion src/plugins/platforms/cocoa/qcocoaglcontext.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@

QT_BEGIN_NAMESPACE

class QCocoaWindow;

class QCocoaGLContext : public QPlatformOpenGLContext
{
public:
Expand All @@ -76,12 +78,12 @@ class QCocoaGLContext : public QPlatformOpenGLContext
static NSOpenGLPixelFormat *pixelFormatForSurfaceFormat(const QSurfaceFormat &format);

bool setDrawable(QPlatformSurface *surface);
void prepareDrawable(QCocoaWindow *platformWindow);
void updateSurfaceFormat();

NSOpenGLContext *m_context = nil;
NSOpenGLContext *m_shareContext = nil;
QSurfaceFormat m_format;
bool m_didCheckForSoftwareContext = false;
QVarLengthArray<QMacNotificationObserver, 3> m_updateObservers;
QAtomicInt m_needsUpdate = false;
};
Expand Down
46 changes: 28 additions & 18 deletions src/plugins/platforms/cocoa/qcocoaglcontext.mm
Original file line number Diff line number Diff line change
Expand Up @@ -368,23 +368,6 @@ static inline QByteArray getGlString(GLenum param)
[m_context makeCurrentContext];

if (surface->surface()->surfaceClass() == QSurface::Window) {
// Disable high-resolution surfaces when using the software renderer, which has the
// problem that the system silently falls back to a to using a low-resolution buffer
// when a high-resolution buffer is requested. This is not detectable using the NSWindow
// convertSizeToBacking and backingScaleFactor APIs. A typical result of this is that Qt
// will display a quarter of the window content when running in a virtual machine.
if (!m_didCheckForSoftwareContext) {
// FIXME: This ensures we check only once per context,
// but the context may be used for multiple surfaces.
m_didCheckForSoftwareContext = true;

const GLubyte* renderer = glGetString(GL_RENDERER);
if (qstrcmp((const char *)renderer, "Apple Software Renderer") == 0) {
NSView *view = static_cast<QCocoaWindow *>(surface)->m_view;
[view setWantsBestResolutionOpenGLSurface:NO];
}
}

if (m_needsUpdate.fetchAndStoreRelaxed(false))
update();
}
Expand Down Expand Up @@ -413,11 +396,14 @@ static inline QByteArray getGlString(GLenum param)
}

Q_ASSERT(surface->surface()->surfaceClass() == QSurface::Window);
QNSView *view = qnsview_cast(static_cast<QCocoaWindow *>(surface)->view());
auto *cocoaWindow = static_cast<QCocoaWindow *>(surface);
QNSView *view = qnsview_cast(cocoaWindow->view());

if (view == m_context.view)
return true;

prepareDrawable(cocoaWindow);

// Setting the drawable may happen on a separate thread as a result of
// a call to makeCurrent, so we need to set up the observers before we
// associate the view with the context. That way we will guarantee that
Expand Down Expand Up @@ -460,6 +446,30 @@ static inline QByteArray getGlString(GLenum param)
return true;
}

void QCocoaGLContext::prepareDrawable(QCocoaWindow *platformWindow)
{
// We generally want high-DPI GL surfaces, unless the user has explicitly disabled them
bool prefersBestResolutionOpenGLSurface = qt_mac_resolveOption(YES,
platformWindow->window(), "_q_mac_wantsBestResolutionOpenGLSurface",
"QT_MAC_WANTS_BEST_RESOLUTION_OPENGL_SURFACE");

auto *view = platformWindow->view();

// The only case we have to opt out ourselves is when using the Apple software renderer
// in combination with surface-backed views, as these together do not support high-DPI.
if (prefersBestResolutionOpenGLSurface) {
int rendererID = 0;
[m_context getValues:&rendererID forParameter:NSOpenGLContextParameterCurrentRendererID];
bool isSoftwareRenderer = (rendererID & kCGLRendererIDMatchingMask) == kCGLRendererGenericFloatID;
if (isSoftwareRenderer && !view.layer) {
qCInfo(lcQpaOpenGLContext) << "Disabling high resolution GL surface due to software renderer";
prefersBestResolutionOpenGLSurface = false;
}
}

view.wantsBestResolutionOpenGLSurface = prefersBestResolutionOpenGLSurface;
}

// NSOpenGLContext is not re-entrant. Even when using separate contexts per thread,
// view, and window, calls into the API will still deadlock. For more information
// see https://openradar.appspot.com/37064579
Expand Down
11 changes: 0 additions & 11 deletions src/plugins/platforms/cocoa/qnsview_drawing.mm
Original file line number Diff line number Diff line change
Expand Up @@ -44,17 +44,6 @@ @implementation QNSView (Drawing)
- (void)initDrawing
{
[self updateLayerBacking];

// Enable high-DPI OpenGL for retina displays. Enabling has the side
// effect that Cocoa will start calling glViewport(0, 0, width, height),
// overriding any glViewport calls in application code. This is usually not a
// problem, except if the application wants to have a "custom" viewport.
// (like the hellogl example)
if (m_platformWindow->window()->supportsOpenGL()) {
self.wantsBestResolutionOpenGLSurface = qt_mac_resolveOption(YES, m_platformWindow->window(),
"_q_mac_wantsBestResolutionOpenGLSurface", "QT_MAC_WANTS_BEST_RESOLUTION_OPENGL_SURFACE");
// See also QCocoaGLContext::makeCurrent for software renderer workarounds.
}
}

- (BOOL)isOpaque
Expand Down

0 comments on commit 444e947

Please sign in to comment.