ic
/
magnum-bindings
6
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity
Browse Source

python: handle Windows path insanities in the bindings directly. 

Otherwise people wouldn't be able to conveniently use os.path, which is
an undesirable pain point. And do this for all currently exposed APIs,
not just the AbstractImporter that caused a problem.

This reverts commit 28497ec2ca.
next
Vladimír Vondruš 3 years ago
parent
b3e24a11b4
commit
a45fbb84da
5 changed files with 94 additions and 44 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Split View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 5
      doc/python/magnum.text.rst
  2. 15
      doc/python/magnum.trade.rst
  3. 60
      src/python/magnum/test/test_trade.py
  4. 18
      src/python/magnum/text.cpp
  5. 40
      src/python/magnum/trade.cpp

5
doc/python/magnum.text.rst
Unescape View File

@ -53,6 +53,11 @@
.. py:function:: magnum.text.AbstractFont.open_file
:raise RuntimeError: If file opening fails
For compatibility with :ref:`os.path`, on Windows this function converts
all backslashes in :p:`filename` to forward slashes before passing it to
:dox:`Text::AbstractFont::openFile()`, which expects forward slashes as
directory separators on all platforms.
.. py:property:: magnum.text.AbstractFont.size
:raise AssertionError: If no file is opened
.. py:property:: magnum.text.AbstractFont.ascent

15
doc/python/magnum.trade.rst
Unescape View File

@ -189,6 +189,11 @@
.. py:function:: magnum.trade.AbstractImporter.open_file
:raise RuntimeError: If file opening fails
For compatibility with :ref:`os.path`, on Windows this function converts
all backslashes in :p:`filename` to forward slashes before passing it to
:dox:`Trade::AbstractImporter::openFile()`, which expects forward slashes
as directory separators on all platforms.
.. py:property:: magnum.trade.AbstractImporter.mesh_count
:raise AssertionError: If no file is opened
.. py:function:: magnum.trade.AbstractImporter.mesh_level_count
@ -297,6 +302,11 @@
.. py:function:: magnum.trade.AbstractImageConverter.convert_to_file
:raise RuntimeError: If image conversion fails
For compatibility with :ref:`os.path`, on Windows this function converts
all backslashes in :p:`filename` to forward slashes before passing it to
:dox:`Trade::AbstractImageConverter::convertToFile()`, which expects
forward slashes as directory separators on all platforms.
.. py:class:: magnum.trade.SceneConverterManager
:summary: Manager for :ref:`AbstractSceneConverter` plugin instances
@ -324,3 +334,8 @@
.. py:function:: magnum.trade.AbstractSceneConverter.convert_to_file
:raise RuntimeError: If scene conversion fails
For compatibility with :ref:`os.path`, on Windows this function converts
all backslashes in :p:`filename` to forward slashes before passing it to
:dox:`Trade::AbstractSceneConverter::convertToFile()`, which expects
forward slashes as directory separators on all platforms.

60
src/python/magnum/test/test_trade.py
Unescape View File

