From f9175d2837579c073709cc4495c8d3dc122f261e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Thu, 6 Mar 2025 21:22:18 +0100 Subject: [PATCH] GL: only allow GL::Attribute::DataType with matching signedness on WebGL. For integer attributes, that is. This never worked and the MeshGLTest even had an explicit test case for this, but it took me until now to realize that this would be best caught at compile time. So now it is. But since this functionality dates back to 2010, just removing the values would likely break a lot of code (well, broken code, but still), so the disallowed values are now deprecated, giving a compile time warning when used, and are removed only on non-deprecated builds. --- doc/changelog.dox | 14 +++++ src/Magnum/GL/Attribute.cpp | 81 ++++++++++++++++++++++-- src/Magnum/GL/Attribute.h | 92 +++++++++++++++++++++++----- src/Magnum/GL/Test/AttributeTest.cpp | 78 ++++++++++++++++++++++- src/Magnum/GL/Test/MeshGLTest.cpp | 20 ++++++ 5 files changed, 264 insertions(+), 21 deletions(-) diff --git a/doc/changelog.dox b/doc/changelog.dox index 7ce45954a..6fb1a91db 100644 --- a/doc/changelog.dox +++ b/doc/changelog.dox @@ -1357,6 +1357,20 @@ See also: - @cpp GL::Mesh::indexTypeSize() @ce is deprecated in favor of a standalone @ref GL::meshIndexTypeSize() utility that you can call on the enum returned from @ref GL::Mesh::indexType() +- On WebGL builds, @ref GL::Attribute::DataType::Byte, + @relativeref{GL::Attribute::DataType,Short} and + @relativeref{GL::Attribute::DataType,Int} values are deprecated for + @ref GL::Attribute with an unsigned integer type, and conversely + @ref GL::Attribute::DataType::UnsignedByte, + @relativeref{GL::Attribute::DataType,UnsignedShort} and + @relativeref{GL::Attribute::DataType,UnsignedInt} values are deprecated for + @ref GL::Attribute with a signed integer type. Those never actually worked, + causing a WebGL error because WebGL requires the signedness to match + between the attribute and the GLSL definition, now it's being checked at + compile time. If @ref MAGNUM_BUILD_DEPRECATED is disabled, only values with + matching signedness are present for integer @ref GL::Attribute on WebGL + builds. There's no such restriction on OpenGL ES and desktop GL, there all + combinations are exposed like before. - The @cpp Array @ce, @cpp Array1D @ce, @cpp Array2D @ce and @cpp Array3D @ce types that were used exclusively for specifying @ref SamplerWrapping in various texture APIs are deprecated in favor of diff --git a/src/Magnum/GL/Attribute.cpp b/src/Magnum/GL/Attribute.cpp index c521880bf..0cb892b81 100644 --- a/src/Magnum/GL/Attribute.cpp +++ b/src/Magnum/GL/Attribute.cpp @@ -134,15 +134,53 @@ UnsignedInt FloatAttribute::size(GLint components, DataType dataType) { #ifndef MAGNUM_TARGET_GLES2 UnsignedInt IntAttribute::size(GLint components, DataType dataType) { switch(dataType) { - case DataType::UnsignedByte: case DataType::Byte: return components; - case DataType::UnsignedShort: case DataType::Short: return 2*components; + case DataType::Int: + return 4*components; + #if !defined(MAGNUM_TARGET_WEBGL) || defined(MAGNUM_BUILD_DEPRECATED) + #ifdef MAGNUM_TARGET_WEBGL + CORRADE_IGNORE_DEPRECATED_PUSH + #endif + case DataType::UnsignedByte: + return components; + case DataType::UnsignedShort: + return 2*components; case DataType::UnsignedInt: + return 4*components; + #ifdef MAGNUM_TARGET_WEBGL + CORRADE_IGNORE_DEPRECATED_POP + #endif + #endif + } + + CORRADE_INTERNAL_ASSERT_UNREACHABLE(); /* LCOV_EXCL_LINE */ +} + +UnsignedInt UnsignedIntAttribute::size(GLint components, DataType dataType) { + switch(dataType) { + case DataType::UnsignedByte: + return components; + case DataType::UnsignedShort: + return 2*components; + case DataType::UnsignedInt: + return 4*components; + #if !defined(MAGNUM_TARGET_WEBGL) || defined(MAGNUM_BUILD_DEPRECATED) + #ifdef MAGNUM_TARGET_WEBGL + CORRADE_IGNORE_DEPRECATED_PUSH + #endif + case DataType::Byte: + return components; + case DataType::Short: + return 2*components; case DataType::Int: return 4*components; + #ifdef MAGNUM_TARGET_WEBGL + CORRADE_IGNORE_DEPRECATED_POP + #endif + #endif } CORRADE_INTERNAL_ASSERT_UNREACHABLE(); /* LCOV_EXCL_LINE */ @@ -381,12 +419,47 @@ Debug& operator<<(Debug& debug, const IntAttribute::DataType value) { switch(value) { /* LCOV_EXCL_START */ #define _c(value) case IntAttribute::DataType::value: return debug << "::" #value; - _c(UnsignedByte) _c(Byte) - _c(UnsignedShort) _c(Short) + _c(Int) + #if !defined(MAGNUM_TARGET_WEBGL) || defined(MAGNUM_BUILD_DEPRECATED) + #ifdef MAGNUM_TARGET_WEBGL + CORRADE_IGNORE_DEPRECATED_PUSH + #endif + _c(UnsignedByte) + _c(UnsignedShort) _c(UnsignedInt) + #ifdef MAGNUM_TARGET_WEBGL + CORRADE_IGNORE_DEPRECATED_POP + #endif + #endif + #undef _c + /* LCOV_EXCL_STOP */ + } + + return debug << "(" << Debug::nospace << Debug::hex << GLenum(value) << Debug::nospace << ")"; +} + +Debug& operator<<(Debug& debug, const UnsignedIntAttribute::DataType value) { + debug << "GL::Attribute::DataType" << Debug::nospace; + + switch(value) { + /* LCOV_EXCL_START */ + #define _c(value) case UnsignedIntAttribute::DataType::value: return debug << "::" #value; + _c(UnsignedByte) + _c(UnsignedShort) + _c(UnsignedInt) + #if !defined(MAGNUM_TARGET_WEBGL) || defined(MAGNUM_BUILD_DEPRECATED) + #ifdef MAGNUM_TARGET_WEBGL + CORRADE_IGNORE_DEPRECATED_PUSH + #endif + _c(Byte) + _c(Short) _c(Int) + #ifdef MAGNUM_TARGET_WEBGL + CORRADE_IGNORE_DEPRECATED_POP + #endif + #endif #undef _c /* LCOV_EXCL_STOP */ } diff --git a/src/Magnum/GL/Attribute.h b/src/Magnum/GL/Attribute.h index c895269f1..6700c64a9 100644 --- a/src/Magnum/GL/Attribute.h +++ b/src/Magnum/GL/Attribute.h @@ -192,12 +192,41 @@ template class Attribute { */ #ifdef DOXYGEN_GENERATING_OUTPUT enum class DataType: GLenum { - UnsignedByte = GL_UNSIGNED_BYTE, /**< Unsigned byte */ - Byte = GL_BYTE, /**< Byte */ - UnsignedShort = GL_UNSIGNED_SHORT, /**< Unsigned short */ - Short = GL_SHORT, /**< Short */ - UnsignedInt = GL_UNSIGNED_INT, /**< Unsigned int */ - Int = GL_INT, /**< Int */ + /** + * Unsigned byte. On WebGL is only for float and unsigned integer + * attributes, cannot be used with signed integer attributes. + */ + UnsignedByte = GL_UNSIGNED_BYTE, + + /** + * Byte. On WebGL is only for float and signed integer attributes, + * cannot be used with unsigned integer attributes. + */ + Byte = GL_BYTE, + + /** + * Unsigned short. On WebGL is only for float and unsigned integer + * attributes, cannot be used with signed integer attributes. + */ + UnsignedShort = GL_UNSIGNED_SHORT, + + /** + * Short. On WebGL is only for float and signed integer attributes, + * cannot be used with unsigned integer attributes. + */ + Short = GL_SHORT, + + /** + * Unsigned int. On WebGL is only for float and unsigned integer + * attributes, cannot be used with signed integer attributes. + */ + UnsignedInt = GL_UNSIGNED_INT, + + /** + * Int. On WebGL is only for float and signed integer attributes, + * cannot be used with unsigned integer attributes. + */ + Int = GL_INT, #ifndef MAGNUM_TARGET_WEBGL /** @@ -890,12 +919,26 @@ struct IntAttribute { typedef Int ScalarType; enum class DataType: GLenum { - UnsignedByte = GL_UNSIGNED_BYTE, Byte = GL_BYTE, - UnsignedShort = GL_UNSIGNED_SHORT, Short = GL_SHORT, - UnsignedInt = GL_UNSIGNED_INT, - Int = GL_INT + Int = GL_INT, + #if !defined(MAGNUM_TARGET_WEBGL) || defined(MAGNUM_BUILD_DEPRECATED) + UnsignedByte + #ifdef MAGNUM_TARGET_WEBGL + CORRADE_DEPRECATED_ENUM("using an unsigned type with a signed attribute isn't supported in WebGL, either make the attribute and the GLSL definition unsigned or use a signed type") + #endif + = GL_UNSIGNED_BYTE, + UnsignedShort + #ifdef MAGNUM_TARGET_WEBGL + CORRADE_DEPRECATED_ENUM("using an unsigned type with a signed attribute isn't supported in WebGL, either make the attribute and the GLSL definition unsigned or use a signed type") + #endif + = GL_UNSIGNED_SHORT, + UnsignedInt + #ifdef MAGNUM_TARGET_WEBGL + CORRADE_DEPRECATED_ENUM("using an unsigned type with a signed attribute isn't supported in WebGL, either make the attribute and the GLSL definition unsigned or use a signed type") + #endif + = GL_UNSIGNED_INT, + #endif }; constexpr static DataType DefaultDataType = DataType::Int; enum: UnsignedInt { DefaultDataTypeSize = 4 }; @@ -914,17 +957,38 @@ MAGNUM_GL_EXPORT Debug& operator<<(Debug& debug, IntAttribute::DataType value); struct UnsignedIntAttribute { typedef UnsignedInt ScalarType; - typedef IntAttribute::DataType DataType; + enum class DataType: GLenum { + UnsignedByte = GL_UNSIGNED_BYTE, + UnsignedShort = GL_UNSIGNED_SHORT, + UnsignedInt = GL_UNSIGNED_INT, + #if !defined(MAGNUM_TARGET_WEBGL) || defined(MAGNUM_BUILD_DEPRECATED) + Byte + #ifdef MAGNUM_TARGET_WEBGL + CORRADE_DEPRECATED_ENUM("using a signed type with an unsigned attribute isn't supported in WebGL, either make the attribute and the GLSL definition signed or use an unsigned type") + #endif + = GL_BYTE, + Short + #ifdef MAGNUM_TARGET_WEBGL + CORRADE_DEPRECATED_ENUM("using a signed type with an unsigned attribute isn't supported in WebGL, either make the attribute and the GLSL definition signed or use an unsigned type") + #endif + = GL_SHORT, + Int + #ifdef MAGNUM_TARGET_WEBGL + CORRADE_DEPRECATED_ENUM("using a signed type with an unsigned attribute isn't supported in WebGL, either make the attribute and the GLSL definition signed or use an unsigned type") + #endif + = GL_INT + #endif + }; constexpr static DataType DefaultDataType = DataType::UnsignedInt; enum: UnsignedInt { DefaultDataTypeSize = 4 }; typedef IntAttribute::DataOption DataOption; typedef IntAttribute::DataOptions DataOptions; - static UnsignedInt size(GLint components, DataType dataType) { - return IntAttribute::size(components, dataType); - } + static UnsignedInt MAGNUM_GL_EXPORT size(GLint components, DataType dataType); }; + +MAGNUM_GL_EXPORT Debug& operator<<(Debug& debug, UnsignedIntAttribute::DataType value); #endif #ifndef MAGNUM_TARGET_GLES diff --git a/src/Magnum/GL/Test/AttributeTest.cpp b/src/Magnum/GL/Test/AttributeTest.cpp index 172234272..99e4102c7 100644 --- a/src/Magnum/GL/Test/AttributeTest.cpp +++ b/src/Magnum/GL/Test/AttributeTest.cpp @@ -96,6 +96,7 @@ struct AttributeTest: TestSuite::Tester { void debugDataTypeFloat(); #ifndef MAGNUM_TARGET_GLES2 void debugDataTypeInt(); + void debugDataTypeUnsignedInt(); #endif #ifndef MAGNUM_TARGET_GLES void debugDataTypeDouble(); @@ -170,6 +171,7 @@ AttributeTest::AttributeTest() { &AttributeTest::debugDataTypeFloat, #ifndef MAGNUM_TARGET_GLES2 &AttributeTest::debugDataTypeInt, + &AttributeTest::debugDataTypeUnsignedInt, #endif #ifndef MAGNUM_TARGET_GLES &AttributeTest::debugDataTypeDouble, @@ -517,6 +519,19 @@ void AttributeTest::attributeVectorInt() { CORRADE_COMPARE(cdb.vectors(), 1); CORRADE_COMPARE(db.dataType(), DynamicAttribute::DataType::Int); CORRADE_COMPARE(cdb.dataType(), DynamicAttribute::DataType::Int); + + /* Unsigned types for signed attributes are not supported on WebGL, exposed + just as deprecated for backwards compatibility */ + #if !defined(MAGNUM_TARGET_WEBGL) || defined(MAGNUM_BUILD_DEPRECATED) + #ifdef MAGNUM_TARGET_WEBGL + CORRADE_IGNORE_DEPRECATED_PUSH + #endif + Attribute c{Attribute::Components::One, Attribute::DataType::UnsignedShort}; + #ifdef MAGNUM_TARGET_WEBGL + CORRADE_IGNORE_DEPRECATED_POP + #endif + CORRADE_COMPARE(c.vectorStride(), 2); + #endif #else CORRADE_SKIP("Integer attributes are not available in OpenGL ES 2."); #endif @@ -574,6 +589,19 @@ void AttributeTest::attributeVectorUnsignedInt() { CORRADE_COMPARE(cdb.vectors(), 1); CORRADE_COMPARE(db.dataType(), DynamicAttribute::DataType::UnsignedShort); CORRADE_COMPARE(cdb.dataType(), DynamicAttribute::DataType::UnsignedShort); + + /* Signed types for unsigned attributes are not supported on WebGL, exposed + just as deprecated for backwards compatibility */ + #if !defined(MAGNUM_TARGET_WEBGL) || defined(MAGNUM_BUILD_DEPRECATED) + #ifdef MAGNUM_TARGET_WEBGL + CORRADE_IGNORE_DEPRECATED_PUSH + #endif + Attribute c{Attribute::Components::One, Attribute::DataType::Short}; + #ifdef MAGNUM_TARGET_WEBGL + CORRADE_IGNORE_DEPRECATED_POP + #endif + CORRADE_COMPARE(c.vectorStride(), 2); + #endif #else CORRADE_SKIP("Integer attributes are not available in OpenGL ES 2."); #endif @@ -1298,9 +1326,53 @@ void AttributeTest::debugDataTypeFloat() { void AttributeTest::debugDataTypeInt() { typedef Attribute<3, Int> Attribute; - Containers::String out; - Debug{&out} << Attribute::DataType::Short << Attribute::DataType(0xdead); - CORRADE_COMPARE(out, "GL::Attribute::DataType::Short GL::Attribute::DataType(0xdead)\n"); + { + Containers::String out; + Debug{&out} << Attribute::DataType::Short << Attribute::DataType(0xdead); + CORRADE_COMPARE(out, "GL::Attribute::DataType::Short GL::Attribute::DataType(0xdead)\n"); + } + + /* Unsigned types for signed attributes are not supported on WebGL, exposed + just as deprecated for backwards compatibility */ + #if !defined(MAGNUM_TARGET_WEBGL) || defined(MAGNUM_BUILD_DEPRECATED) + { + Containers::String out; + #ifdef MAGNUM_TARGET_WEBGL + CORRADE_IGNORE_DEPRECATED_PUSH + #endif + Debug{&out} << Attribute::DataType::UnsignedInt << Attribute::DataType::UnsignedByte; + #ifdef MAGNUM_TARGET_WEBGL + CORRADE_IGNORE_DEPRECATED_POP + #endif + CORRADE_COMPARE(out, "GL::Attribute::DataType::UnsignedInt GL::Attribute::DataType::UnsignedByte\n"); + } + #endif +} + +void AttributeTest::debugDataTypeUnsignedInt() { + typedef Attribute<3, UnsignedInt> Attribute; + + { + Containers::String out; + Debug{&out} << Attribute::DataType::UnsignedShort << Attribute::DataType(0xdead); + CORRADE_COMPARE(out, "GL::Attribute::DataType::UnsignedShort GL::Attribute::DataType(0xdead)\n"); + } + + /* Signed types for unsigned attributes are not supported on WebGL, exposed + just as deprecated for backwards compatibility */ + #if !defined(MAGNUM_TARGET_WEBGL) || defined(MAGNUM_BUILD_DEPRECATED) + { + Containers::String out; + #ifdef MAGNUM_TARGET_WEBGL + CORRADE_IGNORE_DEPRECATED_PUSH + #endif + Debug{&out} << Attribute::DataType::Int << Attribute::DataType::Byte; + #ifdef MAGNUM_TARGET_WEBGL + CORRADE_IGNORE_DEPRECATED_POP + #endif + CORRADE_COMPARE(out, "GL::Attribute::DataType::Int GL::Attribute::DataType::Byte\n"); + } + #endif } #endif diff --git a/src/Magnum/GL/Test/MeshGLTest.cpp b/src/Magnum/GL/Test/MeshGLTest.cpp index 527d58b6c..58f02519d 100644 --- a/src/Magnum/GL/Test/MeshGLTest.cpp +++ b/src/Magnum/GL/Test/MeshGLTest.cpp @@ -1719,6 +1719,9 @@ void MeshGLTest::addVertexBufferUnsignedIntWithShort() { CORRADE_SKIP("WebGL doesn't allow supplying signed data to an unsigned attribute."); #endif + /* Signed DataType is now deprecated for unsigned attributes, so build the + rest only on non-WebGL or on deprecated WebGL builds */ + #if !defined(MAGNUM_TARGET_WEBGL) || defined(MAGNUM_BUILD_DEPRECATED) constexpr Short data[] = { 0, 24563, 2128, 3821, 16583 }; Buffer buffer; buffer.setData(data, BufferUsage::StaticDraw); @@ -1728,7 +1731,13 @@ void MeshGLTest::addVertexBufferUnsignedIntWithShort() { if(testCaseInstanceId() == 0) { setTestCaseDescription("Attribute"); + #ifdef MAGNUM_TARGET_WEBGL + CORRADE_IGNORE_DEPRECATED_PUSH + #endif mesh.addVertexBuffer(buffer, 2, 2, Attribute<0, UnsignedInt>{Attribute<0, UnsignedInt>::DataType::Short}); + #ifdef MAGNUM_TARGET_WEBGL + CORRADE_IGNORE_DEPRECATED_POP + #endif } else if(testCaseInstanceId() == 1) { setTestCaseDescription("DynamicAttribute"); mesh.addVertexBuffer(buffer, 4, 4, DynamicAttribute{ @@ -1749,6 +1758,7 @@ void MeshGLTest::addVertexBufferUnsignedIntWithShort() { MAGNUM_VERIFY_NO_GL_ERROR(); CORRADE_COMPARE(value, 16583); + #endif } void MeshGLTest::addVertexBufferIntWithUnsignedShort() { @@ -1761,6 +1771,9 @@ void MeshGLTest::addVertexBufferIntWithUnsignedShort() { CORRADE_SKIP("WebGL doesn't allow supplying unsigned data to a signed attribute."); #endif + /* Unsigned DataType is now deprecated for signed attributes, so build the + rest only on non-WebGL or on deprecated WebGL builds */ + #if !defined(MAGNUM_TARGET_WEBGL) || defined(MAGNUM_BUILD_DEPRECATED) constexpr UnsignedShort data[] = { 0, 49563, 2128, 3821, 16583 }; Buffer buffer; buffer.setData(data, BufferUsage::StaticDraw); @@ -1770,7 +1783,13 @@ void MeshGLTest::addVertexBufferIntWithUnsignedShort() { if(testCaseInstanceId() == 0) { setTestCaseDescription("Attribute"); + #ifdef MAGNUM_TARGET_WEBGL + CORRADE_IGNORE_DEPRECATED_PUSH + #endif mesh.addVertexBuffer(buffer, 2, 2, Attribute<0, Int>{Attribute<0, Int>::DataType::UnsignedShort}); + #ifdef MAGNUM_TARGET_WEBGL + CORRADE_IGNORE_DEPRECATED_POP + #endif } else if(testCaseInstanceId() == 1) { setTestCaseDescription("DynamicAttribute"); mesh.addVertexBuffer(buffer, 4, 4, DynamicAttribute{ @@ -1791,6 +1810,7 @@ void MeshGLTest::addVertexBufferIntWithUnsignedShort() { MAGNUM_VERIFY_NO_GL_ERROR(); CORRADE_COMPARE(value, 16583); + #endif } void MeshGLTest::addVertexBufferIntWithShort() {