Browse Source

Trade: drop {MeshAttribute,SceneField}::Custom.

I realized those are too annoying when writing a glTF exporter which
contains a lot of switches over enums. And as further shown by the diff,
those were only inflicting additional pain in *all* switch statements,
nothing else, no other added value. And everywhere else the helpers are
the designated way to deal with those, so there's no point in having an
explicit enum value denoting start of a "custom range".

It wasn't even any convenient to have it in the enum, as the extra
effort needed for casting actually made it *exactly* the same length as
if I'd just use a separately-defined constant.
pull/554/head
Vladimír Vondruš 4 years ago
parent
commit
cfc02599e5
  1. 1
      src/Magnum/MeshTools/Compile.cpp
  2. 1
      src/Magnum/MeshTools/RemoveDuplicates.cpp
  3. 11
      src/Magnum/SceneTools/sceneconverter.cpp
  4. 3
      src/Magnum/Trade/MeshData.cpp
  5. 48
      src/Magnum/Trade/MeshData.h
  6. 3
      src/Magnum/Trade/SceneData.cpp
  7. 53
      src/Magnum/Trade/SceneData.h
  8. 4
      src/Magnum/Trade/Test/MeshDataTest.cpp
  9. 4
      src/Magnum/Trade/Test/SceneDataTest.cpp

1
src/Magnum/MeshTools/Compile.cpp

@ -125,7 +125,6 @@ GL::Mesh compileInternal(const Trade::MeshData& meshData, GL::Buffer&& indices,
#ifdef MAGNUM_TARGET_GLES2
case Trade::MeshAttribute::ObjectId:
#endif
case Trade::MeshAttribute::Custom:
break;
/* LCOV_EXCL_STOP */
}

1
src/Magnum/MeshTools/RemoveDuplicates.cpp

