Merge a56b7541ab into 820382767c

doc/changelog.dox

@@ -248,6 +248,10 @@ See also:
     on exit happening with Mesa 24 when the DSA variants of
     @ref GL::Framebuffer::clearColor() and related APIs are used. See
     @ref opengl-workarounds for more information.
+-   A new @cpp "apple-crashy-msaa-default-framebuffer" @ce workaround fixing a
+    crash inside Apple's GL implementation when a window with multisampled
+    default framebuffer is moved or resized. See @ref opengl-workarounds for
+    more information.
 
 @subsubsection changelog-latest-new-math Math library
 

src/Magnum/GL/Context.cpp

@@ -1002,7 +1002,10 @@ bool Context::tryCreate(const Configuration& configuration) {
         _extensionRequiredVersion[extension.index()] = Version::None;
 
     /* Setup driver workarounds (increase required version for particular
-       extensions), see Implementation/driverWorkarounds.cpp */
+       extensions), see Implementation/driverWorkarounds.cpp. Workarounds done
+       here are what affects state creation below, there's another set of
+       workarounds that instead depend on the created state, which are then
+       set up in setupDriverWorkaroundsWithStateCreated() afterwards. */
     setupDriverWorkarounds();
 
     /* Set this context as current */
@@ -1033,6 +1036,10 @@ bool Context::tryCreate(const Configuration& configuration) {
     _stateData = Utility::move(state.first());
     _state = &*state.second();
 
+    /* Setup driver workarounds that need the state to be created already.
+       Counterpart to the setupDriverWorkarounds() call above. */
+    setupDriverWorkaroundsWithStateCreated();
+
     /* Print a list of used workarounds */
     if(!_driverWorkarounds.isEmpty()) {
         Debug{output} << "Using driver workarounds:";

src/Magnum/GL/Context.h

@@ -897,6 +897,7 @@ class MAGNUM_GL_EXPORT Context {
 
         /* Defined in Implementation/driverSpecific.cpp */
         MAGNUM_GL_LOCAL void setupDriverWorkarounds();
+        MAGNUM_GL_LOCAL void setupDriverWorkaroundsWithStateCreated();
 
         #ifndef MAGNUM_TARGET_GLES
         MAGNUM_GL_LOCAL static bool isCoreProfileImplementationDefault(Context& self);

src/Magnum/GL/Implementation/BufferState.cpp

@@ -190,7 +190,9 @@ BufferState::BufferState(Context& context, Containers::StaticArrayView<Implement
     }
 
     #if defined(CORRADE_TARGET_APPLE) && !defined(MAGNUM_TARGET_GLES)
-    if(!context.isDriverWorkaroundDisabled("apple-buffer-texture-unbind-on-buffer-modify"_s)) {
+    /* Enable this only on Apple's own drivers, not alternatives like ANGLE or
+       Zink */
+    if(context.vendorString() == "Apple"_s && !context.isDriverWorkaroundDisabled("apple-buffer-texture-unbind-on-buffer-modify"_s)) {
         dataImplementation = &Buffer::dataImplementationApple;
         subDataImplementation = &Buffer::subDataImplementationApple;
         mapImplementation = &Buffer::mapImplementationApple;

src/Magnum/GL/Implementation/driverSpecific.cpp

@@ -4,6 +4,7 @@
     Copyright © 2010, 2011, 2012, 2013, 2014, 2015, 2016, 2017, 2018, 2019,
                 2020, 2021, 2022, 2023, 2024, 2025
               Vladimír Vondruš <mosra@centrum.cz>
+    Copyright © 2025 David Peicho <david.peicho@gmail.com>
 
     Permission is hereby granted, free of charge, to any person obtaining a
     copy of this software and associated documentation files (the "Software"),
@@ -33,6 +34,13 @@
 #include "Magnum/GL/Extensions.h"
 #include "Magnum/Math/Range.h"
 
+#if defined(CORRADE_TARGET_APPLE) && !defined(MAGNUM_TARGET_GLES)
+/* For the "apple-crashy-msaa-default-framebuffer" workaround */
+#include "Magnum/GL/Framebuffer.h"
+#include "Magnum/GL/Renderbuffer.h"
+#include "Magnum/GL/RenderbufferFormat.h"
+#endif
+
 #if defined(MAGNUM_TARGET_WEBGL) && defined(CORRADE_TARGET_EMSCRIPTEN)
 /* Including any Emscripten header should also make __EMSCRIPTEN_major__ etc
    macros available, independently of whether they're passed implicitly (before
@@ -109,6 +117,20 @@ constexpr Containers::StringView KnownWorkarounds[]{
 "apple-buffer-texture-unbind-on-buffer-modify"_s,
 #endif
 
+#if defined(CORRADE_TARGET_APPLE) && !defined(MAGNUM_TARGET_GLES)
+/* Creating a multisampled default framebuffer in both SDL and GLFW causes a
+   crash in gldUpdateReadFramebuffer() inside Apple's GL-over-Metal
+   implementation when the window gets moved or resized. Disabling
+   multisampling fixes it, but it's also fixed by creating and then immediately
+   destroying a custom framebuffer with a multisampled attachment. Ideally the
+   workaround would only be enabled if the default framebuffer is multisampled,
+   but that's unfortunately only queryable since GL 4.3+, while Apple is stuck
+   on 4.1. So the workaround is enabled always.
+
+   The assumption is that this affects only desktop GL, not GLES on iOS. */
+"apple-crashy-msaa-default-framebuffer"_s,
+#endif
+
 #if defined(MAGNUM_TARGET_GLES) && !defined(MAGNUM_TARGET_GLES2)
 /* Qualcomm Adreno drivers V@0615.65 (and possibly others) report __VERSION__
    as 300 even for GLSL ES 3.10 and 3.20, breaking version-dependent shader
@@ -877,6 +899,29 @@ void Context::setupDriverWorkarounds() {
         }
     }
     #endif
+
+    /* Additional workarounds done in setupDriverWorkaroundsWithStateCreated()
+       below */
+}
+
+void Context::setupDriverWorkaroundsWithStateCreated() {
+    /* Compared to setupDriverWorkarounds(), which is called even before the
+       context is current and the internal state tracker is created, everything
+       here depends on the context being current and state ready. */
+
+    #if defined(CORRADE_TARGET_APPLE) && !defined(MAGNUM_TARGET_GLES)
+    /* Creating and then immediately destructing is what fixes it, yes. Enable
+       this only on Apple's own drivers, not alternatives like ANGLE or Zink.
+       Also ideally would be done only if the default framebuffer is
+       multisampled, but that's only possible to query since GL 4.3, and Apple
+       is stuck at 4.1, so doing this always. */
+    if(vendorString() == "Apple"_s && !isDriverWorkaroundDisabled("apple-crashy-msaa-default-framebuffer"_s)) {
+        GL::Renderbuffer renderbuffer;
+        renderbuffer.setStorageMultisample(4, GL::RenderbufferFormat::RGBA8, Vector2i{1});
+        GL::Framebuffer framebuffer{{{}, Vector2i{1}}};
+        framebuffer.attachRenderbuffer(GL::Framebuffer::ColorAttachment{0}, renderbuffer);
+    }
+    #endif
 }
 
 Context::Configuration& Context::Configuration::addDisabledWorkarounds(const Containers::StringIterable& workarounds) {