From 05699b336c0046bfd2b01522069ec026eb3db96e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Tue, 18 Feb 2025 20:07:14 +0100 Subject: [PATCH 1/3] GL: restrict the "apple-buffer-..." workaround to just Apple drivers. There's an increasing number of projects that use ANGLE or Zink on that platform, and this workaround makes no sense there. --- src/Magnum/GL/Implementation/BufferState.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Magnum/GL/Implementation/BufferState.cpp b/src/Magnum/GL/Implementation/BufferState.cpp index ee1d7ecba..009777648 100644 --- a/src/Magnum/GL/Implementation/BufferState.cpp +++ b/src/Magnum/GL/Implementation/BufferState.cpp @@ -190,7 +190,9 @@ BufferState::BufferState(Context& context, Containers::StaticArrayView Date: Tue, 18 Feb 2025 20:10:02 +0100 Subject: [PATCH 2/3] GL: new "apple-crashy-msaa-default-framebuffer" workaround. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit First spotted in July 2024 with magnum-player, and back then the only reliable solution was to disable MSAA altogether. Fortunately this fixes it as well, unfortunately there's no way to know if the default framebuffer is actually multisampled, so it's done always. Co-authored-by: Vladimír Vondruš --- src/Magnum/GL/Context.cpp | 9 +++- src/Magnum/GL/Context.h | 1 + .../GL/Implementation/driverSpecific.cpp | 44 +++++++++++++++++++ 3 files changed, 53 insertions(+), 1 deletion(-) diff --git a/src/Magnum/GL/Context.cpp b/src/Magnum/GL/Context.cpp index 559b25d64..6f34fc368 100644 --- a/src/Magnum/GL/Context.cpp +++ b/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:"; diff --git a/src/Magnum/GL/Context.h b/src/Magnum/GL/Context.h index b4ababbab..d77e1331d 100644 --- a/src/Magnum/GL/Context.h +++ b/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); diff --git a/src/Magnum/GL/Implementation/driverSpecific.cpp b/src/Magnum/GL/Implementation/driverSpecific.cpp index c8eeaddf1..e13ae6b98 100644 --- a/src/Magnum/GL/Implementation/driverSpecific.cpp +++ b/src/Magnum/GL/Implementation/driverSpecific.cpp @@ -33,6 +33,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 +116,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 +898,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) { From a56b7541abee14a82c7ca45b10cda850c36dd6e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Tue, 18 Feb 2025 20:11:59 +0100 Subject: [PATCH 3/3] doc: updated changelog and credits. --- doc/changelog.dox | 6 +++++- src/Magnum/GL/Implementation/driverSpecific.cpp | 1 + 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/doc/changelog.dox b/doc/changelog.dox index 946c68808..f2ed5fcf8 100644 --- a/doc/changelog.dox +++ b/doc/changelog.dox @@ -246,8 +246,12 @@ See also: information. - A new @cpp "mesa-broken-dsa-framebuffer-clear" @ce workaround for a crash on exit happening with Mesa 24 when the DSA variants of - @ref GL::Framebuffer::clearColor() and related APIs are used. See + @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 diff --git a/src/Magnum/GL/Implementation/driverSpecific.cpp b/src/Magnum/GL/Implementation/driverSpecific.cpp index e13ae6b98..c69047b85 100644 --- a/src/Magnum/GL/Implementation/driverSpecific.cpp +++ b/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š + Copyright © 2025 David Peicho Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"),