Merge remote-tracking branch 'upstream/master' into test_basis_rebase

# Conflicts:
#	src/MagnumPlugins/BasisImporter/BasisImporter.cpp
#	src/MagnumPlugins/BasisImporter/Test/BasisImporterTest.cpp

src/MagnumPlugins/AssimpImporter/AssimpImporter.cpp

@@ -386,7 +386,7 @@ Containers::StringView materialPropertyString(const aiMaterialProperty& property
 
 }
 
-void AssimpImporter::doOpenData(const Containers::ArrayView<const char> data) {
+void AssimpImporter::doOpenData(Containers::Array<char>&& data, DataFlags) {
     /* If we already have the file, we got delegated from doOpenFile() or
        doOpenState(). If we got called from doOpenState(), we don't even have
        the _importer. No need to create it. */
@@ -593,7 +593,7 @@ void AssimpImporter::doOpenState(const void* state, const std::string& filePath)
     _f->scene = static_cast<const aiScene*>(state);
     _f->filePath = filePath;
 
-    doOpenData({});
+    doOpenData({}, {});
 }
 
 void AssimpImporter::doOpenFile(const std::string& filename) {
@@ -608,7 +608,7 @@ void AssimpImporter::doOpenFile(const std::string& filename) {
         return;
     }
 
-    doOpenData({});
+    doOpenData({}, {});
 }
 
 void AssimpImporter::doClose() {
@@ -1786,4 +1786,4 @@ const void* AssimpImporter::doImporterState() const {
 }}
 
 CORRADE_PLUGIN_REGISTER(AssimpImporter, Magnum::Trade::AssimpImporter,
-    "cz.mosra.magnum.Trade.AbstractImporter/0.3.3")
+    "cz.mosra.magnum.Trade.AbstractImporter/0.3.4")

src/MagnumPlugins/AssimpImporter/AssimpImporter.h

@@ -437,7 +437,7 @@ class MAGNUM_ASSIMPIMPORTER_EXPORT AssimpImporter: public AbstractImporter {
 
         MAGNUM_ASSIMPIMPORTER_LOCAL void doSetFileCallback(Containers::Optional<Containers::ArrayView<const char>>(*callback)(const std::string&, InputFileCallbackPolicy, void*), void* userData) override;
 
-        MAGNUM_ASSIMPIMPORTER_LOCAL void doOpenData(Containers::ArrayView<const char> data) override;
+        MAGNUM_ASSIMPIMPORTER_LOCAL void doOpenData(Containers::Array<char>&& data, DataFlags dataFlags) override;
         MAGNUM_ASSIMPIMPORTER_LOCAL void doOpenState(const void* state, const std::string& filePath) override;
         MAGNUM_ASSIMPIMPORTER_LOCAL void doOpenFile(const std::string& filename) override;
         MAGNUM_ASSIMPIMPORTER_LOCAL void doClose() override;

src/MagnumPlugins/AssimpImporter/Test/AssimpImporterTest.cpp

@@ -157,6 +157,9 @@ struct AssimpImporterTest: TestSuite::Tester {
     void fileCallbackImage();
     void fileCallbackImageNotFound();
 
+    void openTwice();
+    void importTwice();
+
     /* Needs to load AnyImageImporter from system-wide location */
     PluginManager::Manager<AbstractImporter> _manager;
 };
@@ -286,7 +289,10 @@ AssimpImporterTest::AssimpImporterTest() {
               &AssimpImporterTest::fileCallbackEmptyFile,
               &AssimpImporterTest::fileCallbackReset,
               &AssimpImporterTest::fileCallbackImage,
-              &AssimpImporterTest::fileCallbackImageNotFound});
+              &AssimpImporterTest::fileCallbackImageNotFound,
+
+              &AssimpImporterTest::openTwice,
+              &AssimpImporterTest::importTwice});
 
     /* Load the plugin directly from the build tree. Otherwise it's static and
        already loaded. It also pulls in the AnyImageImporter dependency. Reset
@@ -3114,6 +3120,37 @@ void AssimpImporterTest::fileCallbackImageNotFound() {
     CORRADE_COMPARE(out.str(), "Trade::AbstractImporter::openFile(): cannot open file diffuse_texture.png\n");
 }
 
+void AssimpImporterTest::openTwice() {
+    Containers::Pointer<AbstractImporter> importer = _manager.instantiate("AssimpImporter");
+
+    CORRADE_VERIFY(importer->openFile(Utility::Directory::join(ASSIMPIMPORTER_TEST_DIR, "scene.dae")));
+    CORRADE_VERIFY(importer->openFile(Utility::Directory::join(ASSIMPIMPORTER_TEST_DIR, "scene.dae")));
+
+    /* Shouldn't crash, leak or anything */
+}
+
+void AssimpImporterTest::importTwice() {
+    Containers::Pointer<AbstractImporter> importer = _manager.instantiate("AssimpImporter");
+    CORRADE_VERIFY(importer->openFile(Utility::Directory::join(ASSIMPIMPORTER_TEST_DIR, "camera.dae")));
+    CORRADE_COMPARE(importer->cameraCount(), 1);
+
+    /* Verify that everything is working the same way on second use. It's only
+       testing a single data type, but better than nothing at all. */
+    {
+        Containers::Optional<CameraData> camera = importer->camera(0);
+        CORRADE_VERIFY(camera);
+        CORRADE_COMPARE(camera->fov(), 49.13434_degf);
+        CORRADE_COMPARE(camera->near(), 0.123f);
+        CORRADE_COMPARE(camera->far(), 123.0f);
+    } {
+        Containers::Optional<CameraData> camera = importer->camera(0);
+        CORRADE_VERIFY(camera);
+        CORRADE_COMPARE(camera->fov(), 49.13434_degf);
+        CORRADE_COMPARE(camera->near(), 0.123f);
+        CORRADE_COMPARE(camera->far(), 123.0f);
+    }
+}
+
 }}}}
 
 CORRADE_TEST_MAIN(Magnum::Trade::Test::AssimpImporterTest)