@ -103,8 +103,7 @@ class ImageData(unittest.TestCase):
class MeshData(unittest.TestCase):
def test(self):
importer = trade.ImporterManager().load_and_instantiate('GltfImporter')
# TODO figure out a less silly way to get forward slashes on Windows
importer.open_file(os.path.join(os.path.dirname(__file__), 'mesh.gltf').replace('\\', '/'))
importer.open_file(os.path.join(os.path.dirname(__file__), 'mesh.gltf'))
mesh = importer.mesh(0)
self.assertEqual(mesh.primitive, MeshPrimitive.TRIANGLES)
@ -197,8 +196,7 @@ class MeshData(unittest.TestCase):
def test_index_data_access(self):
importer = trade.ImporterManager().load_and_instantiate('GltfImporter')
# TODO figure out a less silly way to get forward slashes on Windows
importer.open_file(os.path.join(os.path.dirname(__file__), 'mesh.gltf').replace('\\', '/'))
importer.open_file(os.path.join(os.path.dirname(__file__), 'mesh.gltf'))
mesh = importer.mesh(0)
mesh_refcount = sys.getrefcount(mesh)
@ -221,8 +219,7 @@ class MeshData(unittest.TestCase):
def test_vertex_data_access(self):
importer = trade.ImporterManager().load_and_instantiate('GltfImporter')
# TODO figure out a less silly way to get forward slashes on Windows
importer.open_file(os.path.join(os.path.dirname(__file__), 'mesh.gltf').replace('\\', '/'))
importer.open_file(os.path.join(os.path.dirname(__file__), 'mesh.gltf'))
mesh = importer.mesh(0)
mesh_refcount = sys.getrefcount(mesh)
@ -245,8 +242,7 @@ class MeshData(unittest.TestCase):
def test_indices_access(self):
importer = trade.ImporterManager().load_and_instantiate('GltfImporter')
# TODO figure out a less silly way to get forward slashes on Windows
importer.open_file(os.path.join(os.path.dirname(__file__), 'mesh.gltf').replace('\\', '/'))
importer.open_file(os.path.join(os.path.dirname(__file__), 'mesh.gltf'))
mesh = importer.mesh(0)
mesh_refcount = sys.getrefcount(mesh)
@ -275,8 +271,7 @@ class MeshData(unittest.TestCase):
def test_attribute_access(self):
importer = trade.ImporterManager().load_and_instantiate('GltfImporter')
# TODO figure out a less silly way to get forward slashes on Windows
importer.open_file(os.path.join(os.path.dirname(__file__), 'mesh.gltf').replace('\\', '/'))
importer.open_file(os.path.join(os.path.dirname(__file__), 'mesh.gltf'))
mesh = importer.mesh(0)
mesh_refcount = sys.getrefcount(mesh)
@ -336,8 +331,7 @@ class MeshData(unittest.TestCase):
def test_mutable_index_data_access(self):
importer = trade.ImporterManager().load_and_instantiate('GltfImporter')
# TODO figure out a less silly way to get forward slashes on Windows
importer.open_file(os.path.join(os.path.dirname(__file__), 'mesh.gltf').replace('\\', '/'))
importer.open_file(os.path.join(os.path.dirname(__file__), 'mesh.gltf'))
mesh = importer.mesh(0)
self.assertEqual(mesh.index_data_flags, trade.DataFlag.OWNED|trade.DataFlag.MUTABLE)
@ -354,8 +348,7 @@ class MeshData(unittest.TestCase):
def test_mutable_vertex_data_access(self):
importer = trade.ImporterManager().load_and_instantiate('GltfImporter')
# TODO figure out a less silly way to get forward slashes on Windows
importer.open_file(os.path.join(os.path.dirname(__file__), 'mesh.gltf').replace('\\', '/'))
importer.open_file(os.path.join(os.path.dirname(__file__), 'mesh.gltf'))
mesh = importer.mesh(0)
self.assertEqual(mesh.vertex_data_flags, trade.DataFlag.OWNED|trade.DataFlag.MUTABLE)
@ -372,8 +365,7 @@ class MeshData(unittest.TestCase):
def test_mutable_indices_access(self):
importer = trade.ImporterManager().load_and_instantiate('GltfImporter')
# TODO figure out a less silly way to get forward slashes on Windows
importer.open_file(os.path.join(os.path.dirname(__file__), 'mesh.gltf').replace('\\', '/'))
importer.open_file(os.path.join(os.path.dirname(__file__), 'mesh.gltf'))
mesh = importer.mesh(0)
self.assertEqual(mesh.index_data_flags, trade.DataFlag.OWNED|trade.DataFlag.MUTABLE)
@ -388,8 +380,7 @@ class MeshData(unittest.TestCase):
def test_mutable_attributes_access(self):
importer = trade.ImporterManager().load_and_instantiate('GltfImporter')
# TODO figure out a less silly way to get forward slashes on Windows
importer.open_file(os.path.join(os.path.dirname(__file__), 'mesh.gltf').replace('\\', '/'))
importer.open_file(os.path.join(os.path.dirname(__file__), 'mesh.gltf'))
mesh = importer.mesh(0)
self.assertEqual(mesh.index_data_flags, trade.DataFlag.OWNED|trade.DataFlag.MUTABLE)
@ -431,8 +422,7 @@ class MeshData(unittest.TestCase):
def test_nonindexed(self):
importer = trade.ImporterManager().load_and_instantiate('GltfImporter')
# TODO figure out a less silly way to get forward slashes on Windows
importer.open_file(os.path.join(os.path.dirname(__file__), 'mesh.gltf').replace('\\', '/'))
importer.open_file(os.path.join(os.path.dirname(__file__), 'mesh.gltf'))
mesh = importer.mesh(1)
self.assertFalse(mesh.is_indexed)
@ -456,8 +446,7 @@ class MeshData(unittest.TestCase):
def test_attribute_oob(self):
importer = trade.ImporterManager().load_and_instantiate('GltfImporter')
# TODO figure out a less silly way to get forward slashes on Windows
importer.open_file(os.path.join(os.path.dirname(__file__), 'mesh.gltf').replace('\\', '/'))
importer.open_file(os.path.join(os.path.dirname(__file__), 'mesh.gltf'))
mesh = importer.mesh(0)
@ -513,8 +502,7 @@ class MeshData(unittest.TestCase):
def test_attribute_access_array(self):
importer = trade.ImporterManager().load_and_instantiate('GltfImporter')
# TODO figure out a less silly way to get forward slashes on Windows
importer.open_file(os.path.join(os.path.dirname(__file__), 'mesh.gltf').replace('\\', '/'))
importer.open_file(os.path.join(os.path.dirname(__file__), 'mesh.gltf'))
mesh = importer.mesh(0)
joint_ids_id = mesh.attribute_id(trade.MeshAttribute.JOINT_IDS)
@ -530,8 +518,7 @@ class MeshData(unittest.TestCase):
def test_attribute_access_unsupported_format(self):
importer = trade.ImporterManager().load_and_instantiate('GltfImporter')
# TODO figure out a less silly way to get forward slashes on Windows
importer.open_file(os.path.join(os.path.dirname(__file__), 'mesh.gltf').replace('\\', '/'))
importer.open_file(os.path.join(os.path.dirname(__file__), 'mesh.gltf'))
mesh = importer.mesh(0)
custom_attribute_id = mesh.attribute_id(importer.mesh_attribute_for_name("_CUSTOM_ATTRIBUTE"))
@ -669,8 +656,7 @@ class Importer(unittest.TestCase):
self.assertIsNone(importer.mesh_attribute_name(trade.MeshAttribute(32768 + 7)))
self.assertIsNone(importer.mesh_attribute_for_name("_CUSTOM_ATTRIBUTE"))
# TODO figure out a less silly way to get forward slashes on Windows
importer.open_file(os.path.join(os.path.dirname(__file__), 'mesh.gltf').replace('\\', '/'))
importer.open_file(os.path.join(os.path.dirname(__file__), 'mesh.gltf'))
self.assertEqual(importer.mesh_count, 2)
self.assertEqual(importer.mesh_level_count(0), 1)
self.assertEqual(importer.mesh_name(0), 'Indexed mesh')
@ -693,32 +679,28 @@ class Importer(unittest.TestCase):
def test_mesh_level_oob(self):
importer = trade.ImporterManager().load_and_instantiate('GltfImporter')
# TODO figure out a less silly way to get forward slashes on Windows
importer.open_file(os.path.join(os.path.dirname(__file__), 'mesh.gltf').replace('\\', '/'))
importer.open_file(os.path.join(os.path.dirname(__file__), 'mesh.gltf'))
with self.assertRaises(IndexError):
importer.mesh(0, 1)
def test_mesh_by_name(self):
importer = trade.ImporterManager().load_and_instantiate('GltfImporter')
# TODO figure out a less silly way to get forward slashes on Windows
importer.open_file(os.path.join(os.path.dirname(__file__), 'mesh.gltf').replace('\\', '/'))
importer.open_file(os.path.join(os.path.dirname(__file__), 'mesh.gltf'))
mesh = importer.mesh('Non-indexed mesh')
self.assertEqual(mesh.primitive, MeshPrimitive.TRIANGLES)
def test_mesh_by_name_not_found(self):
importer = trade.ImporterManager().load_and_instantiate('GltfImporter')
# TODO figure out a less silly way to get forward slashes on Windows
importer.open_file(os.path.join(os.path.dirname(__file__), 'mesh.gltf').replace('\\', '/'))
importer.open_file(os.path.join(os.path.dirname(__file__), 'mesh.gltf'))
with self.assertRaises(KeyError):
importer.mesh('Nonexistent')
def test_mesh_by_name_level_oob(self):
importer = trade.ImporterManager().load_and_instantiate('GltfImporter')
# TODO figure out a less silly way to get forward slashes on Windows
importer.open_file(os.path.join(os.path.dirname(__file__), 'mesh.gltf').replace('\\', '/'))
importer.open_file(os.path.join(os.path.dirname(__file__), 'mesh.gltf'))
with self.assertRaises(IndexError):
importer.mesh('Non-indexed mesh', 1)
@ -798,8 +780,7 @@ class ImageConverter(unittest.TestCase):
class SceneConverter(unittest.TestCase):
def test_mesh(self):
importer = trade.ImporterManager().load_and_instantiate('GltfImporter')
# TODO figure out a less silly way to get forward slashes on Windows
importer.open_file(os.path.join(os.path.dirname(__file__), 'mesh.gltf').replace('\\', '/'))
importer.open_file(os.path.join(os.path.dirname(__file__), 'mesh.gltf'))
mesh = importer.mesh(1)
converter = trade.SceneConverterManager().load_and_instantiate('StanfordSceneConverter')
@ -810,8 +791,7 @@ class SceneConverter(unittest.TestCase):
def test_mesh_failed(self):
importer = trade.ImporterManager().load_and_instantiate('GltfImporter')
# TODO figure out a less silly way to get forward slashes on Windows
importer.open_file(os.path.join(os.path.dirname(__file__), 'mesh.gltf').replace('\\', '/'))
importer.open_file(os.path.join(os.path.dirname(__file__), 'mesh.gltf'))
mesh = importer.mesh(1)
converter = trade.SceneConverterManager().load_and_instantiate('AnySceneConverter')

