diff --git a/docs/index.rst b/docs/index.rst index 0df471d..f334d63 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -55,6 +55,7 @@ for the terms and conditions of use. installation tables + modify_existing_model .. toctree:: :caption: API Reference diff --git a/docs/modify_existing_model.rst b/docs/modify_existing_model.rst new file mode 100644 index 0000000..317b4bf --- /dev/null +++ b/docs/modify_existing_model.rst @@ -0,0 +1,60 @@ +.. _modify_existing_model: + +Modify existing models +====================== + +*py3dm* can be used to read, write, and modify ``.3dm`` files. + +Due to the current lightweight implementation of the library, special care is +required when modifying objects that have already been added to a ``Model``. + +Retrieve geometrical objects +---------------------------- +There are two ways to retrieve objects from an existing ``Model``, which mirror +the underlying openNURBS C++ API: + +- ``get_by_uuid`` +- ``get_by_uuid_exclusive`` + +Both methods take only the object's ``ON_UUID`` as input. On the C++ side: + +- ``get_by_uuid`` returns a ``const`` pointer to the geometry, which prevents + modification in C++. +- ``get_by_uuid_exclusive`` returns a raw pointer to the geometry if, and only + if, the retrieved ``ON_ModelGeometryComponent`` is the sole reference to that + geometry. Otherwise, ``nullptr`` is returned. + +Therefore, when retrieving an object with the explicit intent to modify it, +``get_by_uuid_exclusive`` should be used. + +Modify geometrical objects +-------------------------- + +While the distinction between const and non-const access is enforced on the +C++ side, this constness is not preserved when the geometry is exposed to +Python. + +As example, an ``ON_Point`` returned from a const getter is currently bound +as a regular mutable Python ``Point`` object. At the moment there is no simple +mechanism in *nanobind* to indicate that such a returned object should behave +purely as a read-only view. As a consequence, the following code runs +successfully, even though the original C++ API was designed to prevent +modification in this case: + +.. code-block:: python + + import py3dm + + model = py3dm.Model() + pt_uuid = model.point_table.add(0, 0, 0) + pt = model.point_table.get_by_uuid(pt_uuid) + pt.point.x = 1 + + assert pt.point.x == 1 # pass + +This behavior reflects the current implementation: const access in C++ does not +translate to a read-only object in Python. + +A possible future improvement would be to introduce lightweight view types +around the original openNURBS classes and expose those views as read-only +objects on the Python side. This is not implemented yet. diff --git a/py3dm/__init__.py b/py3dm/__init__.py index bfe6373..3a5dce1 100644 --- a/py3dm/__init__.py +++ b/py3dm/__init__.py @@ -12,7 +12,7 @@ OpenNURBSObject, Model, PlotColorSource, - PointGeometry, + Point, PointTable, Point3d, TextLog diff --git a/py3dm/__init__.pyi b/py3dm/__init__.pyi index b32a61a..603a5c9 100644 --- a/py3dm/__init__.pyi +++ b/py3dm/__init__.pyi @@ -894,7 +894,7 @@ class PlotColorSource(Enum): from_parent = 3 -class PointGeometry(Geometry): +class Point(Geometry): """Python bindings for the openNURBS ``ON_Point`` class. """ # read-write member variables @@ -929,7 +929,7 @@ class PointTable: associated ``ONX_Model`` instance. """ # dunder methods - def __iter__(self) -> Iterator[PointGeometry]: ... + def __iter__(self) -> Iterator[Point]: ... def __len__(self) -> int: ... @@ -951,7 +951,7 @@ class PointTable: @overload def add( self, - point: PointGeometry, + point: Point, obj_attr: None | ObjectAttributes = None ) -> UUID: """Returns the ``UUID`` of the point in case of successful addition, or @@ -978,12 +978,29 @@ class PointTable: """ ... - def get_by_uuid(self, object_uuid: UUID) -> PointGeometry | None: + def get_by_uuid(self, object_uuid: UUID) -> Point | None: """Returns the object with the given ``object_uuid`` or ``None`` if ``object_uuid`` is not found. """ ... + def get_by_uuid_exclusive(self, object_uuid: UUID) -> Point | None: + """Returns the object with the given ``object_uuid`` or ``None`` if + ``object_uuid`` is not found. + + Notes + ----- + From opennurbs documentation + (``ON_ModelGeometryComponent::ExclusiveGeometry()``): + Get a pointer to geometry that can be used to modify the geometry. The + returned pointer is not shared at the time it is returned and will not + be shared until a copy of this ``ON_ModelGeometryComponent`` is + created. If this ``ON_ModelGeometryComponent`` is the only reference to + the geometry, then a pointer to the geometry is returned. Otherwise, + ``nullptr`` is returned. + """ + ... + class Point3d: """Python bindings for the openNURBS ``ON_3dPoint`` class. diff --git a/src/bindings/bindings.cpp b/src/bindings/bindings.cpp index 86f0a9f..c445775 100644 --- a/src/bindings/bindings.cpp +++ b/src/bindings/bindings.cpp @@ -15,7 +15,7 @@ #include "model_component_bindings.h" #include "object_attributes_bindings.h" #include "object_bindings.h" -#include "point_geometry_bindings.h" +#include "point_bindings.h" #include "point_table_bindings.h" #include "point3d_bindings.h" #include "text_log_bindings.h" @@ -41,7 +41,7 @@ NB_MODULE(_py3dm, m) { LayerTableBindings(m); LineCurveBindings(m); ModelBindings(m); - PointGeometryBindings(m); + PointBindings(m); TextLogBindings(m); CurveTableBindings(m); diff --git a/src/bindings/point_geometry_bindings.cpp b/src/bindings/point_bindings.cpp similarity index 68% rename from src/bindings/point_geometry_bindings.cpp rename to src/bindings/point_bindings.cpp index ab33882..ef7afd6 100644 --- a/src/bindings/point_geometry_bindings.cpp +++ b/src/bindings/point_bindings.cpp @@ -4,15 +4,15 @@ // Project includes #include "geometry_bindings.h" -#include "point_geometry_bindings.h" +#include "point_bindings.h" -void PointGeometryBindings(nb::module_& m) { - nb::class_(m, "PointGeometry") +void PointBindings(nb::module_& m) { + nb::class_(m, "Point") /*magic methods*/ .def(nb::init<>()) .def(nb::init()) - .def(nb::init()) + .def(nb::init()) /*read-write member variables*/ .def_rw("point", &ON_Point::point) diff --git a/src/bindings/point_geometry_bindings.h b/src/bindings/point_bindings.h similarity index 70% rename from src/bindings/point_geometry_bindings.h rename to src/bindings/point_bindings.h index 78e5839..e69f67b 100644 --- a/src/bindings/point_geometry_bindings.h +++ b/src/bindings/point_bindings.h @@ -1,5 +1,5 @@ /* - src/bindings/point_geometry_bindings.h: Exposing ON_Point to Python + src/bindings/point_bindings.h: Exposing ON_Point to Python Copyright (c) 2025 Studio W Engineers @@ -16,4 +16,4 @@ #include "bindings.h" -void PointGeometryBindings(nb::module_& m); +void PointBindings(nb::module_& m); diff --git a/src/bindings/point_table_bindings.cpp b/src/bindings/point_table_bindings.cpp index f77ecc5..a34710e 100644 --- a/src/bindings/point_table_bindings.cpp +++ b/src/bindings/point_table_bindings.cpp @@ -8,17 +8,14 @@ void PointTableBindings(nb::module_& m) { - nb::class_(m, "__CurveTableIterator") - .def("__iter__", [](PointTable::Iterator& it) -> PointTable::Iterator& { - return it; - }) - .def("__next__", [](PointTable::Iterator& it) { + nb::class_(m, "__PointTableIterator") + .def("__iter__", [](PointTable::Iterator& it) -> PointTable::Iterator& {return it;}) + .def("__next__", [](PointTable::Iterator& it) -> const ON_Point* { while (!it.IsOver()) { - ON_Object* object = *it; + const ON_Point* point = *it; ++it; - - if (object != nullptr) { - return object; + if (point) { + return point; } } throw nb::stop_iteration(); @@ -54,7 +51,8 @@ void PointTableBindings(nb::module_& m) { ) /*getters*/ - .def("get_by_uuid", &PointTable::GetbyUUID, nb::rv_policy::reference_internal) + .def("get_by_uuid", &PointTable::GetByUUID, nb::rv_policy::reference_internal) + .def("get_by_uuid_exclusive", &PointTable::GetByUUIDExclusive, nb::rv_policy::reference_internal) /*other methods*/ .def("count", &PointTable::Count) diff --git a/src/point_table.cpp b/src/point_table.cpp index ec1af00..7330a6c 100644 --- a/src/point_table.cpp +++ b/src/point_table.cpp @@ -14,7 +14,7 @@ ON_UUID PointTable::Add(double x, double y, double z, const ON_3dmObjectAttribut ON_Point point(x, y, z); const ON_ModelComponent* mc = m_model->AddModelGeometryComponent(&point, obj_attr).ModelComponent(); - return (mc != nullptr) ? mc->Id() : ON_nil_uuid; + return mc ? mc->Id() : ON_nil_uuid; } ON_UUID PointTable::Add(const ON_3dPoint& point, const ON_3dmObjectAttributes* obj_attr) const { @@ -26,24 +26,47 @@ ON_UUID PointTable::Add(const ON_Point& point, const ON_3dmObjectAttributes* obj } /*getters*/ -ON_Point* PointTable::GetbyUUID(const ON_UUID on_uuid) { - const ON_ModelComponent* mc = m_model->ComponentFromId(ON_ModelComponent::Type::ModelGeometry, on_uuid).ModelComponent(); +const ON_Point* PointTable::GetByUUID(const ON_UUID on_uuid) const { + const ON_ModelComponent* mc = m_model->ComponentFromId( + ON_ModelComponent::Type::ModelGeometry, + on_uuid + ).ModelComponent(); + const ON_ModelGeometryComponent* mgc = ON_ModelGeometryComponent::Cast(mc); - if (!IsPoint(mc)) { + if (!IsPoint(mgc)) { return nullptr; } + return ON_Point::Cast(mgc->Geometry(nullptr)); +} - const ON_ModelGeometryComponent* mgc = ON_ModelGeometryComponent::Cast(mc); - return const_cast(ON_Point::Cast(mgc->Geometry(nullptr))); +ON_Point* PointTable::GetByUUIDExclusive(const ON_UUID on_uuid) const { + const ON_ModelComponentReference& mcr = m_model->ComponentFromRuntimeSerialNumber( + GetRuntimeSerialNumber(on_uuid) + ); + ON_ModelGeometryComponent* mgc = ON_ModelGeometryComponent::Cast( + mcr.ExclusiveModelComponent() + ); + + if (!IsPoint(mgc)) { + return nullptr; + } + return ON_Point::Cast(mgc->ExclusiveGeometry()); +} + +ON__UINT64 PointTable::GetRuntimeSerialNumber(const ON_UUID on_uuid) const { + return m_model->Manifest().ItemFromId( + ON_ModelComponent::Type::ModelGeometry, + on_uuid + ).ComponentRuntimeSerialNumber(); } /*other methods*/ -int PointTable::Count() { +int PointTable::Count() const { int count = 0; ONX_ModelComponentIterator mci(*m_model.get(), ON_ModelComponent::Type::ModelGeometry); ON_ModelComponentReference mcr = mci.FirstComponentReference(); while (!mcr.IsEmpty()) { - if (PointTable::IsPoint(mcr.ModelComponent())) { + if (IsPoint(ON_ModelGeometryComponent::Cast(mcr.ModelComponent()))) { ++count; } mcr = mci.NextComponentReference(); @@ -52,12 +75,10 @@ int PointTable::Count() { return count; } -bool PointTable::IsPoint(const ON_ModelComponent* mc) { - const ON_ModelGeometryComponent* mgc = ON_ModelGeometryComponent::Cast(mc); - if (mgc == nullptr) { +bool PointTable::IsPoint(const ON_ModelGeometryComponent* mgc) { + if (!mgc) { return false; } - const ON_Geometry* geom = mgc->Geometry(nullptr); return (geom && geom->ObjectType() == ON::point_object); } @@ -69,8 +90,8 @@ PointTable::Iterator::Iterator(PointTable* table) m_current = m_iterator.FirstComponentReference(); } -ON_Point* PointTable::Iterator::operator*() const { - return m_table->GetbyUUID(m_current.ModelComponentId()); +const ON_Point* PointTable::Iterator::operator*() const { + return m_table->GetByUUID(m_current.ModelComponentId()); } PointTable::Iterator& PointTable::Iterator::operator++() { diff --git a/src/point_table.h b/src/point_table.h index 3ea7f95..a933dc1 100644 --- a/src/point_table.h +++ b/src/point_table.h @@ -18,27 +18,32 @@ class PointTable { public: - /*constructors*/ + /*constructor*/ PointTable(std::shared_ptr model); + /*destructor*/ + ~PointTable() = default; + /*add methods*/ ON_UUID Add(double x, double y, double z, const ON_3dmObjectAttributes* obj_attr) const; ON_UUID Add(const ON_Point& point, const ON_3dmObjectAttributes* obj_attr) const; ON_UUID Add(const ON_3dPoint& point, const ON_3dmObjectAttributes* obj_attr) const; /*getters*/ - ON_Point* GetbyUUID(const ON_UUID obj_uuid); + const ON_Point* GetByUUID(const ON_UUID obj_uuid) const; + ON_Point* GetByUUIDExclusive(const ON_UUID obj_uuid) const; + ON__UINT64 GetRuntimeSerialNumber(const ON_UUID on_uuid) const; /*other methods*/ - int Count(); - bool IsPoint(const ON_ModelComponent* mc); + int Count() const; + static bool IsPoint(const ON_ModelGeometryComponent* mgc); /*PointTable Iterator*/ class Iterator { public: Iterator(PointTable* table); - ON_Point* operator*() const; + const ON_Point* operator*() const; Iterator& operator++(); bool IsOver() const; diff --git a/tests/test_point_geometry.py b/tests/test_point.py similarity index 61% rename from tests/test_point_geometry.py rename to tests/test_point.py index 3f3f85c..538a59b 100644 --- a/tests/test_point_geometry.py +++ b/tests/test_point.py @@ -2,30 +2,30 @@ from unittest import TestCase # third party library imports -from py3dm import PointGeometry, Point3d +from py3dm import Point, Point3d # local library specific imports -class PointGeometryTestSuite(TestCase): +class PointTestSuite(TestCase): def test_constructor(self) -> None: with self.subTest(msg="Default constructor"): - self.assertEqual(PointGeometry().point, Point3d(0, 0, 0)) + self.assertEqual(Point().point, Point3d(0, 0, 0)) with self.subTest(msg="Constructor with coordinates"): self.assertEqual( - PointGeometry(1, 2, 1.1).point, + Point(1, 2, 1.1).point, Point3d(1, 2, 1.1) ) with self.subTest(msg="Constructor with Point3d"): self.assertEqual( - PointGeometry(Point3d(1, 2, 1.1)).point, + Point(Point3d(1, 2, 1.1)).point, Point3d(1, 2, 1.1) ) def test_is_valid(self) -> None: - self.assertTrue(PointGeometry().is_valid()) + self.assertTrue(Point().is_valid()) def test_point_member_variable(self) -> None: - self.assertIsInstance(PointGeometry().point, Point3d) + self.assertIsInstance(Point().point, Point3d) diff --git a/tests/test_point_table.py b/tests/test_point_table.py index 8d0d60d..2c6090d 100644 --- a/tests/test_point_table.py +++ b/tests/test_point_table.py @@ -3,7 +3,7 @@ from uuid import UUID # third party library imports -from py3dm import Model, ObjectAttributes, PointGeometry, Point3d +from py3dm import Model, ObjectAttributes, Point, Point3d # local library specific imports @@ -23,16 +23,25 @@ def test_add_with_attributes(self) -> None: point = self.table.get_by_uuid(point_id) self.assertEqual(point.point, Point3d(0, 1, 0)) # type: ignore - point_geo = PointGeometry(0, 2, 0) + point_geo = Point(0, 2, 0) point_geo_id = self.table.add(point_geo, ObjectAttributes()) - with self.subTest(msg="add with PointGeometry part I"): + with self.subTest(msg="add with Point part I"): self.assertNotEqual(point_geo_id, UUID(int=0)) - with self.subTest(msg="add with PointGeometry part II"): + with self.subTest(msg="add with Point part II"): point = self.table.get_by_uuid(point_geo_id) self.assertEqual(point.point, Point3d(0, 2, 0)) # type: ignore + def test_const_get_by_uuid(self) -> None: + pt_uuid = self.table.add(0, 0, 0) + pt = self.table.get_by_uuid(pt_uuid) + pt.point.x = 1 # type: ignore + + # NOTE: The const-ness C++ side is lost when exposing ON_Point to + # Python direcly without a wrapper + self.assertEqual(pt.point.x, 1) # type: ignore + def test_count(self) -> None: with self.subTest(msg="empty table"): self.assertEqual(self.table.count(), 0) @@ -62,6 +71,8 @@ def test_point_iterator(self) -> None: for point in self.model.point_table: point.point.x = 1 + # NOTE: The const-ness C++ side is lost when exposing ON_Point to + # Python direcly without a wrapper for point_index, point in enumerate(self.model.point_table): with self.subTest(point_index=point_index): self.assertEqual(point.point.x, 1)