From b278f00f77ed569eab5ac1dec3e9c177835c6d83 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Wed, 30 Aug 2023 19:09:09 +0200 Subject: [PATCH] GL: implement setIndexOffset() directly on the Mesh as well. So far this was only possible by creating a temporary MeshView, while everything else (index/vertex count, base vertex, base instance, ...) was changeable directly on the Mesh. --- doc/changelog.dox | 2 + src/Magnum/GL/AbstractShaderProgram.h | 14 +++--- src/Magnum/GL/Mesh.cpp | 32 +++++++++----- src/Magnum/GL/Mesh.h | 63 +++++++++++++++++++++++++-- src/Magnum/GL/MeshView.cpp | 6 ++- src/Magnum/GL/MeshView.h | 18 ++++++-- src/Magnum/GL/Test/MeshGLTest.cpp | 50 +++++++++++++++++++++ 7 files changed, 159 insertions(+), 26 deletions(-) diff --git a/doc/changelog.dox b/doc/changelog.dox index 67bd15b96..991de16f4 100644 --- a/doc/changelog.dox +++ b/doc/changelog.dox @@ -166,6 +166,8 @@ See also: - Exposed @gl_extension{ARB,buffer_storage} as @ref GL::Buffer::setStorage() together with additions to @ref GL::Buffer::MapFlag +- It's now possible to modify index offset via @ref GL::Mesh::setIndexOffset() + directly on the mesh itself instead of just through @ref GL::MeshView - Exposed missing @ref GL::Renderer::Feature::SampleAlphaToCoverage, @relativeref{GL::Renderer,Feature::SampleAlphaToOne}, @relativeref{GL::Renderer,Feature::SampleCoverage} and diff --git a/src/Magnum/GL/AbstractShaderProgram.h b/src/Magnum/GL/AbstractShaderProgram.h index f51a70c51..fc254915d 100644 --- a/src/Magnum/GL/AbstractShaderProgram.h +++ b/src/Magnum/GL/AbstractShaderProgram.h @@ -1272,13 +1272,13 @@ class MAGNUM_GL_EXPORT AbstractShaderProgram: public AbstractObject { * and that the output buffer(s) from @p xfb are used as vertex buffers * in the mesh. If its instance count is @cpp 0 @ce, no draw commands * are issued. Everything set by @ref Mesh::setCount(), - * @ref Mesh::setBaseInstance(), @ref Mesh::setBaseVertex() and - * @ref Mesh::setIndexBuffer() is ignored, the mesh is drawn as - * non-indexed and the vertex count is taken from the @p xfb object. If - * @p stream is @cpp 0 @ce, non-stream draw command is used. If - * @gl_extension{ARB,vertex_array_object} (part of OpenGL 3.0) is - * available, the associated vertex array object is bound instead of - * setting up the mesh from scratch. + * @ref Mesh::setBaseInstance(), @ref Mesh::setBaseVertex(), + * @ref Mesh::setIndexOffset() and @ref Mesh::setIndexBuffer() is + * ignored, the mesh is drawn as non-indexed and the vertex count is + * taken from the @p xfb object. If @p stream is @cpp 0 @ce, non-stream + * draw command is used. If @gl_extension{ARB,vertex_array_object} + * (part of OpenGL 3.0) is available, the associated vertex array + * object is bound instead of setting up the mesh from scratch. * @see @ref Mesh::setInstanceCount(), @ref draw(Mesh&), * @ref drawTransformFeedback(MeshView&, TransformFeedback&, UnsignedInt), * @fn_gl_keyword{UseProgram}, @fn_gl_keyword{EnableVertexAttribArray}, diff --git a/src/Magnum/GL/Mesh.cpp b/src/Magnum/GL/Mesh.cpp index 406e9c88e..ef21abaad 100644 --- a/src/Magnum/GL/Mesh.cpp +++ b/src/Magnum/GL/Mesh.cpp @@ -289,7 +289,7 @@ Mesh::Mesh(Mesh&& other) noexcept: _id(other._id), _primitive(other._primitive), #ifndef MAGNUM_TARGET_GLES2 _indexStart(other._indexStart), _indexEnd(other._indexEnd), #endif - _indexOffset(other._indexOffset), _indexType(other._indexType), _indexBuffer{std::move(other._indexBuffer)} + _indexBufferOffset(other._indexBufferOffset), _indexOffset(other._indexOffset), _indexType(other._indexType), _indexBuffer{std::move(other._indexBuffer)} { if(_constructed || other._constructed) Context::current().state().mesh.moveConstructImplementation(*this, std::move(other)); @@ -312,6 +312,7 @@ Mesh& Mesh::operator=(Mesh&& other) noexcept { swap(_indexStart, other._indexStart); swap(_indexEnd, other._indexEnd); #endif + swap(_indexBufferOffset, other._indexBufferOffset); swap(_indexOffset, other._indexOffset); swap(_indexType, other._indexType); swap(_indexBuffer, other._indexBuffer); @@ -372,6 +373,13 @@ UnsignedInt Mesh::indexTypeSize() const { } #endif +Mesh& Mesh::setIndexOffset(Int offset) { + CORRADE_ASSERT(_indexBuffer.id(), + "GL::Mesh::setIndexOffset(): mesh is not indexed", *this); + _indexOffset = offset; + return *this; +} + Mesh& Mesh::addVertexBufferInstanced(Buffer& buffer, const UnsignedInt divisor, const GLintptr offset, const GLsizei stride, const DynamicAttribute& attribute) { for(UnsignedInt i = 0; i != attribute.vectors(); ++i) attributePointerInternal(AttributeLayout{buffer, @@ -399,7 +407,7 @@ Mesh& Mesh::setIndexBuffer(Buffer&& buffer, GLintptr offset, MeshIndexType type, Context::current().state().mesh.bindIndexBufferImplementation(*this, buffer); _indexBuffer = std::move(buffer); - _indexOffset = offset; + _indexBufferOffset = offset; _indexType = type; #ifndef MAGNUM_TARGET_GLES2 _indexStart = start; @@ -798,6 +806,10 @@ void Mesh::drawInternal(Int count, Int baseVertex, Int instanceCount, GLintptr i { const Implementation::MeshState& state = Context::current().state().mesh; + const UnsignedInt indexByteOffset = _indexBuffer.id() ? + _indexBufferOffset + indexOffset*meshIndexTypeSize(_indexType) : + 0; + state.bindImplementation(*this); /* Non-instanced mesh */ @@ -823,7 +835,7 @@ void Mesh::drawInternal(Int count, Int baseVertex, Int instanceCount, GLintptr i #else state.drawRangeElementsBaseVertexImplementation #endif - (GLenum(_primitive), indexStart, indexEnd, count, GLenum(_indexType), reinterpret_cast(indexOffset), baseVertex); + (GLenum(_primitive), indexStart, indexEnd, count, GLenum(_indexType), reinterpret_cast(indexByteOffset), baseVertex); /* Indexed mesh */ } else @@ -834,7 +846,7 @@ void Mesh::drawInternal(Int count, Int baseVertex, Int instanceCount, GLintptr i #else state.drawElementsBaseVertexImplementation #endif - (GLenum(_primitive), count, GLenum(_indexType), reinterpret_cast(indexOffset), baseVertex); + (GLenum(_primitive), count, GLenum(_indexType), reinterpret_cast(indexByteOffset), baseVertex); } #else CORRADE_ASSERT_UNREACHABLE("GL::AbstractShaderProgram::draw(): indexed mesh draw with base vertex specification possible only since WebGL 2.0", ); @@ -848,7 +860,7 @@ void Mesh::drawInternal(Int count, Int baseVertex, Int instanceCount, GLintptr i #if !defined(MAGNUM_TARGET_GLES2) && !defined(MAGNUM_TARGET_WEBGL) /* Indexed mesh with specified range */ if(indexEnd) { - glDrawRangeElements(GLenum(_primitive), indexStart, indexEnd, count, GLenum(_indexType), reinterpret_cast(indexOffset)); + glDrawRangeElements(GLenum(_primitive), indexStart, indexEnd, count, GLenum(_indexType), reinterpret_cast(indexByteOffset)); /* Indexed mesh */ } else @@ -857,7 +869,7 @@ void Mesh::drawInternal(Int count, Int baseVertex, Int instanceCount, GLintptr i static_cast(indexEnd); #endif { - glDrawElements(GLenum(_primitive), count, GLenum(_indexType), reinterpret_cast(indexOffset)); + glDrawElements(GLenum(_primitive), count, GLenum(_indexType), reinterpret_cast(indexByteOffset)); } } @@ -897,7 +909,7 @@ void Mesh::drawInternal(Int count, Int baseVertex, Int instanceCount, GLintptr i #else state.drawElementsInstancedBaseVertexBaseInstanceImplementation #endif - (GLenum(_primitive), count, GLenum(_indexType), reinterpret_cast(indexOffset), instanceCount, baseVertex, baseInstance); + (GLenum(_primitive), count, GLenum(_indexType), reinterpret_cast(indexByteOffset), instanceCount, baseVertex, baseInstance); /* Indexed mesh with base vertex */ } else { @@ -906,7 +918,7 @@ void Mesh::drawInternal(Int count, Int baseVertex, Int instanceCount, GLintptr i #else state.drawElementsInstancedBaseVertexImplementation #endif - (GLenum(_primitive), count, GLenum(_indexType), reinterpret_cast(indexOffset), instanceCount, baseVertex); + (GLenum(_primitive), count, GLenum(_indexType), reinterpret_cast(indexByteOffset), instanceCount, baseVertex); } #else CORRADE_ASSERT_UNREACHABLE("GL::AbstractShaderProgram::draw(): instanced indexed mesh draw with base vertex specification possible only since OpenGL ES 3.0", ); @@ -922,7 +934,7 @@ void Mesh::drawInternal(Int count, Int baseVertex, Int instanceCount, GLintptr i #else state.drawElementsInstancedBaseInstanceImplementation #endif - (GLenum(_primitive), count, GLenum(_indexType), reinterpret_cast(indexOffset), instanceCount, baseInstance); + (GLenum(_primitive), count, GLenum(_indexType), reinterpret_cast(indexByteOffset), instanceCount, baseInstance); /* Instanced mesh */ } else @@ -933,7 +945,7 @@ void Mesh::drawInternal(Int count, Int baseVertex, Int instanceCount, GLintptr i #else state.drawElementsInstancedImplementation #endif - (GLenum(_primitive), count, GLenum(_indexType), reinterpret_cast(indexOffset), instanceCount); + (GLenum(_primitive), count, GLenum(_indexType), reinterpret_cast(indexByteOffset), instanceCount); } } } diff --git a/src/Magnum/GL/Mesh.h b/src/Magnum/GL/Mesh.h index 289bc30fb..870d3bfc3 100644 --- a/src/Magnum/GL/Mesh.h +++ b/src/Magnum/GL/Mesh.h @@ -748,6 +748,51 @@ class MAGNUM_GL_EXPORT Mesh: public AbstractObject { return *this; } + /** + * @brief Index offset + * @m_since_latest + */ + Int indexOffset() const { return _indexOffset; } + + /** + * @brief Set index offset + * @param offset First index + * @param start Minimum array index contained in the buffer + * @param end Maximum array index contained in the buffer + * @return Reference to self (for method chaining) + * @m_since_latest + * + * The offset gets multiplied by index type size and added to the base + * offset that was specified in @ref Mesh::setIndexBuffer(). The + * @p start and @p end parameters may help to improve memory access + * performance, as only a portion of vertex buffer needs to be + * acccessed. On OpenGL ES 2.0 this function behaves the same as + * @ref setIndexOffset(Int), as index range functionality is not + * available there. Ignored when calling + * @ref AbstractShaderProgram::drawTransformFeedback(). + * + * Expects that the mesh is indexed. + * @see @ref setCount(), @ref isIndexed() + */ + /* MinGW/MSVC needs inline also here to avoid linkage conflicts */ + inline Mesh& setIndexOffset(Int offset, UnsignedInt start, UnsignedInt end); + + /** + * @brief Set index offset + * @return Reference to self (for method chaining) + * @m_since_latest + * + * The offset gets multiplied by index type size and added to the base + * offset that was specified in @ref Mesh::setIndexBuffer(). Prefer to + * use @ref setIndexOffset(Int, UnsignedInt, UnsignedInt) for potential + * better performance on certain drivers. Ignored when calling + * @ref AbstractShaderProgram::drawTransformFeedback(). + * + * Expects that the mesh is indexed. + * @see @ref setCount(), @ref isIndexed() + */ + Mesh& setIndexOffset(Int offset); + /** @brief Instance count */ Int instanceCount() const { return _instanceCount; } @@ -1029,8 +1074,8 @@ class MAGNUM_GL_EXPORT Mesh: public AbstractObject { * Ignored when calling @ref AbstractShaderProgram::drawTransformFeedback(). * @see @ref maxElementIndex(), @ref maxElementsIndices(), * @ref maxElementsVertices(), @ref setCount(), @ref isIndexed(), - * @fn_gl{BindVertexArray}, @fn_gl{BindBuffer} or - * @fn_gl_keyword{VertexArrayElementBuffer} + * @ref setIndexOffset(), @fn_gl{BindVertexArray}, + * @fn_gl{BindBuffer} or @fn_gl_keyword{VertexArrayElementBuffer} */ Mesh& setIndexBuffer(Buffer& buffer, GLintptr offset, MeshIndexType type, UnsignedInt start, UnsignedInt end); @@ -1387,7 +1432,7 @@ class MAGNUM_GL_EXPORT Mesh: public AbstractObject { UnsignedInt _baseInstance{}; UnsignedInt _indexStart{}, _indexEnd{}; #endif - GLintptr _indexOffset{}; + GLintptr _indexBufferOffset{}, _indexOffset{}; MeshIndexType _indexType{}; Buffer _indexBuffer{NoCreate}; @@ -1403,6 +1448,18 @@ MAGNUM_GL_EXPORT Debug& operator<<(Debug& debug, MeshPrimitive value); /** @debugoperatorenum{MeshIndexType} */ MAGNUM_GL_EXPORT Debug& operator<<(Debug& debug, MeshIndexType value); +inline Mesh& Mesh::setIndexOffset(Int first, UnsignedInt start, UnsignedInt end) { + setIndexOffset(first); + #ifndef MAGNUM_TARGET_GLES2 + _indexStart = start; + _indexEnd = end; + #else + static_cast(start); + static_cast(end); + #endif + return *this; +} + inline GLuint Mesh::release() { const GLuint id = _id; _id = 0; diff --git a/src/Magnum/GL/MeshView.cpp b/src/Magnum/GL/MeshView.cpp index 596237143..96bb3667f 100644 --- a/src/Magnum/GL/MeshView.cpp +++ b/src/Magnum/GL/MeshView.cpp @@ -59,7 +59,7 @@ void MeshView::draw(AbstractShaderProgram&& shader, std::initializer_list void setIndexBufferTransferOwnership(); template void setIndexBufferRangeTransferOwnership(); + void setIndexOffset(); + void indexTypeSetIndexOffsetNotIndexed(); void unbindVAOWhenSettingIndexBufferData(); @@ -633,6 +635,8 @@ MeshGLTest::MeshGLTest() { &MeshGLTest::setIndexBufferRangeTransferOwnership, &MeshGLTest::setIndexBufferRangeTransferOwnership, + &MeshGLTest::setIndexOffset, + &MeshGLTest::indexTypeSetIndexOffsetNotIndexed, &MeshGLTest::unbindVAOWhenSettingIndexBufferData, @@ -964,6 +968,8 @@ struct DoubleShader: AbstractShaderProgram { }; #endif +/** @todo clean this up, it does too much implicitly and there's no way to + check just a subset, or the getters, or ... */ struct Checker { Checker(AbstractShaderProgram&& shader, RenderbufferFormat format, Mesh& mesh); @@ -2677,6 +2683,48 @@ template void MeshGLTest::setIndexBufferRangeTransferOwnership() { CORRADE_VERIFY(!glIsBuffer(id)); } +void MeshGLTest::setIndexOffset() { + /* Like setIndexBuffer(), but with a four-byte index type and the Checker + internals unwrapped to call setIndexOffset() on the Mesh directly + instead of the view */ + + Buffer vertices; + vertices.setData(indexedVertexData, BufferUsage::StaticDraw); + + constexpr UnsignedInt indexData[] = { 2, 267276, 2653, 282675, 0, 221987 }; + Buffer indices{Buffer::TargetHint::ElementArray}; + indices.setData(indexData, BufferUsage::StaticDraw); + + Mesh mesh; + mesh.addVertexBuffer(vertices, 1*4, MultipleShader::Position(), + MultipleShader::Normal(), MultipleShader::TextureCoordinates()) + .setIndexBuffer(indices, 4, MeshIndexType::UnsignedInt); + + Renderbuffer renderbuffer; + renderbuffer.setStorage( + #ifndef MAGNUM_TARGET_GLES2 + RenderbufferFormat::RGBA8, + #else + RenderbufferFormat::RGBA4, + #endif + Vector2i(1)); + Framebuffer framebuffer{{{}, Vector2i{1}}}; + framebuffer.attachRenderbuffer(Framebuffer::ColorAttachment(0), renderbuffer); + + framebuffer.bind(); + mesh.setPrimitive(MeshPrimitive::Points) + .setCount(1) + .setIndexOffset(3, 0, 1); + CORRADE_COMPARE(mesh.indexOffset(), 3); + + MultipleShader{}.draw(mesh); + + const auto value = framebuffer.read({{}, Vector2i{1}}, {PixelFormat::RGBA, PixelType::UnsignedByte}).pixels()[0][0]; + + MAGNUM_VERIFY_NO_GL_ERROR(); + CORRADE_COMPARE(value, indexedResult); +} + void MeshGLTest::indexTypeSetIndexOffsetNotIndexed() { CORRADE_SKIP_IF_NO_ASSERT(); @@ -2686,9 +2734,11 @@ void MeshGLTest::indexTypeSetIndexOffsetNotIndexed() { std::ostringstream out; Error redirectError{&out}; mesh.indexType(); + mesh.setIndexOffset(3); view.setIndexOffset(3); CORRADE_COMPARE(out.str(), "GL::Mesh::indexType(): mesh is not indexed\n" + "GL::Mesh::setIndexOffset(): mesh is not indexed\n" "GL::MeshView::setIndexOffset(): mesh is not indexed\n"); }