From 0d02fdc1549fbd15a2392ba06eb46c3fa9d0bec4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Mon, 3 Aug 2015 22:40:18 +0200 Subject: [PATCH] Store a copy instead of pointer to Buffer in Mesh. Currently the user had to ensure that buffers added to mesh were not moved at all, which was very annoying, basically each one of them had to be allocated on heap. Now the Mesh stores a weak copy (yes, really) using Buffer::wrap() with no deletion on destruction, so the original instance can be freely moved around without any fear of crash. Thanks to @Squareys for the original idea/request about wrap() functions, really useful part of the API. --- src/Magnum/Implementation/MeshState.h | 2 +- src/Magnum/Mesh.cpp | 60 +++++++++++++++-------- src/Magnum/Mesh.h | 68 +++++++++++---------------- 3 files changed, 69 insertions(+), 61 deletions(-) diff --git a/src/Magnum/Implementation/MeshState.h b/src/Magnum/Implementation/MeshState.h index 2aec542b8..bfae9ef02 100644 --- a/src/Magnum/Implementation/MeshState.h +++ b/src/Magnum/Implementation/MeshState.h @@ -39,7 +39,7 @@ struct MeshState { void(Mesh::*createImplementation)(); void(Mesh::*destroyImplementation)(); - void(Mesh::*attributePointerImplementation)(const Mesh::AttributeLayout&); + void(Mesh::*attributePointerImplementation)(Mesh::AttributeLayout&); #if !defined(MAGNUM_TARGET_GLES) || defined(MAGNUM_TARGET_GLES2) void(Mesh::*vertexAttribDivisorImplementation)(GLuint, GLuint); #endif diff --git a/src/Magnum/Mesh.cpp b/src/Magnum/Mesh.cpp index e1583e727..84d7a8937 100644 --- a/src/Magnum/Mesh.cpp +++ b/src/Magnum/Mesh.cpp @@ -41,6 +41,21 @@ namespace Magnum { +struct Mesh::AttributeLayout { + explicit AttributeLayout(const Buffer& buffer, GLuint location, GLint size, GLenum type, AttributeKind kind, GLintptr offset, GLsizei stride, GLuint divisor) noexcept: buffer{Buffer::wrap(buffer.id())}, location{location}, size{size}, type{type}, kind{kind}, offset{offset}, stride{stride}, divisor{divisor} {} + + explicit AttributeLayout(const AttributeLayout& other): buffer{Buffer::wrap(other.buffer.id())}, location{other.location}, size{other.size}, type{other.type}, kind{other.kind}, offset{other.offset}, stride{other.stride}, divisor{other.divisor} {} + + Buffer buffer; + GLuint location; + GLint size; + GLenum type; + AttributeKind kind; + GLintptr offset; + GLsizei stride; + GLuint divisor; +}; + #ifdef MAGNUM_BUILD_DEPRECATED Int Mesh::maxVertexAttributes() { return AbstractShaderProgram::maxVertexAttributes(); } #endif @@ -174,6 +189,8 @@ Mesh& Mesh::operator=(Mesh&& other) noexcept { return *this; } +Mesh::Mesh(const GLuint id, const MeshPrimitive primitive, const ObjectFlags flags): _id{id}, _primitive{primitive}, _flags{flags} {} + inline void Mesh::createIfNotAlready() { /* If VAO extension is not available, the following is always true */ if(_flags & ObjectFlag::Created) return; @@ -397,23 +414,28 @@ void Mesh::destroyImplementationVAO() { #endif } -void Mesh::attributePointerInternal(const AttributeLayout& attribute) { +void Mesh::attributePointerInternal(const Buffer& buffer, const GLuint location, const GLint size, const GLenum type, const AttributeKind kind, const GLintptr offset, const GLsizei stride, const GLuint divisor) { + AttributeLayout l{buffer, location, size, type, kind, offset, stride, divisor}; + attributePointerInternal(l); +} + +void Mesh::attributePointerInternal(AttributeLayout& attribute) { (this->*Context::current()->state().mesh->attributePointerImplementation)(attribute); } -void Mesh::attributePointerImplementationDefault(const AttributeLayout& attribute) { +void Mesh::attributePointerImplementationDefault(AttributeLayout& attribute) { #if defined(CORRADE_TARGET_NACL) || defined(MAGNUM_TARGET_WEBGL) - CORRADE_ASSERT(attribute.buffer->targetHint() == Buffer::TargetHint::Array, - "Mesh::addVertexBuffer(): the buffer has unexpected target hint, expected" << Buffer::TargetHint::Array << "but got" << attribute.buffer->targetHint(), ); + CORRADE_ASSERT(attribute.buffer.targetHint() == Buffer::TargetHint::Array, + "Mesh::addVertexBuffer(): the buffer has unexpected target hint, expected" << Buffer::TargetHint::Array << "but got" << attribute.buffer.targetHint(), ); #endif _attributes.push_back(attribute); } -void Mesh::attributePointerImplementationVAO(const AttributeLayout& attribute) { +void Mesh::attributePointerImplementationVAO(AttributeLayout& attribute) { #if defined(CORRADE_TARGET_NACL) || defined(MAGNUM_TARGET_WEBGL) - CORRADE_ASSERT(attribute.buffer->targetHint() == Buffer::TargetHint::Array, - "Mesh::addVertexBuffer(): the buffer has unexpected target hint, expected" << Buffer::TargetHint::Array << "but got" << attribute.buffer->targetHint(), ); + CORRADE_ASSERT(attribute.buffer.targetHint() == Buffer::TargetHint::Array, + "Mesh::addVertexBuffer(): the buffer has unexpected target hint, expected" << Buffer::TargetHint::Array << "but got" << attribute.buffer.targetHint(), ); #endif bindVAO(); @@ -421,21 +443,21 @@ void Mesh::attributePointerImplementationVAO(const AttributeLayout& attribute) { } #ifndef MAGNUM_TARGET_GLES -void Mesh::attributePointerImplementationDSAEXT(const AttributeLayout& attribute) { +void Mesh::attributePointerImplementationDSAEXT(AttributeLayout& attribute) { _flags |= ObjectFlag::Created; glEnableVertexArrayAttribEXT(_id, attribute.location); #ifndef MAGNUM_TARGET_GLES2 - if(attribute.kind == AttributeLayout::Kind::Integral) - glVertexArrayVertexAttribIOffsetEXT(_id, attribute.buffer->id(), attribute.location, attribute.size, attribute.type, attribute.stride, attribute.offset); + if(attribute.kind == AttributeKind::Integral) + glVertexArrayVertexAttribIOffsetEXT(_id, attribute.buffer.id(), attribute.location, attribute.size, attribute.type, attribute.stride, attribute.offset); #ifndef MAGNUM_TARGET_GLES - else if(attribute.kind == AttributeLayout::Kind::Long) - glVertexArrayVertexAttribLOffsetEXT(_id, attribute.buffer->id(), attribute.location, attribute.size, attribute.type, attribute.stride, attribute.offset); + else if(attribute.kind == AttributeKind::Long) + glVertexArrayVertexAttribLOffsetEXT(_id, attribute.buffer.id(), attribute.location, attribute.size, attribute.type, attribute.stride, attribute.offset); #endif else #endif { - glVertexArrayVertexAttribOffsetEXT(_id, attribute.buffer->id(), attribute.location, attribute.size, attribute.type, attribute.kind == AttributeLayout::Kind::GenericNormalized, attribute.stride, attribute.offset); + glVertexArrayVertexAttribOffsetEXT(_id, attribute.buffer.id(), attribute.location, attribute.size, attribute.type, attribute.kind == AttributeKind::GenericNormalized, attribute.stride, attribute.offset); } if(attribute.divisor) @@ -443,21 +465,21 @@ void Mesh::attributePointerImplementationDSAEXT(const AttributeLayout& attribute } #endif -void Mesh::vertexAttribPointer(const AttributeLayout& attribute) { +void Mesh::vertexAttribPointer(AttributeLayout& attribute) { glEnableVertexAttribArray(attribute.location); - attribute.buffer->bindInternal(Buffer::TargetHint::Array); + attribute.buffer.bindInternal(Buffer::TargetHint::Array); #ifndef MAGNUM_TARGET_GLES2 - if(attribute.kind == AttributeLayout::Kind::Integral) + if(attribute.kind == AttributeKind::Integral) glVertexAttribIPointer(attribute.location, attribute.size, attribute.type, attribute.stride, reinterpret_cast(attribute.offset)); #ifndef MAGNUM_TARGET_GLES - else if(attribute.kind == AttributeLayout::Kind::Long) + else if(attribute.kind == AttributeKind::Long) glVertexAttribLPointer(attribute.location, attribute.size, attribute.type, attribute.stride, reinterpret_cast(attribute.offset)); #endif else #endif { - glVertexAttribPointer(attribute.location, attribute.size, attribute.type, attribute.kind == AttributeLayout::Kind::GenericNormalized, attribute.stride, reinterpret_cast(attribute.offset)); + glVertexAttribPointer(attribute.location, attribute.size, attribute.type, attribute.kind == AttributeKind::GenericNormalized, attribute.stride, reinterpret_cast(attribute.offset)); } if(attribute.divisor) { @@ -523,7 +545,7 @@ void Mesh::bindIndexBufferImplementationVAO(Buffer& buffer) { void Mesh::bindImplementationDefault() { /* Specify vertex attributes */ - for(const AttributeLayout& attribute: _attributes) + for(AttributeLayout& attribute: _attributes) vertexAttribPointer(attribute); /* Bind index buffer, if the mesh is indexed */ diff --git a/src/Magnum/Mesh.h b/src/Magnum/Mesh.h index 3f862e0ba..4d30ea2b1 100644 --- a/src/Magnum/Mesh.h +++ b/src/Magnum/Mesh.h @@ -856,29 +856,20 @@ class MAGNUM_EXPORT Mesh: public AbstractObject { void draw(AbstractShaderProgram&& shader) { draw(shader); } /**< @overload */ private: - struct MAGNUM_LOCAL AttributeLayout { - enum class Kind { - Generic, - GenericNormalized, - #ifndef MAGNUM_TARGET_GLES2 - Integral, - #ifndef MAGNUM_TARGET_GLES - Long - #endif - #endif - }; - - Buffer* buffer; - GLuint location; - GLint size; - GLenum type; - Kind kind; - GLintptr offset; - GLsizei stride; - GLuint divisor; + enum class AttributeKind { + Generic, + GenericNormalized, + #ifndef MAGNUM_TARGET_GLES2 + Integral, + #ifndef MAGNUM_TARGET_GLES + Long + #endif + #endif }; - explicit Mesh(GLuint id, MeshPrimitive primitive, ObjectFlags flags): _id{id}, _primitive{primitive}, _flags{flags} {} + struct MAGNUM_LOCAL AttributeLayout; + + explicit Mesh(GLuint id, MeshPrimitive primitive, ObjectFlags flags); void MAGNUM_LOCAL createIfNotAlready(); @@ -910,45 +901,39 @@ class MAGNUM_EXPORT Mesh: public AbstractObject { template void addVertexAttribute(typename std::enable_if::ScalarType, Float>::value, Buffer&>::type buffer, const Attribute& attribute, GLintptr offset, GLsizei stride, GLuint divisor) { for(UnsignedInt i = 0; i != Attribute::VectorCount; ++i) - attributePointerInternal(AttributeLayout{ - &buffer, + attributePointerInternal(buffer, location+i, GLint(attribute.components()), GLenum(attribute.dataType()), - attribute.dataOptions() & Attribute::DataOption::Normalized ? AttributeLayout::Kind::GenericNormalized : AttributeLayout::Kind::Generic, + attribute.dataOptions() & Attribute::DataOption::Normalized ? AttributeKind::GenericNormalized : AttributeKind::Generic, GLintptr(offset+i*attribute.vectorSize()), stride, - divisor - }); + divisor); } #ifndef MAGNUM_TARGET_GLES2 template void addVertexAttribute(typename std::enable_if::ScalarType>::value, Buffer&>::type buffer, const Attribute& attribute, GLintptr offset, GLsizei stride, GLuint divisor) { - attributePointerInternal(AttributeLayout{ - &buffer, + attributePointerInternal(buffer, location, GLint(attribute.components()), GLenum(attribute.dataType()), - AttributeLayout::Kind::Integral, + AttributeKind::Integral, offset, stride, - divisor - }); + divisor); } #ifndef MAGNUM_TARGET_GLES template void addVertexAttribute(typename std::enable_if::ScalarType, Double>::value, Buffer&>::type buffer, const Attribute& attribute, GLintptr offset, GLsizei stride, GLuint divisor) { for(UnsignedInt i = 0; i != Attribute::VectorCount; ++i) - attributePointerInternal(AttributeLayout{ - &buffer, + attributePointerInternal(buffer, location+i, GLint(attribute.components()), GLenum(attribute.dataType()), - AttributeLayout::Kind::Long, + AttributeKind::Long, GLintptr(offset+i*attribute.vectorSize()), stride, - divisor - }); + divisor); } #endif #endif @@ -972,13 +957,14 @@ class MAGNUM_EXPORT Mesh: public AbstractObject { void MAGNUM_LOCAL destroyImplementationDefault(); void MAGNUM_LOCAL destroyImplementationVAO(); - void attributePointerInternal(const AttributeLayout& attribute); - void MAGNUM_LOCAL attributePointerImplementationDefault(const AttributeLayout& attribute); - void MAGNUM_LOCAL attributePointerImplementationVAO(const AttributeLayout& attribute); + void attributePointerInternal(const Buffer& buffer, GLuint location, GLint size, GLenum type, AttributeKind kind, GLintptr offset, GLsizei stride, GLuint divisor); + void MAGNUM_LOCAL attributePointerInternal(AttributeLayout& attribute); + void MAGNUM_LOCAL attributePointerImplementationDefault(AttributeLayout& attribute); + void MAGNUM_LOCAL attributePointerImplementationVAO(AttributeLayout& attribute); #ifndef MAGNUM_TARGET_GLES - void MAGNUM_LOCAL attributePointerImplementationDSAEXT(const AttributeLayout& attribute); + void MAGNUM_LOCAL attributePointerImplementationDSAEXT(AttributeLayout& attribute); #endif - void MAGNUM_LOCAL vertexAttribPointer(const AttributeLayout& attribute); + void MAGNUM_LOCAL vertexAttribPointer(AttributeLayout& attribute); #ifndef MAGNUM_TARGET_GLES void MAGNUM_LOCAL vertexAttribDivisorImplementationVAO(GLuint index, GLuint divisor);