Skip to content

Commit

Permalink
MeshTools: option to disable unknown attribute warnings in compile().
Browse files Browse the repository at this point in the history
  • Loading branch information
mosra committed Mar 10, 2020
1 parent 00cc69c commit 6f62217
Show file tree
Hide file tree
Showing 3 changed files with 83 additions and 28 deletions.
48 changes: 32 additions & 16 deletions src/Magnum/MeshTools/Compile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -52,19 +52,11 @@

namespace Magnum { namespace MeshTools {

GL::Mesh compile(const Trade::MeshData& meshData, GL::Buffer& indices, GL::Buffer& vertices) {
return compile(meshData, GL::Buffer::wrap(indices.id(), GL::Buffer::TargetHint::ElementArray), GL::Buffer::wrap(vertices.id(), GL::Buffer::TargetHint::Array));
}

GL::Mesh compile(const Trade::MeshData& meshData, GL::Buffer& indices, GL::Buffer&& vertices) {
return compile(meshData, GL::Buffer::wrap(indices.id(), GL::Buffer::TargetHint::ElementArray), std::move(vertices));
}

GL::Mesh compile(const Trade::MeshData& meshData, GL::Buffer&& indices, GL::Buffer& vertices) {
return compile(meshData, std::move(indices), GL::Buffer::wrap(vertices.id(), GL::Buffer::TargetHint::Array));
}
namespace {

GL::Mesh compile(const Trade::MeshData& meshData, GL::Buffer&& indices, GL::Buffer&& vertices) {
GL::Mesh compileInternal(const Trade::MeshData& meshData, GL::Buffer&& indices, GL::Buffer&& vertices, const CompileFlags flags) {
/* Only this one flag is allowed at this point */
CORRADE_INTERNAL_ASSERT(!(flags & ~CompileFlag::NoWarnOnCustomAttributes));
CORRADE_ASSERT((!meshData.isIndexed() || indices.id()) && vertices.id(),
"MeshTools::compile(): invalid external buffer(s)", GL::Mesh{});

Expand All @@ -82,7 +74,8 @@ GL::Mesh compile(const Trade::MeshData& meshData, GL::Buffer&& indices, GL::Buff
single 32-bit value :( */
const VertexFormat format = meshData.attributeFormat(i);
if(isVertexFormatImplementationSpecific(format)) {
Warning{} << "MeshTools::compile(): ignoring attribute" << meshData.attributeName(i) << "with an implementation-specific format" << reinterpret_cast<void*>(vertexFormatUnwrap(format));
if(!(flags & CompileFlag::NoWarnOnCustomAttributes))
Warning{} << "MeshTools::compile(): ignoring attribute" << meshData.attributeName(i) << "with an implementation-specific format" << reinterpret_cast<void*>(vertexFormatUnwrap(format));
continue;
}

Expand Down Expand Up @@ -113,7 +106,8 @@ GL::Mesh compile(const Trade::MeshData& meshData, GL::Buffer&& indices, GL::Buff
}

if(!attribute) {
Warning{} << "MeshTools::compile(): ignoring unknown attribute" << meshData.attributeName(i);
if(!(flags & CompileFlag::NoWarnOnCustomAttributes))
Warning{} << "MeshTools::compile(): ignoring unknown attribute" << meshData.attributeName(i);
continue;
}

Expand All @@ -134,7 +128,7 @@ GL::Mesh compile(const Trade::MeshData& meshData, GL::Buffer&& indices, GL::Buff
return mesh;
}

GL::Mesh compile(const Trade::MeshData& meshData) {
GL::Mesh compileInternal(const Trade::MeshData& meshData, const CompileFlags flags) {
GL::Buffer indices{NoCreate};
if(meshData.isIndexed()) {
indices = GL::Buffer{GL::Buffer::TargetHint::ElementArray};
Expand All @@ -144,7 +138,29 @@ GL::Mesh compile(const Trade::MeshData& meshData) {
GL::Buffer vertices{GL::Buffer::TargetHint::Array};
vertices.setData(meshData.vertexData());

return compileInternal(meshData, std::move(indices), std::move(vertices));
return compileInternal(meshData, std::move(indices), std::move(vertices), flags);
}

}

GL::Mesh compile(const Trade::MeshData& meshData, GL::Buffer&& indices, GL::Buffer&& vertices) {
return compileInternal(meshData, std::move(indices), std::move(vertices), {});
}

GL::Mesh compile(const Trade::MeshData& meshData, GL::Buffer& indices, GL::Buffer& vertices) {
return compileInternal(meshData, GL::Buffer::wrap(indices.id(), GL::Buffer::TargetHint::ElementArray), GL::Buffer::wrap(vertices.id(), GL::Buffer::TargetHint::Array), CompileFlag::NoWarnOnCustomAttributes);
}

GL::Mesh compile(const Trade::MeshData& meshData, GL::Buffer& indices, GL::Buffer&& vertices) {
return compileInternal(meshData, GL::Buffer::wrap(indices.id(), GL::Buffer::TargetHint::ElementArray), std::move(vertices), CompileFlag::NoWarnOnCustomAttributes);
}

GL::Mesh compile(const Trade::MeshData& meshData, GL::Buffer&& indices, GL::Buffer& vertices) {
return compileInternal(meshData, std::move(indices), GL::Buffer::wrap(vertices.id(), GL::Buffer::TargetHint::Array), CompileFlag::NoWarnOnCustomAttributes);
}

GL::Mesh compile(const Trade::MeshData& meshData) {
return compileInternal(meshData, {});
}

GL::Mesh compile(const Trade::MeshData& meshData, CompileFlags flags) {
Expand Down
21 changes: 19 additions & 2 deletions src/Magnum/MeshTools/Compile.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,18 @@ enum class CompileFlag: UnsignedByte {
* is not a triangle mesh or doesn't have 3D positions, this flag does
* nothing. If the mesh already has its own normals, these get replaced.
*/
GenerateSmoothNormals = 1 << 1
GenerateSmoothNormals = 1 << 1,

/**
* By default, @ref compile() warns when it encounters custom attributes
* and attributes with implementation-specific format, as those get ignored
* by it. If you're binding those manually with
* @ref compile(const Trade::MeshData&, GL::Buffer&, GL::Buffer&) or
* handling them in some other way on the application side already, use
* this flag to suppress the warning messages.
* @m_since_latest
*/
NoWarnOnCustomAttributes = 1 << 2
};

/**
Expand Down Expand Up @@ -105,7 +116,8 @@ possibly also an index buffer, if the mesh is indexed.
their type.
- Custom attributes and known attributes of implementation-specific types
are ignored with a warning. See the @ref compile(const Trade::MeshData&, GL::Buffer&, GL::Buffer&)
for an example showing how to bind them manually.
for an example showing how to bind them manually, and
@ref CompileFlag::NoWarnOnCustomAttributes to suppress the warning.
If normal generation is not requested, @ref Trade::MeshData::indexData() and
@ref Trade::MeshData::vertexData() are uploaded as-is without any further
Expand Down Expand Up @@ -158,6 +170,11 @@ by the mesh or not:
If @p meshData is not indexed, the @p indices parameter is ignored --- in that
case you can pass a @ref NoCreate "NoCreate"-d instance to avoid allocating an
unnecessary OpenGL buffer object.
Compared to @ref compile(const Trade::MeshData&, CompileFlags), this function
implicitly enables the @ref CompileFlag::NoWarnOnCustomAttributes flag,
assuming that custom attributes and attributes with implementation-specific
formats are explicitly handled on the application side.
*/
MAGNUM_MESHTOOLS_EXPORT GL::Mesh compile(const Trade::MeshData& meshData, GL::Buffer& indices, GL::Buffer& vertices);

Expand Down
42 changes: 32 additions & 10 deletions src/Magnum/MeshTools/Test/CompileGLTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ struct CompileGLTest: GL::OpenGLTester {

void packedAttributes();

void unknownAttribute();
void customAttribute();
void implementationSpecificAttributeFormat();
void generateNormalsNoPosition();
void generateNormals2DPosition();
Expand Down Expand Up @@ -170,6 +170,14 @@ constexpr struct {
{"move both", true, true, true}
};

constexpr struct {
const char* name;
CompileFlags flags;
} CustomAttributeWarningData[] {
{"", {}},
{"no warning", CompileFlag::NoWarnOnCustomAttributes}
};

using namespace Math::Literals;

constexpr Color4ub ImageData[] {
Expand Down Expand Up @@ -203,11 +211,13 @@ CompileGLTest::CompileGLTest() {
CORRADE_IGNORE_DEPRECATED_POP
#endif

addTests({&CompileGLTest::packedAttributes,
addTests({&CompileGLTest::packedAttributes});

addInstancedTests({&CompileGLTest::customAttribute,
&CompileGLTest::implementationSpecificAttributeFormat},
Containers::arraySize(CustomAttributeWarningData));

&CompileGLTest::unknownAttribute,
&CompileGLTest::implementationSpecificAttributeFormat,
&CompileGLTest::generateNormalsNoPosition,
addTests({&CompileGLTest::generateNormalsNoPosition,
&CompileGLTest::generateNormals2DPosition,
&CompileGLTest::generateNormalsNoFloats});

Expand Down Expand Up @@ -714,27 +724,39 @@ void CompileGLTest::packedAttributes() {
(DebugTools::CompareImageToFile{_manager, 1.0f, 0.0948f}));
}

void CompileGLTest::unknownAttribute() {
void CompileGLTest::customAttribute() {
auto&& instanceData = CustomAttributeWarningData[testCaseInstanceId()];
setTestCaseDescription(instanceData.name);

Trade::MeshData data{MeshPrimitive::Triangles,
nullptr, {Trade::MeshAttributeData{Trade::meshAttributeCustom(115),
VertexFormat::Short, nullptr}}};

std::ostringstream out;
Warning redirectError{&out};
MeshTools::compile(data);
CORRADE_COMPARE(out.str(),
if(instanceData.flags)
MeshTools::compile(data, instanceData.flags);
else
MeshTools::compile(data);
CORRADE_COMPARE(out.str(), instanceData.flags ? "" :
"MeshTools::compile(): ignoring unknown attribute Trade::MeshAttribute::Custom(115)\n");
}

void CompileGLTest::implementationSpecificAttributeFormat() {
auto&& instanceData = CustomAttributeWarningData[testCaseInstanceId()];
setTestCaseDescription(instanceData.name);

Trade::MeshData data{MeshPrimitive::Triangles,
nullptr, {Trade::MeshAttributeData{Trade::MeshAttribute::Position,
vertexFormatWrap(0xdead), nullptr}}};

std::ostringstream out;
Warning redirectError{&out};
MeshTools::compile(data);
CORRADE_COMPARE(out.str(),
if(instanceData.flags)
MeshTools::compile(data, instanceData.flags);
else
MeshTools::compile(data);
CORRADE_COMPARE(out.str(), instanceData.flags ? "" :
"MeshTools::compile(): ignoring attribute Trade::MeshAttribute::Position with an implementation-specific format 0xdead\n");
}

Expand Down

0 comments on commit 6f62217

Please sign in to comment.