From 68dca658c518eea4f06771c32244a21bbec63c21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Mon, 26 Mar 2018 14:51:24 +0200 Subject: [PATCH] doc: better documented GL driver workarounds and how to add them. --- Doxyfile | 1 + doc/developers.dox | 34 ++++++++++++ doc/opengl-workarounds.dox | 57 ++++++++++++++++++++ doc/opengl.dox | 6 +++ src/Magnum/Context.cpp | 2 +- src/Magnum/Context.h | 3 +- src/Magnum/Implementation/driverSpecific.cpp | 2 + 7 files changed, 102 insertions(+), 3 deletions(-) create mode 100644 doc/opengl-workarounds.dox diff --git a/Doxyfile b/Doxyfile index 61a068c04..bd5718c17 100644 --- a/Doxyfile +++ b/Doxyfile @@ -917,6 +917,7 @@ EXCLUDE_SYMBOLS = Magnum::*Implementation \ # command). EXAMPLE_PATH = doc/snippets/ \ + src/ \ ../magnum-examples/src/ \ ../magnum-integration/doc/snippets/ \ diff --git a/doc/developers.dox b/doc/developers.dox index 006351616..c5d81e901 100644 --- a/doc/developers.dox +++ b/doc/developers.dox @@ -560,6 +560,40 @@ In order to remove GL functionality, be sure to touch all places mentioned above, only in inverse --- but usually @ref developers-deprecation "deprecate first", unless it doesn't affect public API at all. +@section developers-driver-workaround Checklist for adding / removing a driver workaround + +1. Put a descriptive string with even more descriptive comment into the + `KnownWorkarounds` array in `src/Magnum/Implementation/driverSpecific.cpp`, + ideally reuse some of the already existing vendor prefixes +2. If the workaround can be tied down to a particular platform / target, add + apropriate @cpp #ifdef @ce around it +3. Create (or extend) a pair of (member) @cpp private @ce `MAGNUM_LOCAL` + `*Implementation*()` functions in the affected class, `*ImplementationDefault()` + having the optimistic default behavior, the other having the workaround. + Add the appropriate @cpp #ifdef @ce around, if any. +4. Add a new (member) function pointer in `src/Implementation/SomeState.h` and + call it from the affected class instead of executing the implementation + directly. Add the appropriate @cpp #ifdef @ce around, if any. +5. Add a branch into `src/Implementation/SomeState.cpp` (with appropriate + @cpp #ifdef @ce around, if any). First check for driver and other + circumstances and call the @cpp !context.isDriverWorkaroundDisabled("the-workaround-string") @ce + last after you are really sure you need to use it --- calling this + function will append the workaround string to the engine startup log and + it's not desirable to list workarounds that weren't used. +6. Test the affected functionality with the workaround enabled and verify it + works +7. Disable the workaround using `--magnum-disable-workarounds` on command-line + and verify it's still broken --- if not, there's something off! +8. Add a changelog entry. +9. Verify the driver workaround is listed in the snippet in + @ref opengl-workarounds + +Removeing a workaround is simply a matter of searching for its string, removing +all occurences of that string and removing all `*Implementation*()` functions +that were used only if the workaround was in place. No need to deprecate +anything, users explicitly disabling given workaround will only be informed +that such workaround does not exist anymore. + @section developers-dependency Checklist for adding, removing or updating a dependency 1. Verify that there are no important clients stuck on the old version with no diff --git a/doc/opengl-workarounds.dox b/doc/opengl-workarounds.dox new file mode 100644 index 000000000..d9d97971f --- /dev/null +++ b/doc/opengl-workarounds.dox @@ -0,0 +1,57 @@ +/* + This file is part of Magnum. + + Copyright © 2010, 2011, 2012, 2013, 2014, 2015, 2016, 2017, 2018 + 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. +*/ + +namespace Magnum { +/** @page opengl-workarounds Driver workarounds +@brief List of OpenGL driver workarounds used by Magnum + +@m_footernavigation + +Driver workarounds used by a particular app are listed in the engine startup +log such as here: + +@code{.shell-session} +Renderer: GeForce GT 740M/PCIe/SSE2 by NVIDIA Corporation +OpenGL version: 4.6.0 NVIDIA 390.25 +Using optional features: + GL_ARB_ES2_compatibility + GL_ARB_direct_state_access + ... +Using driver workarounds: + no-forward-compatible-core-context + no-layout-qualifiers-on-old-glsl + nv-zero-context-profile-mask + ... +@endcode + +These identifiers correspond to the strings in the below listing. For debugging +and diagnostic purpose it's possible to disable particular workarounds by +passing their identifier string to the `--magnum-disable-workarounds` +command-line option. See @ref Context-command-line for more information. + +@snippet src/Magnum/Implementation/driverSpecific.cpp workarounds + +*/ +} diff --git a/doc/opengl.dox b/doc/opengl.dox index 72efbd825..58faf6e0e 100644 --- a/doc/opengl.dox +++ b/doc/opengl.dox @@ -46,6 +46,12 @@ general they should be avoided. - @subpage opengl-deprecated +The OpenGL driver world is far from ideal and in order to work on majority of +platforms, the engine has to work around some driver bugs. An exhaustive list +is here: + +- @subpage opengl-workarounds + @section opengl-required-extensions Version and extension requirements The engine requires at least OpenGL 2.1 or OpenGL ES 2.0, but some specific diff --git a/src/Magnum/Context.cpp b/src/Magnum/Context.cpp index c335c1273..a064b8344 100644 --- a/src/Magnum/Context.cpp +++ b/src/Magnum/Context.cpp @@ -454,7 +454,7 @@ Context::Context(NoCreateT, Int argc, const char** argv, void functionLoader()): /* Parse arguments */ Utility::Arguments args{"magnum"}; args.addOption("disable-workarounds") - .setHelp("disable-workarounds", "driver workarounds to disable\n (see src/Magnum/Implementation/driverSpecific.cpp for detailed info)", "LIST") + .setHelp("disable-workarounds", "driver workarounds to disable\n (see http://doc.magnum.graphics/magnum/opengl-workarounds.html for detailed info)", "LIST") .addOption("disable-extensions").setHelp("disable-extensions", "OpenGL extensions to disable", "LIST") .addOption("log", "default").setHelp("log", "Console logging", "default|quiet") .setFromEnvironment("disable-workarounds") diff --git a/src/Magnum/Context.h b/src/Magnum/Context.h index 70ac67358..a867c9161 100644 --- a/src/Magnum/Context.h +++ b/src/Magnum/Context.h @@ -124,8 +124,7 @@ Arguments: - `...` --- main application arguments (see `-h` or `--help` for details) - `--magnum-help` --- display this help message and exit - `--magnum-disable-workarounds LIST` --- driver workarounds to disable (see - [src/Magnum/Implementation/driverSpecific.cpp](https://github.com/mosra/magnum/blob/master/src/Magnum/Implementation/driverSpecific.cpp) - for detailed info) (environment: `MAGNUM_DISABLE_WORKAROUNDS`) + @ref opengl-workarounds for detailed info) (environment: `MAGNUM_DISABLE_WORKAROUNDS`) - `--magnum-disable-extensions LIST` --- OpenGL extensions to disable (environment: `MAGNUM_DISABLE_EXTENSIONS`) diff --git a/src/Magnum/Implementation/driverSpecific.cpp b/src/Magnum/Implementation/driverSpecific.cpp index 4b2b0fabe..edcc40be1 100644 --- a/src/Magnum/Implementation/driverSpecific.cpp +++ b/src/Magnum/Implementation/driverSpecific.cpp @@ -35,6 +35,7 @@ namespace Magnum { namespace { /* Search the code for the following strings to see where they are implemented. */ std::vector KnownWorkarounds{ + /* [workarounds] */ #if !defined(MAGNUM_TARGET_GLES) && !defined(CORRADE_TARGET_APPLE) /* Creating core context with specific version on AMD and NV proprietary drivers on Linux/Windows and Intel drivers on Windows @@ -112,6 +113,7 @@ namespace { https://github.com/kripken/emscripten/blob/7f89560101843198787530731f40a65288f6f15f/src/fetch-worker.js#L54-L58 */ "emscripten-pthreads-broken-unicode-shader-sources" #endif + /* [workarounds] */ }; }