Browse Source

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.
pull/504/head
Vladimír Vondruš 5 years ago
parent
commit
06e039dbe7
  1. 4
      doc/changelog.dox
  2. 2
      src/Magnum/MeshTools/sceneconverter.cpp
  3. 16
      src/Magnum/Trade/AbstractSceneConverter.cpp
  4. 23
      src/Magnum/Trade/AbstractSceneConverter.h
  5. 16
      src/Magnum/Trade/Test/AbstractSceneConverterTest.cpp
  6. 8
      src/MagnumPlugins/AnySceneConverter/AnySceneConverter.cpp
  7. 2
      src/MagnumPlugins/AnySceneConverter/AnySceneConverter.h
  8. 5
      src/MagnumPlugins/AnySceneConverter/Test/AnySceneConverterTest.cpp

4
doc/changelog.dox

@ -477,6 +477,10 @@ See also:
@ref MAGNUM_BUILD_DEPRECATED is enabled, the returned type acts as a @ref MAGNUM_BUILD_DEPRECATED is enabled, the returned type acts as a
@ref Corrade::Containers::Optional but has (deprecated) implicit conversion @ref Corrade::Containers::Optional but has (deprecated) implicit conversion
to a @ref Corrade::Containers::Pointer to avoid breaking user code. 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::VertexFormat) @ce,
@cpp Vk::hasVkFormat(Magnum::PixelFormat) @ce, @cpp Vk::hasVkFormat(Magnum::PixelFormat) @ce,
@cpp Vk::hasVkFormat(Magnum::CompressedPixelFormat) @ce, @cpp Vk::hasVkFormat(Magnum::CompressedPixelFormat) @ce,

2
src/Magnum/MeshTools/sceneconverter.cpp

@ -778,7 +778,7 @@ used.)")
Debug{} << "Saving output with" << converterName << Debug::nospace << "..."; Debug{} << "Saving output with" << converterName << Debug::nospace << "...";
Duration d{conversionTime}; Duration d{conversionTime};
if(!converter->convertToFile(args.value("output"), *mesh)) { if(!converter->convertToFile(*mesh, args.value("output"))) {
Error{} << "Cannot save file" << args.value("output"); Error{} << "Cannot save file" << args.value("output");
return 5; return 5;
} }

16
src/Magnum/Trade/AbstractSceneConverter.cpp

