From 4c5d09a1b044c8e15ae72c06a448e986823e3060 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Tue, 18 Feb 2014 01:48:37 +0100 Subject: [PATCH] More robust support for extension-based driver bug workarounds. Previously the extensions were either disabled altogether (e.g. because we don't have yet extension wrangler on ES) or had manually adjusted minimal required versions to avoid issues on older versions (e.g. ARB_explicit_attrib_location is known to give compiler errors when used with GLSL < 1.50 on some drivers), both done basically at compile time unrelated to actual hardware/driver used. Now all the extensions are enabled exactly as the driver advertises them (or when their core version is not larger than the context version) and have minimal required versions as per specification. Given extension is supported in given version when it is marked as such and its minimal required version is not larger than the requested one. The extension disabling is thus done by simply increasing the minimal required version to larger value (or Version::None for disabling it for all versions). Currently no such disabling is put into place, but the existing workarounds scattered all over the place will be gradually converted to this. Simplified the code and tests by marking all extensions from previous versions as supported instead of additional "shorthand" checking whether extension's core version is supported. The check wasn't probably much of a speedup, as it was just another branch. This was also buggy previously, because when the extension would be reported as not being supported in older versions, which is not what we want. Hopefully this won't reintroduce the numerous issues with Mesa and OSX I had in the past :-) Also removed duplicate implementation of Context::isExtensionSupported(), one overload is now calling the other. --- src/Magnum/CMakeLists.txt | 1 + src/Magnum/Context.cpp | 20 +++++++++- src/Magnum/Context.h | 11 ++++-- .../Implementation/setupDriverWorkarounds.cpp | 39 +++++++++++++++++++ 4 files changed, 66 insertions(+), 5 deletions(-) create mode 100644 src/Magnum/Implementation/setupDriverWorkarounds.cpp diff --git a/src/Magnum/CMakeLists.txt b/src/Magnum/CMakeLists.txt index 359fa3301..304705c65 100644 --- a/src/Magnum/CMakeLists.txt +++ b/src/Magnum/CMakeLists.txt @@ -60,6 +60,7 @@ set(Magnum_SRCS Implementation/ShaderProgramState.cpp Implementation/State.cpp Implementation/TextureState.cpp + Implementation/setupDriverWorkarounds.cpp Trade/AbstractImageConverter.cpp Trade/AbstractImporter.cpp diff --git a/src/Magnum/Context.cpp b/src/Magnum/Context.cpp index 100d408b1..be4168385 100644 --- a/src/Magnum/Context.cpp +++ b/src/Magnum/Context.cpp @@ -369,7 +369,6 @@ Context::Context() { glGetIntegerv(GL_CONTEXT_FLAGS, reinterpret_cast(&_flags)); #endif - /* Get first future (not supported) version */ std::vector versions{ #ifndef MAGNUM_TARGET_GLES Version::GL300, @@ -387,10 +386,17 @@ Context::Context() { #endif Version::None }; + + /* Get first future (not supported) version */ std::size_t future = 0; while(versions[future] != Version::None && isVersionSupported(versions[future])) ++future; + /* Mark all extensions from past versions as supported */ + for(std::size_t i = 0; i != future; ++i) + for(const Extension& extension: Extension::extensions(versions[i])) + extensionStatus.set(extension._index); + /* List of extensions from future versions (extensions from current and previous versions should be supported automatically, so we don't need to check for them) */ @@ -483,6 +489,18 @@ Context::Context() { #undef _disable #endif + /* Reset minimal required version to Version::None for whole array */ + for(auto& i: _extensionRequiredVersion) i = Version::None; + + /* Initialize required versions from extension info */ + for(const auto version: versions) + for(const Extension& extension: Extension::extensions(version)) + _extensionRequiredVersion[extension._index] = extension._requiredVersion; + + /* Setup driver workarounds (increase required version for particular + extensions), see Implementation/driverWorkarounds.cpp */ + setupDriverWorkarounds(); + /* Set this context as current */ CORRADE_ASSERT(!_current, "Context: Another context currently active", ); _current = this; diff --git a/src/Magnum/Context.h b/src/Magnum/Context.h index 60afb96ae..081ddca13 100644 --- a/src/Magnum/Context.h +++ b/src/Magnum/Context.h @@ -30,6 +30,7 @@ */ #include +#include #include #include #include @@ -291,7 +292,7 @@ class MAGNUM_EXPORT Context { * @todoc Explicit reference when Doxygen is sane */ template bool isExtensionSupported() const { - return isVersionSupported(T::coreVersion()) || (isVersionSupported(T::requiredVersion()) && extensionStatus[T::Index]); + return isExtensionSupported(version()); } /** @@ -310,7 +311,7 @@ class MAGNUM_EXPORT Context { * @endcode */ template bool isExtensionSupported(Version version) const { - return T::coreVersion() <= version || (T::requiredVersion() <= version && extensionStatus[T::Index]); + return _extensionRequiredVersion[T::Index] <= version && extensionStatus[T::Index]; } /** @@ -319,12 +320,11 @@ class MAGNUM_EXPORT Context { * Can be used e.g. for listing extensions available on current * hardware, but for general usage prefer isExtensionSupported() const, * as it does most operations in compile time. - * * @see @ref supportedExtensions(), @ref Extension::extensions(), * @ref MAGNUM_ASSERT_EXTENSION_SUPPORTED() */ bool isExtensionSupported(const Extension& extension) const { - return isVersionSupported(extension._coreVersion) || (isVersionSupported(extension._requiredVersion) && extensionStatus[extension._index]); + return isVersionSupported(_extensionRequiredVersion[extension._index]) && extensionStatus[extension._index]; } #ifndef DOXYGEN_GENERATING_OUTPUT @@ -334,11 +334,14 @@ class MAGNUM_EXPORT Context { private: static Context* _current; + MAGNUM_LOCAL void setupDriverWorkarounds(); + Version _version; Int _majorVersion; Int _minorVersion; Flags _flags; + std::array _extensionRequiredVersion; std::bitset<160> extensionStatus; std::vector _supportedExtensions; diff --git a/src/Magnum/Implementation/setupDriverWorkarounds.cpp b/src/Magnum/Implementation/setupDriverWorkarounds.cpp new file mode 100644 index 000000000..4978e63e0 --- /dev/null +++ b/src/Magnum/Implementation/setupDriverWorkarounds.cpp @@ -0,0 +1,39 @@ +/* + This file is part of Magnum. + + Copyright © 2010, 2011, 2012, 2013, 2014 + Vladimír Vondruš + + Permission is hereby granted, free of charge, to any person obtaining a + copy of this software and associated documentation files (the "Software"), + to deal in the Software without restriction, including without limitation + the rights to use, copy, modify, merge, publish, distribute, sublicense, + and/or sell copies of the Software, and to permit persons to whom the + Software is furnished to do so, subject to the following conditions: + + The above copyright notice and this permission notice shall be included + in all copies or substantial portions of the Software. + + THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER + DEALINGS IN THE SOFTWARE. +*/ + +#include "Magnum/Context.h" +#include "Magnum/Extensions.h" + +namespace Magnum { + +void Context::setupDriverWorkarounds() { + #define _setRequiredVersion(extension, version) \ + if(_extensionRequiredVersion[Extensions::extension::Index] < Version::version) \ + _extensionRequiredVersion[Extensions::extension::Index] = Version::version + + #undef _setRequiredVersion +} + +}