18
src/python/magnum/text.cpp
Unescape View File

@ -33,6 +33,13 @@
#include "corrade/pluginmanager.h"
#include "magnum/bootstrap.h"
#ifdef CORRADE_TARGET_WINDOWS
/* To allow people to conveniently use Python's os.path, we need to convert
backslashes to forward slashes as all Corrade and Magnum APIs expect
forward */
#include <Corrade/Utility/Path.h>
#endif
namespace magnum {
namespace {
@ -109,7 +116,16 @@ void text(py::module_& m) {
.def("open_file", [](Text::AbstractFont& self, const std::string& filename, Float size) {
/** @todo log redirection -- but we'd need assertions to not be
part of that so when it dies, the user can still see why */
if(self.openFile(filename, size)) return;
if(self.openFile(
#ifdef CORRADE_TARGET_WINDOWS
/* To allow people to conveniently use Python's os.path, we
need to convert backslashes to forward slashes as all
Corrade and Magnum APIs expect forward */
Utility::Path::fromNativeSeparators(filename)
#else
filename
#endif
, size)) return;
PyErr_Format(PyExc_RuntimeError, "opening %s failed", filename.data());
throw py::error_already_set{};

40
src/python/magnum/trade.cpp
Unescape View File

@ -46,6 +46,13 @@
#include "corrade/pluginmanager.h"
#include "magnum/bootstrap.h"
#ifdef CORRADE_TARGET_WINDOWS
/* To allow people to conveniently use Python's os.path, we need to convert
backslashes to forward slashes as all Corrade and Magnum APIs expect
forward */
#include <Corrade/Utility/Path.h>
#endif
namespace magnum {
namespace {
@ -282,7 +289,16 @@ template<class R, Containers::Optional<R>(Trade::AbstractImporter::*f)(UnsignedI
template<class T, bool(Trade::AbstractImageConverter::*f)(const T&, Containers::StringView)> void checkImageConverterResult(Trade::AbstractImageConverter& self, const T& image, const std::string& filename) {
/** @todo log redirection -- but we'd need assertions to not be part of
that so when it dies, the user can still see why */
bool out = (self.*f)(image, filename);
bool out = (self.*f)(image,
#ifdef CORRADE_TARGET_WINDOWS
/* To allow people to conveniently use Python's os.path, we need to
convert backslashes to forward slashes as all Corrade and Magnum
APIs expect forward */
Utility::Path::fromNativeSeparators(filename)
#else
filename
#endif
);
if(!out) {
PyErr_SetString(PyExc_RuntimeError, "conversion failed");
throw py::error_already_set{};
@ -295,7 +311,16 @@ template<class T, bool(Trade::AbstractImageConverter::*f)(const T&, Containers::
template<class T, bool(Trade::AbstractSceneConverter::*f)(const T&, Containers::StringView)> void checkSceneConverterResult(Trade::AbstractSceneConverter& self, const T& mesh, const std::string& filename) {
/** @todo log redirection -- but we'd need assertions to not be part of
that so when it dies, the user can still see why */
bool out = (self.*f)(mesh, filename);
bool out = (self.*f)(mesh,
#ifdef CORRADE_TARGET_WINDOWS
/* To allow people to conveniently use Python's os.path, we need to
convert backslashes to forward slashes as all Corrade and Magnum
APIs expect forward */
Utility::Path::fromNativeSeparators(filename)
#else
filename
#endif
);
if(!out) {
PyErr_SetString(PyExc_RuntimeError, "conversion failed");
throw py::error_already_set{};
@ -691,7 +716,16 @@ void trade(py::module_& m) {
.def("open_file", [](Trade::AbstractImporter& self, const std::string& filename) {
/** @todo log redirection -- but we'd need assertions to not be
part of that so when it dies, the user can still see why */
if(self.openFile(filename)) return;
if(self.openFile(
#ifdef CORRADE_TARGET_WINDOWS
/* To allow people to conveniently use Python's os.path, we
need to convert backslashes to forward slashes as all
Corrade and Magnum APIs expect forward */
Utility::Path::fromNativeSeparators(filename)
#else
filename
#endif
)) return;
PyErr_Format(PyExc_RuntimeError, "opening %s failed", filename.data());
throw py::error_already_set{};

Write Preview
Loading…
Cancel
Save