diff --git a/doc/changelog.dox b/doc/changelog.dox index 786af0d1a..3da51ea8a 100644 --- a/doc/changelog.dox +++ b/doc/changelog.dox @@ -71,6 +71,10 @@ See also: @ref opengl-workarounds for more information). This pseudo-extension can be also explicitly disabled using the usual `--magnum-disable-extensions` @ref GL-Context-command-line "command-line option" for testing purposes. +- New @cpp "intel-windows-buggy-dsa-bufferdata-for-index-buffers" @ce + workaround for a synchronization bug in the Intel Windows drivers when + @ref GL::Buffer::setData() is followed by @ref GL::Mesh::setIndexBuffer() + when @gl_extension{ARB,direct_state_access} is used - New `--magnum-gpu-validation` @ref GL-Context-command-line "command-line option" and a corresponding environment variable to conveniently enable @gl_extension{KHR,debug} debug output diff --git a/src/Magnum/GL/Buffer.cpp b/src/Magnum/GL/Buffer.cpp index 9f06644f7..e90f7d328 100644 --- a/src/Magnum/GL/Buffer.cpp +++ b/src/Magnum/GL/Buffer.cpp @@ -464,6 +464,18 @@ void Buffer::dataImplementationDefault(GLsizeiptr size, const GLvoid* data, Buff void Buffer::dataImplementationDSA(const GLsizeiptr size, const GLvoid* const data, const BufferUsage usage) { glNamedBufferData(_id, size, data, GLenum(usage)); } + +#ifdef CORRADE_TARGET_WINDOWS +void Buffer::dataImplementationDSAIntelWindows(const GLsizeiptr size, const GLvoid* const data, const BufferUsage usage) { + glNamedBufferData(_id, size, data, GLenum(usage)); + /* See the "intel-windows-buggy-dsa-bufferdata-for-index-buffers" + workaround for more information */ + if(_targetHint == TargetHint::ElementArray) { + bindInternal(TargetHint::ElementArray, nullptr); + bindInternal(TargetHint::ElementArray, this); + } +} +#endif #endif void Buffer::subDataImplementationDefault(GLintptr offset, GLsizeiptr size, const GLvoid* data) { diff --git a/src/Magnum/GL/Buffer.h b/src/Magnum/GL/Buffer.h index 323b729e1..eef84717c 100644 --- a/src/Magnum/GL/Buffer.h +++ b/src/Magnum/GL/Buffer.h @@ -1261,6 +1261,7 @@ class MAGNUM_GL_EXPORT Buffer: public AbstractObject { void MAGNUM_GL_LOCAL dataImplementationDefault(GLsizeiptr size, const GLvoid* data, BufferUsage usage); #ifndef MAGNUM_TARGET_GLES + void MAGNUM_GL_LOCAL dataImplementationDSAIntelWindows(GLsizeiptr size, const GLvoid* data, BufferUsage usage); void MAGNUM_GL_LOCAL dataImplementationDSA(GLsizeiptr size, const GLvoid* data, BufferUsage usage); #endif diff --git a/src/Magnum/GL/Implementation/BufferState.cpp b/src/Magnum/GL/Implementation/BufferState.cpp index bef0c06f7..19d6fa1cb 100644 --- a/src/Magnum/GL/Implementation/BufferState.cpp +++ b/src/Magnum/GL/Implementation/BufferState.cpp @@ -107,7 +107,6 @@ BufferState::BufferState(Context& context, std::vector& extensions) copyImplementation = &Buffer::copyImplementationDSA; getParameterImplementation = &Buffer::getParameterImplementationDSA; getSubDataImplementation = &Buffer::getSubDataImplementationDSA; - dataImplementation = &Buffer::dataImplementationDSA; subDataImplementation = &Buffer::subDataImplementationDSA; mapImplementation = &Buffer::mapImplementationDSA; mapRangeImplementation = &Buffer::mapRangeImplementationDSA; @@ -123,7 +122,6 @@ BufferState::BufferState(Context& context, std::vector& extensions) #ifndef MAGNUM_TARGET_GLES getSubDataImplementation = &Buffer::getSubDataImplementationDefault; #endif - dataImplementation = &Buffer::dataImplementationDefault; subDataImplementation = &Buffer::subDataImplementationDefault; #ifndef MAGNUM_TARGET_WEBGL mapImplementation = &Buffer::mapImplementationDefault; @@ -181,6 +179,24 @@ BufferState::BufferState(Context& context, std::vector& extensions) setTargetHintImplementation = &Buffer::setTargetHintImplementationDefault; } + #ifndef MAGNUM_TARGET_GLES + if(context.isExtensionSupported()) { + #ifdef CORRADE_TARGET_WINDOWS + if((context.detectedDriver() & Context::DetectedDriver::IntelWindows) && + !context.isDriverWorkaroundDisabled("intel-windows-buggy-dsa-bufferdata-for-index-buffers")) + { + dataImplementation = &Buffer::dataImplementationDSAIntelWindows; + } else + #endif + { + dataImplementation = &Buffer::dataImplementationDSA; + } + } else + #endif + { + dataImplementation = &Buffer::dataImplementationDefault; + } + #ifdef MAGNUM_TARGET_GLES static_cast(context); static_cast(extensions); diff --git a/src/Magnum/GL/Implementation/driverSpecific.cpp b/src/Magnum/GL/Implementation/driverSpecific.cpp index 7b6c6b6f3..5ba70aebf 100644 --- a/src/Magnum/GL/Implementation/driverSpecific.cpp +++ b/src/Magnum/GL/Implementation/driverSpecific.cpp @@ -200,6 +200,18 @@ namespace { "intel-windows-implementation-color-read-format-completely-broken", #endif + #if !defined(MAGNUM_TARGET_GLES) && defined(CORRADE_TARGET_WINDOWS) + /* Intel drivers on Windows have some synchronization / memory + alignment bug in the DSA glNamedBufferData() when the same buffer is + set as an index buffer to a mesh right after or repeatedly. Calling + glBindBuffer() right before or after the data upload fixes the + issue. Note that this workaround is done only for buffers with + TargetHint::ElementArray, as the issue was not observed elsewhere. + Reproducible with the 2019.01 ImGui example, unfortunately I was not + able to create a standalone minimal repro case. */ + "intel-windows-buggy-dsa-bufferdata-for-index-buffers", + #endif + #ifndef MAGNUM_TARGET_GLES /* NVidia seems to be returning values for the default framebuffer when GL_IMPLEMENTATION_COLOR_READ_FORMAT and _TYPE is queried using diff --git a/src/Magnum/GL/Mesh.cpp b/src/Magnum/GL/Mesh.cpp index 1038c9f34..52dd2bdc3 100644 --- a/src/Magnum/GL/Mesh.cpp +++ b/src/Magnum/GL/Mesh.cpp @@ -678,10 +678,8 @@ void Mesh::attributePointerImplementationVAO(AttributeLayout&& attribute) { #ifndef MAGNUM_TARGET_GLES void Mesh::attributePointerImplementationVAODSA(AttributeLayout&& attribute) { - _flags |= ObjectFlag::Created; glEnableVertexArrayAttrib(_id, attribute.location); - #ifndef MAGNUM_TARGET_GLES2 if(attribute.kind == DynamicAttribute::Kind::Integral) glVertexArrayAttribIFormat(_id, attribute.location, attribute.size, attribute.type, 0);