@ -28,6 +28,8 @@
#include <Corrade/Containers/Array.h> #include <Corrade/Containers/Array.h>
#include <Corrade/Containers/EnumSet.hpp> #include <Corrade/Containers/EnumSet.hpp>
#include <Corrade/Containers/Optional.h> #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/DebugStl.h>
#include <Corrade/Utility/Directory.h> #include <Corrade/Utility/Directory.h>
@ -43,7 +45,7 @@ namespace Magnum { namespace Trade {
std::string AbstractSceneConverter::pluginInterface() { std::string AbstractSceneConverter::pluginInterface() {
return return
/* [interface] */ /* [interface] */
"cz.mosra.magnum.Trade.AbstractSceneConverter/0.1" "cz.mosra.magnum.Trade.AbstractSceneConverter/0.1.1"
/* [interface] */ /* [interface] */
; ;
} }
@ -139,14 +141,20 @@ Containers::Array<char> AbstractSceneConverter::doConvertToData(const MeshData&)
CORRADE_ASSERT_UNREACHABLE("Trade::AbstractSceneConverter::convertToData(): mesh conversion advertised but not implemented", {}); 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, CORRADE_ASSERT(features() >= SceneConverterFeature::ConvertMeshToFile,
"Trade::AbstractSceneConverter::convertToFile(): mesh conversion not supported", {}); "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); CORRADE_ASSERT(features() >= SceneConverterFeature::ConvertMeshToData, "Trade::AbstractSceneConverter::convertToFile(): mesh conversion advertised but not implemented", false);
const auto data = doConvertToData(mesh); const auto data = doConvertToData(mesh);

23
src/Magnum/Trade/AbstractSceneConverter.h

@ -59,7 +59,7 @@ enum class SceneConverterFeature: UnsignedByte {
/** /**
* Converting a mesh to a file with * 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, ConvertMeshToFile = 1 << 2,
@ -171,8 +171,9 @@ checked by the implementation:
@ref SceneConverterFeature::ConvertMeshInPlace is supported. @ref SceneConverterFeature::ConvertMeshInPlace is supported.
- The function @ref doConvertToData(const MeshData&) is called only if - The function @ref doConvertToData(const MeshData&) is called only if
@ref SceneConverterFeature::ConvertMeshToData is supported. @ref SceneConverterFeature::ConvertMeshToData is supported.
- The function @ref doConvertToFile(const std::string&, const MeshData&) is - The function @ref doConvertToFile(const MeshData&, Containers::StringView)
called only if @ref SceneConverterFeature::ConvertMeshToFile is supported. is called only if @ref SceneConverterFeature::ConvertMeshToFile is
supported.
@m_class{m-block m-warning} @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 * @brief Convert a mesh to a file
* @m_since_latest
* *
* Available only if @ref SceneConverterFeature::ConvertMeshToFile or * Available only if @ref SceneConverterFeature::ConvertMeshToFile or
* @ref SceneConverterFeature::ConvertMeshToData is supported. Returns * @ref SceneConverterFeature::ConvertMeshToData is supported. Returns
@ -305,7 +307,16 @@ class MAGNUM_TRADE_EXPORT AbstractSceneConverter: public PluginManager::Abstract
* @cpp false @ce otherwise. * @cpp false @ce otherwise.
* @see @ref features(), @ref convertToData() * @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: private:
/** /**
@ -340,13 +351,13 @@ class MAGNUM_TRADE_EXPORT AbstractSceneConverter: public PluginManager::Abstract
virtual Containers::Array<char> doConvertToData(const MeshData& mesh); virtual Containers::Array<char> 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, * If @ref SceneConverterFeature::ConvertMeshToData is supported,
* default implementation calls @ref doConvertToData(const MeshData&) * default implementation calls @ref doConvertToData(const MeshData&)
* and saves the result to given file. * 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; SceneConverterFlags _flags;
}; };

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

@ -24,6 +24,8 @@
*/ */
#include <sstream> #include <sstream>
#include <Corrade/Containers/StringView.h>
#include <Corrade/Containers/StringStl.h>
#include <Corrade/Containers/Optional.h> #include <Corrade/Containers/Optional.h>
#include <Corrade/TestSuite/Tester.h> #include <Corrade/TestSuite/Tester.h>
#include <Corrade/TestSuite/Compare/Container.h> #include <Corrade/TestSuite/Compare/Container.h>
@ -192,7 +194,7 @@ void AbstractSceneConverterTest::thingNotSupported() {
converter.convert(mesh); converter.convert(mesh);
converter.convertInPlace(mesh); converter.convertInPlace(mesh);
converter.convertToData(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(), CORRADE_COMPARE(out.str(),
"Trade::AbstractSceneConverter::convert(): mesh conversion not supported\n" "Trade::AbstractSceneConverter::convert(): mesh conversion not supported\n"
"Trade::AbstractSceneConverter::convertInPlace(): mesh conversion not supported\n" "Trade::AbstractSceneConverter::convertInPlace(): mesh conversion not supported\n"
@ -440,7 +442,7 @@ void AbstractSceneConverterTest::convertMeshToFile() {
struct: AbstractSceneConverter { struct: AbstractSceneConverter {
SceneConverterFeatures doFeatures() const override { return SceneConverterFeature::ConvertMeshToFile; } 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())})); return Utility::Directory::write(filename, Containers::arrayView( {char(mesh.vertexCount())}));
} }
} converter; } converter;
@ -451,7 +453,7 @@ void AbstractSceneConverterTest::convertMeshToFile() {
Utility::Directory::rm(filename); Utility::Directory::rm(filename);
CORRADE_VERIFY(!Utility::Directory::exists(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, CORRADE_COMPARE_AS(filename,
"\xef", TestSuite::Compare::FileToString); "\xef", TestSuite::Compare::FileToString);
} }
@ -472,7 +474,7 @@ void AbstractSceneConverterTest::convertMeshToFileThroughData() {
CORRADE_VERIFY(!Utility::Directory::exists(filename)); CORRADE_VERIFY(!Utility::Directory::exists(filename));
/* doConvertToFile() should call doConvertToData() */ /* 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, CORRADE_COMPARE_AS(filename,
"\xef", TestSuite::Compare::FileToString); "\xef", TestSuite::Compare::FileToString);
} }
@ -496,7 +498,7 @@ void AbstractSceneConverterTest::convertMeshToFileThroughDataFailed() {
should be printed (the base implementation assumes the plugin does it) */ should be printed (the base implementation assumes the plugin does it) */
std::ostringstream out; std::ostringstream out;
Error redirectError{&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_VERIFY(!Utility::Directory::exists(filename));
CORRADE_COMPARE(out.str(), ""); CORRADE_COMPARE(out.str(), "");
} }
@ -512,7 +514,7 @@ void AbstractSceneConverterTest::convertMeshToFileThroughDataNotWritable() {
std::ostringstream out; std::ostringstream out;
Error redirectError{&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(), CORRADE_COMPARE(out.str(),
"Utility::Directory::write(): can't open /some/path/that/does/not/exist\n" "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"); "Trade::AbstractSceneConverter::convertToFile(): cannot write to file /some/path/that/does/not/exist\n");
@ -529,7 +531,7 @@ void AbstractSceneConverterTest::convertMeshToFileNotImplemented() {
std::ostringstream out; std::ostringstream out;
Error redirectError{&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"); CORRADE_COMPARE(out.str(), "Trade::AbstractSceneConverter::convertToFile(): mesh conversion advertised but not implemented\n");
} }

8
src/MagnumPlugins/AnySceneConverter/AnySceneConverter.cpp

@ -25,6 +25,8 @@
#include "AnySceneConverter.h" #include "AnySceneConverter.h"
#include <Corrade/Containers/StringView.h>
#include <Corrade/Containers/StringStl.h>
#include <Corrade/PluginManager/Manager.h> #include <Corrade/PluginManager/Manager.h>
#include <Corrade/PluginManager/PluginMetadata.h> #include <Corrade/PluginManager/PluginMetadata.h>
#include <Corrade/Utility/Assert.h> #include <Corrade/Utility/Assert.h>
@ -45,7 +47,7 @@ SceneConverterFeatures AnySceneConverter::doFeatures() const {
return SceneConverterFeature::ConvertMeshToFile; 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()); CORRADE_INTERNAL_ASSERT(manager());
/** @todo lowercase only the extension, once Directory::split() is done */ /** @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 /* Try to convert the file (error output should be printed by the plugin
itself) */ itself) */
return converter->convertToFile(filename, mesh); return converter->convertToFile(mesh, filename);
} }
}} }}
CORRADE_PLUGIN_REGISTER(AnySceneConverter, Magnum::Trade::AnySceneConverter, CORRADE_PLUGIN_REGISTER(AnySceneConverter, Magnum::Trade::AnySceneConverter,
"cz.mosra.magnum.Trade.AbstractSceneConverter/0.1") "cz.mosra.magnum.Trade.AbstractSceneConverter/0.1.1")

