From 06e039dbe7fd4c2e95960b97b2001822c79d54b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Thu, 8 Apr 2021 13:41:44 +0200 Subject: [PATCH] Trade: reverse AbstractSceneConverter::convertToFile() argument order. It should be input first, output second, like with all other APIs. I remember I was trying something else here, but that didn't really make sense in the end. Also took that opportunity to get rid of one std::string. The original signature is a deprecated alias to the new one and will be removed in a future release. --- doc/changelog.dox | 4 ++++ src/Magnum/MeshTools/sceneconverter.cpp | 2 +- src/Magnum/Trade/AbstractSceneConverter.cpp | 16 +++++++++---- src/Magnum/Trade/AbstractSceneConverter.h | 23 ++++++++++++++----- .../Trade/Test/AbstractSceneConverterTest.cpp | 16 +++++++------ .../AnySceneConverter/AnySceneConverter.cpp | 8 ++++--- .../AnySceneConverter/AnySceneConverter.h | 2 +- .../Test/AnySceneConverterTest.cpp | 5 ++-- 8 files changed, 52 insertions(+), 24 deletions(-) diff --git a/doc/changelog.dox b/doc/changelog.dox index ba9d293a0d..93ee9a8e89 100644 --- a/doc/changelog.dox +++ b/doc/changelog.dox @@ -477,6 +477,10 @@ See also: @ref MAGNUM_BUILD_DEPRECATED is enabled, the returned type acts as a @ref Corrade::Containers::Optional but has (deprecated) implicit conversion to a @ref Corrade::Containers::Pointer to avoid breaking user code. +- The signature of @ref Trade::AbstractSceneConverter::convertToFile() was + changed to have input first and output second, for consistency with other + interfaces, together with a switch to @ref Containers::StringView. The + original signature is marked as deprecated. - @cpp Vk::hasVkFormat(Magnum::VertexFormat) @ce, @cpp Vk::hasVkFormat(Magnum::PixelFormat) @ce, @cpp Vk::hasVkFormat(Magnum::CompressedPixelFormat) @ce, diff --git a/src/Magnum/MeshTools/sceneconverter.cpp b/src/Magnum/MeshTools/sceneconverter.cpp index 0bdbc45cb0..770f943fc2 100644 --- a/src/Magnum/MeshTools/sceneconverter.cpp +++ b/src/Magnum/MeshTools/sceneconverter.cpp @@ -778,7 +778,7 @@ used.)") Debug{} << "Saving output with" << converterName << Debug::nospace << "..."; Duration d{conversionTime}; - if(!converter->convertToFile(args.value("output"), *mesh)) { + if(!converter->convertToFile(*mesh, args.value("output"))) { Error{} << "Cannot save file" << args.value("output"); return 5; } diff --git a/src/Magnum/Trade/AbstractSceneConverter.cpp b/src/Magnum/Trade/AbstractSceneConverter.cpp index 7807c8d0a2..7af8a68cf5 100644 --- a/src/Magnum/Trade/AbstractSceneConverter.cpp +++ b/src/Magnum/Trade/AbstractSceneConverter.cpp @@ -28,6 +28,8 @@ #include #include #include +#include /* Needed for Directory */ +#include #include #include @@ -43,7 +45,7 @@ namespace Magnum { namespace Trade { std::string AbstractSceneConverter::pluginInterface() { return /* [interface] */ -"cz.mosra.magnum.Trade.AbstractSceneConverter/0.1" +"cz.mosra.magnum.Trade.AbstractSceneConverter/0.1.1" /* [interface] */ ; } @@ -139,14 +141,20 @@ Containers::Array AbstractSceneConverter::doConvertToData(const MeshData&) CORRADE_ASSERT_UNREACHABLE("Trade::AbstractSceneConverter::convertToData(): mesh conversion advertised but not implemented", {}); } -bool AbstractSceneConverter::convertToFile(const std::string& filename, const MeshData& mesh) { +bool AbstractSceneConverter::convertToFile(const MeshData& mesh, const Containers::StringView filename) { CORRADE_ASSERT(features() >= SceneConverterFeature::ConvertMeshToFile, "Trade::AbstractSceneConverter::convertToFile(): mesh conversion not supported", {}); - return doConvertToFile(filename, mesh); + return doConvertToFile(mesh, filename); } -bool AbstractSceneConverter::doConvertToFile(const std::string& filename, const MeshData& mesh) { +#ifdef MAGNUM_BUILD_DEPRECATED +bool AbstractSceneConverter::convertToFile(const std::string& filename, const MeshData& mesh) { + return convertToFile(mesh, filename); +} +#endif + +bool AbstractSceneConverter::doConvertToFile(const MeshData& mesh, const Containers::StringView filename) { CORRADE_ASSERT(features() >= SceneConverterFeature::ConvertMeshToData, "Trade::AbstractSceneConverter::convertToFile(): mesh conversion advertised but not implemented", false); const auto data = doConvertToData(mesh); diff --git a/src/Magnum/Trade/AbstractSceneConverter.h b/src/Magnum/Trade/AbstractSceneConverter.h index 9b2d7aae3d..b5587f4af8 100644 --- a/src/Magnum/Trade/AbstractSceneConverter.h +++ b/src/Magnum/Trade/AbstractSceneConverter.h @@ -59,7 +59,7 @@ enum class SceneConverterFeature: UnsignedByte { /** * Converting a mesh to a file with - * @ref AbstractSceneConverter::convertToFile(const std::string&, const MeshData&). + * @ref AbstractSceneConverter::convertToFile(const MeshData&, Containers::StringView). */ ConvertMeshToFile = 1 << 2, @@ -171,8 +171,9 @@ checked by the implementation: @ref SceneConverterFeature::ConvertMeshInPlace is supported. - The function @ref doConvertToData(const MeshData&) is called only if @ref SceneConverterFeature::ConvertMeshToData is supported. -- The function @ref doConvertToFile(const std::string&, const MeshData&) is - called only if @ref SceneConverterFeature::ConvertMeshToFile is supported. +- The function @ref doConvertToFile(const MeshData&, Containers::StringView) + is called only if @ref SceneConverterFeature::ConvertMeshToFile is + supported. @m_class{m-block m-warning} @@ -298,6 +299,7 @@ class MAGNUM_TRADE_EXPORT AbstractSceneConverter: public PluginManager::Abstract /** * @brief Convert a mesh to a file + * @m_since_latest * * Available only if @ref SceneConverterFeature::ConvertMeshToFile or * @ref SceneConverterFeature::ConvertMeshToData is supported. Returns @@ -305,7 +307,16 @@ class MAGNUM_TRADE_EXPORT AbstractSceneConverter: public PluginManager::Abstract * @cpp false @ce otherwise. * @see @ref features(), @ref convertToData() */ - bool convertToFile(const std::string& filename, const MeshData& mesh); + bool convertToFile(const MeshData& mesh, Containers::StringView filename); + + #ifdef MAGNUM_BUILD_DEPRECATED + /** + * @brief @copybrief convertToFile(const MeshData&, Containers::StringView) + * @m_deprecated_since_latest Use @ref convertToFile(const MeshData&, Containers::StringView) + * instead. + */ + CORRADE_DEPRECATED("use convertToFile(const MeshData&, Containers::StringView) instead") bool convertToFile(const std::string& filename, const MeshData& mesh); + #endif private: /** @@ -340,13 +351,13 @@ class MAGNUM_TRADE_EXPORT AbstractSceneConverter: public PluginManager::Abstract virtual Containers::Array doConvertToData(const MeshData& mesh); /** - * @brief Implementation for @ref convertToFile(const std::string&, const MeshData&) + * @brief Implementation for @ref convertToFile(const MeshData&, Containers::StringView) * * If @ref SceneConverterFeature::ConvertMeshToData is supported, * default implementation calls @ref doConvertToData(const MeshData&) * and saves the result to given file. */ - virtual bool doConvertToFile(const std::string& filename, const MeshData& mesh); + virtual bool doConvertToFile(const MeshData& mesh, Containers::StringView filename); SceneConverterFlags _flags; }; diff --git a/src/Magnum/Trade/Test/AbstractSceneConverterTest.cpp b/src/Magnum/Trade/Test/AbstractSceneConverterTest.cpp index cbf1baecbc..6aca0b7ba5 100644 --- a/src/Magnum/Trade/Test/AbstractSceneConverterTest.cpp +++ b/src/Magnum/Trade/Test/AbstractSceneConverterTest.cpp @@ -24,6 +24,8 @@ */ #include +#include +#include #include #include #include @@ -192,7 +194,7 @@ void AbstractSceneConverterTest::thingNotSupported() { converter.convert(mesh); converter.convertInPlace(mesh); converter.convertToData(mesh); - converter.convertToFile(Utility::Directory::join(TRADE_TEST_OUTPUT_DIR, "mesh.out"), mesh); + converter.convertToFile(mesh, Utility::Directory::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" @@ -440,7 +442,7 @@ void AbstractSceneConverterTest::convertMeshToFile() { struct: AbstractSceneConverter { SceneConverterFeatures doFeatures() const override { return SceneConverterFeature::ConvertMeshToFile; } - bool doConvertToFile(const std::string& filename, const MeshData& mesh) override { + bool doConvertToFile(const MeshData& mesh, Containers::StringView filename) override { return Utility::Directory::write(filename, Containers::arrayView( {char(mesh.vertexCount())})); } } converter; @@ -451,7 +453,7 @@ void AbstractSceneConverterTest::convertMeshToFile() { Utility::Directory::rm(filename); CORRADE_VERIFY(!Utility::Directory::exists(filename)); - CORRADE_VERIFY(converter.convertToFile(filename, MeshData{MeshPrimitive::Triangles, 0xef})); + CORRADE_VERIFY(converter.convertToFile(MeshData{MeshPrimitive::Triangles, 0xef}, filename)); CORRADE_COMPARE_AS(filename, "\xef", TestSuite::Compare::FileToString); } @@ -472,7 +474,7 @@ void AbstractSceneConverterTest::convertMeshToFileThroughData() { CORRADE_VERIFY(!Utility::Directory::exists(filename)); /* doConvertToFile() should call doConvertToData() */ - CORRADE_VERIFY(converter.convertToFile(filename, MeshData{MeshPrimitive::Triangles, 0xef})); + CORRADE_VERIFY(converter.convertToFile(MeshData{MeshPrimitive::Triangles, 0xef}, filename)); CORRADE_COMPARE_AS(filename, "\xef", TestSuite::Compare::FileToString); } @@ -496,7 +498,7 @@ void AbstractSceneConverterTest::convertMeshToFileThroughDataFailed() { should be printed (the base implementation assumes the plugin does it) */ std::ostringstream out; Error redirectError{&out}; - CORRADE_VERIFY(!converter.convertToFile(filename, MeshData{MeshPrimitive::Triangles, 0xef})); + CORRADE_VERIFY(!converter.convertToFile(MeshData{MeshPrimitive::Triangles, 0xef}, filename)); CORRADE_VERIFY(!Utility::Directory::exists(filename)); CORRADE_COMPARE(out.str(), ""); } @@ -512,7 +514,7 @@ void AbstractSceneConverterTest::convertMeshToFileThroughDataNotWritable() { std::ostringstream out; Error redirectError{&out}; - CORRADE_VERIFY(!converter.convertToFile("/some/path/that/does/not/exist", MeshData{MeshPrimitive::Triangles, 0xef})); + CORRADE_VERIFY(!converter.convertToFile(MeshData{MeshPrimitive::Triangles, 0xef}, "/some/path/that/does/not/exist")); CORRADE_COMPARE(out.str(), "Utility::Directory::write(): can't open /some/path/that/does/not/exist\n" "Trade::AbstractSceneConverter::convertToFile(): cannot write to file /some/path/that/does/not/exist\n"); @@ -529,7 +531,7 @@ void AbstractSceneConverterTest::convertMeshToFileNotImplemented() { std::ostringstream out; Error redirectError{&out}; - converter.convertToFile(Utility::Directory::join(TRADE_TEST_OUTPUT_DIR, "mesh.out"), MeshData{MeshPrimitive::Triangles, 6}); + converter.convertToFile(MeshData{MeshPrimitive::Triangles, 6}, Utility::Directory::join(TRADE_TEST_OUTPUT_DIR, "mesh.out")); CORRADE_COMPARE(out.str(), "Trade::AbstractSceneConverter::convertToFile(): mesh conversion advertised but not implemented\n"); } diff --git a/src/MagnumPlugins/AnySceneConverter/AnySceneConverter.cpp b/src/MagnumPlugins/AnySceneConverter/AnySceneConverter.cpp index 2dd20c1d31..b6bd015f35 100644 --- a/src/MagnumPlugins/AnySceneConverter/AnySceneConverter.cpp +++ b/src/MagnumPlugins/AnySceneConverter/AnySceneConverter.cpp @@ -25,6 +25,8 @@ #include "AnySceneConverter.h" +#include +#include #include #include #include @@ -45,7 +47,7 @@ SceneConverterFeatures AnySceneConverter::doFeatures() const { return SceneConverterFeature::ConvertMeshToFile; } -bool AnySceneConverter::doConvertToFile(const std::string& filename, const MeshData& mesh) { +bool AnySceneConverter::doConvertToFile(const MeshData& mesh, const Containers::StringView filename) { CORRADE_INTERNAL_ASSERT(manager()); /** @todo lowercase only the extension, once Directory::split() is done */ @@ -80,10 +82,10 @@ bool AnySceneConverter::doConvertToFile(const std::string& filename, const MeshD /* Try to convert the file (error output should be printed by the plugin itself) */ - return converter->convertToFile(filename, mesh); + return converter->convertToFile(mesh, filename); } }} CORRADE_PLUGIN_REGISTER(AnySceneConverter, Magnum::Trade::AnySceneConverter, - "cz.mosra.magnum.Trade.AbstractSceneConverter/0.1") + "cz.mosra.magnum.Trade.AbstractSceneConverter/0.1.1") diff --git a/src/MagnumPlugins/AnySceneConverter/AnySceneConverter.h b/src/MagnumPlugins/AnySceneConverter/AnySceneConverter.h index 0c613534d8..4dce14d30a 100644 --- a/src/MagnumPlugins/AnySceneConverter/AnySceneConverter.h +++ b/src/MagnumPlugins/AnySceneConverter/AnySceneConverter.h @@ -104,7 +104,7 @@ class MAGNUM_ANYSCENECONVERTER_EXPORT AnySceneConverter: public AbstractSceneCon private: MAGNUM_ANYSCENECONVERTER_LOCAL SceneConverterFeatures doFeatures() const override; - MAGNUM_ANYSCENECONVERTER_LOCAL bool doConvertToFile(const std::string& filename, const MeshData& mesh) override; + MAGNUM_ANYSCENECONVERTER_LOCAL bool doConvertToFile(const MeshData& mesh, Containers::StringView filename) override; }; }} diff --git a/src/MagnumPlugins/AnySceneConverter/Test/AnySceneConverterTest.cpp b/src/MagnumPlugins/AnySceneConverter/Test/AnySceneConverterTest.cpp index 8cc02c5d84..e545d5dbf1 100644 --- a/src/MagnumPlugins/AnySceneConverter/Test/AnySceneConverterTest.cpp +++ b/src/MagnumPlugins/AnySceneConverter/Test/AnySceneConverterTest.cpp @@ -24,6 +24,7 @@ */ #include +#include #include #include #include @@ -92,7 +93,7 @@ void AnySceneConverterTest::detect() { std::ostringstream out; Error redirectError{&out}; - CORRADE_VERIFY(!converter->convertToFile(data.filename, MeshData{MeshPrimitive::Triangles, 0})); + CORRADE_VERIFY(!converter->convertToFile(MeshData{MeshPrimitive::Triangles, 0}, data.filename)); /* Can't use raw string literals in macros on GCC 4.8 */ #ifndef CORRADE_PLUGINMANAGER_NO_DYNAMIC_PLUGIN_SUPPORT CORRADE_COMPARE(out.str(), Utility::formatString( @@ -108,7 +109,7 @@ void AnySceneConverterTest::unknown() { Error redirectError{&output}; Containers::Pointer converter = _manager.instantiate("AnySceneConverter"); - CORRADE_VERIFY(!converter->convertToFile("mesh.obj", MeshData{MeshPrimitive::Triangles, 0})); + CORRADE_VERIFY(!converter->convertToFile(MeshData{MeshPrimitive::Triangles, 0}, "mesh.obj")); CORRADE_COMPARE(output.str(), "Trade::AnySceneConverter::convertToFile(): cannot determine the format of mesh.obj\n"); }