From 1222baf44c41e282eec9843d9dce61d25ee9f0a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Tue, 19 Oct 2021 17:37:06 +0200 Subject: [PATCH] GL: drop compatibility fallback for the new multidraw API. Since the main point of the low-level API is to make use of the EXT_multi_draw_arrays / ANGLE_multi_draw / WEBGL_multi_draw extensions for a reduced driver overhead *and* the builtin gl_DrawID variable, it just doesn't make sense to provide a fallback to draw() in a loop. When the extensions are not available, the user has to do extra work in order to supply proper UBO draw offset for each draw call, and thus the fallback path has basically no practical use. On the other hand, draw() taking the MeshView instances is kept as-is -- this one is rather old and so breaking compatibility would be an unnecessary pain point. Plus, since it has to allocate the contiguous arrays internally, it's not desirable for any fast-path rendering anyway. --- src/Magnum/GL/AbstractShaderProgram.cpp | 14 +--- src/Magnum/GL/AbstractShaderProgram.h | 17 ++--- src/Magnum/GL/Implementation/MeshState.cpp | 8 -- src/Magnum/GL/Implementation/MeshState.h | 4 - src/Magnum/GL/Mesh.cpp | 69 ++--------------- src/Magnum/GL/Mesh.h | 15 +--- src/Magnum/GL/Test/MeshGLTest.cpp | 88 +++++++++++----------- 7 files changed, 61 insertions(+), 154 deletions(-) diff --git a/src/Magnum/GL/AbstractShaderProgram.cpp b/src/Magnum/GL/AbstractShaderProgram.cpp index dae3d3337..7c747e78a 100644 --- a/src/Magnum/GL/AbstractShaderProgram.cpp +++ b/src/Magnum/GL/AbstractShaderProgram.cpp @@ -388,12 +388,7 @@ AbstractShaderProgram& AbstractShaderProgram::draw(Mesh& mesh, const Containers: use(); - #ifndef MAGNUM_TARGET_GLES - Mesh::multiDrawImplementationDefault - #else - Context::current().state().mesh.multiDrawImplementation - #endif - (mesh, counts, vertexOffsets, indexOffsets); + mesh.drawInternalStrided(counts, vertexOffsets, indexOffsets); return *this; } @@ -403,12 +398,7 @@ AbstractShaderProgram& AbstractShaderProgram::draw(Mesh& mesh, const Containers: use(); - #ifndef MAGNUM_TARGET_GLES - Mesh::multiDrawImplementationDefault - #else - Context::current().state().mesh.multiDrawLongImplementation - #endif - (mesh, counts, vertexOffsets, indexOffsets); + mesh.drawInternalStrided(counts, vertexOffsets, indexOffsets); return *this; } diff --git a/src/Magnum/GL/AbstractShaderProgram.h b/src/Magnum/GL/AbstractShaderProgram.h index e9a142c7f..3c1aead66 100644 --- a/src/Magnum/GL/AbstractShaderProgram.h +++ b/src/Magnum/GL/AbstractShaderProgram.h @@ -858,18 +858,6 @@ class MAGNUM_GL_EXPORT AbstractShaderProgram: public AbstractObject { * @return Reference to self (for method chaining) * @m_since_latest * - * On OpenGL ES, if neither @gl_extension{EXT,multi_draw_arrays} nor - * @m_class{m-doc-external} [ANGLE_multi_draw](https://chromium.googlesource.com/angle/angle/+/master/extensions/ANGLE_multi_draw.txt) - * is present, and on WebGL if @webgl_extension{WEBGL,multi_draw} is - * not present, the functionality is emulated equivalently to a - * sequence of @ref draw(MeshView&) calls with items of @p counts used - * for @ref MeshView::setCount(), @p vertexOffsets for - * @ref MeshView::setBaseVertex() and @p indexOffsets divided by size - * of the index type for @ref MeshView::setIndexRange(). Note that - * @webgl_extension{WEBGL,multi_draw} is only implemented since - * Emscripten 2.0.0 and thus it's not even advertised on older - * versions. - * * If @gl_extension{ARB,vertex_array_object} (part of OpenGL 3.0), * OpenGL ES 3.0, WebGL 2.0, @gl_extension{OES,vertex_array_object} in * OpenGL ES 2.0 or @webgl_extension{OES,vertex_array_object} in WebGL @@ -896,10 +884,15 @@ class MAGNUM_GL_EXPORT AbstractShaderProgram: public AbstractObject { * @requires_gl32 Extension @gl_extension{ARB,draw_elements_base_vertex} * if the mesh is indexed and the @p vertexOffsets view is not * empty. + * @requires_es_extension Extension @gl_extension{EXT,multi_draw_arrays} + * or @m_class{m-doc-external} [ANGLE_multi_draw](https://chromium.googlesource.com/angle/angle/+/master/extensions/ANGLE_multi_draw.txt) * @requires_es_extension OpenGL ES 3.0 and extension * @gl_extension{OES,draw_elements_base_vertex} or * @gl_extension{EXT,draw_elements_base_vertex} if the mesh is * indexed and the @p vertexOffsets view is not empty. + * @requires_webgl_extension Extension @webgl_extension{WEBGL,multi_draw}. + * Note that this extension is only implemented since Emscripten + * 2.0.0 and thus it's not even advertised on older versions. * @requires_webgl_extension WebGL 2.0 and extension * @webgl_extension{WEBGL,multi_draw_instanced_base_vertex_base_instance} * if the mesh is indexed and the @p vertexOffsets view is not diff --git a/src/Magnum/GL/Implementation/MeshState.cpp b/src/Magnum/GL/Implementation/MeshState.cpp index d3922bc8b..eefaf2abb 100644 --- a/src/Magnum/GL/Implementation/MeshState.cpp +++ b/src/Magnum/GL/Implementation/MeshState.cpp @@ -301,17 +301,9 @@ MeshState::MeshState(Context& context, ContextState& contextState, Containers::S } #endif - multiDrawImplementation = &Mesh::multiDrawImplementationDefault; - #ifndef CORRADE_TARGET_32BIT - multiDrawLongImplementation = &Mesh::multiDrawImplementationDefault; - #endif multiDrawViewImplementation = &MeshView::multiDrawImplementationDefault; } else { - multiDrawImplementation = &Mesh::multiDrawImplementationFallback; - #ifndef CORRADE_TARGET_32BIT - multiDrawLongImplementation = &Mesh::multiDrawImplementationFallback; - #endif multiDrawViewImplementation = &MeshView::multiDrawImplementationFallback; } #endif diff --git a/src/Magnum/GL/Implementation/MeshState.h b/src/Magnum/GL/Implementation/MeshState.h index da8a6ec13..5f40d1a2d 100644 --- a/src/Magnum/GL/Implementation/MeshState.h +++ b/src/Magnum/GL/Implementation/MeshState.h @@ -77,10 +77,6 @@ struct MeshState { #endif #ifdef MAGNUM_TARGET_GLES - void(*multiDrawImplementation)(Mesh&, const Containers::StridedArrayView1D&, const Containers::StridedArrayView1D&, const Containers::StridedArrayView1D&); - #ifndef CORRADE_TARGET_32BIT - void(*multiDrawLongImplementation)(Mesh&, const Containers::StridedArrayView1D&, const Containers::StridedArrayView1D&, const Containers::StridedArrayView1D&); - #endif void(*multiDrawViewImplementation)(Containers::ArrayView>); void(APIENTRY *multiDrawArraysImplementation)(GLenum, const GLint*, const GLsizei*, GLsizei); void(APIENTRY *multiDrawElementsImplementation)(GLenum, const GLsizei*, GLenum, const void* const*, GLsizei); diff --git a/src/Magnum/GL/Mesh.cpp b/src/Magnum/GL/Mesh.cpp index c0eeaea27..7804b8053 100644 --- a/src/Magnum/GL/Mesh.cpp +++ b/src/Magnum/GL/Mesh.cpp @@ -464,12 +464,12 @@ void Mesh::drawInternal(const Containers::ArrayView& counts, (this->*state.unbindImplementation)(); } -void Mesh::multiDrawImplementationDefault(Mesh& self, const Containers::StridedArrayView1D& counts, const Containers::StridedArrayView1D& vertexOffsets, const Containers::StridedArrayView1D& indexOffsets) { +void Mesh::drawInternalStrided(const Containers::StridedArrayView1D& counts, const Containers::StridedArrayView1D& vertexOffsets, const Containers::StridedArrayView1D& indexOffsets) { #ifdef CORRADE_TARGET_32BIT /* If all views are contiguous and we're on 32-bit, call the implementation directly */ if(counts.isContiguous() && vertexOffsets.isContiguous() && indexOffsets.isContiguous()) - return self.drawInternal(counts.asContiguous(), vertexOffsets.asContiguous(), indexOffsets.asContiguous()); + return drawInternal(counts.asContiguous(), vertexOffsets.asContiguous(), indexOffsets.asContiguous()); #endif /* Otherwise allocate contiguous copies. While it's possible that some @@ -517,18 +517,18 @@ void Mesh::multiDrawImplementationDefault(Mesh& self, const Containers::StridedA #endif ); - self.drawInternal(countsContiguous, vertexOffsetsContiguous, indexOffsetsContiguous); + drawInternal(countsContiguous, vertexOffsetsContiguous, indexOffsetsContiguous); } #ifndef CORRADE_TARGET_32BIT -void Mesh::multiDrawImplementationDefault(Mesh& self, const Containers::StridedArrayView1D& counts, const Containers::StridedArrayView1D& vertexOffsets, const Containers::StridedArrayView1D& indexOffsets) { +void Mesh::drawInternalStrided(const Containers::StridedArrayView1D& counts, const Containers::StridedArrayView1D& vertexOffsets, const Containers::StridedArrayView1D& indexOffsets) { /* If all views are contiguous, call the implementation directly */ if(counts.isContiguous() && vertexOffsets.isContiguous() && indexOffsets.isContiguous()) - return self.drawInternal(counts.asContiguous(), vertexOffsets.asContiguous(), indexOffsets.asContiguous()); + return drawInternal(counts.asContiguous(), vertexOffsets.asContiguous(), indexOffsets.asContiguous()); /* Otherwise delegate into the 32-bit variant, which will allocate a contiguous copy */ - multiDrawImplementationDefault(self, counts, vertexOffsets, + drawInternalStrided(counts, vertexOffsets, /* Get the lower 32 bits of the index offsets, which is the leftmost bits on Little-Endian and rightmost on Big-Endian. On LE it could be just Containers::arrayCast(indexOffsets) but to @@ -545,63 +545,6 @@ void Mesh::multiDrawImplementationDefault(Mesh& self, const Containers::StridedA } #endif -#ifdef MAGNUM_TARGET_GLES -void Mesh::multiDrawImplementationFallback(Mesh& self, const Containers::StridedArrayView1D& counts, const Containers::StridedArrayView1D& vertexOffsets, const Containers::StridedArrayView1D& indexOffsets) { - const UnsignedInt zero[1]{}; - Containers::StridedArrayView1D indexOffsetsNeverEmpty; - Containers::StridedArrayView1D vertexOffsetsNeverEmpty; - if(self._indexBuffer.id()) { - CORRADE_ASSERT(indexOffsets.size() == counts.size(), - "GL::AbstractShaderProgram::draw(): expected" << counts.size() << "index offset items but got" << indexOffsets.size(), ); - indexOffsetsNeverEmpty = indexOffsets; - - if(!vertexOffsets.empty()) { - CORRADE_ASSERT(vertexOffsets.size() == counts.size(), - "GL::AbstractShaderProgram::draw(): expected" << counts.size() << "vertex offset items but got" << vertexOffsets.size(), ); - vertexOffsetsNeverEmpty = vertexOffsets; - } else vertexOffsetsNeverEmpty = Containers::StridedArrayView1D{zero, counts.size(), 0}; - } else { - CORRADE_ASSERT(vertexOffsets.size() == counts.size(), - "GL::AbstractShaderProgram::draw(): expected" << counts.size() << "vertex offset items but got" << vertexOffsets.size(), ); - vertexOffsetsNeverEmpty = vertexOffsets; - indexOffsetsNeverEmpty = Containers::StridedArrayView1D{zero, counts.size(), 0}; - } - - for(std::size_t i = 0; i != counts.size(); ++i) { - if(!counts[i]) continue; - - self.drawInternal( - counts[i], vertexOffsetsNeverEmpty[i], 1, - #ifndef MAGNUM_TARGET_GLES2 - 0, indexOffsetsNeverEmpty[i], 0, 0 - #else - indexOffsetsNeverEmpty[i] - #endif - ); - } -} - -#ifndef CORRADE_TARGET_32BIT -void Mesh::multiDrawImplementationFallback(Mesh& self, const Containers::StridedArrayView1D& counts, const Containers::StridedArrayView1D& vertexOffsets, const Containers::StridedArrayView1D& indexOffsets) { - /* Delegate straight into the 32-bit variant */ - multiDrawImplementationFallback(self, counts, vertexOffsets, - /* Get the lower 32 bits of the index offsets, which is the leftmost - bits on Little-Endian and rightmost on Big-Endian. On LE it could be - just Containers::arrayCast(indexOffsets) but to - minimize a chance of error on BE platforms that are hard to test on, - the same code is used for both. */ - Containers::arrayCast<2, const UnsignedInt>(indexOffsets).transposed<0, 1>()[ - #ifndef CORRADE_TARGET_BIG_ENDIAN - 0 - #else - 1 - #endif - ] - ); -} -#endif -#endif - #ifndef MAGNUM_TARGET_GLES2 void Mesh::drawInternal(Int count, Int baseVertex, Int instanceCount, UnsignedInt baseInstance, GLintptr indexOffset, Int indexStart, Int indexEnd) #else diff --git a/src/Magnum/GL/Mesh.h b/src/Magnum/GL/Mesh.h index be68ce73c..5de7d1347 100644 --- a/src/Magnum/GL/Mesh.h +++ b/src/Magnum/GL/Mesh.h @@ -1096,6 +1096,10 @@ class MAGNUM_GL_EXPORT Mesh: public AbstractObject { const Containers::ArrayView& indexOffsets #endif ); + MAGNUM_GL_LOCAL void drawInternalStrided(const Containers::StridedArrayView1D& counts, const Containers::StridedArrayView1D& vertexOffsets, const Containers::StridedArrayView1D& indexOffsets); + #ifndef CORRADE_TARGET_32BIT + MAGNUM_GL_LOCAL void drawInternalStrided(const Containers::StridedArrayView1D& counts, const Containers::StridedArrayView1D& vertexOffsets, const Containers::StridedArrayView1D& indexOffsets); + #endif #ifndef MAGNUM_TARGET_GLES MAGNUM_GL_LOCAL void drawInternal(TransformFeedback& xfb, UnsignedInt stream, Int instanceCount); @@ -1154,17 +1158,6 @@ class MAGNUM_GL_EXPORT Mesh: public AbstractObject { void MAGNUM_GL_LOCAL unbindImplementationDefault(); void MAGNUM_GL_LOCAL unbindImplementationVAO(); - MAGNUM_GL_LOCAL static void multiDrawImplementationDefault(Mesh& self, const Containers::StridedArrayView1D& counts, const Containers::StridedArrayView1D& vertexOffsets, const Containers::StridedArrayView1D& indexOffsets); - #ifndef CORRADE_TARGET_32BIT - MAGNUM_GL_LOCAL static void multiDrawImplementationDefault(Mesh& self, const Containers::StridedArrayView1D& counts, const Containers::StridedArrayView1D& vertexOffsets, const Containers::StridedArrayView1D& indexOffsets); - #endif - #ifdef MAGNUM_TARGET_GLES - MAGNUM_GL_LOCAL static void multiDrawImplementationFallback(Mesh& self, const Containers::StridedArrayView1D& counts, const Containers::StridedArrayView1D& vertexOffsets, const Containers::StridedArrayView1D& indexOffsets); - #ifndef CORRADE_TARGET_32BIT - MAGNUM_GL_LOCAL static void multiDrawImplementationFallback(Mesh& self, const Containers::StridedArrayView1D& counts, const Containers::StridedArrayView1D& vertexOffsets, const Containers::StridedArrayView1D& indexOffsets); - #endif - #endif - #ifdef MAGNUM_TARGET_GLES #if !(defined(MAGNUM_TARGET_WEBGL) && defined(MAGNUM_TARGET_GLES2)) #if defined(MAGNUM_TARGET_WEBGL) && __EMSCRIPTEN_major__*10000 + __EMSCRIPTEN_minor__*100 + __EMSCRIPTEN_tiny__ >= 13915 diff --git a/src/Magnum/GL/Test/MeshGLTest.cpp b/src/Magnum/GL/Test/MeshGLTest.cpp index 51df5f094..70e3c52a7 100644 --- a/src/Magnum/GL/Test/MeshGLTest.cpp +++ b/src/Magnum/GL/Test/MeshGLTest.cpp @@ -3568,6 +3568,17 @@ void MeshGLTest::multiDraw() { auto&& data = MultiDrawData[testCaseInstanceId()]; setTestCaseDescription(data.name); + #ifdef MAGNUM_TARGET_GLES + #ifndef MAGNUM_TARGET_WEBGL + if(!Context::current().isExtensionSupported() && + !Context::current().isExtensionSupported()) + CORRADE_SKIP("Neither" << Extensions::EXT::multi_draw_arrays::string() << "nor" << Extensions::ANGLE::multi_draw::string() << "is supported."); + #else + if(!Context::current().isExtensionSupported()) + CORRADE_SKIP(Extensions::WEBGL::multi_draw::string() << "is not supported."); + #endif + #endif + #ifndef MAGNUM_TARGET_GLES2 if(data.vertexId && !GL::Context::current().isExtensionSupported()) CORRADE_SKIP("gl_VertexID not supported"); @@ -3586,17 +3597,6 @@ void MeshGLTest::multiDraw() { #endif } - #ifdef MAGNUM_TARGET_GLES - #ifndef MAGNUM_TARGET_WEBGL - if(!Context::current().isExtensionSupported() && - !Context::current().isExtensionSupported()) - CORRADE_INFO("Neither" << Extensions::EXT::multi_draw_arrays::string() << "nor" << Extensions::ANGLE::multi_draw::string() << "is supported, using fallback implementation"); - #else - if(!Context::current().isExtensionSupported()) - CORRADE_INFO(Extensions::WEBGL::multi_draw::string() << "is not supported, using fallback implementation"); - #endif - #endif - const struct { Vector2 position; Vector4 value; @@ -3631,6 +3631,17 @@ void MeshGLTest::multiDrawSparseArrays() { auto&& data = MultiDrawData[testCaseInstanceId()]; setTestCaseDescription(data.name); + #ifdef MAGNUM_TARGET_GLES + #ifndef MAGNUM_TARGET_WEBGL + if(!Context::current().isExtensionSupported() && + !Context::current().isExtensionSupported()) + CORRADE_SKIP("Neither" << Extensions::EXT::multi_draw_arrays::string() << "nor" << Extensions::ANGLE::multi_draw::string() << "is supported."); + #else + if(!Context::current().isExtensionSupported()) + CORRADE_SKIP(Extensions::WEBGL::multi_draw::string() << "is not supported."); + #endif + #endif + #ifndef MAGNUM_TARGET_GLES2 if(data.vertexId && !GL::Context::current().isExtensionSupported()) CORRADE_SKIP("gl_VertexID not supported"); @@ -3649,17 +3660,6 @@ void MeshGLTest::multiDrawSparseArrays() { #endif } - #ifdef MAGNUM_TARGET_GLES - #ifndef MAGNUM_TARGET_WEBGL - if(!Context::current().isExtensionSupported() && - !Context::current().isExtensionSupported()) - CORRADE_INFO("Neither" << Extensions::EXT::multi_draw_arrays::string() << "nor" << Extensions::ANGLE::multi_draw::string() << "is supported, using fallback implementation"); - #else - if(!Context::current().isExtensionSupported()) - CORRADE_INFO(Extensions::WEBGL::multi_draw::string() << "is not supported, using fallback implementation"); - #endif - #endif - const struct { Vector2 position; Vector4 value; @@ -3785,6 +3785,17 @@ template void MeshGLTest::multiDrawIndexed() { auto&& data = MultiDrawIndexedData[testCaseInstanceId()]; setTestCaseDescription(data.name); + #ifdef MAGNUM_TARGET_GLES + #ifndef MAGNUM_TARGET_WEBGL + if(!Context::current().isExtensionSupported() && + !Context::current().isExtensionSupported()) + CORRADE_SKIP("Neither" << Extensions::EXT::multi_draw_arrays::string() << "nor" << Extensions::ANGLE::multi_draw::string() << "is supported."); + #else + if(!Context::current().isExtensionSupported()) + CORRADE_SKIP(Extensions::WEBGL::multi_draw::string() << "is not supported."); + #endif + #endif + #ifndef MAGNUM_TARGET_GLES2 if(data.vertexId && !GL::Context::current().isExtensionSupported()) CORRADE_SKIP("gl_VertexID not supported"); @@ -3807,17 +3818,6 @@ template void MeshGLTest::multiDrawIndexed() { #endif } - #ifdef MAGNUM_TARGET_GLES - #ifndef MAGNUM_TARGET_WEBGL - if(!Context::current().isExtensionSupported() && - !Context::current().isExtensionSupported()) - CORRADE_INFO("Neither" << Extensions::EXT::multi_draw_arrays::string() << "nor" << Extensions::ANGLE::multi_draw::string() << "is supported, using fallback implementation"); - #else - if(!Context::current().isExtensionSupported()) - CORRADE_INFO(Extensions::WEBGL::multi_draw::string() << "is not supported, using fallback implementation"); - #endif - #endif - const struct { Vector2 position; Vector4 value; @@ -3863,6 +3863,17 @@ template void MeshGLTest::multiDrawIndexedSparseArrays() { auto&& data = MultiDrawIndexedData[testCaseInstanceId()]; setTestCaseDescription(data.name); + #ifdef MAGNUM_TARGET_GLES + #ifndef MAGNUM_TARGET_WEBGL + if(!Context::current().isExtensionSupported() && + !Context::current().isExtensionSupported()) + CORRADE_SKIP("Neither" << Extensions::EXT::multi_draw_arrays::string() << "nor" << Extensions::ANGLE::multi_draw::string() << "is supported."); + #else + if(!Context::current().isExtensionSupported()) + CORRADE_SKIP(Extensions::WEBGL::multi_draw::string() << "is not supported."); + #endif + #endif + #ifndef MAGNUM_TARGET_GLES2 if(data.vertexId && !GL::Context::current().isExtensionSupported()) CORRADE_SKIP("gl_VertexID not supported"); @@ -3885,17 +3896,6 @@ template void MeshGLTest::multiDrawIndexedSparseArrays() { #endif } - #ifdef MAGNUM_TARGET_GLES - #ifndef MAGNUM_TARGET_WEBGL - if(!Context::current().isExtensionSupported() && - !Context::current().isExtensionSupported()) - CORRADE_INFO("Neither" << Extensions::EXT::multi_draw_arrays::string() << "nor" << Extensions::ANGLE::multi_draw::string() << "is supported, using fallback implementation"); - #else - if(!Context::current().isExtensionSupported()) - CORRADE_INFO(Extensions::WEBGL::multi_draw::string() << "is not supported, using fallback implementation"); - #endif - #endif - const struct { Vector2 position; Vector4 value;