2
src/MagnumPlugins/AnySceneConverter/AnySceneConverter.h

@ -104,7 +104,7 @@ class MAGNUM_ANYSCENECONVERTER_EXPORT AnySceneConverter: public AbstractSceneCon
private: private:
MAGNUM_ANYSCENECONVERTER_LOCAL SceneConverterFeatures doFeatures() const override; 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;
}; };
}} }}

5
src/MagnumPlugins/AnySceneConverter/Test/AnySceneConverterTest.cpp

@ -24,6 +24,7 @@
*/ */
#include <sstream> #include <sstream>
#include <Corrade/Containers/StringView.h>
#include <Corrade/PluginManager/Manager.h> #include <Corrade/PluginManager/Manager.h>
#include <Corrade/TestSuite/Tester.h> #include <Corrade/TestSuite/Tester.h>
#include <Corrade/Utility/Directory.h> #include <Corrade/Utility/Directory.h>
@ -92,7 +93,7 @@ void AnySceneConverterTest::detect() {
std::ostringstream out; std::ostringstream out;
Error redirectError{&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 */ /* Can't use raw string literals in macros on GCC 4.8 */
#ifndef CORRADE_PLUGINMANAGER_NO_DYNAMIC_PLUGIN_SUPPORT #ifndef CORRADE_PLUGINMANAGER_NO_DYNAMIC_PLUGIN_SUPPORT
CORRADE_COMPARE(out.str(), Utility::formatString( CORRADE_COMPARE(out.str(), Utility::formatString(
@ -108,7 +109,7 @@ void AnySceneConverterTest::unknown() {
Error redirectError{&output}; Error redirectError{&output};
Containers::Pointer<AbstractSceneConverter> converter = _manager.instantiate("AnySceneConverter"); Containers::Pointer<AbstractSceneConverter> 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"); CORRADE_COMPARE(output.str(), "Trade::AnySceneConverter::convertToFile(): cannot determine the format of mesh.obj\n");
} }

Loading…
Cancel
Save