From dc10402c1c72643a9d6aa6f604e5f60f7e8f3118 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Sun, 17 Aug 2014 16:15:19 +0200 Subject: [PATCH] Ensure that the mesh object is properly created before using it. --- src/Magnum/Mesh.cpp | 50 +++++++++++++++++++++++++--------- src/Magnum/Mesh.h | 7 +++-- src/Magnum/Test/MeshGLTest.cpp | 12 ++------ 3 files changed, 45 insertions(+), 24 deletions(-) diff --git a/src/Magnum/Mesh.cpp b/src/Magnum/Mesh.cpp index b7b654da7..0a2794e63 100644 --- a/src/Magnum/Mesh.cpp +++ b/src/Magnum/Mesh.cpp @@ -72,7 +72,7 @@ std::size_t Mesh::indexSize(IndexType type) { CORRADE_ASSERT_UNREACHABLE(); } -Mesh::Mesh(MeshPrimitive primitive): _primitive(primitive), _count(0), _baseVertex(0), _instanceCount{1}, +Mesh::Mesh(const MeshPrimitive primitive): _primitive{primitive}, _count{0}, _baseVertex{0}, _instanceCount{1}, #ifndef MAGNUM_TARGET_GLES _baseInstance{0}, #endif @@ -95,7 +95,7 @@ Mesh::~Mesh() { (this->*Context::current()->state().mesh->destroyImplementation)(); } -Mesh::Mesh(Mesh&& other) noexcept: _id(other._id), _primitive(other._primitive), _count(other._count), _baseVertex{other._baseVertex}, _instanceCount{other._instanceCount}, +Mesh::Mesh(Mesh&& other) noexcept: _id(other._id), _created{other._created}, _primitive(other._primitive), _count(other._count), _baseVertex{other._baseVertex}, _instanceCount{other._instanceCount}, #ifndef MAGNUM_TARGET_GLES _baseInstance{other._baseInstance}, #endif @@ -115,6 +115,7 @@ Mesh::Mesh(Mesh&& other) noexcept: _id(other._id), _primitive(other._primitive), Mesh& Mesh::operator=(Mesh&& other) noexcept { std::swap(_id, other._id); + std::swap(_created, other._created); std::swap(_primitive, other._primitive); std::swap(_count, other._count); std::swap(_baseVertex, other._baseVertex); @@ -140,7 +141,20 @@ Mesh& Mesh::operator=(Mesh&& other) noexcept { return *this; } -std::string Mesh::label() const { +inline void Mesh::createIfNotAlready() { + /* If VAO extension is not available, _created is always true */ + if(_created) return; + + /* glGen*() does not create the object, just reserves the name. Some + commands (such as glObjectLabel()) operate with IDs directly and they + require the object to be created. Binding the VAO finally creates it. + Also all EXT DSA functions implicitly create it. */ + bindVAO(); + CORRADE_INTERNAL_ASSERT(_created); +} + +std::string Mesh::label() { + createIfNotAlready(); #ifndef MAGNUM_TARGET_GLES return Context::current()->state().debug->getLabelImplementation(GL_VERTEX_ARRAY, _id); #else @@ -149,6 +163,7 @@ std::string Mesh::label() const { } Mesh& Mesh::setLabelInternal(const Containers::ArrayReference label) { + createIfNotAlready(); #ifndef MAGNUM_TARGET_GLES Context::current()->state().debug->labelImplementation(GL_VERTEX_ARRAY, _id, label); #else @@ -284,22 +299,28 @@ void Mesh::drawInternal(Int count, Int baseVertex, Int instanceCount, GLintptr i (this->*state.unbindImplementation)(); } -void Mesh::bindVAO(GLuint vao) { +void Mesh::bindVAO() { GLuint& current = Context::current()->state().mesh->currentVAO; - if(current != vao) { + if(current != _id) { + /* Binding the VAO finally creates it */ + _created = true; #ifndef MAGNUM_TARGET_GLES2 - glBindVertexArray(current = vao); + glBindVertexArray(current = _id); #elif !defined(CORRADE_TARGET_EMSCRIPTEN) && !defined(CORRADE_TARGET_NACL) - glBindVertexArrayOES(current = vao); + glBindVertexArrayOES(current = _id); #else CORRADE_ASSERT_UNREACHABLE(); #endif } } -void Mesh::createImplementationDefault() { _id = 0; } +void Mesh::createImplementationDefault() { + _id = 0; + _created = true; +} void Mesh::createImplementationVAO() { + _created = false; #ifndef MAGNUM_TARGET_GLES2 glGenVertexArrays(1, &_id); #elif !defined(CORRADE_TARGET_EMSCRIPTEN) && !defined(CORRADE_TARGET_NACL) @@ -341,12 +362,13 @@ void Mesh::attributePointerImplementationVAO(const Attribute& attribute) { "Mesh::addVertexBuffer(): the buffer has unexpected target hint, expected" << Buffer::Target::Array << "but got" << attribute.buffer->targetHint(), ); #endif - bindVAO(_id); + bindVAO(); vertexAttribPointer(attribute); } #ifndef MAGNUM_TARGET_GLES void Mesh::attributePointerImplementationDSA(const Attribute& attribute) { + _created = true; glEnableVertexArrayAttribEXT(_id, attribute.location); glVertexArrayVertexAttribOffsetEXT(_id, attribute.buffer->id(), attribute.location, attribute.size, attribute.type, attribute.normalized, attribute.stride, attribute.offset); if(attribute.divisor) glVertexArrayVertexAttribDivisorEXT(_id, attribute.location, attribute.divisor); @@ -376,12 +398,13 @@ void Mesh::attributePointerImplementationDefault(const IntegerAttribute& attribu } void Mesh::attributePointerImplementationVAO(const IntegerAttribute& attribute) { - bindVAO(_id); + bindVAO(); vertexAttribPointer(attribute); } #ifndef MAGNUM_TARGET_GLES void Mesh::attributePointerImplementationDSA(const IntegerAttribute& attribute) { + _created = true; glEnableVertexArrayAttribEXT(_id, attribute.location); glVertexArrayVertexAttribIOffsetEXT(_id, attribute.buffer->id(), attribute.location, attribute.size, attribute.type, attribute.stride, attribute.offset); if(attribute.divisor) glVertexArrayVertexAttribDivisorEXT(_id, attribute.location, attribute.divisor); @@ -406,11 +429,12 @@ void Mesh::attributePointerImplementationDefault(const LongAttribute& attribute) } void Mesh::attributePointerImplementationVAO(const LongAttribute& attribute) { - bindVAO(_id); + bindVAO(); vertexAttribPointer(attribute); } void Mesh::attributePointerImplementationDSA(const LongAttribute& attribute) { + _created = true; glEnableVertexArrayAttribEXT(_id, attribute.location); glVertexArrayVertexAttribLOffsetEXT(_id, attribute.buffer->id(), attribute.location, attribute.size, attribute.type, attribute.stride, attribute.offset); if(attribute.divisor) glVertexArrayVertexAttribDivisorEXT(_id, attribute.location, attribute.divisor); @@ -459,7 +483,7 @@ void Mesh::vertexAttribDivisorImplementationNV(const GLuint index, const GLuint void Mesh::bindIndexBufferImplementationDefault(Buffer&) {} void Mesh::bindIndexBufferImplementationVAO(Buffer& buffer) { - bindVAO(_id); + bindVAO(); /* Reset ElementArray binding to force explicit glBindBuffer call later */ /** @todo Do this cleaner way */ @@ -488,7 +512,7 @@ void Mesh::bindImplementationDefault() { } void Mesh::bindImplementationVAO() { - bindVAO(_id); + bindVAO(); } void Mesh::unbindImplementationDefault() { diff --git a/src/Magnum/Mesh.h b/src/Magnum/Mesh.h index eb5bba758..b79df2367 100644 --- a/src/Magnum/Mesh.h +++ b/src/Magnum/Mesh.h @@ -458,7 +458,7 @@ class MAGNUM_EXPORT Mesh: public AbstractObject { * @fn_gl_extension2{GetObjectLabel,EXT,debug_label} with * @def_gl{VERTEX_ARRAY_OBJECT_EXT} */ - std::string label() const; + std::string label(); /** * @brief Set mesh label @@ -860,6 +860,8 @@ class MAGNUM_EXPORT Mesh: public AbstractObject { #endif #endif + void MAGNUM_LOCAL createIfNotAlready(); + Mesh& setLabelInternal(Containers::ArrayReference label); /* Computing stride of interleaved vertex attributes */ @@ -927,7 +929,7 @@ class MAGNUM_EXPORT Mesh: public AbstractObject { #endif #endif - static void MAGNUM_LOCAL bindVAO(GLuint vao); + void MAGNUM_LOCAL bindVAO(); #ifndef MAGNUM_TARGET_GLES void drawInternal(Int count, Int baseVertex, Int instanceCount, UnsignedInt baseInstance, GLintptr indexOffset, Int indexStart, Int indexEnd); @@ -995,6 +997,7 @@ class MAGNUM_EXPORT Mesh: public AbstractObject { #endif GLuint _id; + bool _created; /* see createIfNotAlready() for details */ MeshPrimitive _primitive; Int _count, _baseVertex, _instanceCount; #ifndef MAGNUM_TARGET_GLES diff --git a/src/Magnum/Test/MeshGLTest.cpp b/src/Magnum/Test/MeshGLTest.cpp index 0431d282d..603b3501a 100644 --- a/src/Magnum/Test/MeshGLTest.cpp +++ b/src/Magnum/Test/MeshGLTest.cpp @@ -295,21 +295,15 @@ void MeshGLTest::label() { !Context::current()->isExtensionSupported()) CORRADE_SKIP("Required extension is not available"); - { - /** @todo Is this even legal optimization? */ - CORRADE_EXPECT_FAIL("The object must be used at least once before setting/querying label."); - CORRADE_VERIFY(false); - } - Buffer buffer{Buffer::Target::ElementArray}; Mesh mesh; - mesh.setIndexBuffer(buffer, 0, Mesh::IndexType::UnsignedShort); CORRADE_COMPARE(mesh.label(), ""); + MAGNUM_VERIFY_NO_ERROR(); mesh.setLabel("MyMesh"); - CORRADE_COMPARE(mesh.label(), "MyMesh"); - MAGNUM_VERIFY_NO_ERROR(); + + CORRADE_COMPARE(mesh.label(), "MyMesh"); } namespace {