{Mesh,Scene}Tools: add r-value filter{Attributes,Fields}() overloads.

When writing high-level documentation for those I realized that it's
easier to just add the r-value variants than trying to explain why one
has to be careful to not pass r-values there.

src/Magnum/MeshTools/Filter.cpp

@@ -30,6 +30,7 @@
 #include <Corrade/Containers/BitArrayView.h>
 #include <Corrade/Utility/BitAlgorithms.h>
 
+#include "Magnum/MeshTools/Copy.h"
 #include "Magnum/Trade/MeshData.h"
 
 #ifdef MAGNUM_BUILD_DEPRECATED
@@ -39,7 +40,7 @@
 
 namespace Magnum { namespace MeshTools {
 
-Trade::MeshData filterAttributes(const Trade::MeshData& mesh, const Containers::BitArrayView attributesToKeep) {
+Trade::MeshData filterAttributes(Trade::MeshData&& mesh, const Containers::BitArrayView attributesToKeep) {
     CORRADE_ASSERT(attributesToKeep.size() == mesh.attributeCount(),
         "MeshTools::filterAttributes(): expected" << mesh.attributeCount() << "bits but got" << attributesToKeep.size(), (Trade::MeshData{MeshPrimitive::Triangles, 0}));
 
@@ -61,13 +62,39 @@ Trade::MeshData filterAttributes(const Trade::MeshData& mesh, const Containers::
             mesh.indexCount(),
             mesh.indexStride()}};
 
-    return Trade::MeshData{mesh.primitive(),
+    /* If either the index or vertex buffer is owned, transfer the ownership to
-        {}, mesh.indexData(), indices,
+       the returned instance, otherwise reference the original. Save vertex
-        {}, mesh.vertexData(), Utility::move(filtered),
+       count first to use for the returned instance because releaseVertexData()
-        mesh.vertexCount()};
+       would clear it. */
+    const UnsignedInt vertexCount = mesh.vertexCount();
+    if(mesh.indexDataFlags() >= Trade::DataFlag::Owned &&
+       mesh.vertexDataFlags() >= Trade::DataFlag::Owned)
+        return Trade::MeshData{mesh.primitive(),
+            mesh.releaseIndexData(), indices,
+            mesh.releaseVertexData(), Utility::move(filtered),
+            vertexCount};
+    else if(mesh.indexDataFlags() >= Trade::DataFlag::Owned)
+        return Trade::MeshData{mesh.primitive(),
+            mesh.releaseIndexData(), indices,
+            {}, mesh.vertexData(), Utility::move(filtered),
+            vertexCount};
+    else if(mesh.vertexDataFlags() >= Trade::DataFlag::Owned)
+        return Trade::MeshData{mesh.primitive(),
+            {}, mesh.indexData(), indices,
+            mesh.releaseVertexData(), Utility::move(filtered),
+            vertexCount};
+    else
+        return Trade::MeshData{mesh.primitive(),
+            {}, mesh.indexData(), indices,
+            {}, mesh.vertexData(), Utility::move(filtered),
+            mesh.vertexCount()};
 }
 
-Trade::MeshData filterOnlyAttributes(const Trade::MeshData& mesh, const Containers::ArrayView<const Trade::MeshAttribute> attributes) {
+Trade::MeshData filterAttributes(const Trade::MeshData& mesh, const Containers::BitArrayView attributesToKeep) {
+    return filterAttributes(reference(mesh), attributesToKeep);
+}
+
+Trade::MeshData filterOnlyAttributes(Trade::MeshData&& mesh, const Containers::ArrayView<const Trade::MeshAttribute> attributes) {
     Containers::BitArray attributesToKeep{DirectInit, mesh.attributeCount(), false};
     for(const Trade::MeshAttribute attribute: attributes) {
         /* Can't use findAttributeId() because all instances of the attribute
@@ -77,13 +104,21 @@ Trade::MeshData filterOnlyAttributes(const Trade::MeshData& mesh, const Containe
                 attributesToKeep.set(i);
         }
     }
-    return filterAttributes(mesh, attributesToKeep);
+    return filterAttributes(Utility::move(mesh), attributesToKeep);
+}
+
+Trade::MeshData filterOnlyAttributes(const Trade::MeshData& mesh, const Containers::ArrayView<const Trade::MeshAttribute> attributes) {
+    return filterOnlyAttributes(reference(mesh), attributes);
 }
 
 Trade::MeshData filterOnlyAttributes(const Trade::MeshData& mesh, std::initializer_list<Trade::MeshAttribute> attributes) {
     return filterOnlyAttributes(mesh, Containers::arrayView(attributes));
 }
 
+Trade::MeshData filterOnlyAttributes(Trade::MeshData&& mesh, std::initializer_list<Trade::MeshAttribute> attributes) {
+    return filterOnlyAttributes(Utility::move(mesh), Containers::arrayView(attributes));
+}
+
 #ifdef MAGNUM_BUILD_DEPRECATED
 Trade::MeshData filterOnlyAttributes(const Trade::MeshData& mesh, const Containers::ArrayView<const UnsignedInt> attributes) {
     Containers::BitArray attributesToKeep{DirectInit, mesh.attributeCount(), false};
@@ -104,7 +139,7 @@ Trade::MeshData filterOnlyAttributes(const Trade::MeshData& mesh, std::initializ
 }
 #endif
 
-Trade::MeshData filterExceptAttributes(const Trade::MeshData& mesh, const Containers::ArrayView<const Trade::MeshAttribute> attributes) {
+Trade::MeshData filterExceptAttributes(Trade::MeshData&& mesh, const Containers::ArrayView<const Trade::MeshAttribute> attributes) {
     Containers::BitArray attributesToKeep{DirectInit, mesh.attributeCount(), true};
     for(const Trade::MeshAttribute attribute: attributes) {
         /* Can't use findAttributeId() because all instances of the attribute
@@ -114,13 +149,21 @@ Trade::MeshData filterExceptAttributes(const Trade::MeshData& mesh, const Contai
                 attributesToKeep.reset(i);
         }
     }
-    return filterAttributes(mesh, attributesToKeep);
+    return filterAttributes(Utility::move(mesh), attributesToKeep);
+}
+
+Trade::MeshData filterExceptAttributes(const Trade::MeshData& mesh, const Containers::ArrayView<const Trade::MeshAttribute> attributes) {
+    return filterExceptAttributes(reference(mesh), attributes);
 }
 
 Trade::MeshData filterExceptAttributes(const Trade::MeshData& mesh, std::initializer_list<Trade::MeshAttribute> attributes) {
     return filterExceptAttributes(mesh, Containers::arrayView(attributes));
 }
 
+Trade::MeshData filterExceptAttributes(Trade::MeshData&& mesh, std::initializer_list<Trade::MeshAttribute> attributes) {
+    return filterExceptAttributes(Utility::move(mesh), Containers::arrayView(attributes));
+}
+
 #ifdef MAGNUM_BUILD_DEPRECATED
 Trade::MeshData filterExceptAttributes(const Trade::MeshData& mesh, const Containers::ArrayView<const UnsignedInt> attributes) {
     Containers::BitArray attributesToKeep{DirectInit, mesh.attributeCount(), true};

src/Magnum/MeshTools/Filter.h

@@ -56,6 +56,20 @@ without @ref InterleaveFlag::PreserveInterleavedAttributes set.
 */
 MAGNUM_MESHTOOLS_EXPORT Trade::MeshData filterAttributes(const Trade::MeshData& mesh, Containers::BitArrayView attributesToKeep);
 
+/**
+@brief Filter a mesh to contain only the selected subset of attributes
+@m_since_latest
+
+Compared to @ref filterAttributes(const Trade::MeshData&, Containers::BitArrayView),
+if the @p mesh index or vertex data is owned, the function transfers the data
+ownership to the returned instance instead of returning a non-owning reference.
+If neither the index nor the vertex data is owned, the two overloads behave the
+same.
+@see @ref Trade::MeshData::indexDataFlags(),
+     @ref Trade::MeshData::vertexDataFlags()
+*/
+MAGNUM_MESHTOOLS_EXPORT Trade::MeshData filterAttributes(Trade::MeshData&& mesh, Containers::BitArrayView attributesToKeep);
+
 /**
 @brief Filter a mesh to contain only the selected subset of named attributes
 @m_since_latest
@@ -84,6 +98,26 @@ MAGNUM_MESHTOOLS_EXPORT Trade::MeshData filterOnlyAttributes(const Trade::MeshDa
  */
 MAGNUM_MESHTOOLS_EXPORT Trade::MeshData filterOnlyAttributes(const Trade::MeshData& mesh, std::initializer_list<Trade::MeshAttribute> attributes);
 
+/**
+@brief Filter a mesh to contain only the selected subset of named attributes
+@m_since_latest
+
+Compared to @ref filterOnlyAttributes(const Trade::MeshData&, Containers::ArrayView<const Trade::MeshAttribute>),
+if the @p mesh index or vertex data is owned, the function transfers the data
+ownership to the returned instance instead of returning a non-owning reference.
+If neither the index nor the vertex data is owned, the two overloads behave the
+same.
+@see @ref Trade::MeshData::indexDataFlags(),
+     @ref Trade::MeshData::vertexDataFlags()
+*/
+MAGNUM_MESHTOOLS_EXPORT Trade::MeshData filterOnlyAttributes(Trade::MeshData&& mesh, Containers::ArrayView<const Trade::MeshAttribute> attributes);
+
+/**
+ * @overload
+ * @m_since_latest
+ */
+MAGNUM_MESHTOOLS_EXPORT Trade::MeshData filterOnlyAttributes(Trade::MeshData&& mesh, std::initializer_list<Trade::MeshAttribute> attributes);
+
 /**
 @brief Filter a mesh to contain everything except the selected subset of named attributes
 @m_since_latest
@@ -111,6 +145,26 @@ MAGNUM_MESHTOOLS_EXPORT Trade::MeshData filterExceptAttributes(const Trade::Mesh
  */
 MAGNUM_MESHTOOLS_EXPORT Trade::MeshData filterExceptAttributes(const Trade::MeshData& mesh, std::initializer_list<Trade::MeshAttribute> attributes);
 
+/**
+@brief Filter a mesh to contain everything except the selected subset of named attributes
+@m_since_latest
+
+Compared to @ref filterExceptAttributes(const Trade::MeshData&, Containers::ArrayView<const Trade::MeshAttribute>),
+if the @p mesh index or vertex data is owned, the function transfers the data
+ownership to the returned instance instead of returning a non-owning reference.
+If neither the index nor the vertex data is owned, the two overloads behave the
+same.
+@see @ref Trade::MeshData::indexDataFlags(),
+     @ref Trade::MeshData::vertexDataFlags()
+*/
+MAGNUM_MESHTOOLS_EXPORT Trade::MeshData filterExceptAttributes(Trade::MeshData&& mesh, Containers::ArrayView<const Trade::MeshAttribute> attributes);
+
+/**
+ * @overload
+ * @m_since_latest
+ */
+MAGNUM_MESHTOOLS_EXPORT Trade::MeshData filterExceptAttributes(Trade::MeshData&& mesh, std::initializer_list<Trade::MeshAttribute> attributes);
+
 }}
 
 #endif

src/Magnum/MeshTools/Test/FilterTest.cpp

@@ -27,6 +27,7 @@
 #include <sstream>
 #include <Corrade/Containers/BitArray.h>
 #include <Corrade/Containers/BitArrayView.h>
+#include <Corrade/Containers/Optional.h>
 #include <Corrade/TestSuite/Tester.h>
 #include <Corrade/Utility/DebugStl.h>
 
@@ -46,11 +47,13 @@ struct FilterTest: TestSuite::Tester {
 
     void attributes();
     void attributesNoIndexData();
+    void attributesRvalue();
     void attributesWrongBitCount();
 
     void onlyAttributes();
     void onlyAttributesNoIndexData();
     void onlyAttributesNoAttributeData();
+    void onlyAttributesRvalue();
 
     #ifdef MAGNUM_BUILD_DEPRECATED
     void onlyAttributeIds();
@@ -62,6 +65,7 @@ struct FilterTest: TestSuite::Tester {
     void exceptAttributes();
     void exceptAttributesNoIndexData();
     void exceptAttributesNoAttributeData();
+    void exceptAttributesRvalue();
 
     #ifdef MAGNUM_BUILD_DEPRECATED
     void exceptAttributeIds();
@@ -79,18 +83,44 @@ const struct {
     {"implementation-specific index type", meshIndexTypeWrap(0xcaca)}
 };
 
+const struct {
+    const char* name;
+    Trade::DataFlags indexDataFlags, vertexDataFlags;
+    Trade::DataFlags expectedIndexDataFlags, expectedVertexDataFlags;
+} AttributesRvalueData[]{
+    /* The Global or ExternallyOwned flags are not preserved, because
+       reference() doesn't preserve them either */
+    {"neither owned",
+        {}, Trade::DataFlag::Global,
+        {}, {}},
+    {"index data owned",
+        Trade::DataFlag::Owned, {},
+        Trade::DataFlag::Owned|Trade::DataFlag::Mutable, {}},
+    {"vertex data owned",
+        Trade::DataFlag::ExternallyOwned, Trade::DataFlag::Owned|Trade::DataFlag::Mutable,
+        {}, Trade::DataFlag::Owned|Trade::DataFlag::Mutable},
+    {"both owned",
+        Trade::DataFlag::Owned, Trade::DataFlag::Owned,
+        Trade::DataFlag::Owned|Trade::DataFlag::Mutable, Trade::DataFlag::Owned|Trade::DataFlag::Mutable},
+};
+
 FilterTest::FilterTest() {
     addInstancedTests({&FilterTest::attributes},
         Containers::arraySize(ImplementationSpecificIndexTypeData));
 
-    addTests({&FilterTest::attributesNoIndexData,
+    addTests({&FilterTest::attributesNoIndexData});
-              &FilterTest::attributesWrongBitCount});
+
+    addInstancedTests({&FilterTest::attributesRvalue},
+        Containers::arraySize(AttributesRvalueData));
+
+    addTests({&FilterTest::attributesWrongBitCount});
 
     addInstancedTests({&FilterTest::onlyAttributes},
         Containers::arraySize(ImplementationSpecificIndexTypeData));
 
     addTests({&FilterTest::onlyAttributesNoIndexData,
-              &FilterTest::onlyAttributesNoAttributeData});
+              &FilterTest::onlyAttributesNoAttributeData,
+              &FilterTest::onlyAttributesRvalue});
 
     #ifdef MAGNUM_BUILD_DEPRECATED
     addInstancedTests({&FilterTest::onlyAttributeIds},
@@ -105,7 +135,8 @@ FilterTest::FilterTest() {
         Containers::arraySize(ImplementationSpecificIndexTypeData));
 
     addTests({&FilterTest::exceptAttributesNoIndexData,
-              &FilterTest::exceptAttributesNoAttributeData});
+              &FilterTest::exceptAttributesNoAttributeData,
+              &FilterTest::exceptAttributesRvalue});
 
     #ifdef MAGNUM_BUILD_DEPRECATED
     addInstancedTests({&FilterTest::exceptAttributeIds},
@@ -202,6 +233,66 @@ void FilterTest::attributesNoIndexData() {
     CORRADE_COMPARE(filtered.attributeOffset(0), offsetof(Vertex, textureCoordinates1));
 }
 
+void FilterTest::attributesRvalue() {
+    auto&& data = AttributesRvalueData[testCaseInstanceId()];
+    setTestCaseDescription(data.name);
+
+    /* Subset of attributes() verifying data ownership transfer behavior */
+
+    Containers::Array<char> indexData{5*sizeof(UnsignedShort)};
+    Containers::StridedArrayView1D<UnsignedShort> indices = Containers::arrayCast<UnsignedShort>(indexData);
+    Containers::Array<char> vertexData{3*sizeof(Vertex)};
+    Containers::StridedArrayView1D<Vertex> vertices = Containers::arrayCast<Vertex>(vertexData);
+
+    Containers::Array<Trade::MeshAttributeData> attributes{InPlaceInit, {
+        Trade::MeshAttributeData{Trade::MeshAttribute::Position, vertices.slice(&Vertex::position)},
+        Trade::MeshAttributeData{Trade::MeshAttribute::Tangent, vertices.slice(&Vertex::tangent)},
+        Trade::MeshAttributeData{Trade::MeshAttribute::TextureCoordinates, vertices.slice(&Vertex::textureCoordinates1)},
+        Trade::MeshAttributeData{Trade::MeshAttribute::TextureCoordinates, vertices.slice(&Vertex::textureCoordinates2)},
+    }};
+
+    Containers::Optional<Trade::MeshData> mesh;
+    if(data.indexDataFlags >= Trade::DataFlag::Owned &&
+       data.vertexDataFlags >= Trade::DataFlag::Owned)
+        mesh = Trade::MeshData{MeshPrimitive::Triangles,
+            Utility::move(indexData), Trade::MeshIndexData{indices},
+            Utility::move(vertexData), Utility::move(attributes)};
+    else if(data.indexDataFlags >= Trade::DataFlag::Owned)
+        mesh = Trade::MeshData{MeshPrimitive::Triangles,
+            Utility::move(indexData), Trade::MeshIndexData{indices},
+            data.vertexDataFlags, vertexData, Utility::move(attributes)};
+    else if(data.vertexDataFlags >= Trade::DataFlag::Owned)
+        mesh = Trade::MeshData{MeshPrimitive::Triangles,
+            data.indexDataFlags, indexData, Trade::MeshIndexData{indices},
+            Utility::move(vertexData), Utility::move(attributes)};
+    else
+        mesh = Trade::MeshData{MeshPrimitive::Triangles,
+            data.indexDataFlags, indexData, Trade::MeshIndexData{indices},
+            data.vertexDataFlags, vertexData, Utility::move(attributes)};
+
+    Containers::BitArray attributesToKeep{ValueInit, mesh->attributeCount()};
+    attributesToKeep.set(1);
+    attributesToKeep.set(3);
+
+    Trade::MeshData filtered = filterAttributes(Utility::move(*mesh), attributesToKeep);
+
+    /* The data ownership should be transferred if possible */
+    CORRADE_VERIFY(filtered.isIndexed());
+    CORRADE_COMPARE(filtered.indexCount(), 5);
+    CORRADE_COMPARE(filtered.indexData().data(), indices.data());
+    CORRADE_COMPARE(filtered.indexDataFlags(), data.expectedIndexDataFlags);
+
+    CORRADE_COMPARE(filtered.vertexCount(), 3);
+    CORRADE_COMPARE(filtered.vertexData().data(), vertices.data());
+    CORRADE_COMPARE(filtered.vertexDataFlags(), data.expectedVertexDataFlags);
+
+    /* Just checking that the attributes get actually filtered instead of being
+       passed through verbatim, the actual verification is done in attributes()
+       above */
+    CORRADE_COMPARE(filtered.attributeCount(), 2);
+    CORRADE_COMPARE(filtered.attributeName(0), Trade::MeshAttribute::Tangent);
+}
+
 void FilterTest::attributesWrongBitCount() {
     CORRADE_SKIP_IF_NO_ASSERT();
 
@@ -332,6 +423,45 @@ void FilterTest::onlyAttributesNoAttributeData() {
     CORRADE_COMPARE(filtered.attributeCount(), 0);
 }
 
+void FilterTest::onlyAttributesRvalue() {
+    /* Subset of onlyAttributes() verifying data ownership transfer behavior.
+       All cases of ownership transfer are verified in attributesRvalue(), this
+       only checks that the r-value gets correctly passed through all overloads
+       to keep the index data owned and vertex data not. */
+
+    Containers::Array<char> indexData{5*sizeof(UnsignedShort)};
+    Containers::StridedArrayView1D<UnsignedShort> indices = Containers::arrayCast<UnsignedShort>(indexData);
+    Vertex vertexData[3];
+    Containers::StridedArrayView1D<Vertex> vertices = vertexData;
+
+    Trade::MeshData mesh{MeshPrimitive::TriangleStrip,
+        Utility::move(indexData), Trade::MeshIndexData{indices},
+        {}, vertexData, {
+            Trade::MeshAttributeData{Trade::MeshAttribute::Position, vertices.slice(&Vertex::position)},
+            Trade::MeshAttributeData{Trade::MeshAttribute::Tangent, vertices.slice(&Vertex::tangent)},
+            Trade::MeshAttributeData{Trade::MeshAttribute::TextureCoordinates, vertices.slice(&Vertex::textureCoordinates1)},
+            Trade::MeshAttributeData{Trade::MeshAttribute::TextureCoordinates, vertices.slice(&Vertex::textureCoordinates2)},
+        }};
+
+    Trade::MeshData filtered = filterOnlyAttributes(Utility::move(mesh), {
+        Trade::MeshAttribute::TextureCoordinates,
+        Trade::MeshAttribute::Position
+    });
+    CORRADE_COMPARE(filtered.primitive(), MeshPrimitive::TriangleStrip);
+
+    CORRADE_VERIFY(filtered.isIndexed());
+    CORRADE_COMPARE(filtered.indexCount(), 5);
+    CORRADE_COMPARE(filtered.indexData().data(), indices.data());
+    CORRADE_COMPARE(filtered.indexDataFlags(), Trade::DataFlag::Owned|Trade::DataFlag::Mutable);
+
+    CORRADE_COMPARE(filtered.vertexCount(), 3);
+    CORRADE_COMPARE(filtered.vertexData().data(), vertices.data());
+    CORRADE_COMPARE(filtered.vertexDataFlags(), Trade::DataFlags{});
+
+    CORRADE_COMPARE(filtered.attributeCount(), 3);
+    CORRADE_COMPARE(filtered.attributeName(0), Trade::MeshAttribute::Position);
+}
+
 #ifdef MAGNUM_BUILD_DEPRECATED
 void FilterTest::onlyAttributeIds() {
     auto&& data = ImplementationSpecificIndexTypeData[testCaseInstanceId()];
@@ -576,6 +706,46 @@ void FilterTest::exceptAttributesNoAttributeData() {
     CORRADE_COMPARE(filtered.attributeCount(), 0);
 }
 
+void FilterTest::exceptAttributesRvalue() {
+    /* Subset of onlyAttributes() verifying data ownership transfer behavior.
+       All cases of ownership transfer are verified in attributesRvalue(), this
+       only checks that the r-value gets correctly passed through all overloads
+       to keep the vertex data owned and index data not. */
+
+    UnsignedShort indices[5];
+    Containers::Array<char> vertexData{3*sizeof(Vertex)};
+    Containers::StridedArrayView1D<Vertex> vertices = Containers::arrayCast<Vertex>(vertexData);
+
+    Trade::MeshData mesh{MeshPrimitive::TriangleStrip,
+        {}, indices, Trade::MeshIndexData{indices},
+        Utility::move(vertexData), {
+            Trade::MeshAttributeData{Trade::MeshAttribute::Position, vertices.slice(&Vertex::position)},
+            Trade::MeshAttributeData{Trade::MeshAttribute::Tangent, vertices.slice(&Vertex::tangent)},
+            Trade::MeshAttributeData{Trade::MeshAttribute::TextureCoordinates, vertices.slice(&Vertex::textureCoordinates1)},
+            Trade::MeshAttributeData{Trade::MeshAttribute::TextureCoordinates, vertices.slice(&Vertex::textureCoordinates2)},
+            /* Positions again, just under a different name. Should be kept. */
+            Trade::MeshAttributeData{Trade::meshAttributeCustom(0xbaf), vertices.slice(&Vertex::position)},
+        }};
+
+    Trade::MeshData filtered = filterExceptAttributes(Utility::move(mesh), {
+        Trade::MeshAttribute::Position,
+        Trade::MeshAttribute::TextureCoordinates
+    });
+    CORRADE_COMPARE(filtered.primitive(), MeshPrimitive::TriangleStrip);
+
+    CORRADE_VERIFY(filtered.isIndexed());
+    CORRADE_COMPARE(filtered.indexCount(), 5);
+    CORRADE_COMPARE(filtered.indexData().data(), static_cast<void*>(indices));
+    CORRADE_COMPARE(filtered.indexDataFlags(), Trade::DataFlags{});
+
+    CORRADE_COMPARE(filtered.vertexCount(), 3);
+    CORRADE_COMPARE(filtered.vertexData().data(), vertices.data());
+    CORRADE_COMPARE(filtered.vertexDataFlags(), Trade::DataFlag::Owned|Trade::DataFlag::Mutable);
+
+    CORRADE_COMPARE(filtered.attributeCount(), 2);
+    CORRADE_COMPARE(filtered.attributeName(0), Trade::MeshAttribute::Tangent);
+}
+
 #ifdef MAGNUM_BUILD_DEPRECATED
 void FilterTest::exceptAttributeIds() {
     auto&& data = ImplementationSpecificIndexTypeData[testCaseInstanceId()];

src/Magnum/SceneTools/Filter.cpp

@@ -35,11 +35,12 @@
 #include <Corrade/Utility/BitAlgorithms.h>
 
 #include "Magnum/SceneTools/Combine.h"
+#include "Magnum/SceneTools/Copy.h"
 #include "Magnum/Trade/SceneData.h"
 
 namespace Magnum { namespace SceneTools {
 
-Trade::SceneData filterFields(const Trade::SceneData& scene, const Containers::BitArrayView fieldsToKeep) {
+Trade::SceneData filterFields(Trade::SceneData&& scene, const Containers::BitArrayView fieldsToKeep) {
     CORRADE_ASSERT(fieldsToKeep.size() == scene.fieldCount(),
         "SceneTools::filterFields(): expected" << scene.fieldCount() << "bits but got" << fieldsToKeep.size(), (Trade::SceneData{Trade::SceneMappingType::UnsignedInt, 0, nullptr, {}}));
 
@@ -48,36 +49,60 @@ Trade::SceneData filterFields(const Trade::SceneData& scene, const Containers::B
     Containers::Array<Trade::SceneFieldData> filtered{ValueInit, fieldsToKeep.count()};
     Utility::copyMasked(scene.fieldData(), fieldsToKeep, filtered);
 
-    return Trade::SceneData{scene.mappingType(), scene.mappingBound(),
+    if(scene.dataFlags() >= Trade::DataFlag::Owned)
-        {}, scene.data(), Utility::move(filtered)};
+        return Trade::SceneData{scene.mappingType(), scene.mappingBound(),
+            scene.releaseData(), Utility::move(filtered)};
+    else
+        return Trade::SceneData{scene.mappingType(), scene.mappingBound(),
+            {}, scene.data(), Utility::move(filtered)};
 }
 
-Trade::SceneData filterOnlyFields(const Trade::SceneData& scene, const Containers::ArrayView<const Trade::SceneField> fields) {
+Trade::SceneData filterFields(const Trade::SceneData& scene, const Containers::BitArrayView fieldsToKeep) {
+    return filterFields(reference(scene), fieldsToKeep);
+}
+
+Trade::SceneData filterOnlyFields(Trade::SceneData&& scene, const Containers::ArrayView<const Trade::SceneField> fields) {
     Containers::BitArray fieldsToKeep{DirectInit, scene.fieldCount(), false};
     for(const Trade::SceneField field: fields) {
         if(const Containers::Optional<UnsignedInt> fieldId = scene.findFieldId(field))
             fieldsToKeep.set(*fieldId);
     }
-    return filterFields(scene, fieldsToKeep);
+    return filterFields(Utility::move(scene), fieldsToKeep);
+}
+
+Trade::SceneData filterOnlyFields(const Trade::SceneData& scene, const Containers::ArrayView<const Trade::SceneField> fields) {
+    return filterOnlyFields(reference(scene), fields);
 }
 
 Trade::SceneData filterOnlyFields(const Trade::SceneData& scene, std::initializer_list<Trade::SceneField> fields) {
     return filterOnlyFields(scene, Containers::arrayView(fields));
 }
 
-Trade::SceneData filterExceptFields(const Trade::SceneData& scene, const Containers::ArrayView<const Trade::SceneField> fields) {
+Trade::SceneData filterOnlyFields(Trade::SceneData&& scene, std::initializer_list<Trade::SceneField> fields) {
+    return filterOnlyFields(Utility::move(scene), Containers::arrayView(fields));
+}
+
+Trade::SceneData filterExceptFields(Trade::SceneData&& scene, const Containers::ArrayView<const Trade::SceneField> fields) {
     Containers::BitArray fieldsToKeep{DirectInit, scene.fieldCount(), true};
     for(const Trade::SceneField field: fields) {
         if(const Containers::Optional<UnsignedInt> fieldId = scene.findFieldId(field))
             fieldsToKeep.reset(*fieldId);
     }
-    return filterFields(scene, fieldsToKeep);
+    return filterFields(Utility::move(scene), fieldsToKeep);
+}
+
+Trade::SceneData filterExceptFields(const Trade::SceneData& scene, const Containers::ArrayView<const Trade::SceneField> fields) {
+    return filterExceptFields(reference(scene), fields);
 }
 
 Trade::SceneData filterExceptFields(const Trade::SceneData& scene, std::initializer_list<Trade::SceneField> fields) {
     return filterExceptFields(scene, Containers::arrayView(fields));
 }
 
+Trade::SceneData filterExceptFields(Trade::SceneData&& scene, std::initializer_list<Trade::SceneField> fields) {
+    return filterExceptFields(Utility::move(scene), Containers::arrayView(fields));
+}
+
 Trade::SceneData filterFieldEntries(const Trade::SceneData& scene, const Containers::ArrayView<const Containers::Pair<UnsignedInt, Containers::BitArrayView>> entriesToKeep) {
     /* Track unique mapping views (pointer, size, stride) so fields that shared
        a mapping before stay shared after as well -- if they're filtered, they

src/Magnum/SceneTools/Filter.h

@@ -52,6 +52,18 @@ the output to @ref combineFields(const Trade::SceneData&).
 */
 MAGNUM_SCENETOOLS_EXPORT Trade::SceneData filterFields(const Trade::SceneData& scene, Containers::BitArrayView fieldsToKeep);
 
+/**
+@brief Filter a scene to contain only the selected subset of fields
+@m_since_latest
+
+Compared to @ref filterFields(const Trade::SceneData&, Containers::BitArrayView),
+if the @p scene data is owned, this function transfers the data ownership to
+the returned instance instead of returning a non-owning reference. If the data
+is not owned, the two overloads behave the same.
+@see @ref Trade::SceneData::dataFlags()
+*/
+MAGNUM_SCENETOOLS_EXPORT Trade::SceneData filterFields(Trade::SceneData&& scene, Containers::BitArrayView fieldsToKeep);
+
 /**
 @brief Filter a scene to contain only the selected subset of named fields
 @m_since_latest
@@ -73,6 +85,24 @@ MAGNUM_SCENETOOLS_EXPORT Trade::SceneData filterOnlyFields(const Trade::SceneDat
 */
 MAGNUM_SCENETOOLS_EXPORT Trade::SceneData filterOnlyFields(const Trade::SceneData& scene, std::initializer_list<Trade::SceneField> fields);
 
+/**
+@brief Filter a scene to contain only the selected subset of named fields
+@m_since_latest
+
+Compared to @ref filterOnlyFields(const Trade::SceneData&, Containers::ArrayView<const Trade::SceneField>),
+if the @p scene data is owned, this function transfers the data ownership to
+the returned instance instead of returning a non-owning reference. If the data
+is not owned, the two overloads behave the same.
+@see @ref Trade::SceneData::dataFlags()
+*/
+MAGNUM_SCENETOOLS_EXPORT Trade::SceneData filterOnlyFields(Trade::SceneData&& scene, Containers::ArrayView<const Trade::SceneField> fields);
+
+/**
+@overload
+@m_since_latest
+*/
+MAGNUM_SCENETOOLS_EXPORT Trade::SceneData filterOnlyFields(Trade::SceneData&& scene, std::initializer_list<Trade::SceneField> fields);
+
 /**
 @brief Filter a scene to contain everything except the selected subset of named fields
 @m_since_latest
@@ -94,6 +124,24 @@ MAGNUM_SCENETOOLS_EXPORT Trade::SceneData filterExceptFields(const Trade::SceneD
 */
 MAGNUM_SCENETOOLS_EXPORT Trade::SceneData filterExceptFields(const Trade::SceneData& scene, std::initializer_list<Trade::SceneField> fields);
 
+/**
+@brief Filter a scene to contain everything except the selected subset of named fields
+@m_since_latest
+
+Compared to @ref filterExceptFields(const Trade::SceneData&, Containers::ArrayView<const Trade::SceneField>),
+if the @p scene data is owned, this function transfers the data ownership to
+the returned instance instead of returning a non-owning reference. If the data
+is not owned, the two overloads behave the same.
+@see @ref Trade::SceneData::dataFlags()
+*/
+MAGNUM_SCENETOOLS_EXPORT Trade::SceneData filterExceptFields(Trade::SceneData&& scene, Containers::ArrayView<const Trade::SceneField> fields);
+
+/**
+@overload
+@m_since_latest
+*/
+MAGNUM_SCENETOOLS_EXPORT Trade::SceneData filterExceptFields(Trade::SceneData&& scene, std::initializer_list<Trade::SceneField> fields);
+
 /**
 @brief Filter individual entries of fields in a scene
 @m_since_latest

src/Magnum/SceneTools/Test/FilterTest.cpp

@@ -26,6 +26,7 @@
 
 #include <sstream>
 #include <Corrade/Containers/BitArray.h>
+#include <Corrade/Containers/Optional.h>
 #include <Corrade/Containers/Pair.h>
 #include <Corrade/Containers/StridedBitArrayView.h>
 #include <Corrade/TestSuite/Tester.h>
@@ -43,13 +44,16 @@ struct FilterTest: TestSuite::Tester {
     explicit FilterTest();
 
     void fields();
+    void fieldsRvalue();
     void fieldsWrongBitCount();
 
     void onlyFields();
     void onlyFieldsNoFieldData();
+    void onlyFieldsRvalue();
 
     void exceptFields();
     void exceptFieldsNoFieldData();
+    void exceptFieldsRvalue();
 
     void fieldEntries();
     void fieldEntriesFieldNotFound();
@@ -70,6 +74,20 @@ struct FilterTest: TestSuite::Tester {
 
 using namespace Math::Literals;
 
+const struct {
+    const char* name;
+    Trade::DataFlags dataFlags, expectedDataFlags;
+} FieldsRvalueData[]{
+    /* The Global or ExternallyOwned flags are not preserved, because
+       reference() doesn't preserve them either */
+    {"not owned",
+        Trade::DataFlag::Global|Trade::DataFlag::ExternallyOwned,
+        {}},
+    {"owned",
+        Trade::DataFlag::Owned,
+        Trade::DataFlag::Owned|Trade::DataFlag::Mutable}
+};
+
 const struct {
     const char* name;
     bool byName;
@@ -79,14 +97,20 @@ const struct {
 };
 
 FilterTest::FilterTest() {
-    addTests({&FilterTest::fields,
+    addTests({&FilterTest::fields});
-              &FilterTest::fieldsWrongBitCount,
+
+    addInstancedTests({&FilterTest::fieldsRvalue},
+        Containers::arraySize(FieldsRvalueData));
+
+    addTests({&FilterTest::fieldsWrongBitCount,
 
               &FilterTest::onlyFields,
               &FilterTest::onlyFieldsNoFieldData,
+              &FilterTest::onlyFieldsRvalue,
 
               &FilterTest::exceptFields,
-              &FilterTest::exceptFieldsNoFieldData});
+              &FilterTest::exceptFieldsNoFieldData,
+              &FilterTest::exceptFieldsRvalue});
 
     addInstancedTests({&FilterTest::fieldEntries},
         Containers::arraySize(FieldEntriesData));
@@ -165,6 +189,62 @@ void FilterTest::fields() {
     CORRADE_VERIFY(!fieldData.deleter());
 }
 
+void FilterTest::fieldsRvalue() {
+    auto&& data = FieldsRvalueData[testCaseInstanceId()];
+    setTestCaseDescription(data.name);
+
+    /* Subset of attributes() verifying data ownership transfer behavior */
+
+    struct Data {
+        UnsignedShort meshMaterialMapping[5];
+        UnsignedByte mesh[5];
+        Byte meshMaterial[5];
+        UnsignedShort lightVisibilityMapping[3];
+        UnsignedInt light[3];
+        bool visible[3];
+    };
+    Containers::Array<char> sceneData{sizeof(Data)};
+    Data& d = *reinterpret_cast<Data*>(sceneData.data());
+    Containers::Array<Trade::SceneFieldData> fields{InPlaceInit, {
+        Trade::SceneFieldData{Trade::SceneField::Mesh,
+            Containers::arrayView(d.meshMaterialMapping),
+            Containers::arrayView(d.mesh)},
+        Trade::SceneFieldData{Trade::SceneField::MeshMaterial,
+            Containers::arrayView(d.meshMaterialMapping),
+            Containers::arrayView(d.meshMaterial)},
+        Trade::SceneFieldData{Trade::SceneField::Light,
+            Containers::arrayView(d.lightVisibilityMapping),
+            Containers::arrayView(d.light)},
+        Trade::SceneFieldData{Trade::sceneFieldCustom(15),
+            Containers::arrayView(d.lightVisibilityMapping),
+            Containers::stridedArrayView(d.visible).sliceBit(0)},
+    }};
+
+    Containers::Optional<Trade::SceneData> scene;
+    if(data.dataFlags >= Trade::DataFlag::Owned)
+        scene = Trade::SceneData{Trade::SceneMappingType::UnsignedShort, 76, Utility::move(sceneData), Utility::move(fields)};
+    else
+        scene = Trade::SceneData{Trade::SceneMappingType::UnsignedShort, 76, data.dataFlags, sceneData, Utility::move(fields)};
+
+    Containers::BitArray attributesToKeep{ValueInit, scene->fieldCount()};
+    attributesToKeep.set(0);
+    attributesToKeep.set(1);
+    attributesToKeep.set(3);
+
+    /* The data ownership should be transferred if possible */
+    Trade::SceneData filtered = filterFields(Utility::move(*scene), attributesToKeep);
+    CORRADE_COMPARE(filtered.mappingType(), Trade::SceneMappingType::UnsignedShort);
+    CORRADE_COMPARE(filtered.mappingBound(), 76);
+    CORRADE_COMPARE(filtered.data().data(), static_cast<const void*>(&d));
+    CORRADE_COMPARE(filtered.dataFlags(), data.expectedDataFlags);
+
+    /* Just checking that the fields get actually filtered instead of being
+       passed through verbatim, the actual verification is done in fields()
+       above */
+    CORRADE_COMPARE(filtered.fieldCount(), 3);
+    CORRADE_COMPARE(filtered.fieldName(0), Trade::SceneField::Mesh);
+}
+
 void FilterTest::fieldsWrongBitCount() {
     CORRADE_SKIP_IF_NO_ASSERT();
 
@@ -236,8 +316,9 @@ void FilterTest::onlyFields() {
 void FilterTest::onlyFieldsNoFieldData() {
     /* Just to verify it doesn't crash */
 
-    Trade::SceneData filtered = filterOnlyFields(Trade::SceneData{Trade::SceneMappingType::UnsignedShort, 331, nullptr, {}}, {
+    Trade::SceneData scene{Trade::SceneMappingType::UnsignedShort, 331, nullptr, {}};
-        Trade::SceneField::MeshMaterial,
+    Trade::SceneData filtered = filterOnlyFields(scene, {
+        Trade::SceneField::MeshMaterial
     });
     CORRADE_COMPARE(filtered.mappingType(), Trade::SceneMappingType::UnsignedShort);
     CORRADE_COMPARE(filtered.mappingBound(), 331);
@@ -245,6 +326,47 @@ void FilterTest::onlyFieldsNoFieldData() {
     CORRADE_COMPARE(filtered.dataFlags(), Trade::DataFlags{});
 }
 
+void FilterTest::onlyFieldsRvalue() {
+    /* Subset of onlyFields() verifying data ownership transfer behavior. All
+       cases of ownership transfer are verified in fieldsRvalue(), this only
+       checks that the r-value gets correctly passed through all overloads to
+       keep the data owned. */
+
+    struct Data {
+        UnsignedByte meshMaterialMapping[5];
+        UnsignedByte mesh[5];
+        Byte meshMaterial[5];
+        UnsignedByte lightMapping[3];
+        UnsignedInt light[3];
+    };
+    Containers::Array<char> data{sizeof(Data)};
+    Data& d = *reinterpret_cast<Data*>(data.data());
+
+    Trade::SceneData scene{Trade::SceneMappingType::UnsignedByte, 133, Utility::move(data), {
+        Trade::SceneFieldData{Trade::SceneField::Mesh,
+            Containers::arrayView(d.meshMaterialMapping),
+            Containers::arrayView(d.mesh)},
+        Trade::SceneFieldData{Trade::SceneField::MeshMaterial,
+            Containers::arrayView(d.meshMaterialMapping),
+            Containers::arrayView(d.meshMaterial)},
+        Trade::SceneFieldData{Trade::SceneField::Light,
+            Containers::arrayView(d.lightMapping),
+            Containers::arrayView(d.light)},
+    }};
+
+    Trade::SceneData filtered = filterOnlyFields(Utility::move(scene), {
+        Trade::SceneField::Light,
+        Trade::SceneField::MeshMaterial
+    });
+    CORRADE_COMPARE(filtered.mappingType(), Trade::SceneMappingType::UnsignedByte);
+    CORRADE_COMPARE(filtered.mappingBound(), 133);
+    CORRADE_COMPARE(filtered.data().data(), static_cast<const void*>(&d));
+    CORRADE_COMPARE(filtered.dataFlags(), Trade::DataFlag::Owned|Trade::DataFlag::Mutable);
+
+    CORRADE_COMPARE(filtered.fieldCount(), 2);
+    CORRADE_COMPARE(filtered.fieldName(0), Trade::SceneField::MeshMaterial);
+}
+
 void FilterTest::exceptFields() {
     const struct {
         UnsignedLong meshMaterialMapping[5];
@@ -302,7 +424,8 @@ void FilterTest::exceptFields() {
 void FilterTest::exceptFieldsNoFieldData() {
     /* Just to verify it doesn't crash */
 
-    Trade::SceneData filtered = filterOnlyFields(Trade::SceneData{Trade::SceneMappingType::UnsignedShort, 331, nullptr, {}}, {
+    Trade::SceneData scene{Trade::SceneMappingType::UnsignedShort, 331, nullptr, {}};
+    Trade::SceneData filtered = filterOnlyFields(scene, {
         Trade::SceneField::MeshMaterial,
     });
     CORRADE_COMPARE(filtered.mappingType(), Trade::SceneMappingType::UnsignedShort);
@@ -311,6 +434,51 @@ void FilterTest::exceptFieldsNoFieldData() {
     CORRADE_COMPARE(filtered.dataFlags(), Trade::DataFlags{});
 }
 
+void FilterTest::exceptFieldsRvalue() {
+    /* Subset of exceptFields() verifying data ownership transfer behavior. All
+       cases of ownership transfer are verified in fieldsRvalue(), this only
+       checks that the r-value gets correctly passed through all overloads to
+       keep the data owned. */
+
+    struct Data {
+        UnsignedLong meshMaterialMapping[5];
+        UnsignedByte mesh[5];
+        Byte meshMaterial[5];
+        UnsignedLong lightVisibilityMapping[3];
+        UnsignedInt light[3];
+        bool visible[3];
+    };
+    Containers::Array<char> data{sizeof(Data)};
+    Data& d = *reinterpret_cast<Data*>(data.data());
+
+    Trade::SceneData scene{Trade::SceneMappingType::UnsignedLong, 12, Utility::move(data), {
+        Trade::SceneFieldData{Trade::SceneField::Mesh,
+            Containers::arrayView(d.meshMaterialMapping),
+            Containers::arrayView(d.mesh)},
+        Trade::SceneFieldData{Trade::SceneField::MeshMaterial,
+            Containers::arrayView(d.meshMaterialMapping),
+            Containers::arrayView(d.meshMaterial)},
+        Trade::SceneFieldData{Trade::SceneField::Light,
+            Containers::arrayView(d.lightVisibilityMapping),
+            Containers::arrayView(d.light)},
+        Trade::SceneFieldData{Trade::sceneFieldCustom(15),
+            Containers::arrayView(d.lightVisibilityMapping),
+            Containers::stridedArrayView(d.visible).sliceBit(0)},
+    }};
+
+    Trade::SceneData filtered = filterExceptFields(Utility::move(scene), {
+        Trade::SceneField::Light,
+        Trade::SceneField::MeshMaterial
+    });
+    CORRADE_COMPARE(filtered.mappingType(), Trade::SceneMappingType::UnsignedLong);
+    CORRADE_COMPARE(filtered.mappingBound(), 12);
+    CORRADE_COMPARE(filtered.data().data(), static_cast<const void*>(&d));
+    CORRADE_COMPARE(filtered.dataFlags(), Trade::DataFlag::Owned|Trade::DataFlag::Mutable);
+
+    CORRADE_COMPARE(filtered.fieldCount(), 2);
+    CORRADE_COMPARE(filtered.fieldName(0), Trade::SceneField::Mesh);
+}
+
 void FilterTest::fieldEntries() {
     auto&& data = FieldEntriesData[testCaseInstanceId()];
     setTestCaseDescription(data.name);