From c93cc0ba083b4bd49fabf028051da1ee1c58c633 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Thu, 6 Oct 2016 16:26:55 +0200 Subject: [PATCH] Added nv-windows-dangling-transform-feedback-varying-names workaround. On Windows NVidia drivers the glTransformFeedbackVaryings() does not make a copy of its char* arguments so it fails at link time when the original char arrays are not in scope anymore. Enabling *synchronous* debug output circumvents this bug. Can be triggered by running TransformFeedbackGLTest with GL_KHR_debug extension disabled. --- src/Magnum/AbstractShaderProgram.cpp | 17 +++++++++++++++++ src/Magnum/AbstractShaderProgram.h | 17 +++++++++++++++++ .../Implementation/ShaderProgramState.cpp | 13 +++++++++++++ src/Magnum/Implementation/ShaderProgramState.h | 9 +++++---- src/Magnum/Implementation/driverSpecific.cpp | 8 ++++++++ 5 files changed, 60 insertions(+), 4 deletions(-) diff --git a/src/Magnum/AbstractShaderProgram.cpp b/src/Magnum/AbstractShaderProgram.cpp index 7fc4228f9..ce19cb1a6 100644 --- a/src/Magnum/AbstractShaderProgram.cpp +++ b/src/Magnum/AbstractShaderProgram.cpp @@ -368,6 +368,10 @@ void AbstractShaderProgram::bindFragmentDataLocationIndexedInternal(const Unsign #ifndef MAGNUM_TARGET_GLES2 void AbstractShaderProgram::setTransformFeedbackOutputs(const std::initializer_list outputs, const TransformFeedbackBufferMode bufferMode) { + (this->*Context::current().state().shaderProgram->transformFeedbackVaryingsImplementation)({outputs.begin(), outputs.size()}, bufferMode); +} + +void AbstractShaderProgram::transformFeedbackVaryingsImplementationDefault(const Containers::ArrayView outputs, const TransformFeedbackBufferMode bufferMode) { /** @todo VLAs */ Containers::Array names{outputs.size()}; @@ -376,6 +380,19 @@ void AbstractShaderProgram::setTransformFeedbackOutputs(const std::initializer_l glTransformFeedbackVaryings(_id, outputs.size(), names, GLenum(bufferMode)); } + +#ifdef CORRADE_TARGET_WINDOWS +void AbstractShaderProgram::transformFeedbackVaryingsImplementationDanglingWorkaround(const Containers::ArrayView outputs, const TransformFeedbackBufferMode bufferMode) { + /* NVidia on Windows doesn't copy the names when calling + glTransformFeedbackVaryings() so it then fails at link time because the + char* are dangling. We have to do the copy on the engine side and keep + the values until link time (which can happen any time and multiple + times, so basically for the remaining lifetime of the shader program) */ + _transformFeedbackVaryingNames.assign(outputs.begin(), outputs.end()); + + transformFeedbackVaryingsImplementationDefault({_transformFeedbackVaryingNames.data(), _transformFeedbackVaryingNames.size()}, bufferMode); +} +#endif #endif bool AbstractShaderProgram::link(std::initializer_list> shaders) { diff --git a/src/Magnum/AbstractShaderProgram.h b/src/Magnum/AbstractShaderProgram.h index 74538e0b9..9c5aed167 100644 --- a/src/Magnum/AbstractShaderProgram.h +++ b/src/Magnum/AbstractShaderProgram.h @@ -36,6 +36,10 @@ #include "Magnum/AbstractObject.h" #include "Magnum/Attribute.h" +#if defined(CORRADE_TARGET_WINDOWS) && !defined(MAGNUM_TARGET_GLES2) +#include +#endif + namespace Magnum { namespace Implementation { struct ShaderProgramState; } @@ -1189,6 +1193,13 @@ class MAGNUM_EXPORT AbstractShaderProgram: public AbstractObject { Int uniformLocationInternal(Containers::ArrayView name); UnsignedInt uniformBlockIndexInternal(Containers::ArrayView name); + #ifndef MAGNUM_TARGET_GLES2 + void MAGNUM_LOCAL transformFeedbackVaryingsImplementationDefault(Containers::ArrayView outputs, TransformFeedbackBufferMode bufferMode); + #ifdef CORRADE_TARGET_WINDOWS + void MAGNUM_LOCAL transformFeedbackVaryingsImplementationDanglingWorkaround(Containers::ArrayView outputs, TransformFeedbackBufferMode bufferMode); + #endif + #endif + #ifndef MAGNUM_BUILD_DEPRECATED void use(); #endif @@ -1347,6 +1358,12 @@ class MAGNUM_EXPORT AbstractShaderProgram: public AbstractObject { #endif GLuint _id; + + #if defined(CORRADE_TARGET_WINDOWS) && !defined(MAGNUM_TARGET_GLES2) + /* Needed for the nv-windows-dangling-transform-feedback-varying-names + workaround */ + std::vector _transformFeedbackVaryingNames; + #endif }; } diff --git a/src/Magnum/Implementation/ShaderProgramState.cpp b/src/Magnum/Implementation/ShaderProgramState.cpp index 2d16fa201..80e753a8b 100644 --- a/src/Magnum/Implementation/ShaderProgramState.cpp +++ b/src/Magnum/Implementation/ShaderProgramState.cpp @@ -47,6 +47,19 @@ ShaderProgramState::ShaderProgramState(Context& context, std::vector()) diff --git a/src/Magnum/Implementation/ShaderProgramState.h b/src/Magnum/Implementation/ShaderProgramState.h index 689875174..92aa939a2 100644 --- a/src/Magnum/Implementation/ShaderProgramState.h +++ b/src/Magnum/Implementation/ShaderProgramState.h @@ -32,10 +32,7 @@ #include "Magnum/OpenGL.h" #include "Magnum/Math/Vector3.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! */ +#ifndef MAGNUM_TARGET_GLES2 #include "Magnum/AbstractShaderProgram.h" #endif @@ -46,6 +43,10 @@ struct ShaderProgramState { void reset(); + #ifndef MAGNUM_TARGET_GLES2 + void(AbstractShaderProgram::*transformFeedbackVaryingsImplementation)(Containers::ArrayView, AbstractShaderProgram::TransformFeedbackBufferMode); + #endif + void(AbstractShaderProgram::*uniform1fvImplementation)(GLint, GLsizei, const GLfloat*); void(AbstractShaderProgram::*uniform2fvImplementation)(GLint, GLsizei, const Math::Vector<2, GLfloat>*); void(AbstractShaderProgram::*uniform3fvImplementation)(GLint, GLsizei, const Math::Vector<3, GLfloat>*); diff --git a/src/Magnum/Implementation/driverSpecific.cpp b/src/Magnum/Implementation/driverSpecific.cpp index b405dc12d..32c726a2e 100644 --- a/src/Magnum/Implementation/driverSpecific.cpp +++ b/src/Magnum/Implementation/driverSpecific.cpp @@ -48,6 +48,14 @@ namespace { GLSL even though the extension (e.g. binding keyword) is not supported */ "intel-windows-glsl-exposes-unsupported-shading-language-420pack", + + /* On Windows NVidia drivers the glTransformFeedbackVaryings() does not + make a copy of its char* arguments so it fails at link time when the + original char arrays are not in scope anymore. Enabling + *synchronous* debug output circumvents this bug. Can be triggered by + running TransformFeedbackGLTest with GL_KHR_debug extension + disabled. */ + "nv-windows-dangling-transform-feedback-varying-names", #endif /* Layout qualifier causes compiler error with GLSL 1.20 on Mesa, GLSL