@ -533,7 +533,6 @@ Trade::MeshData removeDuplicatesFuzzy(const Trade::MeshData& data, const Float f
/* These have unbounded range. Do nothing but enumerate all
these here to silence warnings about unused enum values. */
case Trade::MeshAttribute::Position:
case Trade::MeshAttribute::Custom:
break;
/* These shouldn't be floating point */

11
src/Magnum/SceneTools/sceneconverter.cpp

@ -680,8 +680,9 @@ is specified as well, the IDs reference attributes of the first mesh.)")
for(UnsignedInt k = 0; k != mesh->attributeCount(); ++k) {
const Trade::MeshAttribute name = mesh->attributeName(k);
/* Calculate bounds, if requested and this is not an
implementation-specific format */
/* Calculate bounds, if requested, if this is not an
implementation-specific format and if it's not a custom
attribute */
Containers::String bounds;
if(args.isSet("bounds") && !isVertexFormatImplementationSpecific(mesh->attributeFormat(k))) switch(name) {
case Trade::MeshAttribute::Position:
@ -706,12 +707,6 @@ is specified as well, the IDs reference attributes of the first mesh.)")
Debug{} << mesh->objectIdsAsArray(namedAttributeId(*mesh, k));
bounds = calculateBounds(mesh->objectIdsAsArray(namedAttributeId(*mesh, k)));
break;
/* And also all other custom attribs. Not saying
default: here so we get notified when we forget to
handle newly added attribute names */
case Trade::MeshAttribute::Custom:
break;
}
arrayAppend(info.attributes, InPlaceInit,

3
src/Magnum/Trade/MeshData.cpp

@ -899,9 +899,6 @@ Debug& operator<<(Debug& debug, const MeshAttribute value) {
_c(ObjectId)
#undef _c
/* LCOV_EXCL_STOP */
/* To silence compiler warning about unhandled values */
case MeshAttribute::Custom: CORRADE_INTERNAL_ASSERT_UNREACHABLE(); /* LCOV_EXCL_LINE */
}
return debug << (packed ? "" : "(") << Debug::nospace << reinterpret_cast<void*>(UnsignedShort(value)) << Debug::nospace << (packed ? "" : ")");

48
src/Magnum/Trade/MeshData.h

@ -40,13 +40,26 @@
namespace Magnum { namespace Trade {
namespace Implementation {
enum: UnsignedShort { MeshAttributeCustom = 32768 };
}
/**
@brief Mesh attribute name
@m_since{2020,06}
@see @ref MeshData, @ref MeshAttributeData, @ref VertexFormat,
@ref AbstractImporter::meshAttributeForName(),
@ref AbstractImporter::meshAttributeName()
See @ref MeshData for more information.
Apart from the builtin attribute names it's possible to have custom ones,
which use the upper 15 bits of the enum range. Those are detected via
@ref isMeshAttributeCustom() and can be converted to and from a numeric
identifier using @ref meshAttributeCustom(MeshAttribute) and
@ref meshAttributeCustom(UnsignedShort). Unlike the builtin ones, these can
be of any type. An importer that exposes custom attributes then may also
provide a string mapping using @ref AbstractImporter::meshAttributeForName()
and @ref AbstractImporter::meshAttributeName(). See documentation of a
particular importer for details.
@see @ref MeshAttributeData, @ref VertexFormat
*/
/* 16 bits because 8 bits is not enough to cover all potential per-edge,
per-face, per-instance and per-meshlet attributes */
@ -145,16 +158,7 @@ enum class MeshAttribute: UnsignedShort {
* Corresponds to @ref Shaders::GenericGL::ObjectId.
* @see @ref MeshData::objectIdsAsArray()
*/
ObjectId,
/**
* This and all higher values are for importer-specific attributes. Can be
* of any type. See documentation of a particular importer for details.
* @see @ref isMeshAttributeCustom(),
* @ref meshAttributeCustom(MeshAttribute),
* @ref meshAttributeCustom(UnsignedShort)
*/
Custom = 32768
ObjectId
};
/**
@ -167,30 +171,30 @@ MAGNUM_TRADE_EXPORT Debug& operator<<(Debug& debug, MeshAttribute value);
@brief Whether a mesh attribute is custom
@m_since{2020,06}
Returns @cpp true @ce if @p name has a value larger or equal to
@ref MeshAttribute::Custom, @cpp false @ce otherwise.
Returns @cpp true @ce if @p name has a value in the upper 15 bits of the enum
range, @cpp false @ce otherwise.
@see @ref meshAttributeCustom(UnsignedShort),
@ref meshAttributeCustom(MeshAttribute),
@ref AbstractImporter::meshAttributeName()
*/
constexpr bool isMeshAttributeCustom(MeshAttribute name) {
return UnsignedShort(name) >= UnsignedShort(MeshAttribute::Custom);
return UnsignedShort(name) >= Implementation::MeshAttributeCustom;
}
/**
@brief Create a custom mesh attribute
@m_since{2020,06}
Returns a custom mesh attribute with index @p id. The index is expected to be
less than the value of @ref MeshAttribute::Custom. Use
@ref meshAttributeCustom(MeshAttribute) to get the index back.
Returns a custom mesh attribute with index @p id. The index is expected to fit
into 15 bits. Use @ref meshAttributeCustom(MeshAttribute) to get the index
back.
*/
/* Constexpr so it's usable for creating compile-time MeshAttributeData
instances */
constexpr MeshAttribute meshAttributeCustom(UnsignedShort id) {
return CORRADE_CONSTEXPR_ASSERT(id < UnsignedShort(MeshAttribute::Custom),
return CORRADE_CONSTEXPR_ASSERT(id < Implementation::MeshAttributeCustom,
"Trade::meshAttributeCustom(): index" << id << "too large"),
MeshAttribute(UnsignedShort(MeshAttribute::Custom) + id);
MeshAttribute(Implementation::MeshAttributeCustom + id);
}
/**
@ -204,7 +208,7 @@ is custom.
constexpr UnsignedShort meshAttributeCustom(MeshAttribute name) {
return CORRADE_CONSTEXPR_ASSERT(isMeshAttributeCustom(name),
"Trade::meshAttributeCustom():" << name << "is not custom"),
UnsignedShort(name) - UnsignedShort(MeshAttribute::Custom);
UnsignedShort(name) - Implementation::MeshAttributeCustom;
}
/**

3
src/Magnum/Trade/SceneData.cpp

@ -131,9 +131,6 @@ Debug& operator<<(Debug& debug, const SceneField value) {
_c(ImporterState)
#undef _c
/* LCOV_EXCL_STOP */
/* To silence compiler warning about unhandled values */
case SceneField::Custom: CORRADE_INTERNAL_ASSERT_UNREACHABLE(); /* LCOV_EXCL_LINE */
}
return debug << (packed ? "" : "(") << Debug::nospace << reinterpret_cast<void*>(UnsignedInt(value)) << Debug::nospace << (packed ? "" : ")");

53
src/Magnum/Trade/SceneData.h

@ -85,14 +85,30 @@ Returns the same value as @ref sceneMappingTypeSize().
*/
MAGNUM_TRADE_EXPORT UnsignedInt sceneMappingTypeAlignment(SceneMappingType type);
namespace Implementation {
enum: UnsignedInt { SceneFieldCustom = 0x80000000u };
}
/**
@brief Scene field name
@m_since_latest
See @ref SceneData for more information.
@see @ref SceneFieldData, @ref SceneFieldType,
@ref AbstractImporter::sceneFieldForName(),
@ref AbstractImporter::sceneFieldName()
Apart from the builtin field names it's possible to have custom ones, which use
the upper 31 bits of the enum range. While it's unlikely to have billions of
custom fields, the enum intentionally reserves a full 31-bit range to avoid the
need to remap field identifiers coming from 3rd party ECS frameworks, for
example.
Custom fields are detected via @ref isSceneFieldCustom() and can be converted
to and from a numeric identifier using @ref sceneFieldCustom(SceneField) and
@ref sceneFieldCustom(UnsignedInt). Unlike the builtin ones, these can be of
any type. An importer that exposes custom fields then may also provide a string
mapping using @ref AbstractImporter::sceneFieldForName() and
@ref AbstractImporter::sceneFieldName(). See documentation of a particular
importer for details.
@see @ref SceneFieldData, @ref SceneFieldType
*/
enum class SceneField: UnsignedInt {
/* Zero used for an invalid value */
@ -303,19 +319,7 @@ enum class SceneField: UnsignedInt {
* @see @ref SceneData::importerStateAsArray(),
* @ref SceneData::importerStateFor()
*/
ImporterState,
/**
* This and all higher values are for importer-specific fields. Can be
* of any type. See documentation of a particular importer for details.
*
* While it's unlikely to have billions of custom fields, the enum
* intentionally reserves a full 31-bit range to avoid the need to remap
* field identifiers coming from 3rd party ECS frameworks, for example.
* @see @ref isSceneFieldCustom(), @ref sceneFieldCustom(SceneField),
* @ref sceneFieldCustom(UnsignedInt)
*/
Custom = 0x80000000u
ImporterState
};
/**
@ -328,29 +332,28 @@ MAGNUM_TRADE_EXPORT Debug& operator<<(Debug& debug, SceneField value);
@brief Whether a scene field is custom
@m_since_latest
Returns @cpp true @ce if @p name has a value larger or equal to
@ref SceneField::Custom, @cpp false @ce otherwise.
Returns @cpp true @ce if @p name has a value in the upper 31 bits of the enum
range, @cpp false @ce otherwise.
@see @ref sceneFieldCustom(UnsignedInt), @ref sceneFieldCustom(SceneField),
@ref AbstractImporter::sceneFieldName()
*/
constexpr bool isSceneFieldCustom(SceneField name) {
return UnsignedInt(name) >= UnsignedInt(SceneField::Custom);
return UnsignedInt(name) >= Implementation::SceneFieldCustom;
}
/**
@brief Create a custom scene field
@m_since_latest
Returns a custom scene field with index @p id. The index is expected to be less
than the value of @ref SceneField::Custom. Use @ref sceneFieldCustom(SceneField)
to get the index back.
Returns a custom scene field with index @p id. The index is expected to fit
into 31 bits. Use @ref sceneFieldCustom(SceneField) to get the index back.
*/
/* Constexpr so it's usable for creating compile-time SceneFieldData
instances */
constexpr SceneField sceneFieldCustom(UnsignedInt id) {
return CORRADE_CONSTEXPR_ASSERT(id < UnsignedInt(SceneField::Custom),
return CORRADE_CONSTEXPR_ASSERT(id < Implementation::SceneFieldCustom,
"Trade::sceneFieldCustom(): index" << id << "too large"),
SceneField(UnsignedInt(SceneField::Custom) + id);
SceneField(Implementation::SceneFieldCustom + id);
}
/**
@ -364,7 +367,7 @@ custom.
constexpr UnsignedInt sceneFieldCustom(SceneField name) {
return CORRADE_CONSTEXPR_ASSERT(isSceneFieldCustom(name),
"Trade::sceneFieldCustom():" << name << "is not custom"),
UnsignedInt(name) - UnsignedInt(SceneField::Custom);
UnsignedInt(name) - Implementation::SceneFieldCustom;
}
/**

4
src/Magnum/Trade/Test/MeshDataTest.cpp

@ -419,14 +419,14 @@ MeshDataTest::MeshDataTest() {
void MeshDataTest::customAttributeName() {
CORRADE_VERIFY(!isMeshAttributeCustom(MeshAttribute::Position));
CORRADE_VERIFY(!isMeshAttributeCustom(MeshAttribute(32767)));
CORRADE_VERIFY(isMeshAttributeCustom(MeshAttribute::Custom));
CORRADE_VERIFY(isMeshAttributeCustom(MeshAttribute(Implementation::MeshAttributeCustom)));
CORRADE_VERIFY(isMeshAttributeCustom(MeshAttribute(65535)));
CORRADE_COMPARE(UnsignedShort(meshAttributeCustom(0)), 32768);
CORRADE_COMPARE(UnsignedShort(meshAttributeCustom(8290)), 41058);
CORRADE_COMPARE(UnsignedShort(meshAttributeCustom(32767)), 65535);
CORRADE_COMPARE(meshAttributeCustom(MeshAttribute::Custom), 0);
CORRADE_COMPARE(meshAttributeCustom(MeshAttribute(Implementation::MeshAttributeCustom)), 0);
CORRADE_COMPARE(meshAttributeCustom(MeshAttribute(41058)), 8290);
CORRADE_COMPARE(meshAttributeCustom(MeshAttribute(65535)), 32767);

4
src/Magnum/Trade/Test/SceneDataTest.cpp

@ -625,14 +625,14 @@ void SceneDataTest::debugMappingTypePacked() {
void SceneDataTest::customFieldName() {
CORRADE_VERIFY(!isSceneFieldCustom(SceneField::Rotation));
CORRADE_VERIFY(!isSceneFieldCustom(SceneField(0x0fffffffu)));
CORRADE_VERIFY(isSceneFieldCustom(SceneField::Custom));
CORRADE_VERIFY(isSceneFieldCustom(SceneField(Implementation::SceneFieldCustom)));
CORRADE_VERIFY(isSceneFieldCustom(SceneField(0x80000000u)));
CORRADE_COMPARE(UnsignedInt(sceneFieldCustom(0)), 0x80000000u);
CORRADE_COMPARE(UnsignedInt(sceneFieldCustom(0xabcd)), 0x8000abcdu);
CORRADE_COMPARE(UnsignedInt(sceneFieldCustom(0x7fffffff)), 0xffffffffu);
CORRADE_COMPARE(sceneFieldCustom(SceneField::Custom), 0);
CORRADE_COMPARE(sceneFieldCustom(SceneField(Implementation::SceneFieldCustom)), 0);
CORRADE_COMPARE(sceneFieldCustom(SceneField(0x8000abcdu)), 0xabcd);
CORRADE_COMPARE(sceneFieldCustom(SceneField(0xffffffffu)), 0x7fffffffu);

Loading…
Cancel
Save