diff --git a/doc/changelog.dox b/doc/changelog.dox index 8093a19bc..a787486df 100644 --- a/doc/changelog.dox +++ b/doc/changelog.dox @@ -57,6 +57,14 @@ See also: @subsection changelog-latest-changes Changes and improvements +@subsubsection changelog-latest-changes-gl GL library + +- To prevent nothing being rendered by accident, @ref GL::Mesh::setCount() + and @ref GL::MeshView::setCount() now has always to be called, even just to + set the count to @cpp 0 @ce. + +@subsubsection changelog-latest-changes-platform Platform libraries + - @ref Platform::GlfwApplication now behaves the same as @ref Platform::Sdl2Application when creating an OpenGL context: first it attempts to create a forward-compatible core OpenGL 3.2+ context and if diff --git a/src/Magnum/GL/CMakeLists.txt b/src/Magnum/GL/CMakeLists.txt index c45ae2711..bff17f6a3 100644 --- a/src/Magnum/GL/CMakeLists.txt +++ b/src/Magnum/GL/CMakeLists.txt @@ -34,7 +34,6 @@ set(MagnumGL_SRCS Context.cpp DefaultFramebuffer.cpp Framebuffer.cpp - MeshView.cpp OpenGL.cpp Renderbuffer.cpp Renderer.cpp @@ -56,6 +55,7 @@ set(MagnumGL_SRCS set(MagnumGL_GracefulAssert_SRCS Mesh.cpp + MeshView.cpp PixelFormat.cpp Sampler.cpp) diff --git a/src/Magnum/GL/Mesh.cpp b/src/Magnum/GL/Mesh.cpp index d0237b5ae..dc2b3bc85 100644 --- a/src/Magnum/GL/Mesh.cpp +++ b/src/Magnum/GL/Mesh.cpp @@ -221,7 +221,7 @@ Mesh::~Mesh() { (this->*Context::current().state().mesh->destroyImplementation)(); } -Mesh::Mesh(Mesh&& other) noexcept: _id(other._id), _primitive(other._primitive), _flags{other._flags}, _count(other._count), _baseVertex{other._baseVertex}, _instanceCount{other._instanceCount}, +Mesh::Mesh(Mesh&& other) noexcept: _id(other._id), _primitive(other._primitive), _flags{other._flags}, _countSet{other._countSet}, _count(other._count), _baseVertex{other._baseVertex}, _instanceCount{other._instanceCount}, #ifndef MAGNUM_TARGET_GLES _baseInstance{other._baseInstance}, #endif @@ -239,6 +239,7 @@ Mesh& Mesh::operator=(Mesh&& other) noexcept { swap(_id, other._id); swap(_flags, other._flags); swap(_primitive, other._primitive); + swap(_countSet, other._countSet); swap(_count, other._count); swap(_baseVertex, other._baseVertex); swap(_instanceCount, other._instanceCount); @@ -345,6 +346,8 @@ Mesh& Mesh::setIndexBuffer(Buffer& buffer, GLintptr offset, MeshIndexType type, } void Mesh::draw(AbstractShaderProgram& shader) { + CORRADE_ASSERT(_countSet, "GL::Mesh::draw(): setCount() was never called, probably a mistake?", ); + /* Nothing to draw, exit without touching any state */ if(!_count || !_instanceCount) return; diff --git a/src/Magnum/GL/Mesh.h b/src/Magnum/GL/Mesh.h index 8aab32ed6..43489c172 100644 --- a/src/Magnum/GL/Mesh.h +++ b/src/Magnum/GL/Mesh.h @@ -545,10 +545,15 @@ class MAGNUM_GL_EXPORT Mesh: public AbstractObject { * otherwise the value is vertex count. If set to @cpp 0 @ce, no draw * commands are issued when calling @ref draw(AbstractShaderProgram&). * Ignored when calling @ref draw(AbstractShaderProgram&, TransformFeedback&, UnsignedInt). - * Default is @cpp 0 @ce. + * + * @attention To prevent nothing being rendered by accident, this + * function has to be always called, even to just set the count to + * @cpp 0 @ce. + * * @see @ref isIndexed(), @ref setBaseVertex(), @ref setInstanceCount() */ Mesh& setCount(Int count) { + _countSet = true; _count = count; return *this; } @@ -1035,6 +1040,9 @@ class MAGNUM_GL_EXPORT Mesh: public AbstractObject { GLuint _id; MeshPrimitive _primitive; ObjectFlags _flags; + /* using a separate bool instead of Optional to make use of the 3-byte + gap after _flags */ + bool _countSet{}; Int _count{}, _baseVertex{}, _instanceCount{1}; #ifndef MAGNUM_TARGET_GLES UnsignedInt _baseInstance{}; diff --git a/src/Magnum/GL/MeshView.cpp b/src/Magnum/GL/MeshView.cpp index 057d1f34a..a17b5e1be 100644 --- a/src/Magnum/GL/MeshView.cpp +++ b/src/Magnum/GL/MeshView.cpp @@ -146,6 +146,8 @@ MeshView& MeshView::setIndexRange(Int first) { } void MeshView::draw(AbstractShaderProgram& shader) { + CORRADE_ASSERT(_countSet, "GL::MeshView::draw(): setCount() was never called, probably a mistake?", ); + /* Nothing to draw, exit without touching any state */ if(!_count || !_instanceCount) return; diff --git a/src/Magnum/GL/MeshView.h b/src/Magnum/GL/MeshView.h index f77bf3966..87aa55972 100644 --- a/src/Magnum/GL/MeshView.h +++ b/src/Magnum/GL/MeshView.h @@ -125,9 +125,13 @@ class MAGNUM_GL_EXPORT MeshView { * @return Reference to self (for method chaining) * * Ignored when calling @ref draw(AbstractShaderProgram&, TransformFeedback&, UnsignedInt). - * Default is @cpp 0 @ce. + * + * @attention To prevent nothing being rendered by accident, this + * function has to be always called, even to just set the count to + * @cpp 0 @ce. */ MeshView& setCount(Int count) { + _countSet = true; _count = count; return *this; } @@ -286,6 +290,7 @@ class MAGNUM_GL_EXPORT MeshView { std::reference_wrapper _original; + bool _countSet{}; Int _count{}, _baseVertex{}, _instanceCount{1}; #ifndef MAGNUM_TARGET_GLES UnsignedInt _baseInstance{}; diff --git a/src/Magnum/GL/Test/MeshTest.cpp b/src/Magnum/GL/Test/MeshTest.cpp index ef3a88647..7083408a0 100644 --- a/src/Magnum/GL/Test/MeshTest.cpp +++ b/src/Magnum/GL/Test/MeshTest.cpp @@ -28,15 +28,22 @@ #include #include "Magnum/Mesh.h" +#include "Magnum/GL/AbstractShaderProgram.h" #include "Magnum/GL/Mesh.h" +#include "Magnum/GL/MeshView.h" namespace Magnum { namespace GL { namespace Test { +/* Tests MeshView as well */ + struct MeshTest: TestSuite::Tester { explicit MeshTest(); void constructNoCreate(); + void drawCountNotSet(); + void drawViewCountNotSet(); + #ifdef MAGNUM_BUILD_DEPRECATED void indexSizeDeprecated(); #endif @@ -56,6 +63,9 @@ struct MeshTest: TestSuite::Tester { MeshTest::MeshTest() { addTests({&MeshTest::constructNoCreate, + &MeshTest::drawCountNotSet, + &MeshTest::drawViewCountNotSet, + #ifdef MAGNUM_BUILD_DEPRECATED &MeshTest::indexSizeDeprecated, #endif @@ -81,6 +91,35 @@ void MeshTest::constructNoCreate() { CORRADE_VERIFY(true); } +namespace { + struct Shader: AbstractShaderProgram { + explicit Shader(NoCreateT): AbstractShaderProgram{NoCreate} {} + }; +} + +void MeshTest::drawCountNotSet() { + std::ostringstream out; + Error redirectError{&out}; + + Mesh mesh{NoCreate}; + mesh.draw(Shader{NoCreate}); + + CORRADE_COMPARE(out.str(), + "GL::Mesh::draw(): setCount() was never called, probably a mistake?\n"); +} + +void MeshTest::drawViewCountNotSet() { + std::ostringstream out; + Error redirectError{&out}; + + Mesh mesh{NoCreate}; + MeshView view{mesh}; + view.draw(Shader{NoCreate}); + + CORRADE_COMPARE(out.str(), + "GL::MeshView::draw(): setCount() was never called, probably a mistake?\n"); +} + #ifdef MAGNUM_BUILD_DEPRECATED void MeshTest::indexSizeDeprecated() { CORRADE_IGNORE_DEPRECATED_PUSH