Browse Source

Trade: port to the new Utility::Path.

For file opening there's no longer an unatomic pair of exists() +
read(), but since Path::read() now returns an Optional, it means we can
reliably distinguish between empty files and failures.

While at it, also added TODOs for removal of the StringStl.h header
that's needed in various places for compatibility with APIs still using
STL strings.
pull/556/head
Vladimír Vondruš 4 years ago
parent
commit
98296567a7
  1. 15
      doc/snippets/MagnumTrade.cpp
  2. 31
      src/Magnum/Trade/AbstractImageConverter.cpp
  3. 12
      src/Magnum/Trade/AbstractImporter.cpp
  4. 12
      src/Magnum/Trade/AbstractSceneConverter.cpp
  5. 522
      src/Magnum/Trade/Test/AbstractImageConverterTest.cpp
  6. 12
      src/Magnum/Trade/Test/AbstractImporterTest.cpp
  7. 35
      src/Magnum/Trade/Test/AbstractSceneConverterTest.cpp

15
doc/snippets/MagnumTrade.cpp

@ -26,10 +26,11 @@
#include <unordered_map>
#include <Corrade/Containers/ArrayTuple.h>
#include <Corrade/Containers/Optional.h>
#include <Corrade/Containers/StringStl.h> /** @todo remove once file callbacks are <string>-free */
#include <Corrade/Containers/Pair.h>
#include <Corrade/Utility/Algorithms.h>
#include <Corrade/Utility/DebugStl.h>
#include <Corrade/Utility/Directory.h>
#include <Corrade/Utility/Path.h>
#include <Corrade/Utility/Resource.h>
#include "Magnum/FileCallback.h"
@ -162,8 +163,8 @@ if(!importer->openData(data)) /* or openMemory() */
Containers::Pointer<Trade::AbstractImporter> importer;
/* [AbstractImporter-usage-callbacks] */
struct Data {
std::unordered_map<std::string,
Containers::Array<const char, Utility::Directory::MapDeleter>> files;
std::unordered_map<std::string, Containers::Optional<
Containers::Array<const char, Utility::Path::MapDeleter>>> files;
} data;
importer->setFileCallback([](const std::string& filename,
@ -178,11 +179,13 @@ importer->setFileCallback([](const std::string& filename,
return {};
}
/* Load if not there yet */
/* Load if not there yet. If the mapping fails, remember that to not
attempt to load the same file again next time. */
if(found == data.files.end()) found = data.files.emplace(
filename, Utility::Directory::mapRead(filename)).first;
filename, Utility::Path::mapRead(filename)).first;
return Containers::arrayView(found->second);
if(!found->second) return {};
return Containers::arrayView(*found->second);
}, data);
importer->openFile("scene.gltf"); // memory-maps all files

31
src/Magnum/Trade/AbstractImageConverter.cpp

