From c57325385f426bddd11540bc2ee8a549ef56a2ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Mon, 7 Mar 2022 20:12:56 +0100 Subject: [PATCH] *converter: port command-line utilities to the new Utility::Path. And resolve TODOs waiting for Directory::read() to stop being shitty. --- src/Magnum/SceneTools/sceneconverter.cpp | 14 +++---- src/Magnum/ShaderTools/shaderconverter.cpp | 17 +++++--- src/Magnum/Text/fontconverter.cpp | 14 +++---- .../TextureTools/distancefieldconverter.cpp | 10 ++--- src/Magnum/Trade/imageconverter.cpp | 41 +++++++++++-------- 5 files changed, 53 insertions(+), 43 deletions(-) diff --git a/src/Magnum/SceneTools/sceneconverter.cpp b/src/Magnum/SceneTools/sceneconverter.cpp index 8f28413a9..e380bd909 100644 --- a/src/Magnum/SceneTools/sceneconverter.cpp +++ b/src/Magnum/SceneTools/sceneconverter.cpp @@ -30,8 +30,8 @@ #include #include #include -#include #include +#include #include #include "Magnum/PixelFormat.h" @@ -309,8 +309,8 @@ is specified as well, the IDs reference attributes of the first mesh.)") } PluginManager::Manager importerManager{ - args.value("plugin-dir").empty() ? std::string{} : - Utility::Directory::join(args.value("plugin-dir"), Trade::AbstractImporter::pluginSearchPaths()[0])}; + args.value("plugin-dir").empty() ? Containers::String{} : + Utility::Path::join(args.value("plugin-dir"), Trade::AbstractImporter::pluginSearchPaths()[0])}; Containers::Pointer importer = importerManager.loadAndInstantiate(args.value("importer")); if(!importer) { @@ -327,10 +327,10 @@ is specified as well, the IDs reference attributes of the first mesh.)") /* Open the file or map it if requested */ #if defined(CORRADE_TARGET_UNIX) || (defined(CORRADE_TARGET_WINDOWS) && !defined(CORRADE_TARGET_WINDOWS_RT)) - Containers::Array mapped; + Containers::Optional> mapped; if(args.isSet("map")) { Trade::Implementation::Duration d{importTime}; - if(!(mapped = Utility::Directory::mapRead(args.value("input"))) || !importer->openMemory(mapped)) { + if(!(mapped = Utility::Path::mapRead(args.value("input"))) || !importer->openMemory(*mapped)) { Error() << "Cannot memory-map file" << args.value("input"); return 3; } @@ -1099,8 +1099,8 @@ is specified as well, the IDs reference attributes of the first mesh.)") /* Load converter plugin */ PluginManager::Manager converterManager{ - args.value("plugin-dir").empty() ? std::string{} : - Utility::Directory::join(args.value("plugin-dir"), Trade::AbstractSceneConverter::pluginSearchPaths()[0])}; + args.value("plugin-dir").empty() ? Containers::String{} : + Utility::Path::join(args.value("plugin-dir"), Trade::AbstractSceneConverter::pluginSearchPaths()[0])}; /* Assume there's always one passed --converter option less, and the last is implicitly AnySceneConverter. All converters except the last one are diff --git a/src/Magnum/ShaderTools/shaderconverter.cpp b/src/Magnum/ShaderTools/shaderconverter.cpp index 8d8c86a49..f0f9c2b8c 100644 --- a/src/Magnum/ShaderTools/shaderconverter.cpp +++ b/src/Magnum/ShaderTools/shaderconverter.cpp @@ -27,7 +27,7 @@ #include #include #include -#include +#include #include #include "Magnum/Implementation/converterUtilities.h" @@ -337,10 +337,15 @@ see documentation of a particular converter for more information.)") /* If we want just SPIR-V info and the input looks like a SPIR-V binary, do that right away without going through any plugin. If it doesn't, we - try again after using a converter.s */ + try again after using a converter. */ if(args.isSet("info")) { - const Containers::Array data = Utility::Directory::read(args.arrayValue("input", 0)); - if(Containers::ArrayView spirv = ShaderTools::Implementation::spirvData(data, data.size())) { + const Containers::Optional> data = Utility::Path::read(args.arrayValue("input", 0)); + if(!data) { + Error{} << "Can't read" << args.arrayValue("input", 0); + return 24; + } + + if(Containers::ArrayView spirv = ShaderTools::Implementation::spirvData(*data, data->size())) { printSpirvInfo(spirv); return 0; } @@ -348,8 +353,8 @@ see documentation of a particular converter for more information.)") /* Set up a converter manager */ PluginManager::Manager converterManager{ - args.value("plugin-dir").empty() ? std::string{} : - Utility::Directory::join(args.value("plugin-dir"), ShaderTools::AbstractConverter::pluginSearchPaths()[0])}; + args.value("plugin-dir").empty() ? Containers::String{} : + Utility::Path::join(args.value("plugin-dir"), ShaderTools::AbstractConverter::pluginSearchPaths()[0])}; /* Data passed from one converter to another in case there's more than one */ Containers::Array data; diff --git a/src/Magnum/Text/fontconverter.cpp b/src/Magnum/Text/fontconverter.cpp index 039c2d6ab..d1d2c3a0a 100644 --- a/src/Magnum/Text/fontconverter.cpp +++ b/src/Magnum/Text/fontconverter.cpp @@ -26,7 +26,7 @@ #include #include #include -#include +#include #include "Magnum/Math/ConfigurationValue.h" #include "Magnum/Text/AbstractFont.h" @@ -163,21 +163,21 @@ FontConverter::FontConverter(const Arguments& arguments): Platform::WindowlessAp int FontConverter::exec() { /* Font converter dependencies */ PluginManager::Manager imageConverterManager{ - args.value("plugin-dir").empty() ? std::string{} : - Utility::Directory::join(args.value("plugin-dir"), Trade::AbstractImageConverter::pluginSearchPaths()[0])}; + args.value("plugin-dir").empty() ? Containers::String{} : + Utility::Path::join(args.value("plugin-dir"), Trade::AbstractImageConverter::pluginSearchPaths()[0])}; /* Load font */ PluginManager::Manager fontManager{ - args.value("plugin-dir").empty() ? std::string{} : - Utility::Directory::join(args.value("plugin-dir"), Text::AbstractFont::pluginSearchPaths()[0])}; + args.value("plugin-dir").empty() ? Containers::String{} : + Utility::Path::join(args.value("plugin-dir"), Text::AbstractFont::pluginSearchPaths()[0])}; Containers::Pointer font = fontManager.loadAndInstantiate(args.value("font")); if(!font) return 1; /* Register the image converter manager for potential dependencies (MagnumFontConverter needs TgaImageConverter, for example) */ PluginManager::Manager converterManager{ - args.value("plugin-dir").empty() ? std::string{} : - Utility::Directory::join(args.value("plugin-dir"), Text::AbstractFontConverter::pluginSearchPaths()[0])}; + args.value("plugin-dir").empty() ? Containers::String{} : + Utility::Path::join(args.value("plugin-dir"), Text::AbstractFontConverter::pluginSearchPaths()[0])}; converterManager.registerExternalManager(imageConverterManager); /* Load font converter */ diff --git a/src/Magnum/TextureTools/distancefieldconverter.cpp b/src/Magnum/TextureTools/distancefieldconverter.cpp index 9e8799482..0c5c92ea4 100644 --- a/src/Magnum/TextureTools/distancefieldconverter.cpp +++ b/src/Magnum/TextureTools/distancefieldconverter.cpp @@ -26,7 +26,7 @@ #include #include #include -#include +#include #include #include "Magnum/Image.h" @@ -161,15 +161,15 @@ DistanceFieldConverter::DistanceFieldConverter(const Arguments& arguments): Plat int DistanceFieldConverter::exec() { /* Load importer plugin */ PluginManager::Manager importerManager{ - args.value("plugin-dir").empty() ? std::string{} : - Utility::Directory::join(args.value("plugin-dir"), Trade::AbstractImporter::pluginSearchPaths()[0])}; + args.value("plugin-dir").empty() ? Containers::String{} : + Utility::Path::join(args.value("plugin-dir"), Trade::AbstractImporter::pluginSearchPaths()[0])}; Containers::Pointer importer = importerManager.loadAndInstantiate(args.value("importer")); if(!importer) return 1; /* Load converter plugin */ PluginManager::Manager converterManager{ - args.value("plugin-dir").empty() ? std::string{} : - Utility::Directory::join(args.value("plugin-dir"), Trade::AbstractImageConverter::pluginSearchPaths()[0])}; + args.value("plugin-dir").empty() ? Containers::String{} : + Utility::Path::join(args.value("plugin-dir"), Trade::AbstractImageConverter::pluginSearchPaths()[0])}; Containers::Pointer converter = converterManager.loadAndInstantiate(args.value("converter")); if(!converter) return 2; diff --git a/src/Magnum/Trade/imageconverter.cpp b/src/Magnum/Trade/imageconverter.cpp index 9fdca270d..2c343b832 100644 --- a/src/Magnum/Trade/imageconverter.cpp +++ b/src/Magnum/Trade/imageconverter.cpp @@ -32,7 +32,7 @@ #include #include #include -#include +#include #include #include "Magnum/ImageView.h" @@ -362,8 +362,8 @@ key=true; configuration subgroups are delimited with /.)") } PluginManager::Manager importerManager{ - args.value("plugin-dir").empty() ? std::string{} : - Utility::Directory::join(args.value("plugin-dir"), Trade::AbstractImporter::pluginSearchPaths()[0])}; + args.value("plugin-dir").empty() ? Containers::String{} : + Utility::Path::join(args.value("plugin-dir"), Trade::AbstractImporter::pluginSearchPaths()[0])}; const Int dimensions = args.value("dimensions"); /** @todo make them array options as well? */ @@ -371,7 +371,7 @@ key=true; configuration subgroups are delimited with /.)") Containers::Optional level; if(!args.value("level").empty()) level = args.value("level"); #if defined(CORRADE_TARGET_UNIX) || (defined(CORRADE_TARGET_WINDOWS) && !defined(CORRADE_TARGET_WINDOWS_RT)) - Containers::Array> mapped; + Containers::Array> mapped; #endif Containers::Array images1D; Containers::Array images2D; @@ -401,12 +401,6 @@ key=true; configuration subgroups are delimited with /.)") return 4; } - /** @todo simplify once read() reliably returns an Optional */ - if(!Utility::Directory::exists(input)) { - Error{} << "Cannot open file" << input; - return 3; - } - /* Read the file or map it if requested */ Containers::Array data; #if defined(CORRADE_TARGET_UNIX) || (defined(CORRADE_TARGET_WINDOWS) && !defined(CORRADE_TARGET_WINDOWS_RT)) @@ -414,20 +408,28 @@ key=true; configuration subgroups are delimited with /.)") arrayAppend(mapped, InPlaceInit); Trade::Implementation::Duration d{importTime}; - if(!(mapped.back() = Utility::Directory::mapRead(input))) { + Containers::Optional> mappedMaybe = Utility::Path::mapRead(input); + if(!mappedMaybe) { Error() << "Cannot memory-map file" << input; return 3; } /* Fake a mutable array with a non-owning deleter to have the - same type as from Directory::read(). The actual memory is - owned by the `mapped` array. */ + same type as from Path::read(). The actual memory is owned + by the `mapped` array. */ + mapped.back() = *std::move(mappedMaybe); data = Containers::Array{const_cast(mapped.back().data()), mapped.back().size(), [](char*, std::size_t){}}; } else #endif { Trade::Implementation::Duration d{importTime}; - data = Utility::Directory::read(input); + Containers::Optional> dataMaybe = Utility::Path::read(input); + if(!dataMaybe) { + Error{} << "Cannot read file" << input; + return 3; + } + + data = *std::move(dataMaybe); } auto side = Int(std::sqrt(data.size()/pixelSize)); @@ -467,10 +469,13 @@ key=true; configuration subgroups are delimited with /.)") arrayAppend(mapped, InPlaceInit); Trade::Implementation::Duration d{importTime}; - if(!(mapped.back() = Utility::Directory::mapRead(input)) || !importer->openMemory(mapped.back())) { + Containers::Optional> mappedMaybe = Utility::Path::mapRead(input); + if(!mappedMaybe || !importer->openMemory(*mappedMaybe)) { Error() << "Cannot memory-map file" << input; return 3; } + + mapped.back() = *std::move(mappedMaybe); } else #endif { @@ -882,7 +887,7 @@ key=true; configuration subgroups are delimited with /.)") { Trade::Implementation::Duration d{conversionTime}; - if(!Utility::Directory::write(output, data)) return 1; + if(!Utility::Path::write(output, data)) return 1; } if(args.isSet("profile")) { @@ -895,8 +900,8 @@ key=true; configuration subgroups are delimited with /.)") /* Load converter plugin */ PluginManager::Manager converterManager{ - args.value("plugin-dir").empty() ? std::string{} : - Utility::Directory::join(args.value("plugin-dir"), Trade::AbstractImageConverter::pluginSearchPaths()[0])}; + args.value("plugin-dir").empty() ? Containers::String{} : + Utility::Path::join(args.value("plugin-dir"), Trade::AbstractImageConverter::pluginSearchPaths()[0])}; Containers::Pointer converter = converterManager.loadAndInstantiate(args.value("converter")); if(!converter) { Debug{} << "Available converter plugins:" << Utility::String::join(converterManager.aliasList(), ", ");