From fff756ad063c9687ea03e5952c1c5e4665d48b55 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Sun, 25 Mar 2018 20:25:35 +0200 Subject: [PATCH] Work around issue with Unicode shaders with threaded Emscripten. --- doc/changelog.dox | 4 ++ src/Magnum/CMakeLists.txt | 1 + src/Magnum/Implementation/ShaderState.cpp | 64 ++++++++++++++++++++ src/Magnum/Implementation/ShaderState.h | 30 +++++---- src/Magnum/Implementation/State.cpp | 2 +- src/Magnum/Implementation/driverSpecific.cpp | 10 ++- src/Magnum/Shader.cpp | 21 ++++++- src/Magnum/Shader.h | 9 +++ 8 files changed, 120 insertions(+), 21 deletions(-) create mode 100644 src/Magnum/Implementation/ShaderState.cpp diff --git a/doc/changelog.dox b/doc/changelog.dox index d22ede14f..e08a64307 100644 --- a/doc/changelog.dox +++ b/doc/changelog.dox @@ -76,6 +76,10 @@ See also: - Engine startup info was not properly printed to Android log since introducing the `--magnum-log` option in version 2018.02 +- Working around Emscripten issue where `-s USE_PTHREADS=1` would cause all + shader sources containing Unicode characters to be truncated to empty + strings. See the @cpp "emscripten-pthreads-broken-unicode-shader-sources" @ce + workaround description for details. @subsection changelog-latest-deprecated Deprecated APIs diff --git a/src/Magnum/CMakeLists.txt b/src/Magnum/CMakeLists.txt index 82b7dc436..801dfe209 100644 --- a/src/Magnum/CMakeLists.txt +++ b/src/Magnum/CMakeLists.txt @@ -59,6 +59,7 @@ set(Magnum_SRCS Implementation/MeshState.cpp Implementation/RendererState.cpp Implementation/ShaderProgramState.cpp + Implementation/ShaderState.cpp Implementation/State.cpp Implementation/TextureState.cpp Implementation/driverSpecific.cpp diff --git a/src/Magnum/Implementation/ShaderState.cpp b/src/Magnum/Implementation/ShaderState.cpp new file mode 100644 index 000000000..65b46f546 --- /dev/null +++ b/src/Magnum/Implementation/ShaderState.cpp @@ -0,0 +1,64 @@ +/* + 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. +*/ + +#include "ShaderState.h" + +#include "Magnum/Shader.h" + +#if defined(CORRADE_TARGET_EMSCRIPTEN) && defined(__EMSCRIPTEN_PTHREADS__) +#include "Magnum/Context.h" +#endif + +namespace Magnum { namespace Implementation { + +ShaderState::ShaderState(Context& context, std::vector&): + maxVertexOutputComponents{}, maxFragmentInputComponents{}, + #if !defined(MAGNUM_TARGET_GLES2) && !defined(MAGNUM_TARGET_WEBGL) + maxTessellationControlInputComponents{}, maxTessellationControlOutputComponents{}, maxTessellationControlTotalOutputComponents{}, maxTessellationEvaluationInputComponents{}, maxTessellationEvaluationOutputComponents{}, maxGeometryInputComponents{}, maxGeometryOutputComponents{}, maxGeometryTotalOutputComponents{}, maxAtomicCounterBuffers{}, maxCombinedAtomicCounterBuffers{}, maxAtomicCounters{}, maxCombinedAtomicCounters{}, maxImageUniforms{}, maxCombinedImageUniforms{}, maxShaderStorageBlocks{}, maxCombinedShaderStorageBlocks{}, + #endif + maxTextureImageUnits{}, maxTextureImageUnitsCombined{}, + #ifndef MAGNUM_TARGET_GLES2 + maxUniformBlocks{}, maxCombinedUniformBlocks{}, + #endif + maxUniformComponents{}, maxUniformComponentsCombined{} + #ifndef MAGNUM_TARGET_GLES2 + , maxCombinedUniformComponents{} + #endif +{ + #if defined(CORRADE_TARGET_EMSCRIPTEN) && defined(__EMSCRIPTEN_PTHREADS__) + if(!context.isDriverWorkaroundDisabled("emscripten-pthreads-broken-unicode-shader-sources")) { + addSourceImplementation = &Shader::addSourceImplementationEmscriptenPthread; + } else + #endif + { + addSourceImplementation = &Shader::addSourceImplementationDefault; + } + + #if !defined(CORRADE_TARGET_EMSCRIPTEN) || !defined(__EMSCRIPTEN_PTHREADS__) + static_cast(context); + #endif +} + +}} diff --git a/src/Magnum/Implementation/ShaderState.h b/src/Magnum/Implementation/ShaderState.h index bebaf1717..cabd08c73 100644 --- a/src/Magnum/Implementation/ShaderState.h +++ b/src/Magnum/Implementation/ShaderState.h @@ -25,27 +25,23 @@ DEALINGS IN THE SOFTWARE. */ +#include +#include + +#include "Magnum/Magnum.h" #include "Magnum/OpenGL.h" -#include "Magnum/Types.h" -#include "Magnum/configure.h" + +#ifdef _MSC_VER +/* Otherwise the member function pointers will have different size based on + whether the header was included or not. CAUSES SERIOUS MEMORY CORRUPTION AND + IS NOT CAUGHT BY ANY WARNING WHATSOEVER! AARGH! */ +#include "Magnum/Shader.h" +#endif namespace Magnum { namespace Implementation { struct ShaderState { - explicit ShaderState(): - maxVertexOutputComponents{}, maxFragmentInputComponents{}, - #if !defined(MAGNUM_TARGET_GLES2) && !defined(MAGNUM_TARGET_WEBGL) - maxTessellationControlInputComponents{}, maxTessellationControlOutputComponents{}, maxTessellationControlTotalOutputComponents{}, maxTessellationEvaluationInputComponents{}, maxTessellationEvaluationOutputComponents{}, maxGeometryInputComponents{}, maxGeometryOutputComponents{}, maxGeometryTotalOutputComponents{}, maxAtomicCounterBuffers{}, maxCombinedAtomicCounterBuffers{}, maxAtomicCounters{}, maxCombinedAtomicCounters{}, maxImageUniforms{}, maxCombinedImageUniforms{}, maxShaderStorageBlocks{}, maxCombinedShaderStorageBlocks{}, - #endif - maxTextureImageUnits{}, maxTextureImageUnitsCombined{}, - #ifndef MAGNUM_TARGET_GLES2 - maxUniformBlocks{}, maxCombinedUniformBlocks{}, - #endif - maxUniformComponents{}, maxUniformComponentsCombined{} - #ifndef MAGNUM_TARGET_GLES2 - , maxCombinedUniformComponents{} - #endif - {} + explicit ShaderState(Context& context, std::vector& extensions); enum: std::size_t { #if !defined(MAGNUM_TARGET_GLES2) && !defined(MAGNUM_TARGET_WEBGL) @@ -55,6 +51,8 @@ struct ShaderState { #endif }; + void(Shader::*addSourceImplementation)(std::string); + GLint maxVertexOutputComponents, maxFragmentInputComponents; #if !defined(MAGNUM_TARGET_GLES2) && !defined(MAGNUM_TARGET_WEBGL) diff --git a/src/Magnum/Implementation/State.cpp b/src/Magnum/Implementation/State.cpp index d5e3bb0e6..988087bfa 100644 --- a/src/Magnum/Implementation/State.cpp +++ b/src/Magnum/Implementation/State.cpp @@ -71,7 +71,7 @@ State::State(Context& context, std::ostream* const out) { query.reset(new QueryState{context, extensions}); #endif renderer.reset(new RendererState{context, extensions}); - shader.reset(new ShaderState); + shader.reset(new ShaderState(context, extensions)); shaderProgram.reset(new ShaderProgramState{context, extensions}); texture.reset(new TextureState{context, extensions}); #ifndef MAGNUM_TARGET_GLES2 diff --git a/src/Magnum/Implementation/driverSpecific.cpp b/src/Magnum/Implementation/driverSpecific.cpp index c13ddd3c6..4b2b0fabe 100644 --- a/src/Magnum/Implementation/driverSpecific.cpp +++ b/src/Magnum/Implementation/driverSpecific.cpp @@ -103,7 +103,15 @@ namespace { with copies from host memory, not with buffer images. Seems to be fixed in Mesa 13, but I have no such system to verify that on. https://github.com/mesa3d/mesa/commit/2aa9ff0cda1f6ad97c83d5583fab7a84efabe19e */ - "svga3d-texture-upload-slice-by-slice" + "svga3d-texture-upload-slice-by-slice", + + #if defined(CORRADE_TARGET_EMSCRIPTEN) && defined(__EMSCRIPTEN_PTHREADS__) + /* Shader sources containing UTF-8 characters are converted to empty + strings when running on Emscripten with -s USE_PTHREADS=1. Working + around that by replacing all chars > 127 with spaces. Relevant code: + https://github.com/kripken/emscripten/blob/7f89560101843198787530731f40a65288f6f15f/src/fetch-worker.js#L54-L58 */ + "emscripten-pthreads-broken-unicode-shader-sources" + #endif }; } diff --git a/src/Magnum/Shader.cpp b/src/Magnum/Shader.cpp index b2c43c589..10eb7239c 100644 --- a/src/Magnum/Shader.cpp +++ b/src/Magnum/Shader.cpp @@ -704,6 +704,8 @@ std::vector Shader::sources() const { return _sources; } Shader& Shader::addSource(std::string source) { if(!source.empty()) { + auto addSource = Context::current().state().shader->addSourceImplementation; + /* Fix line numbers, so line 41 of third added file is marked as 3(41) in case shader version was not Version::None, because then source 0 is the #version directive added in constructor. @@ -715,15 +717,28 @@ Shader& Shader::addSource(std::string source) { order to avoid complex logic in compile() where we assert for at least some user-provided source, an empty string is added here instead. */ - if(!_sources.empty()) _sources.push_back("#line 1 " + std::to_string((_sources.size()+1)/2) + '\n'); - else _sources.emplace_back(); + if(!_sources.empty()) (this->*addSource)("#line 1 " + std::to_string((_sources.size()+1)/2) + '\n'); + else (this->*addSource)({}); - _sources.push_back(std::move(source)); + (this->*addSource)(std::move(source)); } return *this; } +void Shader::addSourceImplementationDefault(std::string source) { + _sources.push_back(std::move(source)); +} + +#if defined(CORRADE_TARGET_EMSCRIPTEN) && defined(__EMSCRIPTEN_PTHREADS__) +void Shader::addSourceImplementationEmscriptenPthread(std::string source) { + /* See the "emscripten-pthreads-broken-unicode-shader-sources" + workaround description for details */ + for(char& c: source) if(c < 0) c = ' '; + _sources.push_back(std::move(source)); +} +#endif + Shader& Shader::addFile(const std::string& filename) { CORRADE_ASSERT(Utility::Directory::fileExists(filename), "Shader file " << '\'' + filename + '\'' << " cannot be read.", *this); diff --git a/src/Magnum/Shader.h b/src/Magnum/Shader.h index 5e7e5fe1c..1e9a6dd80 100644 --- a/src/Magnum/Shader.h +++ b/src/Magnum/Shader.h @@ -39,6 +39,8 @@ namespace Magnum { +namespace Implementation { struct ShaderState; } + /** @brief Shader @@ -50,6 +52,8 @@ Shader limits and implementation-defined values (such as @ref maxUniformComponen are cached, so repeated queries don't result in repeated @fn_gl{Get} calls. */ class MAGNUM_EXPORT Shader: public AbstractObject { + friend Implementation::ShaderState; + public: /** * @brief Shader type @@ -615,6 +619,11 @@ class MAGNUM_EXPORT Shader: public AbstractObject { private: Shader& setLabelInternal(Containers::ArrayView label); + void MAGNUM_LOCAL addSourceImplementationDefault(std::string source); + #if defined(CORRADE_TARGET_EMSCRIPTEN) && defined(__EMSCRIPTEN_PTHREADS__) + void MAGNUM_LOCAL addSourceImplementationEmscriptenPthread(std::string source); + #endif + Type _type; GLuint _id;