@ -29,9 +29,9 @@
#include <Corrade/Containers/EnumSet.hpp>
#include <Corrade/Containers/Optional.h>
#include <Corrade/Containers/StringView.h>
#include <Corrade/Containers/StringStl.h> /* for Directory */
#include <Corrade/Containers/StringStl.h> /** @todo remove once PluginManager is <string>-free */
#include <Corrade/Utility/Assert.h>
#include <Corrade/Utility/Directory.h>
#include <Corrade/Utility/Path.h>
#include <Corrade/Utility/DebugStl.h>
#include "Magnum/Image.h"
@ -55,9 +55,10 @@ std::string AbstractImageConverter::pluginInterface() {
#ifndef CORRADE_PLUGINMANAGER_NO_DYNAMIC_PLUGIN_SUPPORT
std::vector<std::string> AbstractImageConverter::pluginSearchPaths() {
const Containers::Optional<Containers::String> libraryLocation = Utility::Path::libraryLocation(&pluginInterface);
return PluginManager::implicitPluginSearchPaths(
#ifndef MAGNUM_BUILD_STATIC
Utility::Directory::libraryLocation(&pluginInterface),
libraryLocation ? *libraryLocation : Containers::String{},
#else
{},
#endif
@ -647,7 +648,7 @@ bool AbstractImageConverter::doConvertToFile(const ImageView1D& image, const Con
/* No deleter checks as it doesn't matter here */
if(!data) return false;
if(!Utility::Directory::write(filename, data)) {
if(!Utility::Path::write(filename, data)) {
Error() << "Trade::AbstractImageConverter::convertToFile(): cannot write to file" << filename;
return false;
}
@ -681,7 +682,7 @@ bool AbstractImageConverter::doConvertToFile(const ImageView2D& image, const Con
/* No deleter checks as it doesn't matter here */
if(!data) return false;
if(!Utility::Directory::write(filename, data)) {
if(!Utility::Path::write(filename, data)) {
Error() << "Trade::AbstractImageConverter::convertToFile(): cannot write to file" << filename;
return false;
}
@ -721,7 +722,7 @@ bool AbstractImageConverter::doConvertToFile(const ImageView3D& image, const Con
/* No deleter checks as it doesn't matter here */
if(!data) return false;
if(!Utility::Directory::write(filename, data)) {
if(!Utility::Path::write(filename, data)) {
Error() << "Trade::AbstractImageConverter::convertToFile(): cannot write to file" << filename;
return false;
}
@ -755,7 +756,7 @@ bool AbstractImageConverter::doConvertToFile(const CompressedImageView1D& image,
/* No deleter checks as it doesn't matter here */
if(!data) return false;
if(!Utility::Directory::write(filename, data)) {
if(!Utility::Path::write(filename, data)) {
Error() << "Trade::AbstractImageConverter::convertToFile(): cannot write to file" << filename;
return false;
}
@ -789,7 +790,7 @@ bool AbstractImageConverter::doConvertToFile(const CompressedImageView2D& image,
/* No deleter checks as it doesn't matter here */
if(!data) return false;
if(!Utility::Directory::write(filename, data)) {
if(!Utility::Path::write(filename, data)) {
Error() << "Trade::AbstractImageConverter::convertToFile(): cannot write to file" << filename;
return false;
}
@ -829,7 +830,7 @@ bool AbstractImageConverter::doConvertToFile(const CompressedImageView3D& image,
/* No deleter checks as it doesn't matter here */
if(!data) return false;
if(!Utility::Directory::write(filename, data)) {
if(!Utility::Path::write(filename, data)) {
Error() << "Trade::AbstractImageConverter::convertToFile(): cannot write to file" << filename;
return false;
}
@ -879,7 +880,7 @@ bool AbstractImageConverter::doConvertToFile(const Containers::ArrayView<const I
/* No deleter checks as it doesn't matter here */
if(!data) return false;
if(!Utility::Directory::write(filename, data)) {
if(!Utility::Path::write(filename, data)) {
Error() << "Trade::AbstractImageConverter::convertToFile(): cannot write to file" << filename;
return false;
}
@ -911,7 +912,7 @@ bool AbstractImageConverter::doConvertToFile(const Containers::ArrayView<const I
/* No deleter checks as it doesn't matter here */
if(!data) return false;
if(!Utility::Directory::write(filename, data)) {
if(!Utility::Path::write(filename, data)) {
Error() << "Trade::AbstractImageConverter::convertToFile(): cannot write to file" << filename;
return false;
}
@ -943,7 +944,7 @@ bool AbstractImageConverter::doConvertToFile(const Containers::ArrayView<const I
/* No deleter checks as it doesn't matter here */
if(!data) return false;
if(!Utility::Directory::write(filename, data)) {
if(!Utility::Path::write(filename, data)) {
Error() << "Trade::AbstractImageConverter::convertToFile(): cannot write to file" << filename;
return false;
}
@ -975,7 +976,7 @@ bool AbstractImageConverter::doConvertToFile(const Containers::ArrayView<const C
/* No deleter checks as it doesn't matter here */
if(!data) return false;
if(!Utility::Directory::write(filename, data)) {
if(!Utility::Path::write(filename, data)) {
Error() << "Trade::AbstractImageConverter::convertToFile(): cannot write to file" << filename;
return false;
}
@ -1007,7 +1008,7 @@ bool AbstractImageConverter::doConvertToFile(const Containers::ArrayView<const C
/* No deleter checks as it doesn't matter here */
if(!data) return false;
if(!Utility::Directory::write(filename, data)) {
if(!Utility::Path::write(filename, data)) {
Error() << "Trade::AbstractImageConverter::convertToFile(): cannot write to file" << filename;
return false;
}
@ -1039,7 +1040,7 @@ bool AbstractImageConverter::doConvertToFile(const Containers::ArrayView<const C
/* No deleter checks as it doesn't matter here */
if(!data) return false;
if(!Utility::Directory::write(filename, data)) {
if(!Utility::Path::write(filename, data)) {
Error() << "Trade::AbstractImageConverter::convertToFile(): cannot write to file" << filename;
return false;
}

12
src/Magnum/Trade/AbstractImporter.cpp

@ -28,9 +28,11 @@
#include <Corrade/Containers/Array.h>
#include <Corrade/Containers/EnumSet.hpp>
#include <Corrade/Containers/Optional.h>
#include <Corrade/Containers/String.h>
#include <Corrade/Containers/StringStl.h> /** @todo remove once PluginManager is <string>-free */
#include <Corrade/Utility/Assert.h>
#include <Corrade/Utility/DebugStl.h>
#include <Corrade/Utility/Directory.h>
#include <Corrade/Utility/Path.h>
#include "Magnum/FileCallback.h"
#include "Magnum/Trade/AnimationData.h"
@ -77,9 +79,10 @@ std::string AbstractImporter::pluginInterface() {
#ifndef CORRADE_PLUGINMANAGER_NO_DYNAMIC_PLUGIN_SUPPORT
std::vector<std::string> AbstractImporter::pluginSearchPaths() {
const Containers::Optional<Containers::String> libraryLocation = Utility::Path::libraryLocation(&pluginInterface);
return PluginManager::implicitPluginSearchPaths(
#ifndef MAGNUM_BUILD_STATIC
Utility::Directory::libraryLocation(&pluginInterface),
libraryLocation ? *libraryLocation : Containers::String{},
#else
{},
#endif
@ -249,12 +252,13 @@ void AbstractImporter::doOpenFile(const std::string& filename) {
/* Otherwise open the file directly */
} else {
if(!Utility::Directory::exists(filename)) {
Containers::Optional<Containers::Array<char>> data = Utility::Path::read(filename);
if(!data) {
Error() << "Trade::AbstractImporter::openFile(): cannot open file" << filename;
return;
}
doOpenData(Utility::Directory::read(filename), DataFlag::Owned|DataFlag::Mutable);
doOpenData(*std::move(data), DataFlag::Owned|DataFlag::Mutable);
}
}

12
src/Magnum/Trade/AbstractSceneConverter.cpp

@ -28,10 +28,9 @@
#include <Corrade/Containers/Array.h>
#include <Corrade/Containers/EnumSet.hpp>
#include <Corrade/Containers/Optional.h>
#include <Corrade/Containers/StringStl.h> /* Needed for Directory */
#include <Corrade/Containers/StringView.h>
#include <Corrade/Utility/DebugStl.h>
#include <Corrade/Utility/Directory.h>
#include <Corrade/Containers/String.h>
#include <Corrade/Containers/StringStl.h> /** @todo remove once PluginManager is <string>-free */
#include <Corrade/Utility/Path.h>
#include "Magnum/Trade/ArrayAllocator.h"
#include "Magnum/Trade/MeshData.h"
@ -52,9 +51,10 @@ std::string AbstractSceneConverter::pluginInterface() {
#ifndef CORRADE_PLUGINMANAGER_NO_DYNAMIC_PLUGIN_SUPPORT
std::vector<std::string> AbstractSceneConverter::pluginSearchPaths() {
const Containers::Optional<Containers::String> libraryLocation = Utility::Path::libraryLocation(&pluginInterface);
return PluginManager::implicitPluginSearchPaths(
#ifndef MAGNUM_BUILD_STATIC
Utility::Directory::libraryLocation(&pluginInterface),
libraryLocation ? *libraryLocation : Containers::String{},
#else
{},
#endif
@ -161,7 +161,7 @@ bool AbstractSceneConverter::doConvertToFile(const MeshData& mesh, const Contain
/* No deleter checks as it doesn't matter here */
if(!data) return false;
if(!Utility::Directory::write(filename, data)) {
if(!Utility::Path::write(filename, data)) {
Error() << "Trade::AbstractSceneConverter::convertToFile(): cannot write to file" << filename;
return false;
}

522
src/Magnum/Trade/Test/AbstractImageConverterTest.cpp

File diff suppressed because it is too large Load Diff

12
src/Magnum/Trade/Test/AbstractImporterTest.cpp

@ -26,11 +26,14 @@
#include <sstream>
#include <Corrade/Containers/ArrayView.h>
#include <Corrade/Containers/Optional.h>
#include <Corrade/Containers/String.h>
#include <Corrade/Containers/StringStl.h> /** @todo remove once Debug is stream-free and AbstractImporter is <string>-free */
#include <Corrade/TestSuite/Tester.h>
#include <Corrade/TestSuite/Compare/Container.h>
#include <Corrade/TestSuite/Compare/Numeric.h>
#include <Corrade/TestSuite/Compare/String.h>
#include <Corrade/Utility/DebugStl.h>
#include <Corrade/Utility/Directory.h>
#include <Corrade/Utility/Path.h>
#include "Magnum/PixelFormat.h"
#include "Magnum/FileCallback.h"
@ -831,7 +834,7 @@ void AbstractImporterTest::openFileAsData() {
/* doOpenFile() should call doOpenData() */
CORRADE_VERIFY(!importer.isOpened());
CORRADE_VERIFY(importer.openFile(Utility::Directory::join(TRADE_TEST_DIR, "file.bin")));
CORRADE_VERIFY(importer.openFile(Utility::Path::join(TRADE_TEST_DIR, "file.bin")));
CORRADE_VERIFY(importer.isOpened());
importer.close();
@ -856,7 +859,10 @@ void AbstractImporterTest::openFileAsDataNotFound() {
CORRADE_VERIFY(!importer.openFile("nonexistent.bin"));
CORRADE_VERIFY(!importer.isOpened());
CORRADE_COMPARE(out.str(), "Trade::AbstractImporter::openFile(): cannot open file nonexistent.bin\n");
/* There's an error message from Path::read() before */
CORRADE_COMPARE_AS(out.str(),
"\nTrade::AbstractImporter::openFile(): cannot open file nonexistent.bin\n",
TestSuite::Compare::StringHasSuffix);
}
void AbstractImporterTest::openFileNotImplemented() {

35
src/Magnum/Trade/Test/AbstractSceneConverterTest.cpp

@ -25,14 +25,14 @@
#include <sstream>
#include <Corrade/Containers/StringView.h>
#include <Corrade/Containers/StringStl.h>
#include <Corrade/Containers/StringStl.h> /** @todo remove once Debug is stream-free */
#include <Corrade/Containers/Optional.h>
#include <Corrade/TestSuite/Tester.h>
#include <Corrade/TestSuite/Compare/Container.h>
#include <Corrade/TestSuite/Compare/FileToString.h>
#include <Corrade/TestSuite/Compare/String.h>
#include <Corrade/Utility/DebugStl.h>
#include <Corrade/Utility/Directory.h>
#include <Corrade/Utility/Path.h>
#include "Magnum/Math/Vector3.h"
#include "Magnum/Trade/ArrayAllocator.h"
@ -117,7 +117,7 @@ AbstractSceneConverterTest::AbstractSceneConverterTest() {
&AbstractSceneConverterTest::debugFlags});
/* Create testing dir */
Utility::Directory::mkpath(TRADE_TEST_OUTPUT_DIR);
Utility::Path::make(TRADE_TEST_OUTPUT_DIR);
}
void AbstractSceneConverterTest::featuresNone() {
@ -197,7 +197,7 @@ void AbstractSceneConverterTest::thingNotSupported() {
converter.convert(mesh);
converter.convertInPlace(mesh);
converter.convertToData(mesh);
converter.convertToFile(mesh, Utility::Directory::join(TRADE_TEST_OUTPUT_DIR, "mesh.out"));
converter.convertToFile(mesh, Utility::Path::join(TRADE_TEST_OUTPUT_DIR, "mesh.out"));
CORRADE_COMPARE(out.str(),
"Trade::AbstractSceneConverter::convert(): mesh conversion not supported\n"
"Trade::AbstractSceneConverter::convertInPlace(): mesh conversion not supported\n"
@ -446,15 +446,14 @@ void AbstractSceneConverterTest::convertMeshToFile() {
SceneConverterFeatures doFeatures() const override { return SceneConverterFeature::ConvertMeshToFile; }
bool doConvertToFile(const MeshData& mesh, Containers::StringView filename) override {
return Utility::Directory::write(filename, Containers::arrayView( {char(mesh.vertexCount())}));
return Utility::Path::write(filename, Containers::arrayView( {char(mesh.vertexCount())}));
}
} converter;
const std::string filename = Utility::Directory::join(TRADE_TEST_OUTPUT_DIR, "mesh.out");
/* Remove previous file, if any */
if(Utility::Directory::exists(filename))
CORRADE_VERIFY(Utility::Directory::rm(filename));
Containers::String filename = Utility::Path::join(TRADE_TEST_OUTPUT_DIR, "mesh.out");
if(Utility::Path::exists(filename))
CORRADE_VERIFY(Utility::Path::remove(filename));
CORRADE_VERIFY(converter.convertToFile(MeshData{MeshPrimitive::Triangles, 0xef}, filename));
CORRADE_COMPARE_AS(filename,
@ -470,11 +469,10 @@ void AbstractSceneConverterTest::convertMeshToFileThroughData() {
}
} converter;
const std::string filename = Utility::Directory::join(TRADE_TEST_OUTPUT_DIR, "mesh.out");
/* Remove previous file, if any */
if(Utility::Directory::exists(filename))
CORRADE_VERIFY(Utility::Directory::rm(filename));
Containers::String filename = Utility::Path::join(TRADE_TEST_OUTPUT_DIR, "mesh.out");
if(Utility::Path::exists(filename))
CORRADE_VERIFY(Utility::Path::remove(filename));
/* doConvertToFile() should call doConvertToData() */
CORRADE_VERIFY(converter.convertToFile(MeshData{MeshPrimitive::Triangles, 0xef}, filename));
@ -491,18 +489,17 @@ void AbstractSceneConverterTest::convertMeshToFileThroughDataFailed() {
}
} converter;
const std::string filename = Utility::Directory::join(TRADE_TEST_OUTPUT_DIR, "mesh.out");
/* Remove previous file, if any */
if(Utility::Directory::exists(filename))
CORRADE_VERIFY(Utility::Directory::rm(filename));
Containers::String filename = Utility::Path::join(TRADE_TEST_OUTPUT_DIR, "mesh.out");
if(Utility::Path::exists(filename))
CORRADE_VERIFY(Utility::Path::remove(filename));
/* Function should fail, no file should get written and no error output
should be printed (the base implementation assumes the plugin does it) */
std::ostringstream out;
Error redirectError{&out};
CORRADE_VERIFY(!converter.convertToFile(MeshData{MeshPrimitive::Triangles, 0xef}, filename));
CORRADE_VERIFY(!Utility::Directory::exists(filename));
CORRADE_VERIFY(!Utility::Path::exists(filename));
CORRADE_COMPARE(out.str(), "");
}
@ -534,7 +531,7 @@ void AbstractSceneConverterTest::convertMeshToFileNotImplemented() {
std::ostringstream out;
Error redirectError{&out};
converter.convertToFile(MeshData{MeshPrimitive::Triangles, 6}, Utility::Directory::join(TRADE_TEST_OUTPUT_DIR, "mesh.out"));
converter.convertToFile(MeshData{MeshPrimitive::Triangles, 6}, Utility::Path::join(TRADE_TEST_OUTPUT_DIR, "mesh.out"));
CORRADE_COMPARE(out.str(), "Trade::AbstractSceneConverter::convertToFile(): mesh conversion advertised but not implemented\n");
}

Loading…
Cancel
Save