Skip to content

Commit

Permalink
GL: finalize the apple-buffer-texture-unbind-on-buffer-modify workaro…
Browse files Browse the repository at this point in the history
…und.

Followup to 24cc971, covering the
remaining case.
  • Loading branch information
mosra committed Jun 26, 2020
1 parent 5147377 commit 36f51e3
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 10 deletions.
19 changes: 14 additions & 5 deletions src/Magnum/GL/AbstractTexture.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,14 @@ void AbstractTexture::bindImplementationDSAIntelWindows(const GLint textureUnit)
else bindImplementationDSA(textureUnit);
}
#endif

#ifdef CORRADE_TARGET_APPLE
void AbstractTexture::bindImplementationAppleBufferTextureWorkaround(const GLint textureUnit) {
bindImplementationDefault(textureUnit);
if(_target == GL_TEXTURE_BUFFER)
Context::current().state().texture->bufferTextureBound.set(textureUnit, true);
}
#endif
#endif

#ifndef MAGNUM_TARGET_GLES2
Expand Down Expand Up @@ -526,13 +534,14 @@ void AbstractTexture::bindInternal() {
if(textureState.currentTextureUnit != internalTextureUnit)
glActiveTexture(GL_TEXTURE0 + (textureState.currentTextureUnit = internalTextureUnit));

/* Bind the texture to internal unit if not already, update state tracker */
/* If already bound in given texture unit, nothing to do */
if(textureState.bindings[internalTextureUnit].second == _id) return;
textureState.bindings[internalTextureUnit] = {_target, _id};

/* Binding the texture finally creates it */
_flags |= ObjectFlag::Created;
glBindTexture(_target, _id);
/* Update state tracker, bind the texture to the unit. Not directly calling
glBindTexture() here because we may need to include various
platform-specific workarounds (Apple, Intel Windpws) */
textureState.bindings[internalTextureUnit] = {_target, _id};
(this->*textureState.bindImplementation)(internalTextureUnit);
}

#if !defined(MAGNUM_TARGET_GLES) || defined(MAGNUM_TARGET_GLES2)
Expand Down
3 changes: 3 additions & 0 deletions src/Magnum/GL/AbstractTexture.h
Original file line number Diff line number Diff line change
Expand Up @@ -551,6 +551,9 @@ class MAGNUM_GL_EXPORT AbstractTexture: public AbstractObject {
#ifdef CORRADE_TARGET_WINDOWS
void MAGNUM_GL_LOCAL bindImplementationDSAIntelWindows(GLint textureUnit);
#endif
#ifdef CORRADE_TARGET_APPLE
void MAGNUM_GL_LOCAL bindImplementationAppleBufferTextureWorkaround(GLint textureUnit);
#endif
#endif

void MAGNUM_GL_LOCAL parameterImplementationDefault(GLenum parameter, GLint value);
Expand Down
18 changes: 13 additions & 5 deletions src/Magnum/GL/Buffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -553,12 +553,19 @@ bool Buffer::unmapImplementationDSA() {
#if defined(CORRADE_TARGET_APPLE) && !defined(CORRADE_TARGET_IOS)
/* See apple-buffer-texture-detach-on-data-modify for the gory details. */
void Buffer::textureWorkaroundAppleBefore() {
/* Apple "fortunately" supports just 16 texture units, so this doesn't take
too long. */
/* My Mac Mini reports 80 texture units, which means this thing can have a
pretty significant overhead. Skipping the whole thing if no buffer
texture is known to be bound. */
Implementation::TextureState& textureState = *Context::current().state().texture;
if(textureState.bufferTextureBound.none()) return;
for(GLint textureUnit = 0; textureUnit != GLint(textureState.bindings.size()); ++textureUnit) {
std::pair<GLenum, GLuint>& binding = textureState.bindings[textureUnit];
if(binding.first != GL_TEXTURE_BUFFER) continue;
/* Checking just
textureState.bindings[textureUnit].first != GL_TEXTURE_BUFFER
isn't enough, as for GL allows to bind different texture types under
the same texture unit. Magnum's state tracker ignores that (as it
would mean having to maintain a state cache of 128 units times 12
targets) and so this state is tracked separately. */
if(!textureState.bufferTextureBound[textureUnit]) continue;

/* Activate given texture unit if not already active, update state
tracker */
Expand All @@ -569,7 +576,8 @@ void Buffer::textureWorkaroundAppleBefore() {
glBindTexture(GL_TEXTURE_BUFFER, 0);
/* libstdc++ since GCC 6.3 can't handle just = {} (ambiguous overload
of operator=) */
binding = std::pair<GLenum, GLuint>{};
textureState.bindings[textureUnit] = std::pair<GLenum, GLuint>{};
textureState.bufferTextureBound.set(textureUnit, false);
}
}

Expand Down
10 changes: 10 additions & 0 deletions src/Magnum/GL/Implementation/TextureState.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -488,6 +488,16 @@ TextureState::TextureState(Context& context, std::vector<std::string>& extension
CORRADE_INTERNAL_ASSERT(maxTextureUnits > 0);
bindings = Containers::Array<std::pair<GLenum, GLuint>>{Containers::ValueInit, std::size_t(maxTextureUnits)};

#if defined(CORRADE_TARGET_APPLE) && !defined(CORRADE_TARGET_IOS)
if(!context.isDriverWorkaroundDisabled("apple-buffer-texture-unbind-on-buffer-modify")) {
CORRADE_INTERNAL_ASSERT(std::size_t(maxTextureUnits) <= decltype(bufferTextureBound)::Size);
/* Assume ARB_multi_bind is not supported, otherwise we'd need to
implement the workaround also for bindMultiImplementation */
CORRADE_INTERNAL_ASSERT(!context.isExtensionSupported<Extensions::ARB::multi_bind>());
bindImplementation = &AbstractTexture::bindImplementationAppleBufferTextureWorkaround;
}
#endif

#if !defined(MAGNUM_TARGET_GLES2) && !defined(MAGNUM_TARGET_WEBGL)
/* Allocate image bindings array to hold all possible image units */
#ifndef MAGNUM_TARGET_GLES
Expand Down
7 changes: 7 additions & 0 deletions src/Magnum/GL/Implementation/TextureState.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,10 @@
#endif
#endif

#if defined(CORRADE_TARGET_APPLE) && !defined(MAGNUM_TARGET_GLES)
#include "Magnum/Math/BoolVector.h"
#endif

namespace Magnum { namespace GL { namespace Implementation {

struct TextureState {
Expand Down Expand Up @@ -153,6 +157,9 @@ struct TextureState {
#endif

Containers::Array<std::pair<GLenum, GLuint>> bindings;
#if defined(CORRADE_TARGET_APPLE) && !defined(CORRADE_TARGET_IOS)
Math::BoolVector<80> bufferTextureBound;
#endif
#if !defined(MAGNUM_TARGET_GLES2) && !defined(MAGNUM_TARGET_WEBGL)
/* Texture object ID, level, layered, layer, access */
Containers::Array<std::tuple<GLuint, GLint, GLboolean, GLint, GLenum>> imageBindings;
Expand Down

0 comments on commit 36f51e3

Please sign in to comment.