src/MagnumPlugins/BasisImporter/BasisImporter.cpp

@@ -206,7 +206,7 @@ void BasisImporter::doClose() {
     _state->in = nullptr;
 }
 
-void BasisImporter::doOpenData(const Containers::ArrayView<const char> data) {
+void BasisImporter::doOpenData(Containers::Array<char>&& data, DataFlags dataFlags) {
     /* Because here we're copying the data and using the _in to check if file
        is opened, having them nullptr would mean openData() would fail without
        any error message. It's not possible to do this check on the importer
@@ -226,13 +226,18 @@ void BasisImporter::doOpenData(const Containers::ArrayView<const char> data) {
     const bool isKTX2 = data.size() >= sizeof(KtxFileIdentifier) &&
         std::memcmp(data.begin(), KtxFileIdentifier, sizeof(KtxFileIdentifier)) == 0;
 
-    /* Keep a copy of the data. We have to do this first because transcoders
-       may hold on to the pointer we pass into init()/start_transcoding().
-       While basis_transcoder currently doesn't keep the pointer around, it
-       might in the future and ktx2_transcoder already does. */
+    /* Keep the original data, take over the existing array or copy the data if
+       we can't. We have to do this first because transcoders may hold on to
+       the pointer we pass into init()/start_transcoding(). While
+       basis_transcoder currently doesn't keep the pointer around, it might in
+       the future and ktx2_transcoder already does. */
     Containers::Pointer<State> state{InPlaceInit};
-    state->in = Containers::Array<char>{NoInit, data.size()};
-    Utility::copy(data, state->in);
+    if(dataFlags & (DataFlag::Owned|DataFlag::ExternallyOwned)) {
+        state->in = std::move(data);
+    } else {
+        state->in = Containers::Array<char>{data.size()};
+        Utility::copy(data, state->in);
+    }
 
     if(isKTX2) {
         #if !BASISD_SUPPORT_KTX2
@@ -644,4 +649,4 @@ BasisImporter::TargetFormat BasisImporter::targetFormat() const {
 }}
 
 CORRADE_PLUGIN_REGISTER(BasisImporter, Magnum::Trade::BasisImporter,
-    "cz.mosra.magnum.Trade.AbstractImporter/0.3.3")
+    "cz.mosra.magnum.Trade.AbstractImporter/0.3.4")

src/MagnumPlugins/BasisImporter/BasisImporter.h

@@ -372,7 +372,7 @@ class MAGNUM_BASISIMPORTER_EXPORT BasisImporter: public AbstractImporter {
         MAGNUM_BASISIMPORTER_LOCAL ImporterFeatures doFeatures() const override;
         MAGNUM_BASISIMPORTER_LOCAL bool doIsOpened() const override;
         MAGNUM_BASISIMPORTER_LOCAL void doClose() override;
-        MAGNUM_BASISIMPORTER_LOCAL void doOpenData(Containers::ArrayView<const char> data) override;
+        MAGNUM_BASISIMPORTER_LOCAL void doOpenData(Containers::Array<char>&& data, DataFlags dataFlags) override;
 
         MAGNUM_BASISIMPORTER_LOCAL UnsignedInt doImage2DCount() const override;
         MAGNUM_BASISIMPORTER_LOCAL UnsignedInt doImage2DLevelCount(UnsignedInt id) override;

src/MagnumPlugins/BasisImporter/Test/BasisImporterTest.cpp

@@ -28,6 +28,7 @@
 #include <Corrade/Containers/Optional.h>
 #include <Corrade/Containers/StridedArrayView.h>
 #include <Corrade/TestSuite/Tester.h>
+#include <Corrade/Utility/Algorithms.h>
 #include <Corrade/Utility/ConfigurationGroup.h>
 #include <Corrade/Utility/DebugStl.h>
 #include <Corrade/Utility/Directory.h>
@@ -84,6 +85,7 @@ struct BasisImporterTest: TestSuite::Tester {
     void videoSeeking();
     void videoVerbose();
 
+    void openMemory();
     void openSameTwice();
     void openDifferent();
     void importMultipleFormats();
@@ -203,6 +205,22 @@ const struct {
     {"KTX2 UASTC", "rgba-video-uastc.ktx2"}
 };
 
+/* Shared among all plugins that implement data copying optimizations */
+const struct {
+    const char* name;
+    bool(*open)(AbstractImporter&, Containers::ArrayView<const void>);
+} OpenMemoryData[]{
+    {"data", [](AbstractImporter& importer, Containers::ArrayView<const void> data) {
+        /* Copy to ensure the original memory isn't referenced */
+        Containers::Array<char> copy{NoInit, data.size()};
+        Utility::copy(Containers::arrayCast<const char>(data), copy);
+        return importer.openData(copy);
+    }},
+    {"memory", [](AbstractImporter& importer, Containers::ArrayView<const void> data) {
+        return importer.openMemory(data);
+    }},
+};
+
 BasisImporterTest::BasisImporterTest() {
     addTests({&BasisImporterTest::empty});
 
@@ -250,9 +268,12 @@ BasisImporterTest::BasisImporterTest() {
 addInstancedTests({&BasisImporterTest::videoSeeking},
                       Containers::arraySize(VideoSeekingData));
 
-    addTests({&BasisImporterTest::videoVerbose,
+    addTests({&BasisImporterTest::videoVerbose});
+
+    addInstancedTests({&BasisImporterTest::openMemory},
+        Containers::arraySize(OpenMemoryData));
 
-              &BasisImporterTest::openSameTwice,
+    addTests({&BasisImporterTest::openSameTwice,
               &BasisImporterTest::openDifferent,
               &BasisImporterTest::importMultipleFormats});
 
@@ -1150,6 +1171,39 @@ void BasisImporterTest::videoVerbose() {
     CORRADE_COMPARE(out.str(), "Trade::BasisImporter::openData(): file contains video frames, images must be transcoded sequentially\n");
 }
 
+void BasisImporterTest::openMemory() {
+    /* Same as rgbaUncompressed() except that it uses openData() & openMemory()
+       instead of openFile() to test data copying on import */
+
+    auto&& data = OpenMemoryData[testCaseInstanceId()];
+    setTestCaseDescription(data.name);
+
+    Containers::Pointer<AbstractImporter> importer = _manager.instantiate("BasisImporterRGBA8");
+    CORRADE_VERIFY(importer);
+    CORRADE_COMPARE(importer->configuration().value<std::string>("format"),
+        "RGBA8");
+    Containers::Array<char> memory = Utility::Directory::read(Utility::Directory::join(BASISIMPORTER_TEST_DIR,
+        "rgba.basis"));
+    CORRADE_VERIFY(data.open(*importer, memory));
+    CORRADE_COMPARE(importer->image2DCount(), 1);
+
+    Containers::Optional<Trade::ImageData2D> image = importer->image2D(0);
+    CORRADE_VERIFY(image);
+    CORRADE_VERIFY(!image->isCompressed());
+    CORRADE_COMPARE(image->format(), PixelFormat::RGBA8Unorm);
+    CORRADE_COMPARE(image->size(), (Vector2i{63, 27}));
+
+    if(_manager.loadState("AnyImageImporter") == PluginManager::LoadState::NotFound)
+        CORRADE_SKIP("AnyImageImporter plugin not found, cannot test contents");
+    if(_manager.loadState("PngImporter") == PluginManager::LoadState::NotFound)
+        CORRADE_SKIP("PngImporter plugin not found, cannot test contents");
+
+    CORRADE_COMPARE_WITH(image->pixels<Color4ub>(),
+        Utility::Directory::join(BASISIMPORTER_TEST_DIR, "rgba-63x27.png"),
+        /* There are moderately significant compression artifacts */
+        (DebugTools::CompareImageToFile{_manager, 78.3f, 8.31f}));
+}
+
 void BasisImporterTest::openSameTwice() {
     Containers::Pointer<AbstractImporter> importer = _manager.instantiate("BasisImporterEtc2RGBA");
     CORRADE_VERIFY(importer->openFile(

src/MagnumPlugins/CgltfImporter/CgltfImporter.cpp

@@ -517,14 +517,19 @@ void CgltfImporter::doOpenFile(const std::string& filename) {
     AbstractImporter::doOpenFile(filename);
 }
 
-void CgltfImporter::doOpenData(const Containers::ArrayView<const char> data) {
+void CgltfImporter::doOpenData(Containers::Array<char>&& data, const DataFlags dataFlags) {
     if(!_d) _d.reset(new Document);
 
-    /* Copy file content. We need to keep the data around for .glb binary blobs
-       and extension data which cgltf stores as pointers into the original
-       memory passed to cgltf_parse. */
-    _d->fileData = Containers::Array<char>{data.size()};
-    Utility::copy(data, _d->fileData);
+    /* Copy file content. Take over the existing array or copy the data if we
+       can't. We need to keep the data around for .glb binary blobs and
+       extension data which cgltf stores as pointers into the original memory
+       passed to cgltf_parse. */
+    if(dataFlags & (DataFlag::Owned|DataFlag::ExternallyOwned)) {
+        _d->fileData = std::move(data);
+    } else {
+        _d->fileData = Containers::Array<char>{data.size()};
+        Utility::copy(data, _d->fileData);
+    }
 
     /* Auto-detect glb/gltf */
     _d->options.type = cgltf_file_type::cgltf_file_type_invalid;
@@ -901,7 +906,7 @@ Containers::Optional<AnimationData> CgltfImporter::doAnimation(UnsignedInt id) {
      *      postprocess them and can't just use the memory directly.
      */
     Containers::Array<char> data{dataSize};
-    for(const std::pair<const cgltf_accessor*, SamplerData>& view: samplerData) {
+    for(const std::pair<const cgltf_accessor* const, SamplerData>& view: samplerData) {
         Containers::StridedArrayView2D<const char> src = view.second.src;
         Containers::StridedArrayView2D<char> dst{data.suffix(view.second.outputOffset),
             src.size()};
@@ -2648,4 +2653,4 @@ Containers::Optional<ImageData2D> CgltfImporter::doImage2D(const UnsignedInt id,
 }}
 
 CORRADE_PLUGIN_REGISTER(CgltfImporter, Magnum::Trade::CgltfImporter,
-    "cz.mosra.magnum.Trade.AbstractImporter/0.3.3")
+    "cz.mosra.magnum.Trade.AbstractImporter/0.3.4")

src/MagnumPlugins/CgltfImporter/CgltfImporter.h

@@ -407,7 +407,7 @@ class MAGNUM_CGLTFIMPORTER_EXPORT CgltfImporter: public AbstractImporter {
 
         MAGNUM_CGLTFIMPORTER_LOCAL bool doIsOpened() const override;
 
-        MAGNUM_CGLTFIMPORTER_LOCAL void doOpenData(Containers::ArrayView<const char> data) override;
+        MAGNUM_CGLTFIMPORTER_LOCAL void doOpenData(Containers::Array<char>&& data, DataFlags dataFlags) override;
         MAGNUM_CGLTFIMPORTER_LOCAL void doOpenFile(const std::string& filename) override;
         MAGNUM_CGLTFIMPORTER_LOCAL void doClose() override;