From 5f54cc4702ac101c6d598933b2179d659b0c83b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Sat, 19 Feb 2022 12:07:56 +0100 Subject: [PATCH] GL: drop a silly from state tracker internals. Using a tuple was very useful, helpful and exciting as I had to make an explicit comment about what element is what, and then having to remember the order in all places that accessed the tuple. Using a struct however is rather boring as the fields are just named there, I don't need any complex std::get<4>() and extra comments explaining what is where and it's just not so adventurous anymore. The build time of a non-deprecated MagnumGL library also dropped from 5.9 seconds to 5.85. SAD! --- src/Magnum/GL/AbstractTexture.cpp | 17 ++++++----- src/Magnum/GL/Implementation/State.cpp | 3 +- src/Magnum/GL/Implementation/TextureState.cpp | 7 ++--- src/Magnum/GL/Implementation/TextureState.h | 28 +++++++++++++++++-- 4 files changed, 37 insertions(+), 18 deletions(-) diff --git a/src/Magnum/GL/AbstractTexture.cpp b/src/Magnum/GL/AbstractTexture.cpp index a06479b2f..9754b65d6 100644 --- a/src/Magnum/GL/AbstractTexture.cpp +++ b/src/Magnum/GL/AbstractTexture.cpp @@ -25,7 +25,6 @@ #include "AbstractTexture.h" -#include #include #ifndef MAGNUM_TARGET_WEBGL #include @@ -245,7 +244,7 @@ AbstractTexture::~AbstractTexture() { /* Remove all image bindings */ for(auto& binding: Context::current().state().texture.imageBindings) { /* MSVC 2015 needs the parentheses around */ - if(std::get<0>(binding) == _id) binding = {}; + if(binding.id == _id) binding = {}; } #endif @@ -281,10 +280,10 @@ void AbstractTexture::unbindImage(const Int imageUnit) { Implementation::TextureState& textureState = Context::current().state().texture; /* If already unbound in given image unit, nothing to do */ - if(std::get<0>(textureState.imageBindings[imageUnit]) == 0) return; + if(textureState.imageBindings[imageUnit].id == 0) return; /* Update state tracker, bind the texture to the unit */ - std::get<0>(textureState.imageBindings[imageUnit]) = 0; + textureState.imageBindings[imageUnit].id = 0; glBindImageTexture(imageUnit, 0, 0, false, 0, GL_READ_ONLY, GL_RGBA8); } @@ -297,15 +296,15 @@ void AbstractTexture::bindImages(const Int firstImageUnit, Containers::ArrayView Containers::Array ids{textures ? textures.size() : 0}; bool different = false; for(std::size_t i = 0; i != textures.size(); ++i) { - const std::tuple state = textures && textures[i] ? - std::tuple(textures[i]->_id, 0, true, 0, GL_READ_WRITE) : - std::tuple(0, 0, false, 0, GL_READ_ONLY); + const Implementation::TextureState::ImageBinding state = textures && textures[i] ? + Implementation::TextureState::ImageBinding{textures[i]->_id, 0, true, 0, GL_READ_WRITE} : + Implementation::TextureState::ImageBinding{0, 0, false, 0, GL_READ_ONLY}; if(textures) { if(textures[i]) { textures[i]->createIfNotAlready(); } - ids[i] = std::get<0>(state); + ids[i] = state.id; } if(textureState.imageBindings[firstImageUnit + i] != state) { @@ -321,7 +320,7 @@ void AbstractTexture::bindImages(const Int firstImageUnit, Containers::ArrayView void AbstractTexture::bindImageInternal(const Int imageUnit, const Int level, const bool layered, const Int layer, const ImageAccess access, const ImageFormat format) { Implementation::TextureState& textureState = Context::current().state().texture; - const std::tuple state{_id, level, layered, layer, GLenum(access)}; + const Implementation::TextureState::ImageBinding state{_id, level, layered, layer, GLenum(access)}; /* If already bound in given texture unit, nothing to do */ if(textureState.imageBindings[imageUnit] == state) return; diff --git a/src/Magnum/GL/Implementation/State.cpp b/src/Magnum/GL/Implementation/State.cpp index f1af6dfd3..e3eb1149b 100644 --- a/src/Magnum/GL/Implementation/State.cpp +++ b/src/Magnum/GL/Implementation/State.cpp @@ -25,7 +25,6 @@ #include "State.h" -#include #include #include @@ -86,7 +85,7 @@ std::pair State::allocate(Context& context, std: Containers::ArrayView textureStateView; Containers::ArrayView> textureBindings; #if !defined(MAGNUM_TARGET_GLES2) && !defined(MAGNUM_TARGET_WEBGL) - Containers::ArrayView> imageBindings; + Containers::ArrayView imageBindings; #endif #ifndef MAGNUM_TARGET_GLES2 Containers::ArrayView transformFeedbackStateView; diff --git a/src/Magnum/GL/Implementation/TextureState.cpp b/src/Magnum/GL/Implementation/TextureState.cpp index 077397a83..8d51cea6e 100644 --- a/src/Magnum/GL/Implementation/TextureState.cpp +++ b/src/Magnum/GL/Implementation/TextureState.cpp @@ -25,7 +25,6 @@ #include "TextureState.h" -#include #include #include @@ -46,7 +45,7 @@ using namespace Containers::Literals; TextureState::TextureState(Context& context, Containers::ArrayView> bindings, #if !defined(MAGNUM_TARGET_GLES2) && !defined(MAGNUM_TARGET_WEBGL) - Containers::ArrayView> imageBindings, + Containers::ArrayView imageBindings, #endif Containers::StaticArrayView extensions): maxSize{}, @@ -551,8 +550,8 @@ void TextureState::reset() { for(std::pair& i: bindings) i = {{}, State::DisengagedBinding}; #if !defined(MAGNUM_TARGET_GLES2) && !defined(MAGNUM_TARGET_WEBGL) - for(std::tuple& i: imageBindings) - i = std::make_tuple(State::DisengagedBinding, 0, false, 0, 0); + for(ImageBinding& i: imageBindings) + i = {State::DisengagedBinding, 0, false, 0, 0}; #endif } diff --git a/src/Magnum/GL/Implementation/TextureState.h b/src/Magnum/GL/Implementation/TextureState.h index 90efe6ef7..8ded1dbf2 100644 --- a/src/Magnum/GL/Implementation/TextureState.h +++ b/src/Magnum/GL/Implementation/TextureState.h @@ -25,8 +25,8 @@ DEALINGS IN THE SOFTWARE. */ +#include /* std::pair */ #include -#include #include "Magnum/Magnum.h" #include "Magnum/GL/GL.h" @@ -51,10 +51,32 @@ namespace Magnum { namespace GL { namespace Implementation { struct TextureState { + #if !defined(MAGNUM_TARGET_GLES2) && !defined(MAGNUM_TARGET_WEBGL) + struct ImageBinding { + GLuint id; + GLint level; + GLboolean layered; + GLint layer; + GLenum access; + + /* Used inside AbstractTexture to check if the state changed */ + bool operator==(const ImageBinding& other) { + return other.id == id && + other.level == level && + other.layered == layered && + other.layer == layer && + other.access == access; + } + bool operator!=(const ImageBinding& other) { + return !operator==(other); + } + }; + #endif + explicit TextureState(Context& context, Containers::ArrayView> bindings, #if !defined(MAGNUM_TARGET_GLES2) && !defined(MAGNUM_TARGET_WEBGL) - Containers::ArrayView> imageBindings, + Containers::ArrayView imageBindings, #endif Containers::StaticArrayView extensions); @@ -171,7 +193,7 @@ struct TextureState { #endif #if !defined(MAGNUM_TARGET_GLES2) && !defined(MAGNUM_TARGET_WEBGL) /* Texture object ID, level, layered, layer, access */ - Containers::ArrayView> imageBindings; + Containers::ArrayView imageBindings; #endif };