From 32caf4c102a2502888a6dc1c244a72a22aabbe7c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Wed, 20 Oct 2021 19:06:25 +0200 Subject: [PATCH 1/9] CgltfImporter: fix a compiler warning. --- src/MagnumPlugins/CgltfImporter/CgltfImporter.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/MagnumPlugins/CgltfImporter/CgltfImporter.cpp b/src/MagnumPlugins/CgltfImporter/CgltfImporter.cpp index ff94f6aea..715fe0044 100644 --- a/src/MagnumPlugins/CgltfImporter/CgltfImporter.cpp +++ b/src/MagnumPlugins/CgltfImporter/CgltfImporter.cpp @@ -901,7 +901,7 @@ Containers::Optional CgltfImporter::doAnimation(UnsignedInt id) { * postprocess them and can't just use the memory directly. */ Containers::Array data{dataSize}; - for(const std::pair& view: samplerData) { + for(const std::pair& view: samplerData) { Containers::StridedArrayView2D src = view.second.src; Containers::StridedArrayView2D dst{data.suffix(view.second.outputOffset), src.size()}; From 5c16c8dd0229ed2b82102b01b2c3f01b965b7ffb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Wed, 20 Oct 2021 19:06:40 +0200 Subject: [PATCH 2/9] CgltfImporter: GCC said this test was unused. Thanks, GCC! --- src/MagnumPlugins/CgltfImporter/Test/CgltfImporterTest.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/MagnumPlugins/CgltfImporter/Test/CgltfImporterTest.cpp b/src/MagnumPlugins/CgltfImporter/Test/CgltfImporterTest.cpp index 243819017..c3af2e905 100644 --- a/src/MagnumPlugins/CgltfImporter/Test/CgltfImporterTest.cpp +++ b/src/MagnumPlugins/CgltfImporter/Test/CgltfImporterTest.cpp @@ -764,6 +764,9 @@ CgltfImporterTest::CgltfImporterTest() { addInstancedTests({&CgltfImporterTest::texture}, Containers::arraySize(SingleFileData)); + addInstancedTests({&CgltfImporterTest::textureOutOfBounds}, + Containers::arraySize(TextureOutOfBoundsData)); + addInstancedTests({&CgltfImporterTest::textureInvalid}, Containers::arraySize(TextureInvalidData)); From f28267f51e52d9e111d3291a529908599b450887 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Wed, 20 Oct 2021 23:29:58 +0200 Subject: [PATCH 3/9] DdsImporter: no need to put everything in a resource. Leaving the DXT10 files there as there's many and I remember there were some issues with Emscripten command line length when bundling these. Or something. --- .../DdsImporter/Test/CMakeLists.txt | 16 ++++-- .../DdsImporter/Test/DdsImporterTest.cpp | 50 +++++-------------- .../DdsImporter/Test/resources.conf | 33 ------------ 3 files changed, 25 insertions(+), 74 deletions(-) delete mode 100644 src/MagnumPlugins/DdsImporter/Test/resources.conf diff --git a/src/MagnumPlugins/DdsImporter/Test/CMakeLists.txt b/src/MagnumPlugins/DdsImporter/Test/CMakeLists.txt index 56d93b076..07f690fe7 100644 --- a/src/MagnumPlugins/DdsImporter/Test/CMakeLists.txt +++ b/src/MagnumPlugins/DdsImporter/Test/CMakeLists.txt @@ -44,16 +44,24 @@ configure_file(${CMAKE_CURRENT_SOURCE_DIR}/configure.h.cmake file(GENERATE OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/$/configure.h INPUT ${CMAKE_CURRENT_BINARY_DIR}/configure.h.in) -corrade_add_resource(DDS_TEST_FILES_RESOURCE - ${CMAKE_CURRENT_SOURCE_DIR}/resources.conf) corrade_add_resource(DXT10_TEST_FILES_RESOURCE ${CMAKE_CURRENT_SOURCE_DIR}/Dxt10TestFiles/resources.conf) corrade_add_test(DdsImporterTest DdsImporterTest.cpp - ${DDS_TEST_FILES_RESOURCE} ${DXT10_TEST_FILES_RESOURCE} - LIBRARIES Magnum::Trade) + LIBRARIES Magnum::Trade + FILES + rgb_uncompressed.dds + rgb_uncompressed_mips.dds + rgb_uncompressed_volume.dds + rgba_dxt1.dds + rgba_dxt3.dds + rgba_dxt5.dds + too_short_dxt10.dds + unknown_compression.dds + unknown_format.dds + wrong_signature.dds) target_include_directories(DdsImporterTest PRIVATE ${CMAKE_CURRENT_BINARY_DIR}/$) if(MAGNUM_DDSIMPORTER_BUILD_STATIC) target_link_libraries(DdsImporterTest PRIVATE DdsImporter) diff --git a/src/MagnumPlugins/DdsImporter/Test/DdsImporterTest.cpp b/src/MagnumPlugins/DdsImporter/Test/DdsImporterTest.cpp index d0b8e6962..738271d60 100644 --- a/src/MagnumPlugins/DdsImporter/Test/DdsImporterTest.cpp +++ b/src/MagnumPlugins/DdsImporter/Test/DdsImporterTest.cpp @@ -188,10 +188,8 @@ void DdsImporterTest::unknownCompression() { std::ostringstream out; Error redirectError{&out}; - Utility::Resource resource{"DdsTestFiles"}; - Containers::Pointer importer = _manager.instantiate("DdsImporter"); - CORRADE_VERIFY(!importer->openData(resource.getRaw("unknown_compression.dds"))); + CORRADE_VERIFY(!importer->openFile(Utility::Directory::join(DDSIMPORTER_TEST_DIR, "unknown_compression.dds"))); CORRADE_COMPARE(out.str(), "Trade::DdsImporter::openData(): unknown compression DXT4\n"); } @@ -199,10 +197,8 @@ void DdsImporterTest::wrongSignature() { std::ostringstream out; Error redirectError{&out}; - Utility::Resource resource{"DdsTestFiles"}; - Containers::Pointer importer = _manager.instantiate("DdsImporter"); - CORRADE_VERIFY(!importer->openData(resource.getRaw("wrong_signature.dds"))); + CORRADE_VERIFY(!importer->openFile(Utility::Directory::join(DDSIMPORTER_TEST_DIR, "wrong_signature.dds"))); CORRADE_COMPARE(out.str(), "Trade::DdsImporter::openData(): wrong file signature\n"); } @@ -210,10 +206,8 @@ void DdsImporterTest::unknownFormat() { std::ostringstream out; Error redirectError{&out}; - Utility::Resource resource{"DdsTestFiles"}; - Containers::Pointer importer = _manager.instantiate("DdsImporter"); - CORRADE_VERIFY(!importer->openData(resource.getRaw("unknown_format.dds"))); + CORRADE_VERIFY(!importer->openFile(Utility::Directory::join(DDSIMPORTER_TEST_DIR, "unknown_format.dds"))); CORRADE_COMPARE(out.str(), "Trade::DdsImporter::openData(): unknown format\n"); } @@ -221,11 +215,9 @@ void DdsImporterTest::insufficientData() { std::ostringstream out; Error redirectError{&out}; - Utility::Resource resource{"DdsTestFiles"}; - Containers::Pointer importer = _manager.instantiate("DdsImporter"); - auto data = resource.getRaw("rgb_uncompressed.dds"); - CORRADE_VERIFY(!importer->openData(data.prefix(data.size()-1))); + Containers::Array data = Utility::Directory::read(Utility::Directory::join(DDSIMPORTER_TEST_DIR, "rgb_uncompressed.dds")); + CORRADE_VERIFY(!importer->openData(data.except(1))); CORRADE_COMPARE(out.str(), "Trade::DdsImporter::openData(): not enough image data\n"); } @@ -233,11 +225,9 @@ void DdsImporterTest::rgb() { auto&& data = VerboseData[testCaseInstanceId()]; setTestCaseDescription(data.name); - Utility::Resource resource{"DdsTestFiles"}; - Containers::Pointer importer = _manager.instantiate("DdsImporter"); importer->setFlags(data.flags); - CORRADE_VERIFY(importer->openData(resource.getRaw("rgb_uncompressed.dds"))); + CORRADE_VERIFY(importer->openFile(Utility::Directory::join(DDSIMPORTER_TEST_DIR, "rgb_uncompressed.dds"))); CORRADE_COMPARE(importer->image2DCount(), 1); CORRADE_COMPARE(importer->image2DLevelCount(0), 1); CORRADE_COMPARE(importer->image3DCount(), 0); @@ -268,11 +258,9 @@ void DdsImporterTest::rgbWithMips() { auto&& data = VerboseData[testCaseInstanceId()]; setTestCaseDescription(data.name); - Utility::Resource resource{"DdsTestFiles"}; - Containers::Pointer importer = _manager.instantiate("DdsImporter"); importer->setFlags(data.flags); - CORRADE_VERIFY(importer->openData(resource.getRaw("rgb_uncompressed_mips.dds"))); + CORRADE_VERIFY(importer->openFile(Utility::Directory::join(DDSIMPORTER_TEST_DIR, "rgb_uncompressed_mips.dds"))); CORRADE_COMPARE(importer->image2DCount(), 1); CORRADE_COMPARE(importer->image2DLevelCount(0), 2); CORRADE_COMPARE(importer->image3DCount(), 0); @@ -322,11 +310,9 @@ void DdsImporterTest::rgbVolume() { auto&& data = VerboseData[testCaseInstanceId()]; setTestCaseDescription(data.name); - Utility::Resource resource{"DdsTestFiles"}; - Containers::Pointer importer = _manager.instantiate("DdsImporter"); importer->setFlags(data.flags); - CORRADE_VERIFY(importer->openData(resource.getRaw("rgb_uncompressed_volume.dds"))); + CORRADE_VERIFY(importer->openFile(Utility::Directory::join(DDSIMPORTER_TEST_DIR, "rgb_uncompressed_volume.dds"))); CORRADE_COMPARE(importer->image2DCount(), 0); CORRADE_COMPARE(importer->image3DCount(), 1); CORRADE_COMPARE(importer->image3DLevelCount(0), 1); @@ -371,10 +357,8 @@ void DdsImporterTest::rgbVolume() { } void DdsImporterTest::dxt1() { - Utility::Resource resource{"DdsTestFiles"}; - Containers::Pointer importer = _manager.instantiate("DdsImporter"); - CORRADE_VERIFY(importer->openData(resource.getRaw("rgba_dxt1.dds"))); + CORRADE_VERIFY(importer->openFile(Utility::Directory::join(DDSIMPORTER_TEST_DIR, "rgba_dxt1.dds"))); Containers::Optional image = importer->image2D(0); CORRADE_VERIFY(image); @@ -387,10 +371,8 @@ void DdsImporterTest::dxt1() { } void DdsImporterTest::dxt3() { - Utility::Resource resource{"DdsTestFiles"}; - Containers::Pointer importer = _manager.instantiate("DdsImporter"); - CORRADE_VERIFY(importer->openData(resource.getRaw("rgba_dxt3.dds"))); + CORRADE_VERIFY(importer->openFile(Utility::Directory::join(DDSIMPORTER_TEST_DIR, "rgba_dxt3.dds"))); Containers::Optional image = importer->image2D(0); CORRADE_VERIFY(image); @@ -404,10 +386,8 @@ void DdsImporterTest::dxt3() { } void DdsImporterTest::dxt5() { - Utility::Resource resource{"DdsTestFiles"}; - Containers::Pointer importer = _manager.instantiate("DdsImporter"); - CORRADE_VERIFY(importer->openData(resource.getRaw("rgba_dxt5.dds"))); + CORRADE_VERIFY(importer->openFile(Utility::Directory::join(DDSIMPORTER_TEST_DIR, "rgba_dxt5.dds"))); Containers::Optional image = importer->image2D(0); CORRADE_VERIFY(image); @@ -471,13 +451,11 @@ void DdsImporterTest::dxt10Data() { } void DdsImporterTest::dxt10TooShort() { - Utility::Resource resource{"DdsTestFiles"}; - std::ostringstream out; Error redirectError{&out}; Containers::Pointer importer = _manager.instantiate("DdsImporter"); - CORRADE_VERIFY(!importer->openData(resource.getRaw("too_short_dxt10.dds"))); + CORRADE_VERIFY(!importer->openFile(Utility::Directory::join(DDSIMPORTER_TEST_DIR, "too_short_dxt10.dds"))); CORRADE_COMPARE(out.str(), "Trade::DdsImporter::openData(): fourcc was DX10 but file is too short to contain DXT10 header\n"); } @@ -493,10 +471,8 @@ void DdsImporterTest::dxt10UnsupportedFormat() { } void DdsImporterTest::useTwice() { - Utility::Resource resource{"DdsTestFiles"}; - Containers::Pointer importer = _manager.instantiate("DdsImporter"); - CORRADE_VERIFY(importer->openData(resource.getRaw("rgba_dxt5.dds"))); + CORRADE_VERIFY(importer->openFile(Utility::Directory::join(DDSIMPORTER_TEST_DIR, "rgba_dxt5.dds"))); /* Verify that the file is rewinded for second use */ { diff --git a/src/MagnumPlugins/DdsImporter/Test/resources.conf b/src/MagnumPlugins/DdsImporter/Test/resources.conf deleted file mode 100644 index 2a0bd1a28..000000000 --- a/src/MagnumPlugins/DdsImporter/Test/resources.conf +++ /dev/null @@ -1,33 +0,0 @@ -group=DdsTestFiles - - -[file] -filename=rgba_dxt1.dds - -[file] -filename=rgba_dxt3.dds - -[file] -filename=rgba_dxt5.dds - -[file] -filename=rgb_uncompressed.dds - -[file] -filename=rgb_uncompressed_mips.dds - -[file] -filename=rgb_uncompressed_volume.dds - -[file] -filename=too_short_dxt10.dds - -[file] -filename=unknown_compression.dds - -[file] -filename=unknown_format.dds - -[file] -filename=wrong_signature.dds - From 87dbee095c101a26f6902badd98cb05816205549 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Wed, 20 Oct 2021 23:44:16 +0200 Subject: [PATCH 4/9] {Assimp,Dds,Cgltf,TinyGltf}Importer: test double opening & double import. Like with all other plugins. --- .../Test/AssimpImporterTest.cpp | 39 +++++++++++++++++- .../CgltfImporter/Test/CgltfImporterTest.cpp | 41 +++++++++++++++++++ .../DdsImporter/Test/DdsImporterTest.cpp | 17 ++++++-- .../Test/TinyGltfImporterTest.cpp | 41 +++++++++++++++++++ 4 files changed, 134 insertions(+), 4 deletions(-) diff --git a/src/MagnumPlugins/AssimpImporter/Test/AssimpImporterTest.cpp b/src/MagnumPlugins/AssimpImporter/Test/AssimpImporterTest.cpp index ea71bd68c..c51371de9 100644 --- a/src/MagnumPlugins/AssimpImporter/Test/AssimpImporterTest.cpp +++ b/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 _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 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 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 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 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) diff --git a/src/MagnumPlugins/CgltfImporter/Test/CgltfImporterTest.cpp b/src/MagnumPlugins/CgltfImporter/Test/CgltfImporterTest.cpp index c3af2e905..c5c1f84d5 100644 --- a/src/MagnumPlugins/CgltfImporter/Test/CgltfImporterTest.cpp +++ b/src/MagnumPlugins/CgltfImporter/Test/CgltfImporterTest.cpp @@ -188,6 +188,9 @@ struct CgltfImporterTest: TestSuite::Tester { void versionSupported(); void versionUnsupported(); + void openTwice(); + void importTwice(); + /* Needs to load AnyImageImporter from system-wide location */ PluginManager::Manager _manager; }; @@ -815,6 +818,9 @@ CgltfImporterTest::CgltfImporterTest() { addInstancedTests({&CgltfImporterTest::versionUnsupported}, Containers::arraySize(UnsupportedVersionData)); + addTests({&CgltfImporterTest::openTwice, + &CgltfImporterTest::importTwice}); + /* Load the plugin directly from the build tree. Otherwise it's static and already loaded. It also pulls in the AnyImageImporter dependency. Reset the plugin dir after so it doesn't load anything else from the filesystem. */ @@ -4601,6 +4607,41 @@ void CgltfImporterTest::versionUnsupported() { CORRADE_COMPARE(out.str(), Utility::formatString("Trade::CgltfImporter::openData(): {}\n", data.message)); } +void CgltfImporterTest::openTwice() { + Containers::Pointer importer = _manager.instantiate("CgltfImporter"); + + CORRADE_VERIFY(importer->openFile(Utility::Directory::join(TINYGLTFIMPORTER_TEST_DIR, "camera.gltf"))); + CORRADE_VERIFY(importer->openFile(Utility::Directory::join(TINYGLTFIMPORTER_TEST_DIR, "camera.gltf"))); + + /* Shouldn't crash, leak or anything */ +} + +void CgltfImporterTest::importTwice() { + Containers::Pointer importer = _manager.instantiate("CgltfImporter"); + CORRADE_VERIFY(importer->openFile(Utility::Directory::join(TINYGLTFIMPORTER_TEST_DIR, "camera.gltf"))); + CORRADE_COMPARE(importer->cameraCount(), 4); + + /* 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. */ + { + auto cam = importer->camera(0); + CORRADE_VERIFY(cam); + CORRADE_COMPARE(cam->type(), CameraType::Orthographic3D); + CORRADE_COMPARE(cam->size(), (Vector2{4.0f, 3.0f})); + CORRADE_COMPARE(cam->aspectRatio(), 1.333333f); + CORRADE_COMPARE(cam->near(), 0.01f); + CORRADE_COMPARE(cam->far(), 100.0f); + } { + auto cam = importer->camera(0); + CORRADE_VERIFY(cam); + CORRADE_COMPARE(cam->type(), CameraType::Orthographic3D); + CORRADE_COMPARE(cam->size(), (Vector2{4.0f, 3.0f})); + CORRADE_COMPARE(cam->aspectRatio(), 1.333333f); + CORRADE_COMPARE(cam->near(), 0.01f); + CORRADE_COMPARE(cam->far(), 100.0f); + } +} + }}}} CORRADE_TEST_MAIN(Magnum::Trade::Test::CgltfImporterTest) diff --git a/src/MagnumPlugins/DdsImporter/Test/DdsImporterTest.cpp b/src/MagnumPlugins/DdsImporter/Test/DdsImporterTest.cpp index 738271d60..5c36892bf 100644 --- a/src/MagnumPlugins/DdsImporter/Test/DdsImporterTest.cpp +++ b/src/MagnumPlugins/DdsImporter/Test/DdsImporterTest.cpp @@ -62,7 +62,8 @@ struct DdsImporterTest: TestSuite::Tester { void dxt10TooShort(); void dxt10UnsupportedFormat(); - void useTwice(); + void openTwice(); + void importTwice(); /* Explicitly forbid system-wide plugin dependencies */ PluginManager::Manager _manager{"nonexistent"}; @@ -175,7 +176,8 @@ DdsImporterTest::DdsImporterTest() { &DdsImporterTest::dxt10TooShort, &DdsImporterTest::dxt10UnsupportedFormat, - &DdsImporterTest::useTwice}); + &DdsImporterTest::openTwice, + &DdsImporterTest::importTwice}); /* Load the plugin directly from the build tree. Otherwise it's static and already loaded. */ @@ -470,7 +472,16 @@ void DdsImporterTest::dxt10UnsupportedFormat() { CORRADE_COMPARE(out.str(), "Trade::DdsImporter::openData(): unsupported DXGI format 100\n"); } -void DdsImporterTest::useTwice() { +void DdsImporterTest::openTwice() { + Containers::Pointer importer = _manager.instantiate("DdsImporter"); + + CORRADE_VERIFY(importer->openFile(Utility::Directory::join(DDSIMPORTER_TEST_DIR, "rgba_dxt5.dds"))); + CORRADE_VERIFY(importer->openFile(Utility::Directory::join(DDSIMPORTER_TEST_DIR, "rgba_dxt5.dds"))); + + /* Shouldn't crash, leak or anything */ +} + +void DdsImporterTest::importTwice() { Containers::Pointer importer = _manager.instantiate("DdsImporter"); CORRADE_VERIFY(importer->openFile(Utility::Directory::join(DDSIMPORTER_TEST_DIR, "rgba_dxt5.dds"))); diff --git a/src/MagnumPlugins/TinyGltfImporter/Test/TinyGltfImporterTest.cpp b/src/MagnumPlugins/TinyGltfImporter/Test/TinyGltfImporterTest.cpp index 0918b1941..11c6b250d 100644 --- a/src/MagnumPlugins/TinyGltfImporter/Test/TinyGltfImporterTest.cpp +++ b/src/MagnumPlugins/TinyGltfImporter/Test/TinyGltfImporterTest.cpp @@ -174,6 +174,9 @@ struct TinyGltfImporterTest: TestSuite::Tester { void versionSupported(); void versionUnsupported(); + void openTwice(); + void importTwice(); + /* Needs to load AnyImageImporter from system-wide location */ PluginManager::Manager _manager; }; @@ -689,6 +692,9 @@ TinyGltfImporterTest::TinyGltfImporterTest() { addInstancedTests({&TinyGltfImporterTest::versionUnsupported}, Containers::arraySize(UnsupportedVersionData)); + addTests({&TinyGltfImporterTest::openTwice, + &TinyGltfImporterTest::importTwice}); + /* Load the plugin directly from the build tree. Otherwise it's static and already loaded. It also pulls in the AnyImageImporter dependency. Reset the plugin dir after so it doesn't load anything else from the filesystem. */ @@ -4168,6 +4174,41 @@ void TinyGltfImporterTest::versionUnsupported() { CORRADE_COMPARE(out.str(), Utility::formatString("Trade::CgltfImporter::openData(): {}\n", data.message)); } +void TinyGltfImporterTest::openTwice() { + Containers::Pointer importer = _manager.instantiate("TinyGltfImporter"); + + CORRADE_VERIFY(importer->openFile(Utility::Directory::join(TINYGLTFIMPORTER_TEST_DIR, "camera.gltf"))); + CORRADE_VERIFY(importer->openFile(Utility::Directory::join(TINYGLTFIMPORTER_TEST_DIR, "camera.gltf"))); + + /* Shouldn't crash, leak or anything */ +} + +void TinyGltfImporterTest::importTwice() { + Containers::Pointer importer = _manager.instantiate("TinyGltfImporter"); + CORRADE_VERIFY(importer->openFile(Utility::Directory::join(TINYGLTFIMPORTER_TEST_DIR, "camera.gltf"))); + CORRADE_COMPARE(importer->cameraCount(), 4); + + /* 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. */ + { + auto cam = importer->camera(0); + CORRADE_VERIFY(cam); + CORRADE_COMPARE(cam->type(), CameraType::Orthographic3D); + CORRADE_COMPARE(cam->size(), (Vector2{4.0f, 3.0f})); + CORRADE_COMPARE(cam->aspectRatio(), 1.333333f); + CORRADE_COMPARE(cam->near(), 0.01f); + CORRADE_COMPARE(cam->far(), 100.0f); + } { + auto cam = importer->camera(0); + CORRADE_VERIFY(cam); + CORRADE_COMPARE(cam->type(), CameraType::Orthographic3D); + CORRADE_COMPARE(cam->size(), (Vector2{4.0f, 3.0f})); + CORRADE_COMPARE(cam->aspectRatio(), 1.333333f); + CORRADE_COMPARE(cam->near(), 0.01f); + CORRADE_COMPARE(cam->far(), 100.0f); + } +} + }}}} CORRADE_TEST_MAIN(Magnum::Trade::Test::TinyGltfImporterTest) From 152c7ed874d485a4addb411be469a33ee23ea95a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Wed, 20 Oct 2021 23:58:38 +0200 Subject: [PATCH 5/9] Adapt to doOpenData() changes, first trivial cases. These plugins either populate a custom state from the input data and don't need it anymore afterwards, or they ignore the data altogether. Nothing to do here, nothing to test. --- src/MagnumPlugins/AssimpImporter/AssimpImporter.cpp | 8 ++++---- src/MagnumPlugins/AssimpImporter/AssimpImporter.h | 2 +- .../DevIlImageImporter/DevIlImageImporter.cpp | 4 ++-- src/MagnumPlugins/DevIlImageImporter/DevIlImageImporter.h | 2 +- src/MagnumPlugins/OpenGexImporter/OpenGexImporter.cpp | 4 ++-- src/MagnumPlugins/OpenGexImporter/OpenGexImporter.h | 2 +- src/MagnumPlugins/PrimitiveImporter/PrimitiveImporter.cpp | 4 ++-- src/MagnumPlugins/PrimitiveImporter/PrimitiveImporter.h | 2 +- src/MagnumPlugins/TinyGltfImporter/TinyGltfImporter.cpp | 4 ++-- src/MagnumPlugins/TinyGltfImporter/TinyGltfImporter.h | 2 +- 10 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/MagnumPlugins/AssimpImporter/AssimpImporter.cpp b/src/MagnumPlugins/AssimpImporter/AssimpImporter.cpp index 098f553a4..c7bc935bf 100644 --- a/src/MagnumPlugins/AssimpImporter/AssimpImporter.cpp +++ b/src/MagnumPlugins/AssimpImporter/AssimpImporter.cpp @@ -386,7 +386,7 @@ Containers::StringView materialPropertyString(const aiMaterialProperty& property } -void AssimpImporter::doOpenData(const Containers::ArrayView data) { +void AssimpImporter::doOpenData(Containers::Array&& 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(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") diff --git a/src/MagnumPlugins/AssimpImporter/AssimpImporter.h b/src/MagnumPlugins/AssimpImporter/AssimpImporter.h index 49cd72431..d23113c2e 100644 --- a/src/MagnumPlugins/AssimpImporter/AssimpImporter.h +++ b/src/MagnumPlugins/AssimpImporter/AssimpImporter.h @@ -437,7 +437,7 @@ class MAGNUM_ASSIMPIMPORTER_EXPORT AssimpImporter: public AbstractImporter { MAGNUM_ASSIMPIMPORTER_LOCAL void doSetFileCallback(Containers::Optional>(*callback)(const std::string&, InputFileCallbackPolicy, void*), void* userData) override; - MAGNUM_ASSIMPIMPORTER_LOCAL void doOpenData(Containers::ArrayView data) override; + MAGNUM_ASSIMPIMPORTER_LOCAL void doOpenData(Containers::Array&& 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; diff --git a/src/MagnumPlugins/DevIlImageImporter/DevIlImageImporter.cpp b/src/MagnumPlugins/DevIlImageImporter/DevIlImageImporter.cpp index f6dddabe2..8da612c5f 100644 --- a/src/MagnumPlugins/DevIlImageImporter/DevIlImageImporter.cpp +++ b/src/MagnumPlugins/DevIlImageImporter/DevIlImageImporter.cpp @@ -65,7 +65,7 @@ void DevIlImageImporter::doClose() { /* So we can use the shorter if(!ilFoo()) */ static_assert(!IL_FALSE, "IL_FALSE doesn't have a zero value"); -void DevIlImageImporter::doOpenData(const Containers::ArrayView data) { +void DevIlImageImporter::doOpenData(Containers::Array&& data, DataFlags) { UnsignedInt image; ilGenImages(1, &image); ilBindImage(image); @@ -209,4 +209,4 @@ Containers::Optional DevIlImageImporter::doImage2D(UnsignedInt id, }} CORRADE_PLUGIN_REGISTER(DevIlImageImporter, Magnum::Trade::DevIlImageImporter, - "cz.mosra.magnum.Trade.AbstractImporter/0.3.3") + "cz.mosra.magnum.Trade.AbstractImporter/0.3.4") diff --git a/src/MagnumPlugins/DevIlImageImporter/DevIlImageImporter.h b/src/MagnumPlugins/DevIlImageImporter/DevIlImageImporter.h index e01e748d8..fd151dfae 100644 --- a/src/MagnumPlugins/DevIlImageImporter/DevIlImageImporter.h +++ b/src/MagnumPlugins/DevIlImageImporter/DevIlImageImporter.h @@ -241,7 +241,7 @@ class MAGNUM_DEVILIMAGEIMPORTER_EXPORT DevIlImageImporter: public AbstractImport MAGNUM_DEVILIMAGEIMPORTER_LOCAL bool doIsOpened() const override; MAGNUM_DEVILIMAGEIMPORTER_LOCAL void doClose() override; MAGNUM_DEVILIMAGEIMPORTER_LOCAL void doOpenFile(const std::string& filename) override; - MAGNUM_DEVILIMAGEIMPORTER_LOCAL void doOpenData(Containers::ArrayView data) override; + MAGNUM_DEVILIMAGEIMPORTER_LOCAL void doOpenData(Containers::Array&& data, DataFlags dataFlags) override; MAGNUM_DEVILIMAGEIMPORTER_LOCAL UnsignedInt doImage2DCount() const override; MAGNUM_DEVILIMAGEIMPORTER_LOCAL Containers::Optional doImage2D(UnsignedInt id, UnsignedInt level) override; diff --git a/src/MagnumPlugins/OpenGexImporter/OpenGexImporter.cpp b/src/MagnumPlugins/OpenGexImporter/OpenGexImporter.cpp index 184f86229..63cf97a3b 100644 --- a/src/MagnumPlugins/OpenGexImporter/OpenGexImporter.cpp +++ b/src/MagnumPlugins/OpenGexImporter/OpenGexImporter.cpp @@ -127,7 +127,7 @@ void gatherNodes(OpenDdl::Structure node, std::vector& nodes } -void OpenGexImporter::doOpenData(const Containers::ArrayView data) { +void OpenGexImporter::doOpenData(Containers::Array&& data, DataFlags) { Containers::Pointer d{InPlaceInit}; /* Parse the document */ @@ -873,4 +873,4 @@ const void* OpenGexImporter::doImporterState() const { }} CORRADE_PLUGIN_REGISTER(OpenGexImporter, Magnum::Trade::OpenGexImporter, - "cz.mosra.magnum.Trade.AbstractImporter/0.3.3") + "cz.mosra.magnum.Trade.AbstractImporter/0.3.4") diff --git a/src/MagnumPlugins/OpenGexImporter/OpenGexImporter.h b/src/MagnumPlugins/OpenGexImporter/OpenGexImporter.h index 02d24cd26..b2ab1b0af 100644 --- a/src/MagnumPlugins/OpenGexImporter/OpenGexImporter.h +++ b/src/MagnumPlugins/OpenGexImporter/OpenGexImporter.h @@ -236,7 +236,7 @@ class MAGNUM_OPENGEXIMPORTER_EXPORT OpenGexImporter: public AbstractImporter { MAGNUM_OPENGEXIMPORTER_LOCAL ImporterFeatures doFeatures() const override; MAGNUM_OPENGEXIMPORTER_LOCAL bool doIsOpened() const override; - MAGNUM_OPENGEXIMPORTER_LOCAL void doOpenData(Containers::ArrayView data) override; + MAGNUM_OPENGEXIMPORTER_LOCAL void doOpenData(Containers::Array&& data, DataFlags dataFlags) override; MAGNUM_OPENGEXIMPORTER_LOCAL void doOpenFile(const std::string& filename) override; MAGNUM_OPENGEXIMPORTER_LOCAL void doClose() override; diff --git a/src/MagnumPlugins/PrimitiveImporter/PrimitiveImporter.cpp b/src/MagnumPlugins/PrimitiveImporter/PrimitiveImporter.cpp index add054f72..b4b01a999 100644 --- a/src/MagnumPlugins/PrimitiveImporter/PrimitiveImporter.cpp +++ b/src/MagnumPlugins/PrimitiveImporter/PrimitiveImporter.cpp @@ -62,7 +62,7 @@ bool PrimitiveImporter::doIsOpened() const { return _opened; } void PrimitiveImporter::doClose() { _opened = false; } -void PrimitiveImporter::doOpenData(Containers::ArrayView) { +void PrimitiveImporter::doOpenData(Containers::Array&&, DataFlags) { _opened = true; } @@ -574,4 +574,4 @@ Containers::Optional PrimitiveImporter::doMesh(UnsignedInt id, Unsigne }} CORRADE_PLUGIN_REGISTER(PrimitiveImporter, Magnum::Trade::PrimitiveImporter, - "cz.mosra.magnum.Trade.AbstractImporter/0.3.3") + "cz.mosra.magnum.Trade.AbstractImporter/0.3.4") diff --git a/src/MagnumPlugins/PrimitiveImporter/PrimitiveImporter.h b/src/MagnumPlugins/PrimitiveImporter/PrimitiveImporter.h index 11cb7abb2..93cd41180 100644 --- a/src/MagnumPlugins/PrimitiveImporter/PrimitiveImporter.h +++ b/src/MagnumPlugins/PrimitiveImporter/PrimitiveImporter.h @@ -133,7 +133,7 @@ class MAGNUM_PRIMITIVEIMPORTER_EXPORT PrimitiveImporter: public AbstractImporter MAGNUM_PRIMITIVEIMPORTER_LOCAL ImporterFeatures doFeatures() const override; MAGNUM_PRIMITIVEIMPORTER_LOCAL bool doIsOpened() const override; - MAGNUM_PRIMITIVEIMPORTER_LOCAL void doOpenData(Containers::ArrayView data) override; + MAGNUM_PRIMITIVEIMPORTER_LOCAL void doOpenData(Containers::Array&& data, DataFlags dataFlags) override; MAGNUM_PRIMITIVEIMPORTER_LOCAL void doClose() override; MAGNUM_PRIMITIVEIMPORTER_LOCAL Int doDefaultScene() const override; diff --git a/src/MagnumPlugins/TinyGltfImporter/TinyGltfImporter.cpp b/src/MagnumPlugins/TinyGltfImporter/TinyGltfImporter.cpp index 6986bb34f..35d5dba6f 100644 --- a/src/MagnumPlugins/TinyGltfImporter/TinyGltfImporter.cpp +++ b/src/MagnumPlugins/TinyGltfImporter/TinyGltfImporter.cpp @@ -285,7 +285,7 @@ void TinyGltfImporter::doOpenFile(const std::string& filename) { AbstractImporter::doOpenFile(filename); } -void TinyGltfImporter::doOpenData(const Containers::ArrayView data) { +void TinyGltfImporter::doOpenData(Containers::Array&& data, DataFlags) { tinygltf::TinyGLTF loader; std::string err; @@ -2474,4 +2474,4 @@ const void* TinyGltfImporter::doImporterState() const { }} CORRADE_PLUGIN_REGISTER(TinyGltfImporter, Magnum::Trade::TinyGltfImporter, - "cz.mosra.magnum.Trade.AbstractImporter/0.3.3") + "cz.mosra.magnum.Trade.AbstractImporter/0.3.4") diff --git a/src/MagnumPlugins/TinyGltfImporter/TinyGltfImporter.h b/src/MagnumPlugins/TinyGltfImporter/TinyGltfImporter.h index 7bd7c0d99..43afbc309 100644 --- a/src/MagnumPlugins/TinyGltfImporter/TinyGltfImporter.h +++ b/src/MagnumPlugins/TinyGltfImporter/TinyGltfImporter.h @@ -475,7 +475,7 @@ class MAGNUM_TINYGLTFIMPORTER_EXPORT TinyGltfImporter: public AbstractImporter { MAGNUM_TINYGLTFIMPORTER_LOCAL bool doIsOpened() const override; - MAGNUM_TINYGLTFIMPORTER_LOCAL void doOpenData(Containers::ArrayView data) override; + MAGNUM_TINYGLTFIMPORTER_LOCAL void doOpenData(Containers::Array&& data, DataFlags dataFlags) override; MAGNUM_TINYGLTFIMPORTER_LOCAL void doOpenFile(const std::string& filename) override; MAGNUM_TINYGLTFIMPORTER_LOCAL void doClose() override; From e84dadeb9324bfe02317fd28f24ecf10ad0d296f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Thu, 21 Oct 2021 00:28:48 +0200 Subject: [PATCH 6/9] Adapt data-copying plugins to doOpenData() changes. Most of these will now have the openFile() call use as little as half the memory. --- .../BasisImporter/BasisImporter.cpp | 15 ++++-- .../BasisImporter/BasisImporter.h | 2 +- .../BasisImporter/Test/BasisImporterTest.cpp | 54 +++++++++++++++++++ .../CgltfImporter/CgltfImporter.cpp | 19 ++++--- .../CgltfImporter/CgltfImporter.h | 2 +- .../CgltfImporter/Test/CgltfImporterTest.cpp | 42 +++++++++++++++ src/MagnumPlugins/DdsImporter/DdsImporter.cpp | 14 +++-- src/MagnumPlugins/DdsImporter/DdsImporter.h | 2 +- .../DdsImporter/Test/DdsImporterTest.cpp | 46 +++++++++++++++- src/MagnumPlugins/IcoImporter/IcoImporter.cpp | 14 +++-- src/MagnumPlugins/IcoImporter/IcoImporter.h | 2 +- .../IcoImporter/Test/IcoImporterTest.cpp | 44 ++++++++++++++- .../JpegImporter/JpegImporter.cpp | 15 ++++-- src/MagnumPlugins/JpegImporter/JpegImporter.h | 4 +- .../JpegImporter/Test/JpegImporterTest.cpp | 52 +++++++++++++++++- src/MagnumPlugins/KtxImporter/KtxImporter.cpp | 18 ++++--- src/MagnumPlugins/KtxImporter/KtxImporter.h | 2 +- .../KtxImporter/Test/KtxImporterTest.cpp | 54 ++++++++++++++++++- .../OpenExrImporter/OpenExrImporter.cpp | 15 ++++-- .../OpenExrImporter/OpenExrImporter.h | 2 +- .../Test/OpenExrImporterTest.cpp | 45 ++++++++++++++++ src/MagnumPlugins/PngImporter/PngImporter.cpp | 19 ++++--- src/MagnumPlugins/PngImporter/PngImporter.h | 4 +- .../PngImporter/Test/PngImporterTest.cpp | 44 +++++++++++++++ .../StbImageImporter/StbImageImporter.cpp | 13 +++-- .../StbImageImporter/StbImageImporter.h | 2 +- .../Test/StbImageImporterTest.cpp | 47 +++++++++++++++- 27 files changed, 522 insertions(+), 70 deletions(-) diff --git a/src/MagnumPlugins/BasisImporter/BasisImporter.cpp b/src/MagnumPlugins/BasisImporter/BasisImporter.cpp index 191687b9d..c53cdef30 100644 --- a/src/MagnumPlugins/BasisImporter/BasisImporter.cpp +++ b/src/MagnumPlugins/BasisImporter/BasisImporter.cpp @@ -173,7 +173,7 @@ void BasisImporter::doClose() { _state->in = nullptr; } -void BasisImporter::doOpenData(const Containers::ArrayView data) { +void BasisImporter::doOpenData(Containers::Array&& 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 @@ -203,10 +203,15 @@ void BasisImporter::doOpenData(const Containers::ArrayView data) { return; } - /* All good, release the transcoder guard and keep a copy of the data */ + /* All good, release the transcoder guard. Keep the original data, take + over the existing array or copy the data if we can't. */ transcoderGuard.release(); - _state->in = Containers::Array{NoInit, data.size()}; - Utility::copy(data, _state->in); + if(dataFlags & (DataFlag::Owned|DataFlag::ExternallyOwned)) { + _state->in = std::move(data); + } else { + _state->in = Containers::Array{data.size()}; + Utility::copy(data, _state->in); + } } UnsignedInt BasisImporter::doImage2DCount() const { @@ -294,4 +299,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") diff --git a/src/MagnumPlugins/BasisImporter/BasisImporter.h b/src/MagnumPlugins/BasisImporter/BasisImporter.h index adf47e101..8347ed501 100644 --- a/src/MagnumPlugins/BasisImporter/BasisImporter.h +++ b/src/MagnumPlugins/BasisImporter/BasisImporter.h @@ -305,7 +305,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 data) override; + MAGNUM_BASISIMPORTER_LOCAL void doOpenData(Containers::Array&& data, DataFlags dataFlags) override; MAGNUM_BASISIMPORTER_LOCAL UnsignedInt doImage2DCount() const override; MAGNUM_BASISIMPORTER_LOCAL UnsignedInt doImage2DLevelCount(UnsignedInt id) override; diff --git a/src/MagnumPlugins/BasisImporter/Test/BasisImporterTest.cpp b/src/MagnumPlugins/BasisImporter/Test/BasisImporterTest.cpp index 966e8b1f4..4e343a8ea 100644 --- a/src/MagnumPlugins/BasisImporter/Test/BasisImporterTest.cpp +++ b/src/MagnumPlugins/BasisImporter/Test/BasisImporterTest.cpp @@ -28,6 +28,7 @@ #include #include #include +#include #include #include #include @@ -60,6 +61,7 @@ struct BasisImporterTest: TestSuite::Tester { void rgb(); void rgba(); + void openMemory(); void openSameTwice(); void openDifferent(); void importMultipleFormats(); @@ -101,6 +103,22 @@ constexpr struct { "EacRG", CompressedPixelFormat::EacRG11Unorm, {63, 27}} }; +/* Shared among all plugins that implement data copying optimizations */ +const struct { + const char* name; + bool(*open)(AbstractImporter&, Containers::ArrayView); +} OpenMemoryData[]{ + {"data", [](AbstractImporter& importer, Containers::ArrayView data) { + /* Copy to ensure the original memory isn't referenced */ + Containers::Array copy{NoInit, data.size()}; + Utility::copy(Containers::arrayCast(data), copy); + return importer.openData(copy); + }}, + {"memory", [](AbstractImporter& importer, Containers::ArrayView data) { + return importer.openMemory(data); + }}, +}; + BasisImporterTest::BasisImporterTest() { addTests({&BasisImporterTest::empty, &BasisImporterTest::invalid, @@ -118,6 +136,9 @@ BasisImporterTest::BasisImporterTest() { &BasisImporterTest::rgba}, Containers::arraySize(FormatData)); + addInstancedTests({&BasisImporterTest::openMemory}, + Containers::arraySize(OpenMemoryData)); + addTests({&BasisImporterTest::openSameTwice, &BasisImporterTest::openDifferent, &BasisImporterTest::importMultipleFormats}); @@ -457,6 +478,39 @@ void BasisImporterTest::rgba() { CORRADE_COMPARE(image->data().size(), compressedBlockDataSize(formatData.expectedFormat)*((image->size() + compressedBlockSize(formatData.expectedFormat).xy() - Vector2i{1})/compressedBlockSize(formatData.expectedFormat).xy()).product()); } +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 importer = _manager.instantiate("BasisImporterRGBA8"); + CORRADE_VERIFY(importer); + CORRADE_COMPARE(importer->configuration().value("format"), + "RGBA8"); + Containers::Array 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 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(), + 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 importer = _manager.instantiate("BasisImporterEtc2RGBA"); CORRADE_VERIFY(importer->openFile( diff --git a/src/MagnumPlugins/CgltfImporter/CgltfImporter.cpp b/src/MagnumPlugins/CgltfImporter/CgltfImporter.cpp index 715fe0044..477844e0a 100644 --- a/src/MagnumPlugins/CgltfImporter/CgltfImporter.cpp +++ b/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 data) { +void CgltfImporter::doOpenData(Containers::Array&& 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{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{data.size()}; + Utility::copy(data, _d->fileData); + } /* Auto-detect glb/gltf */ _d->options.type = cgltf_file_type::cgltf_file_type_invalid; @@ -2648,4 +2653,4 @@ Containers::Optional 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") diff --git a/src/MagnumPlugins/CgltfImporter/CgltfImporter.h b/src/MagnumPlugins/CgltfImporter/CgltfImporter.h index 7125e305c..ca6ed34ac 100644 --- a/src/MagnumPlugins/CgltfImporter/CgltfImporter.h +++ b/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 data) override; + MAGNUM_CGLTFIMPORTER_LOCAL void doOpenData(Containers::Array&& data, DataFlags dataFlags) override; MAGNUM_CGLTFIMPORTER_LOCAL void doOpenFile(const std::string& filename) override; MAGNUM_CGLTFIMPORTER_LOCAL void doClose() override; diff --git a/src/MagnumPlugins/CgltfImporter/Test/CgltfImporterTest.cpp b/src/MagnumPlugins/CgltfImporter/Test/CgltfImporterTest.cpp index c5c1f84d5..770ca9c4d 100644 --- a/src/MagnumPlugins/CgltfImporter/Test/CgltfImporterTest.cpp +++ b/src/MagnumPlugins/CgltfImporter/Test/CgltfImporterTest.cpp @@ -32,6 +32,7 @@ #include #include #include +#include #include #include #include @@ -188,6 +189,7 @@ struct CgltfImporterTest: TestSuite::Tester { void versionSupported(); void versionUnsupported(); + void openMemory(); void openTwice(); void importTwice(); @@ -611,6 +613,22 @@ constexpr struct { {"unknown minor version", "version-unsupported-min.gltf", "unsupported minVersion 2.1, expected 2.0"} }; +/* Shared among all plugins that implement data copying optimizations */ +const struct { + const char* name; + bool(*open)(AbstractImporter&, Containers::ArrayView); +} OpenMemoryData[]{ + {"data", [](AbstractImporter& importer, Containers::ArrayView data) { + /* Copy to ensure the original memory isn't referenced */ + Containers::Array copy{NoInit, data.size()}; + Utility::copy(Containers::arrayCast(data), copy); + return importer.openData(copy); + }}, + {"memory", [](AbstractImporter& importer, Containers::ArrayView data) { + return importer.openMemory(data); + }}, +}; + CgltfImporterTest::CgltfImporterTest() { addInstancedTests({&CgltfImporterTest::open}, Containers::arraySize(SingleFileData)); @@ -818,6 +836,9 @@ CgltfImporterTest::CgltfImporterTest() { addInstancedTests({&CgltfImporterTest::versionUnsupported}, Containers::arraySize(UnsupportedVersionData)); + addInstancedTests({&CgltfImporterTest::openMemory}, + Containers::arraySize(OpenMemoryData)); + addTests({&CgltfImporterTest::openTwice, &CgltfImporterTest::importTwice}); @@ -4607,6 +4628,27 @@ void CgltfImporterTest::versionUnsupported() { CORRADE_COMPARE(out.str(), Utility::formatString("Trade::CgltfImporter::openData(): {}\n", data.message)); } +void CgltfImporterTest::openMemory() { + /* Same as (a subset of) camera() except that it uses openData() & + openMemory() instead of openFile() to test data copying on import */ + + auto&& data = OpenMemoryData[testCaseInstanceId()]; + setTestCaseDescription(data.name); + + Containers::Pointer importer = _manager.instantiate("CgltfImporter"); + Containers::Array memory = Utility::Directory::read(Utility::Directory::join(TINYGLTFIMPORTER_TEST_DIR, "camera.gltf")); + CORRADE_VERIFY(data.open(*importer, memory)); + CORRADE_COMPARE(importer->cameraCount(), 4); + + auto cam = importer->camera(0); + CORRADE_VERIFY(cam); + CORRADE_COMPARE(cam->type(), CameraType::Orthographic3D); + CORRADE_COMPARE(cam->size(), (Vector2{4.0f, 3.0f})); + CORRADE_COMPARE(cam->aspectRatio(), 1.333333f); + CORRADE_COMPARE(cam->near(), 0.01f); + CORRADE_COMPARE(cam->far(), 100.0f); +} + void CgltfImporterTest::openTwice() { Containers::Pointer importer = _manager.instantiate("CgltfImporter"); diff --git a/src/MagnumPlugins/DdsImporter/DdsImporter.cpp b/src/MagnumPlugins/DdsImporter/DdsImporter.cpp index 416cd126d..7072073b1 100644 --- a/src/MagnumPlugins/DdsImporter/DdsImporter.cpp +++ b/src/MagnumPlugins/DdsImporter/DdsImporter.cpp @@ -489,12 +489,16 @@ bool DdsImporter::doIsOpened() const { return !!_f; } void DdsImporter::doClose() { _f = nullptr; } -void DdsImporter::doOpenData(const Containers::ArrayView data) { +void DdsImporter::doOpenData(Containers::Array&& data, const DataFlags dataFlags) { Containers::Pointer f{new File}; - /* clear previous data */ - f->in = Containers::Array{NoInit, data.size()}; - Utility::copy(data, f->in); + /* Take over the existing array or copy the data if we can't */ + if(dataFlags & (DataFlag::Owned|DataFlag::ExternallyOwned)) { + f->in = std::move(data); + } else { + f->in = Containers::Array{NoInit, data.size()}; + Utility::copy(data, f->in); + } constexpr size_t MagicNumberSize = 4; /* read magic number to verify this is a dds file. */ @@ -690,4 +694,4 @@ Containers::Optional DdsImporter::doImage3D(UnsignedInt, const Unsi }} CORRADE_PLUGIN_REGISTER(DdsImporter, Magnum::Trade::DdsImporter, - "cz.mosra.magnum.Trade.AbstractImporter/0.3.3") + "cz.mosra.magnum.Trade.AbstractImporter/0.3.4") diff --git a/src/MagnumPlugins/DdsImporter/DdsImporter.h b/src/MagnumPlugins/DdsImporter/DdsImporter.h index 58bd0bad0..699c37916 100644 --- a/src/MagnumPlugins/DdsImporter/DdsImporter.h +++ b/src/MagnumPlugins/DdsImporter/DdsImporter.h @@ -158,7 +158,7 @@ class MAGNUM_DDSIMPORTER_EXPORT DdsImporter: public AbstractImporter { MAGNUM_DDSIMPORTER_LOCAL ImporterFeatures doFeatures() const override; MAGNUM_DDSIMPORTER_LOCAL bool doIsOpened() const override; MAGNUM_DDSIMPORTER_LOCAL void doClose() override; - MAGNUM_DDSIMPORTER_LOCAL void doOpenData(Containers::ArrayView data) override; + MAGNUM_DDSIMPORTER_LOCAL void doOpenData(Containers::Array&& data, DataFlags dataFlags) override; MAGNUM_DDSIMPORTER_LOCAL UnsignedInt doImage2DCount() const override; MAGNUM_DDSIMPORTER_LOCAL UnsignedInt doImage2DLevelCount(UnsignedInt id) override; diff --git a/src/MagnumPlugins/DdsImporter/Test/DdsImporterTest.cpp b/src/MagnumPlugins/DdsImporter/Test/DdsImporterTest.cpp index 5c36892bf..213884ce2 100644 --- a/src/MagnumPlugins/DdsImporter/Test/DdsImporterTest.cpp +++ b/src/MagnumPlugins/DdsImporter/Test/DdsImporterTest.cpp @@ -28,6 +28,7 @@ #include #include #include +#include #include #include #include @@ -62,6 +63,7 @@ struct DdsImporterTest: TestSuite::Tester { void dxt10TooShort(); void dxt10UnsupportedFormat(); + void openMemory(); void openTwice(); void importTwice(); @@ -151,6 +153,22 @@ constexpr struct { {"3D_R32G32B32_UINT.dds", PixelFormat::RGB32UI} }; +/* Shared among all plugins that implement data copying optimizations */ +const struct { + const char* name; + bool(*open)(AbstractImporter&, Containers::ArrayView); +} OpenMemoryData[]{ + {"data", [](AbstractImporter& importer, Containers::ArrayView data) { + /* Copy to ensure the original memory isn't referenced */ + Containers::Array copy{NoInit, data.size()}; + Utility::copy(Containers::arrayCast(data), copy); + return importer.openData(copy); + }}, + {"memory", [](AbstractImporter& importer, Containers::ArrayView data) { + return importer.openMemory(data); + }}, +}; + DdsImporterTest::DdsImporterTest() { addTests({&DdsImporterTest::wrongSignature, &DdsImporterTest::unknownFormat, @@ -174,9 +192,12 @@ DdsImporterTest::DdsImporterTest() { addTests({&DdsImporterTest::dxt10Data, &DdsImporterTest::dxt10TooShort, - &DdsImporterTest::dxt10UnsupportedFormat, + &DdsImporterTest::dxt10UnsupportedFormat}); - &DdsImporterTest::openTwice, + addInstancedTests({&DdsImporterTest::openMemory}, + Containers::arraySize(OpenMemoryData)); + + addTests({&DdsImporterTest::openTwice, &DdsImporterTest::importTwice}); /* Load the plugin directly from the build tree. Otherwise it's static and @@ -472,6 +493,27 @@ void DdsImporterTest::dxt10UnsupportedFormat() { CORRADE_COMPARE(out.str(), "Trade::DdsImporter::openData(): unsupported DXGI format 100\n"); } +void DdsImporterTest::openMemory() { + /* same as dxt1() except that it uses openData() & openMemory() instead of + openFile() to test data copying on import */ + + auto&& data = OpenMemoryData[testCaseInstanceId()]; + setTestCaseDescription(data.name); + + Containers::Pointer importer = _manager.instantiate("DdsImporter"); + Containers::Array memory = Utility::Directory::read(Utility::Directory::join(DDSIMPORTER_TEST_DIR, "rgba_dxt1.dds")); + CORRADE_VERIFY(data.open(*importer, memory)); + + Containers::Optional image = importer->image2D(0); + CORRADE_VERIFY(image); + CORRADE_VERIFY(image->isCompressed()); + CORRADE_COMPARE(image->size(), Vector2i(3, 2)); + CORRADE_COMPARE(image->compressedFormat(), CompressedPixelFormat::Bc1RGBAUnorm); + CORRADE_COMPARE_AS(image->data(), Containers::arrayView({ + '\x76', '\xdd', '\xee', '\xcf', '\x04', '\x51', '\x04', '\x51' + }), TestSuite::Compare::Container); +} + void DdsImporterTest::openTwice() { Containers::Pointer importer = _manager.instantiate("DdsImporter"); diff --git a/src/MagnumPlugins/IcoImporter/IcoImporter.cpp b/src/MagnumPlugins/IcoImporter/IcoImporter.cpp index bfeb1f741..0c8c06947 100644 --- a/src/MagnumPlugins/IcoImporter/IcoImporter.cpp +++ b/src/MagnumPlugins/IcoImporter/IcoImporter.cpp @@ -86,7 +86,7 @@ namespace { }; } -void IcoImporter::doOpenData(const Containers::ArrayView data) { +void IcoImporter::doOpenData(Containers::Array&& data, const DataFlags dataFlags) { if(data.size() < sizeof(IconDir)) { Error{} << "Trade::IcoImporter::openData(): file header too short, expected at least" << sizeof(IconDir) << "bytes but got" << data.size(); return; @@ -96,9 +96,15 @@ void IcoImporter::doOpenData(const Containers::ArrayView data) { Utility::Endianness::littleEndianInPlace(header.imageType, header.imageCount); Containers::Pointer state{InPlaceInit}; - state->data = Containers::Array{NoInit, data.size()}; state->levels = Containers::Array>{header.imageCount}; - Utility::copy(data, state->data); + + /* Take over the existing array or copy the data if we can't */ + if(dataFlags & (DataFlag::Owned|DataFlag::ExternallyOwned)) { + state->data = std::move(data); + } else { + state->data = Containers::Array{NoInit, data.size()}; + Utility::copy(data, state->data); + } for(UnsignedInt i = 0; i != header.imageCount; ++i) { std::size_t iconDirEntryOffset = sizeof(IconDir) + sizeof(IconDirEntry)*i; @@ -156,4 +162,4 @@ Containers::Optional IcoImporter::doImage2D(UnsignedInt, UnsignedIn }} CORRADE_PLUGIN_REGISTER(IcoImporter, Magnum::Trade::IcoImporter, - "cz.mosra.magnum.Trade.AbstractImporter/0.3.3") + "cz.mosra.magnum.Trade.AbstractImporter/0.3.4") diff --git a/src/MagnumPlugins/IcoImporter/IcoImporter.h b/src/MagnumPlugins/IcoImporter/IcoImporter.h index b3d91afd9..8358cf2ee 100644 --- a/src/MagnumPlugins/IcoImporter/IcoImporter.h +++ b/src/MagnumPlugins/IcoImporter/IcoImporter.h @@ -117,7 +117,7 @@ class MAGNUM_ICOIMPORTER_EXPORT IcoImporter: public AbstractImporter { MAGNUM_ICOIMPORTER_LOCAL ImporterFeatures doFeatures() const override; MAGNUM_ICOIMPORTER_LOCAL bool doIsOpened() const override; MAGNUM_ICOIMPORTER_LOCAL void doClose() override; - MAGNUM_ICOIMPORTER_LOCAL void doOpenData(Containers::ArrayView data) override; + MAGNUM_ICOIMPORTER_LOCAL void doOpenData(Containers::Array&& data, DataFlags dataFlags) override; MAGNUM_ICOIMPORTER_LOCAL UnsignedInt doImage2DCount() const override; MAGNUM_ICOIMPORTER_LOCAL UnsignedInt doImage2DLevelCount(UnsignedInt id) override; diff --git a/src/MagnumPlugins/IcoImporter/Test/IcoImporterTest.cpp b/src/MagnumPlugins/IcoImporter/Test/IcoImporterTest.cpp index 55071b5d4..72931e5dc 100644 --- a/src/MagnumPlugins/IcoImporter/Test/IcoImporterTest.cpp +++ b/src/MagnumPlugins/IcoImporter/Test/IcoImporterTest.cpp @@ -30,6 +30,7 @@ #include #include #include +#include #include #include #include @@ -52,6 +53,7 @@ struct IcoImporterTest: TestSuite::Tester { void bmp(); void png(); + void openMemory(); void openTwice(); void importTwice(); @@ -72,6 +74,22 @@ const struct { "image too short, expected at least 974 bytes but got 973"} }; +/* Shared among all plugins that implement data copying optimizations */ +const struct { + const char* name; + bool(*open)(AbstractImporter&, Containers::ArrayView); +} OpenMemoryData[]{ + {"data", [](AbstractImporter& importer, Containers::ArrayView data) { + /* Copy to ensure the original memory isn't referenced */ + Containers::Array copy{NoInit, data.size()}; + Utility::copy(Containers::arrayCast(data), copy); + return importer.openData(copy); + }}, + {"memory", [](AbstractImporter& importer, Containers::ArrayView data) { + return importer.openMemory(data); + }}, +}; + IcoImporterTest::IcoImporterTest() { addInstancedTests({&IcoImporterTest::tooShort}, Containers::arraySize(TooShortData)); @@ -80,9 +98,12 @@ IcoImporterTest::IcoImporterTest() { &IcoImporterTest::pngLoadFailed, &IcoImporterTest::bmp, - &IcoImporterTest::png, + &IcoImporterTest::png}); - &IcoImporterTest::openTwice, + addInstancedTests({&IcoImporterTest::openMemory}, + Containers::arraySize(OpenMemoryData)); + + addTests({&IcoImporterTest::openTwice, &IcoImporterTest::importTwice}); /* Load the plugin directly from the build tree. Otherwise it's static and @@ -205,6 +226,25 @@ void IcoImporterTest::png() { } } +void IcoImporterTest::openMemory() { + /* same as (a subset of) png() except that it uses openData() & + openMemory() instead of openFile() to test data copying on import */ + + auto&& data = OpenMemoryData[testCaseInstanceId()]; + setTestCaseDescription(data.name); + + Containers::Pointer importer = _manager.instantiate("IcoImporter"); + Containers::Array memory = Utility::Directory::read(Utility::Directory::join(ICOIMPORTER_TEST_DIR, "pngs.ico")); + CORRADE_VERIFY(data.open(*importer, memory)); + CORRADE_COMPARE(importer->image2DCount(), 1); + + Containers::Optional image = importer->image2D(0); + CORRADE_VERIFY(image); + CORRADE_COMPARE(image->format(), PixelFormat::RGB8Unorm); + CORRADE_COMPARE(image->size(), (Vector2i{16, 8})); + CORRADE_COMPARE(image->pixels()[0][0], 0x00ff00_rgb); +} + void IcoImporterTest::openTwice() { Containers::Pointer importer = _manager.instantiate("IcoImporter"); diff --git a/src/MagnumPlugins/JpegImporter/JpegImporter.cpp b/src/MagnumPlugins/JpegImporter/JpegImporter.cpp index 7d971964f..5a9623cc5 100644 --- a/src/MagnumPlugins/JpegImporter/JpegImporter.cpp +++ b/src/MagnumPlugins/JpegImporter/JpegImporter.cpp @@ -66,7 +66,7 @@ bool JpegImporter::doIsOpened() const { return _in; } void JpegImporter::doClose() { _in = nullptr; } -void JpegImporter::doOpenData(const Containers::ArrayView data) { +void JpegImporter::doOpenData(Containers::Array&& data, const 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 @@ -79,8 +79,13 @@ void JpegImporter::doOpenData(const Containers::ArrayView data) { return; } - _in = Containers::Array{NoInit, data.size()}; - Utility::copy(Containers::arrayCast(data), _in); + /* Take over the existing array or copy the data if we can't */ + if(dataFlags & (DataFlag::Owned|DataFlag::ExternallyOwned)) { + _in = std::move(data); + } else { + _in = Containers::Array{NoInit, data.size()}; + Utility::copy(data, _in); + } } UnsignedInt JpegImporter::doImage2DCount() const { return 1; } @@ -111,7 +116,7 @@ Containers::Optional JpegImporter::doImage2D(UnsignedInt, UnsignedI /* Open file */ jpeg_create_decompress(&file); - jpeg_mem_src(&file, _in.begin(), _in.size()); + jpeg_mem_src(&file, reinterpret_cast(_in.begin()), _in.size()); /* Read file header, start decompression. On macOS (Travis, with Xcode 7.3) the compilation fails because "no known conversion from 'bool' to @@ -164,4 +169,4 @@ Containers::Optional JpegImporter::doImage2D(UnsignedInt, UnsignedI }} CORRADE_PLUGIN_REGISTER(JpegImporter, Magnum::Trade::JpegImporter, - "cz.mosra.magnum.Trade.AbstractImporter/0.3.3") + "cz.mosra.magnum.Trade.AbstractImporter/0.3.4") diff --git a/src/MagnumPlugins/JpegImporter/JpegImporter.h b/src/MagnumPlugins/JpegImporter/JpegImporter.h index 88a1cee9c..6d5532b5f 100644 --- a/src/MagnumPlugins/JpegImporter/JpegImporter.h +++ b/src/MagnumPlugins/JpegImporter/JpegImporter.h @@ -124,12 +124,12 @@ class MAGNUM_JPEGIMPORTER_EXPORT JpegImporter: public AbstractImporter { MAGNUM_JPEGIMPORTER_LOCAL ImporterFeatures doFeatures() const override; MAGNUM_JPEGIMPORTER_LOCAL bool doIsOpened() const override; MAGNUM_JPEGIMPORTER_LOCAL void doClose() override; - MAGNUM_JPEGIMPORTER_LOCAL void doOpenData(Containers::ArrayView data) override; + MAGNUM_JPEGIMPORTER_LOCAL void doOpenData(Containers::Array&& data, DataFlags dataFlags) override; MAGNUM_JPEGIMPORTER_LOCAL UnsignedInt doImage2DCount() const override; MAGNUM_JPEGIMPORTER_LOCAL Containers::Optional doImage2D(UnsignedInt id, UnsignedInt level) override; - Containers::Array _in; + Containers::Array _in; }; }} diff --git a/src/MagnumPlugins/JpegImporter/Test/JpegImporterTest.cpp b/src/MagnumPlugins/JpegImporter/Test/JpegImporterTest.cpp index 5fbc62515..92735d5c6 100644 --- a/src/MagnumPlugins/JpegImporter/Test/JpegImporterTest.cpp +++ b/src/MagnumPlugins/JpegImporter/Test/JpegImporterTest.cpp @@ -27,6 +27,7 @@ #include #include #include +#include #include #include #include @@ -46,6 +47,7 @@ struct JpegImporterTest: TestSuite::Tester { void gray(); void rgb(); + void openMemory(); void openTwice(); void importTwice(); @@ -53,14 +55,33 @@ struct JpegImporterTest: TestSuite::Tester { PluginManager::Manager _manager{"nonexistent"}; }; +/* Shared among all plugins that implement data copying optimizations */ +const struct { + const char* name; + bool(*open)(AbstractImporter&, Containers::ArrayView); +} OpenMemoryData[]{ + {"data", [](AbstractImporter& importer, Containers::ArrayView data) { + /* Copy to ensure the original memory isn't referenced */ + Containers::Array copy{NoInit, data.size()}; + Utility::copy(Containers::arrayCast(data), copy); + return importer.openData(copy); + }}, + {"memory", [](AbstractImporter& importer, Containers::ArrayView data) { + return importer.openMemory(data); + }}, +}; + JpegImporterTest::JpegImporterTest() { addTests({&JpegImporterTest::empty, &JpegImporterTest::invalid, &JpegImporterTest::gray, - &JpegImporterTest::rgb, + &JpegImporterTest::rgb}); + + addInstancedTests({&JpegImporterTest::openMemory}, + Containers::arraySize(OpenMemoryData)); - &JpegImporterTest::openTwice, + addTests({&JpegImporterTest::openTwice, &JpegImporterTest::importTwice}); /* Load the plugin directly from the build tree. Otherwise it's static and @@ -146,6 +167,33 @@ void JpegImporterTest::rgb() { }), TestSuite::Compare::Container); } +void JpegImporterTest::openMemory() { + /* same as gray() except that it uses openData() & openMemory() instead of + openFile() to test data copying on import */ + + auto&& data = OpenMemoryData[testCaseInstanceId()]; + setTestCaseDescription(data.name); + + Containers::Pointer importer = _manager.instantiate("JpegImporter"); + Containers::Array memory = Utility::Directory::read(Utility::Directory::join(JPEGIMPORTER_TEST_DIR, "gray.jpg")); + CORRADE_VERIFY(data.open(*importer, memory)); + + Containers::Optional image = importer->image2D(0); + CORRADE_VERIFY(image); + CORRADE_COMPARE(image->size(), Vector2i(3, 2)); + CORRADE_COMPARE(image->format(), PixelFormat::R8Unorm); + + /* The image has four-byte aligned rows, clear the padding to deterministic + values */ + CORRADE_COMPARE(image->data().size(), 8); + image->mutableData()[3] = image->mutableData()[7] = 0; + + CORRADE_COMPARE_AS(image->data(), Containers::arrayView({ + '\xff', '\x88', '\x00', 0, + '\x88', '\x00', '\xff', 0 + }), TestSuite::Compare::Container); +} + void JpegImporterTest::openTwice() { Containers::Pointer importer = _manager.instantiate("JpegImporter"); diff --git a/src/MagnumPlugins/KtxImporter/KtxImporter.cpp b/src/MagnumPlugins/KtxImporter/KtxImporter.cpp index 33612c965..1043eb6f2 100644 --- a/src/MagnumPlugins/KtxImporter/KtxImporter.cpp +++ b/src/MagnumPlugins/KtxImporter/KtxImporter.cpp @@ -286,7 +286,7 @@ bool KtxImporter::doIsOpened() const { return !!_f; } void KtxImporter::doClose() { _f = nullptr; } -void KtxImporter::doOpenData(const Containers::ArrayView data) { +void KtxImporter::doOpenData(Containers::Array&& data, DataFlags dataFlags) { /* Check if the file is long enough for the header */ if(data.size() < sizeof(Implementation::KtxHeader)) { Error{} << "Trade::KtxImporter::openData(): file too short, expected" @@ -457,9 +457,13 @@ void KtxImporter::doOpenData(const Containers::ArrayView data) { } f->pixelFormat.typeSize = header.typeSize; - /* Make an owned copy of the data */ - f->in = Containers::Array{NoInit, data.size()}; - Utility::copy(data, f->in); + /* Take over the existing array or copy the data if we can't */ + if(dataFlags & (DataFlag::Owned|DataFlag::ExternallyOwned)) { + f->in = std::move(data); + } else { + f->in = Containers::Array{NoInit, data.size()}; + Utility::copy(data, f->in); + } /* The level index contains byte ranges for each mipmap, from largest to smallest. Each mipmap contains tightly packed images ordered by @@ -494,9 +498,9 @@ void KtxImporter::doOpenData(const Containers::ArrayView data) { } const std::size_t levelEnd = level.byteOffset + level.byteLength; - if(data.size() < levelEnd) { + if(f->in.size() < levelEnd) { Error{} << "Trade::KtxImporter::openData(): file too short, expected" - << levelEnd << "bytes for level data but got only" << data.size(); + << levelEnd << "bytes for level data but got only" << f->in.size(); return; } @@ -809,4 +813,4 @@ Containers::Optional KtxImporter::doTexture(UnsignedInt id) { }} CORRADE_PLUGIN_REGISTER(KtxImporter, Magnum::Trade::KtxImporter, - "cz.mosra.magnum.Trade.AbstractImporter/0.3.3") + "cz.mosra.magnum.Trade.AbstractImporter/0.3.4") diff --git a/src/MagnumPlugins/KtxImporter/KtxImporter.h b/src/MagnumPlugins/KtxImporter/KtxImporter.h index 677a4eb6b..270a5eab9 100644 --- a/src/MagnumPlugins/KtxImporter/KtxImporter.h +++ b/src/MagnumPlugins/KtxImporter/KtxImporter.h @@ -168,7 +168,7 @@ class MAGNUM_KTXIMPORTER_EXPORT KtxImporter: public AbstractImporter { MAGNUM_KTXIMPORTER_LOCAL ImporterFeatures doFeatures() const override; MAGNUM_KTXIMPORTER_LOCAL bool doIsOpened() const override; MAGNUM_KTXIMPORTER_LOCAL void doClose() override; - MAGNUM_KTXIMPORTER_LOCAL void doOpenData(Containers::ArrayView data) override; + MAGNUM_KTXIMPORTER_LOCAL void doOpenData(Containers::Array&& data, DataFlags dataFlags) override; MAGNUM_KTXIMPORTER_LOCAL UnsignedInt doImage1DCount() const override; MAGNUM_KTXIMPORTER_LOCAL UnsignedInt doImage1DLevelCount(UnsignedInt id) override; diff --git a/src/MagnumPlugins/KtxImporter/Test/KtxImporterTest.cpp b/src/MagnumPlugins/KtxImporter/Test/KtxImporterTest.cpp index 79508bb3b..660afb5be 100644 --- a/src/MagnumPlugins/KtxImporter/Test/KtxImporterTest.cpp +++ b/src/MagnumPlugins/KtxImporter/Test/KtxImporterTest.cpp @@ -109,6 +109,7 @@ struct KtxImporterTest: TestSuite::Tester { void swizzleUnsupported(); void swizzleCompressed(); + void openMemory(); void openTwice(); void importTwice(); @@ -396,6 +397,22 @@ const struct { nullptr, Containers::arrayCast(PatternRgba2DData)} }; +/* Shared among all plugins that implement data copying optimizations */ +const struct { + const char* name; + bool(*open)(AbstractImporter&, Containers::ArrayView); +} OpenMemoryData[]{ + {"data", [](AbstractImporter& importer, Containers::ArrayView data) { + /* Copy to ensure the original memory isn't referenced */ + Containers::Array copy{NoInit, data.size()}; + Utility::copy(Containers::arrayCast(data), copy); + return importer.openData(copy); + }}, + {"memory", [](AbstractImporter& importer, Containers::ArrayView data) { + return importer.openMemory(data); + }}, +}; + Containers::Array createKeyValueData(Containers::StringView key, Containers::ArrayView value, bool terminatingZero = false) { UnsignedInt size = key.size() + 1 + value.size() + UnsignedInt(terminatingZero); size = (size + 3)/4*4; @@ -502,9 +519,12 @@ KtxImporterTest::KtxImporterTest() { addTests({&KtxImporterTest::swizzleMultipleBytes, &KtxImporterTest::swizzleIdentity, &KtxImporterTest::swizzleUnsupported, - &KtxImporterTest::swizzleCompressed, + &KtxImporterTest::swizzleCompressed}); - &KtxImporterTest::openTwice, + addInstancedTests({&KtxImporterTest::openMemory}, + Containers::arraySize(OpenMemoryData)); + + addTests({&KtxImporterTest::openTwice, &KtxImporterTest::importTwice}); /* Load the plugin directly from the build tree. Otherwise it's static and @@ -1757,6 +1777,36 @@ void KtxImporterTest::swizzleCompressed() { CORRADE_COMPARE(out.str(), "Trade::KtxImporter::openData(): unsupported channel mapping bgra\n"); } +void KtxImporterTest::openMemory() { + /* Same as imageRgba() except that it uses openData() & openMemory() + instead of openFile() to test data copying on import */ + + auto&& data = OpenMemoryData[testCaseInstanceId()]; + setTestCaseDescription(data.name); + + Containers::Pointer importer = _manager.instantiate("KtxImporter"); + Containers::Array memory = Utility::Directory::read(Utility::Directory::join(KTXIMPORTER_TEST_DIR, "2d-rgba.ktx2")); + CORRADE_VERIFY(data.open(*importer, memory)); + + CORRADE_COMPARE(importer->image2DCount(), 1); + CORRADE_COMPARE(importer->image2DLevelCount(0), 1); + + auto image = importer->image2D(0); + CORRADE_VERIFY(image); + + CORRADE_VERIFY(!image->isCompressed()); + CORRADE_COMPARE(image->format(), PixelFormat::RGBA8Srgb); + CORRADE_COMPARE(image->size(), (Vector2i{4, 3})); + + const PixelStorage storage = image->storage(); + CORRADE_COMPARE(storage.alignment(), 4); + CORRADE_COMPARE(storage.rowLength(), 0); + CORRADE_COMPARE(storage.imageHeight(), 0); + CORRADE_COMPARE(storage.skip(), Vector3i{}); + + CORRADE_COMPARE_AS(image->data(), Containers::arrayCast(PatternRgba2DData), TestSuite::Compare::Container); +} + void KtxImporterTest::openTwice() { Containers::Pointer importer = _manager.instantiate("KtxImporter"); diff --git a/src/MagnumPlugins/OpenExrImporter/OpenExrImporter.cpp b/src/MagnumPlugins/OpenExrImporter/OpenExrImporter.cpp index 46145644c..fc95c1736 100644 --- a/src/MagnumPlugins/OpenExrImporter/OpenExrImporter.cpp +++ b/src/MagnumPlugins/OpenExrImporter/OpenExrImporter.cpp @@ -131,10 +131,15 @@ bool OpenExrImporter::doIsOpened() const { return !!_state; } void OpenExrImporter::doClose() { _state = nullptr; } -void OpenExrImporter::doOpenData(const Containers::ArrayView data) { - /* Make an owned copy of the data */ - Containers::Array dataCopy{NoInit, data.size()}; - Utility::copy(data, dataCopy); +void OpenExrImporter::doOpenData(Containers::Array&& data, const DataFlags dataFlags) { + /* Take over the existing array or copy the data if we can't */ + Containers::Array dataCopy; + if(dataFlags & (DataFlag::Owned|DataFlag::ExternallyOwned)) { + dataCopy = std::move(data); + } else { + dataCopy = Containers::Array{NoInit, data.size()}; + Utility::copy(data, dataCopy); + } /* Set up the input stream using the MemoryIStream class above */ Containers::Pointer state{InPlaceInit, std::move(dataCopy)}; @@ -631,4 +636,4 @@ Containers::Optional OpenExrImporter::doImage3D(UnsignedInt, const }} CORRADE_PLUGIN_REGISTER(OpenExrImporter, Magnum::Trade::OpenExrImporter, - "cz.mosra.magnum.Trade.AbstractImporter/0.3.3") + "cz.mosra.magnum.Trade.AbstractImporter/0.3.4") diff --git a/src/MagnumPlugins/OpenExrImporter/OpenExrImporter.h b/src/MagnumPlugins/OpenExrImporter/OpenExrImporter.h index af7693961..cef989996 100644 --- a/src/MagnumPlugins/OpenExrImporter/OpenExrImporter.h +++ b/src/MagnumPlugins/OpenExrImporter/OpenExrImporter.h @@ -228,7 +228,7 @@ class MAGNUM_OPENEXRIMPORTER_EXPORT OpenExrImporter: public AbstractImporter { MAGNUM_OPENEXRIMPORTER_LOCAL ImporterFeatures doFeatures() const override; MAGNUM_OPENEXRIMPORTER_LOCAL bool doIsOpened() const override; MAGNUM_OPENEXRIMPORTER_LOCAL void doClose() override; - MAGNUM_OPENEXRIMPORTER_LOCAL void doOpenData(Containers::ArrayView data) override; + MAGNUM_OPENEXRIMPORTER_LOCAL void doOpenData(Containers::Array&& data, DataFlags dataFlags) override; MAGNUM_OPENEXRIMPORTER_LOCAL UnsignedInt doImage2DCount() const override; MAGNUM_OPENEXRIMPORTER_LOCAL UnsignedInt doImage2DLevelCount(UnsignedInt id) override; diff --git a/src/MagnumPlugins/OpenExrImporter/Test/OpenExrImporterTest.cpp b/src/MagnumPlugins/OpenExrImporter/Test/OpenExrImporterTest.cpp index a07c085fe..6b346d8b5 100644 --- a/src/MagnumPlugins/OpenExrImporter/Test/OpenExrImporterTest.cpp +++ b/src/MagnumPlugins/OpenExrImporter/Test/OpenExrImporterTest.cpp @@ -29,6 +29,7 @@ #include #include #include +#include #include #include #include @@ -74,6 +75,7 @@ struct OpenExrImporterTest: TestSuite::Tester { void levelsCubeMap(); void levelsCubeMapIncomplete(); + void openMemory(); void openTwice(); void importTwice(); @@ -145,6 +147,22 @@ const struct { "Trade::OpenExrImporter::openData(): last 3 levels are missing in the file, capping at 2 levels\n"}, }; +/* Shared among all plugins that implement data copying optimizations */ +const struct { + const char* name; + bool(*open)(AbstractImporter&, Containers::ArrayView); +} OpenMemoryData[]{ + {"data", [](AbstractImporter& importer, Containers::ArrayView data) { + /* Copy to ensure the original memory isn't referenced */ + Containers::Array copy{NoInit, data.size()}; + Utility::copy(Containers::arrayCast(data), copy); + return importer.openData(copy); + }}, + {"memory", [](AbstractImporter& importer, Containers::ArrayView data) { + return importer.openMemory(data); + }}, +}; + OpenExrImporterTest::OpenExrImporterTest() { addTests({&OpenExrImporterTest::emptyFile, &OpenExrImporterTest::shortFile, @@ -185,6 +203,9 @@ OpenExrImporterTest::OpenExrImporterTest() { addInstancedTests({&OpenExrImporterTest::levelsCubeMapIncomplete}, Containers::arraySize(IncompletelCubeMapData)); + addInstancedTests({&OpenExrImporterTest::openMemory}, + Containers::arraySize(OpenMemoryData)); + addTests({&OpenExrImporterTest::openTwice, &OpenExrImporterTest::importTwice}); @@ -809,6 +830,30 @@ void OpenExrImporterTest::levelsCubeMapIncomplete() { } } +void OpenExrImporterTest::openMemory() { + /* Same as rgba32f() except that it uses openData() & openMemory() instead + of openFile() to test data copying on import */ + + auto&& data = OpenMemoryData[testCaseInstanceId()]; + setTestCaseDescription(data.name); + + Containers::Pointer importer = _manager.instantiate("OpenExrImporter"); + Containers::Array memory = Utility::Directory::read(Utility::Directory::join(OPENEXRIMPORTER_TEST_DIR, "rgba32f.exr")); + CORRADE_VERIFY(data.open(*importer, memory)); + + Containers::Optional image = importer->image2D(0); + CORRADE_VERIFY(image); + CORRADE_COMPARE(image->size(), Vector2i(1, 3)); + CORRADE_COMPARE(image->format(), PixelFormat::RGBA32F); + + /* Data should be tightly packed here */ + CORRADE_COMPARE_AS(Containers::arrayCast(image->data()), Containers::arrayView({ + 0.0f, 1.0f, 2.0f, 3.0f, + 4.0f, 5.0f, 6.0f, 7.0f, + 8.0f, 9.0f, 10.0f, 11.0f + }), TestSuite::Compare::Container); +} + void OpenExrImporterTest::openTwice() { Containers::Pointer importer = _manager.instantiate("OpenExrImporter"); diff --git a/src/MagnumPlugins/PngImporter/PngImporter.cpp b/src/MagnumPlugins/PngImporter/PngImporter.cpp index 2d91b100a..d4b1ab387 100644 --- a/src/MagnumPlugins/PngImporter/PngImporter.cpp +++ b/src/MagnumPlugins/PngImporter/PngImporter.cpp @@ -60,7 +60,7 @@ bool PngImporter::doIsOpened() const { return _in; } void PngImporter::doClose() { _in = nullptr; } -void PngImporter::doOpenData(const Containers::ArrayView data) { +void PngImporter::doOpenData(Containers::Array&& 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 @@ -73,8 +73,13 @@ void PngImporter::doOpenData(const Containers::ArrayView data) { return; } - _in = Containers::Array{NoInit, data.size()}; - Utility::copy(Containers::arrayCast(data), _in); + /* Take over the existing array or copy the data if we can't */ + if(dataFlags & (DataFlag::Owned|DataFlag::ExternallyOwned)) { + _in = std::move(data); + } else { + _in = Containers::Array{NoInit, data.size()}; + Utility::copy(data, _in); + } } UnsignedInt PngImporter::doImage2DCount() const { return 1; } @@ -84,7 +89,7 @@ Containers::Optional PngImporter::doImage2D(UnsignedInt, UnsignedIn "Trade::PngImporter::image2D(): libpng version mismatch, got" << png_libpng_ver << "but expected" << PNG_LIBPNG_VER_STRING, Containers::NullOpt); /* Verify file signature */ - if(png_sig_cmp(_in, 0, Math::min(std::size_t(8), _in.size())) != 0) { + if(png_sig_cmp(reinterpret_cast(_in.data()), 0, Math::min(std::size_t(8), _in.size())) != 0) { Error() << "Trade::PngImporter::image2D(): wrong file signature"; return Containers::NullOpt; } @@ -121,11 +126,11 @@ Containers::Optional PngImporter::doImage2D(UnsignedInt, UnsignedIn Error{} << "Trade::PngImporter::image2D(): signature too short"; return Containers::NullOpt; } - Containers::ArrayView input = _in.suffix(8); + Containers::ArrayView input = _in.suffix(8); /* Set functions for reading */ png_set_read_fn(file, &input, [](const png_structp file, const png_bytep data, const png_size_t length) { - auto&& input = *reinterpret_cast*>(png_get_io_ptr(file)); + auto&& input = *reinterpret_cast*>(png_get_io_ptr(file)); if(input.size() < length) png_error(file, "file too short"); std::copy_n(input.begin(), length, data); input = input.suffix(length); @@ -255,4 +260,4 @@ Containers::Optional PngImporter::doImage2D(UnsignedInt, UnsignedIn }} CORRADE_PLUGIN_REGISTER(PngImporter, Magnum::Trade::PngImporter, - "cz.mosra.magnum.Trade.AbstractImporter/0.3.3") + "cz.mosra.magnum.Trade.AbstractImporter/0.3.4") diff --git a/src/MagnumPlugins/PngImporter/PngImporter.h b/src/MagnumPlugins/PngImporter/PngImporter.h index cc1cc882a..8af74c8d0 100644 --- a/src/MagnumPlugins/PngImporter/PngImporter.h +++ b/src/MagnumPlugins/PngImporter/PngImporter.h @@ -138,12 +138,12 @@ class MAGNUM_PNGIMPORTER_EXPORT PngImporter: public AbstractImporter { MAGNUM_PNGIMPORTER_LOCAL ImporterFeatures doFeatures() const override; MAGNUM_PNGIMPORTER_LOCAL bool doIsOpened() const override; MAGNUM_PNGIMPORTER_LOCAL void doClose() override; - MAGNUM_PNGIMPORTER_LOCAL void doOpenData(Containers::ArrayView data) override; + MAGNUM_PNGIMPORTER_LOCAL void doOpenData(Containers::Array&& data, DataFlags dataFlags) override; MAGNUM_PNGIMPORTER_LOCAL UnsignedInt doImage2DCount() const override; MAGNUM_PNGIMPORTER_LOCAL Containers::Optional doImage2D(UnsignedInt id, UnsignedInt level) override; - Containers::Array _in; + Containers::Array _in; }; }} diff --git a/src/MagnumPlugins/PngImporter/Test/PngImporterTest.cpp b/src/MagnumPlugins/PngImporter/Test/PngImporterTest.cpp index 71abb7fa6..c811e7ef0 100644 --- a/src/MagnumPlugins/PngImporter/Test/PngImporterTest.cpp +++ b/src/MagnumPlugins/PngImporter/Test/PngImporterTest.cpp @@ -28,6 +28,7 @@ #include #include #include +#include #include #include #include @@ -54,6 +55,7 @@ struct PngImporterTest: TestSuite::Tester { void rgbPalette1bit(); void rgba(); + void openMemory(); void openTwice(); void importTwice(); @@ -123,6 +125,22 @@ constexpr struct { {"tRNS alpha mask", "rgba-trns.png"}, }; +/* Shared among all plugins that implement data copying optimizations */ +const struct { + const char* name; + bool(*open)(AbstractImporter&, Containers::ArrayView); +} OpenMemoryData[]{ + {"data", [](AbstractImporter& importer, Containers::ArrayView data) { + /* Copy to ensure the original memory isn't referenced */ + Containers::Array copy{NoInit, data.size()}; + Utility::copy(Containers::arrayCast(data), copy); + return importer.openData(copy); + }}, + {"memory", [](AbstractImporter& importer, Containers::ArrayView data) { + return importer.openMemory(data); + }}, +}; + PngImporterTest::PngImporterTest() { addTests({&PngImporterTest::empty}); @@ -146,6 +164,9 @@ PngImporterTest::PngImporterTest() { addInstancedTests({&PngImporterTest::rgba}, Containers::arraySize(RgbaData)); + addInstancedTests({&PngImporterTest::openMemory}, + Containers::arraySize(OpenMemoryData)); + addTests({&PngImporterTest::openTwice, &PngImporterTest::importTwice}); @@ -335,6 +356,29 @@ void PngImporterTest::rgba() { }), TestSuite::Compare::Container); } +void PngImporterTest::openMemory() { + /* Same as gray16() except that it uses openData() & openMemory() instead + of openFile() to test data copying on import */ + + auto&& data = OpenMemoryData[testCaseInstanceId()]; + setTestCaseDescription(data.name); + + Containers::Pointer importer = _manager.instantiate("PngImporter"); + Containers::Array memory = Utility::Directory::read(Utility::Directory::join(PNGIMPORTER_TEST_DIR, "gray16.png")); + CORRADE_VERIFY(data.open(*importer, memory)); + + Containers::Optional image = importer->image2D(0); + CORRADE_VERIFY(image); + CORRADE_COMPARE(image->size(), Vector2i(2, 3)); + CORRADE_COMPARE(image->format(), PixelFormat::R16Unorm); + + CORRADE_COMPARE_AS(image->pixels().asContiguous(), Containers::arrayView({ + 1, 2, + 3, 4, + 5, 6 + }), TestSuite::Compare::Container); +} + void PngImporterTest::openTwice() { Containers::Pointer importer = _manager.instantiate("PngImporter"); diff --git a/src/MagnumPlugins/StbImageImporter/StbImageImporter.cpp b/src/MagnumPlugins/StbImageImporter/StbImageImporter.cpp index f48df1d30..305e4f8b7 100644 --- a/src/MagnumPlugins/StbImageImporter/StbImageImporter.cpp +++ b/src/MagnumPlugins/StbImageImporter/StbImageImporter.cpp @@ -76,7 +76,7 @@ void StbImageImporter::doClose() { _in = nullptr; } -void StbImageImporter::doOpenData(const Containers::ArrayView data) { +void StbImageImporter::doOpenData(Containers::Array&& data, const 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 @@ -137,9 +137,14 @@ void StbImageImporter::doOpenData(const Containers::ArrayView data) } } + /* Take over the existing array or copy the data if we can't */ _in.emplace(); - _in->data = Containers::Array{data.size()}; - Utility::copy(data, _in->data); + if(dataFlags & (DataFlag::Owned|DataFlag::ExternallyOwned)) { + _in->data = std::move(data); + } else { + _in->data = Containers::Array{NoInit, data.size()}; + Utility::copy(data, _in->data); + } } const void* StbImageImporter::doImporterState() const { @@ -233,4 +238,4 @@ Containers::Optional StbImageImporter::doImage2D(const UnsignedInt }} CORRADE_PLUGIN_REGISTER(StbImageImporter, Magnum::Trade::StbImageImporter, - "cz.mosra.magnum.Trade.AbstractImporter/0.3.3") + "cz.mosra.magnum.Trade.AbstractImporter/0.3.4") diff --git a/src/MagnumPlugins/StbImageImporter/StbImageImporter.h b/src/MagnumPlugins/StbImageImporter/StbImageImporter.h index 19743c131..8c4acd608 100644 --- a/src/MagnumPlugins/StbImageImporter/StbImageImporter.h +++ b/src/MagnumPlugins/StbImageImporter/StbImageImporter.h @@ -210,7 +210,7 @@ class MAGNUM_STBIMAGEIMPORTER_EXPORT StbImageImporter: public AbstractImporter { MAGNUM_STBIMAGEIMPORTER_LOCAL ImporterFeatures doFeatures() const override; MAGNUM_STBIMAGEIMPORTER_LOCAL bool doIsOpened() const override; MAGNUM_STBIMAGEIMPORTER_LOCAL void doClose() override; - MAGNUM_STBIMAGEIMPORTER_LOCAL void doOpenData(Containers::ArrayView data) override; + MAGNUM_STBIMAGEIMPORTER_LOCAL void doOpenData(Containers::Array&& data, DataFlags dataFlags) override; MAGNUM_STBIMAGEIMPORTER_LOCAL UnsignedInt doImage2DCount() const override; MAGNUM_STBIMAGEIMPORTER_LOCAL Containers::Optional doImage2D(UnsignedInt id, UnsignedInt level) override; diff --git a/src/MagnumPlugins/StbImageImporter/Test/StbImageImporterTest.cpp b/src/MagnumPlugins/StbImageImporter/Test/StbImageImporterTest.cpp index 70de167e9..829c1bbdd 100644 --- a/src/MagnumPlugins/StbImageImporter/Test/StbImageImporterTest.cpp +++ b/src/MagnumPlugins/StbImageImporter/Test/StbImageImporterTest.cpp @@ -28,6 +28,7 @@ #include #include #include +#include #include #include #include @@ -71,6 +72,7 @@ struct StbImageImporterTest: TestSuite::Tester { void animatedGif(); + void openMemory(); void openTwice(); void importTwice(); @@ -90,6 +92,22 @@ const struct { {"CgBI BGRA", "rgba-iphone.png"} }; +/* Shared among all plugins that implement data copying optimizations */ +const struct { + const char* name; + bool(*open)(AbstractImporter&, Containers::ArrayView); +} OpenMemoryData[]{ + {"data", [](AbstractImporter& importer, Containers::ArrayView data) { + /* Copy to ensure the original memory isn't referenced */ + Containers::Array copy{NoInit, data.size()}; + Utility::copy(Containers::arrayCast(data), copy); + return importer.openData(copy); + }}, + {"memory", [](AbstractImporter& importer, Containers::ArrayView data) { + return importer.openMemory(data); + }}, +}; + StbImageImporterTest::StbImageImporterTest() { addTests({&StbImageImporterTest::empty, &StbImageImporterTest::invalid, @@ -113,9 +131,12 @@ StbImageImporterTest::StbImageImporterTest() { addInstancedTests({&StbImageImporterTest::rgbaPng}, Containers::arraySize(RgbaPngTestData)); - addTests({&StbImageImporterTest::animatedGif, + addTests({&StbImageImporterTest::animatedGif}); + + addInstancedTests({&StbImageImporterTest::openMemory}, + Containers::arraySize(OpenMemoryData)); - &StbImageImporterTest::openTwice, + addTests({&StbImageImporterTest::openTwice, &StbImageImporterTest::importTwice}); #ifndef CORRADE_TARGET_EMSCRIPTEN @@ -479,6 +500,28 @@ void StbImageImporterTest::animatedGif() { } } +void StbImageImporterTest::openMemory() { + /* Same as grayPng() except that it uses openData() & openMemory() instead + of openFile() to test data copying on import */ + + auto&& data = OpenMemoryData[testCaseInstanceId()]; + setTestCaseDescription(data.name); + + Containers::Pointer importer = _manager.instantiate("StbImageImporter"); + Containers::Array memory = Utility::Directory::read(Utility::Directory::join(PNGIMPORTER_TEST_DIR, "gray.png")); + CORRADE_VERIFY(data.open(*importer, memory)); + + Containers::Optional image = importer->image2D(0); + CORRADE_VERIFY(image); + CORRADE_COMPARE(image->storage().alignment(), 1); + CORRADE_COMPARE(image->size(), Vector2i(3, 2)); + CORRADE_COMPARE(image->format(), PixelFormat::R8Unorm); + CORRADE_COMPARE_AS(image->data(), Containers::arrayView({ + '\xff', '\x88', '\x00', + '\x88', '\x00', '\xff' + }), TestSuite::Compare::Container); +} + void StbImageImporterTest::openTwice() { Containers::Pointer importer = _manager.instantiate("StbImageImporter"); From d121d1d3791bbc29d6a518c2c76db2c6823f3f59 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Thu, 21 Oct 2021 00:29:56 +0200 Subject: [PATCH 7/9] StanfordImporter,StlImporter: adapt to doOpenData() changes. In those two I already went further to avoid the data copy on file opening, which was mostly for import speed reasons. Good to see that code is now obsoleted by an API available at a higher level. --- .../StanfordImporter/StanfordImporter.cpp | 32 ++++------ .../StanfordImporter/StanfordImporter.h | 4 +- .../Test/StanfordImporterTest.cpp | 53 ++++++++++++---- src/MagnumPlugins/StlImporter/StlImporter.cpp | 27 +++------ src/MagnumPlugins/StlImporter/StlImporter.h | 4 +- .../StlImporter/Test/StlImporterTest.cpp | 60 +++++++++++++++---- 6 files changed, 113 insertions(+), 67 deletions(-) diff --git a/src/MagnumPlugins/StanfordImporter/StanfordImporter.cpp b/src/MagnumPlugins/StanfordImporter/StanfordImporter.cpp index 6d7413369..4db23c562 100644 --- a/src/MagnumPlugins/StanfordImporter/StanfordImporter.cpp +++ b/src/MagnumPlugins/StanfordImporter/StanfordImporter.cpp @@ -74,21 +74,6 @@ bool StanfordImporter::doIsOpened() const { return !!_state; } void StanfordImporter::doClose() { _state = nullptr; } -void StanfordImporter::doOpenFile(const std::string& filename) { - if(!Utility::Directory::exists(filename)) { - Error{} << "Trade::StanfordImporter::openFile(): cannot open file" << filename; - return; - } - - openDataInternal(Utility::Directory::read(filename)); -} - -void StanfordImporter::doOpenData(Containers::ArrayView data) { - Containers::Array copy{NoInit, data.size()}; - Utility::copy(data, copy); - openDataInternal(std::move(copy)); -} - namespace { enum class PropertyType { @@ -188,7 +173,7 @@ template bool checkVectorAttributeValidity(const Math::Vector< } -void StanfordImporter::openDataInternal(Containers::Array&& data) { +void StanfordImporter::doOpenData(Containers::Array&& data, const 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 @@ -201,9 +186,18 @@ void StanfordImporter::openDataInternal(Containers::Array&& data) { return; } + /* Take over the existing array or copy the data if we can't */ + Containers::Array dataCopy; + if(dataFlags & (DataFlag::Owned|DataFlag::ExternallyOwned)) { + dataCopy = std::move(data); + } else { + dataCopy = Containers::Array{NoInit, data.size()}; + Utility::copy(data, dataCopy); + } + /* Initialize the state */ auto state = Containers::pointer(); - Containers::ArrayView in = data; + Containers::ArrayView in = dataCopy; /* Check file signature */ { @@ -651,7 +645,7 @@ void StanfordImporter::openDataInternal(Containers::Array&& data) { /* All good, move the data to the state struct and save it. Remember header size so we can directly access the binary data in doMesh(). */ - state->data = std::move(data); + state->data = std::move(dataCopy); state->headerSize = state->data.size() - in.size(); _state = std::move(state); } @@ -897,4 +891,4 @@ MeshAttribute StanfordImporter::doMeshAttributeForName(const std::string& name) }} CORRADE_PLUGIN_REGISTER(StanfordImporter, Magnum::Trade::StanfordImporter, - "cz.mosra.magnum.Trade.AbstractImporter/0.3.3") + "cz.mosra.magnum.Trade.AbstractImporter/0.3.4") diff --git a/src/MagnumPlugins/StanfordImporter/StanfordImporter.h b/src/MagnumPlugins/StanfordImporter/StanfordImporter.h index 998814867..4997ba0b6 100644 --- a/src/MagnumPlugins/StanfordImporter/StanfordImporter.h +++ b/src/MagnumPlugins/StanfordImporter/StanfordImporter.h @@ -207,9 +207,7 @@ class MAGNUM_STANFORDIMPORTER_EXPORT StanfordImporter: public AbstractImporter { MAGNUM_STANFORDIMPORTER_LOCAL ImporterFeatures doFeatures() const override; MAGNUM_STANFORDIMPORTER_LOCAL bool doIsOpened() const override; - MAGNUM_STANFORDIMPORTER_LOCAL void doOpenFile(const std::string& filename) override; - MAGNUM_STANFORDIMPORTER_LOCAL void doOpenData(Containers::ArrayView data) override; - MAGNUM_STANFORDIMPORTER_LOCAL void openDataInternal(Containers::Array&& data); + MAGNUM_STANFORDIMPORTER_LOCAL void doOpenData(Containers::Array&& data, DataFlags dataFlags) override; MAGNUM_STANFORDIMPORTER_LOCAL void doClose() override; diff --git a/src/MagnumPlugins/StanfordImporter/Test/StanfordImporterTest.cpp b/src/MagnumPlugins/StanfordImporter/Test/StanfordImporterTest.cpp index 51100804b..1b3b89252 100644 --- a/src/MagnumPlugins/StanfordImporter/Test/StanfordImporterTest.cpp +++ b/src/MagnumPlugins/StanfordImporter/Test/StanfordImporterTest.cpp @@ -30,6 +30,7 @@ #include #include #include +#include #include #include #include @@ -47,7 +48,6 @@ struct StanfordImporterTest: TestSuite::Tester { explicit StanfordImporterTest(); void invalid(); - void fileNotFound(); void fileEmpty(); void fileTooShort(); @@ -64,6 +64,7 @@ struct StanfordImporterTest: TestSuite::Tester { void triangleFastPath(); void triangleFastPathPerFaceToPerVertex(); + void openMemory(); void openTwice(); void importTwice(); @@ -248,12 +249,27 @@ constexpr struct { {"disabled", false} }; +/* Shared among all plugins that implement data copying optimizations */ +const struct { + const char* name; + bool(*open)(AbstractImporter&, Containers::ArrayView); +} OpenMemoryData[]{ + {"data", [](AbstractImporter& importer, Containers::ArrayView data) { + /* Copy to ensure the original memory isn't referenced */ + Containers::Array copy{NoInit, data.size()}; + Utility::copy(Containers::arrayCast(data), copy); + return importer.openData(copy); + }}, + {"memory", [](AbstractImporter& importer, Containers::ArrayView data) { + return importer.openMemory(data); + }}, +}; + StanfordImporterTest::StanfordImporterTest() { addInstancedTests({&StanfordImporterTest::invalid}, Containers::arraySize(InvalidData)); - addTests({&StanfordImporterTest::fileNotFound, - &StanfordImporterTest::fileEmpty}); + addTests({&StanfordImporterTest::fileEmpty}); addInstancedTests({&StanfordImporterTest::fileTooShort}, Containers::arraySize(ShortFileData)); @@ -279,6 +295,9 @@ StanfordImporterTest::StanfordImporterTest() { &StanfordImporterTest::triangleFastPathPerFaceToPerVertex}, Containers::arraySize(FastTrianglePathData)); + addInstancedTests({&StanfordImporterTest::openMemory}, + Containers::arraySize(OpenMemoryData)); + addTests({&StanfordImporterTest::openTwice, &StanfordImporterTest::importTwice}); @@ -307,15 +326,6 @@ void StanfordImporterTest::invalid() { data.message)); } -void StanfordImporterTest::fileNotFound() { - Containers::Pointer importer = _manager.instantiate("StanfordImporter"); - - std::ostringstream out; - Error redirectError{&out}; - CORRADE_VERIFY(!importer->openFile("nonexistent.ply")); - CORRADE_COMPARE(out.str(), Utility::formatString("Trade::StanfordImporter::openFile(): cannot open file nonexistent.ply\n")); -} - void StanfordImporterTest::fileEmpty() { Containers::Pointer importer = _manager.instantiate("StanfordImporter"); @@ -1005,6 +1015,25 @@ void StanfordImporterTest::triangleFastPathPerFaceToPerVertex() { }), TestSuite::Compare::Container); } +void StanfordImporterTest::openMemory() { + /* Same as (a subset of) parse() except that it uses openData() & + openMemory() instead of openFile() to test data copying on import */ + + auto&& data = OpenMemoryData[testCaseInstanceId()]; + setTestCaseDescription(data.name); + + Containers::Pointer importer = _manager.instantiate("StanfordImporter"); + Containers::Array memory = Utility::Directory::read(Utility::Directory::join(STANFORDIMPORTER_TEST_DIR, "positions-float-indices-uint.ply")); + CORRADE_VERIFY(data.open(*importer, memory)); + CORRADE_COMPARE(importer->meshCount(), 1); + + auto mesh = importer->mesh(0); + CORRADE_VERIFY(mesh); + CORRADE_COMPARE_AS(mesh->attribute(MeshAttribute::Position), + Containers::arrayView(Positions), + TestSuite::Compare::Container); +} + void StanfordImporterTest::openTwice() { Containers::Pointer importer = _manager.instantiate("StanfordImporter"); diff --git a/src/MagnumPlugins/StlImporter/StlImporter.cpp b/src/MagnumPlugins/StlImporter/StlImporter.cpp index 44c1aadc0..21c5d67b3 100644 --- a/src/MagnumPlugins/StlImporter/StlImporter.cpp +++ b/src/MagnumPlugins/StlImporter/StlImporter.cpp @@ -51,28 +51,13 @@ bool StlImporter::doIsOpened() const { return !!_in; } void StlImporter::doClose() { _in = Containers::NullOpt; } -void StlImporter::doOpenFile(const std::string& filename) { - if(!Utility::Directory::exists(filename)) { - Error{} << "Trade::StlImporter::openFile(): cannot open file" << filename; - return; - } - - openDataInternal(Utility::Directory::read(filename)); -} - -void StlImporter::doOpenData(Containers::ArrayView data) { - Containers::Array copy{NoInit, data.size()}; - Utility::copy(data, copy); - openDataInternal(std::move(copy)); -} - namespace { /* In the input file, the triangle is represented by 12 floats (3D normal followed by three 3D vertices) and 2 extra bytes. */ constexpr std::ptrdiff_t InputTriangleStride = 12*4 + 2; } -void StlImporter::openDataInternal(Containers::Array&& data) { +void StlImporter::doOpenData(Containers::Array&& data, const DataFlags dataFlags) { /* At this point we can't even check if it's an ASCII or binary file, bail out */ if(data.size() < 5) { @@ -97,7 +82,13 @@ void StlImporter::openDataInternal(Containers::Array&& data) { return; } - _in = std::move(data); + /* Take over the existing array or copy the data if we can't */ + if(dataFlags & (DataFlag::Owned|DataFlag::ExternallyOwned)) { + _in = std::move(data); + } else { + _in = Containers::Array{NoInit, data.size()}; + Utility::copy(data, *_in); + } } UnsignedInt StlImporter::doMeshCount() const { return 1; } @@ -196,4 +187,4 @@ Containers::Optional StlImporter::doMesh(UnsignedInt, UnsignedInt leve }} CORRADE_PLUGIN_REGISTER(StlImporter, Magnum::Trade::StlImporter, - "cz.mosra.magnum.Trade.AbstractImporter/0.3.3") + "cz.mosra.magnum.Trade.AbstractImporter/0.3.4") diff --git a/src/MagnumPlugins/StlImporter/StlImporter.h b/src/MagnumPlugins/StlImporter/StlImporter.h index f7d695f64..b4b4a283d 100644 --- a/src/MagnumPlugins/StlImporter/StlImporter.h +++ b/src/MagnumPlugins/StlImporter/StlImporter.h @@ -134,9 +134,7 @@ class MAGNUM_STLIMPORTER_EXPORT StlImporter: public AbstractImporter { MAGNUM_STLIMPORTER_LOCAL ImporterFeatures doFeatures() const override; MAGNUM_STLIMPORTER_LOCAL bool doIsOpened() const override; - MAGNUM_STLIMPORTER_LOCAL void doOpenFile(const std::string& filename) override; - MAGNUM_STLIMPORTER_LOCAL void doOpenData(Containers::ArrayView data) override; - MAGNUM_STLIMPORTER_LOCAL void openDataInternal(Containers::Array&& data); + MAGNUM_STLIMPORTER_LOCAL void doOpenData(Containers::Array&& data, DataFlags dataFlags) override; MAGNUM_STLIMPORTER_LOCAL void doClose() override; MAGNUM_STLIMPORTER_LOCAL UnsignedInt doMeshCount() const override; diff --git a/src/MagnumPlugins/StlImporter/Test/StlImporterTest.cpp b/src/MagnumPlugins/StlImporter/Test/StlImporterTest.cpp index 17e7c4f10..2894d16f2 100644 --- a/src/MagnumPlugins/StlImporter/Test/StlImporterTest.cpp +++ b/src/MagnumPlugins/StlImporter/Test/StlImporterTest.cpp @@ -28,6 +28,7 @@ #include #include #include +#include #include #include #include @@ -44,12 +45,12 @@ struct StlImporterTest: TestSuite::Tester { explicit StlImporterTest(); void invalid(); - void fileNotFound(); void ascii(); void almostAsciiButNotActually(); void emptyBinary(); void binary(); + void openMemory(); void openTwice(); void importTwice(); @@ -115,18 +116,36 @@ const struct { false, 1, 2, MeshPrimitive::Faces, 2, 1, false, false} }; +/* Shared among all plugins that implement data copying optimizations */ +const struct { + const char* name; + bool(*open)(AbstractImporter&, Containers::ArrayView); +} OpenMemoryData[]{ + {"data", [](AbstractImporter& importer, Containers::ArrayView data) { + /* Copy to ensure the original memory isn't referenced */ + Containers::Array copy{NoInit, data.size()}; + Utility::copy(Containers::arrayCast(data), copy); + return importer.openData(copy); + }}, + {"memory", [](AbstractImporter& importer, Containers::ArrayView data) { + return importer.openMemory(data); + }}, +}; + StlImporterTest::StlImporterTest() { addInstancedTests({&StlImporterTest::invalid}, Containers::arraySize(InvalidData)); - addTests({&StlImporterTest::fileNotFound, - &StlImporterTest::ascii, + addTests({&StlImporterTest::ascii, &StlImporterTest::almostAsciiButNotActually, &StlImporterTest::emptyBinary}); addInstancedTests({&StlImporterTest::binary}, Containers::arraySize(BinaryData)); + addInstancedTests({&StlImporterTest::openMemory}, + Containers::arraySize(OpenMemoryData)); + addTests({&StlImporterTest::openTwice, &StlImporterTest::importTwice}); @@ -150,15 +169,6 @@ void StlImporterTest::invalid() { Utility::formatString("Trade::StlImporter::openData(): {}\n", data.message)); } -void StlImporterTest::fileNotFound() { - Containers::Pointer importer = _manager.instantiate("StlImporter"); - - std::ostringstream out; - Error redirectError{&out}; - CORRADE_VERIFY(!importer->openFile("nonexistent.stl")); - CORRADE_COMPARE(out.str(), Utility::formatString("Trade::StlImporter::openFile(): cannot open file nonexistent.stl\n")); -} - void StlImporterTest::ascii() { Containers::Pointer importer = _manager.instantiate("StlImporter"); @@ -273,6 +283,32 @@ void StlImporterTest::binary() { } else CORRADE_INTERNAL_ASSERT_UNREACHABLE(); } +void StlImporterTest::openMemory() { + /* Same as (a subset of) binary() except that it uses openData() & + openMemory() instead of openFile() to test data copying on import */ + + auto&& data = OpenMemoryData[testCaseInstanceId()]; + setTestCaseDescription(data.name); + + Containers::Pointer importer = _manager.instantiate("StlImporter"); + Containers::Array memory = Utility::Directory::read(Utility::Directory::join(STLIMPORTER_TEST_DIR, "binary.stl")); + CORRADE_VERIFY(data.open(*importer, memory)); + CORRADE_COMPARE(importer->meshCount(), 1); + + Containers::Optional mesh = importer->mesh(0); + CORRADE_VERIFY(mesh); + CORRADE_COMPARE_AS(mesh->attribute(MeshAttribute::Position), + Containers::arrayView({ + {1.0f, 2.0f, 3.0f}, + {4.0f, 5.0f, 6.0f}, + {7.0f, 8.0f, 9.0f}, + + {1.1f, 2.1f, 3.1f}, + {4.1f, 5.1f, 6.1f}, + {7.1f, 8.1f, 9.1f} + }), TestSuite::Compare::Container); +} + void StlImporterTest::openTwice() { Containers::Pointer importer = _manager.instantiate("StlImporter"); From af1121828e3080e0800472bdc2862f627bca2fd9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Thu, 21 Oct 2021 00:31:21 +0200 Subject: [PATCH 8/9] DevIlImageImporter: this comment makes no sense, kinda. --- src/MagnumPlugins/DevIlImageImporter/DevIlImageImporter.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/MagnumPlugins/DevIlImageImporter/DevIlImageImporter.cpp b/src/MagnumPlugins/DevIlImageImporter/DevIlImageImporter.cpp index 8da612c5f..adbc0c210 100644 --- a/src/MagnumPlugins/DevIlImageImporter/DevIlImageImporter.cpp +++ b/src/MagnumPlugins/DevIlImageImporter/DevIlImageImporter.cpp @@ -71,7 +71,7 @@ void DevIlImageImporter::doOpenData(Containers::Array&& data, DataFlags) { ilBindImage(image); /* The documentation doesn't state if the data needs to stay in scope. - Let's assume so to avoid a copy on the importer side. */ + Let's assume it doesn't. */ if(!ilLoadL(configuration().value("type", Utility::ConfigurationValueFlag::Hex), data.begin(), data.size())) { /* iluGetString() returns empty string for 0x512, which is even more useless than just returning the error ID */ From 7a1b1a8571822eb56fc77625cb04e51fc930ac2b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Sat, 23 Oct 2021 23:09:55 +0200 Subject: [PATCH 9/9] JpegImporter,PngImporter: ah, can't const here, sorry. Amazing, the 1970s weren't good. But also, thanks for finally fixing this in the new versions. --- src/MagnumPlugins/JpegImporter/JpegImporter.cpp | 3 ++- src/MagnumPlugins/PngImporter/PngImporter.cpp | 5 +++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/MagnumPlugins/JpegImporter/JpegImporter.cpp b/src/MagnumPlugins/JpegImporter/JpegImporter.cpp index 5a9623cc5..c925c600a 100644 --- a/src/MagnumPlugins/JpegImporter/JpegImporter.cpp +++ b/src/MagnumPlugins/JpegImporter/JpegImporter.cpp @@ -116,7 +116,8 @@ Containers::Optional JpegImporter::doImage2D(UnsignedInt, UnsignedI /* Open file */ jpeg_create_decompress(&file); - jpeg_mem_src(&file, reinterpret_cast(_in.begin()), _in.size()); + /* Older libjpegs want a mutable pointer, can't const here */ + jpeg_mem_src(&file, reinterpret_cast(_in.begin()), _in.size()); /* Read file header, start decompression. On macOS (Travis, with Xcode 7.3) the compilation fails because "no known conversion from 'bool' to diff --git a/src/MagnumPlugins/PngImporter/PngImporter.cpp b/src/MagnumPlugins/PngImporter/PngImporter.cpp index d4b1ab387..e466a77ac 100644 --- a/src/MagnumPlugins/PngImporter/PngImporter.cpp +++ b/src/MagnumPlugins/PngImporter/PngImporter.cpp @@ -88,8 +88,9 @@ Containers::Optional PngImporter::doImage2D(UnsignedInt, UnsignedIn CORRADE_ASSERT(std::strcmp(PNG_LIBPNG_VER_STRING, png_libpng_ver) == 0, "Trade::PngImporter::image2D(): libpng version mismatch, got" << png_libpng_ver << "but expected" << PNG_LIBPNG_VER_STRING, Containers::NullOpt); - /* Verify file signature */ - if(png_sig_cmp(reinterpret_cast(_in.data()), 0, Math::min(std::size_t(8), _in.size())) != 0) { + /* Verify file signature. Older libpngs want a mutable pointer, can't + const. */ + if(png_sig_cmp(reinterpret_cast(_in.data()), 0, Math::min(std::size_t(8), _in.size())) != 0) { Error() << "Trade::PngImporter::image2D(): wrong file signature"; return Containers::NullOpt; }