Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Surface from rhino #1350

Closed
wants to merge 30 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
c41b638
added plugin for Surface.from_plane
chenkasirer May 2, 2024
1d0b2c6
made argument `box` optional in `RhinoSurface.from_plane`
chenkasirer May 2, 2024
27a20d9
Merge branch 'main' of https://github.com/compas-dev/compas into surf…
chenkasirer May 2, 2024
b2a18e3
added `NurbsSurface.from_plane`
chenkasirer May 2, 2024
fdd97a9
Merge branch 'main' into surface_from_rhino
chenkasirer Jun 21, 2024
4dd4294
surface domain instead of box
chenkasirer Jun 21, 2024
4354d35
user specifies domain in RhinoNurbsSurface.from_plane
chenkasirer Jun 21, 2024
605a95e
default domains
chenkasirer Jun 21, 2024
7249f1e
noqa on typing imports
chenkasirer Jun 21, 2024
4242ee3
added from_native and native_surface to Surface
chenkasirer Jun 21, 2024
b78432a
Surface() create a RhinoSurface object in Rhino
chenkasirer Jun 21, 2024
5ed8791
added RhinoSurface.from_native plugin with intention to replace from_…
chenkasirer Jun 21, 2024
3e5017c
updated changelog
chenkasirer Jun 21, 2024
5566f95
unused import
chenkasirer Jun 21, 2024
de97233
WIP
chenkasirer Jun 24, 2024
0d72fe5
protect Surface(). Surface object can only be created via alternative…
chenkasirer Jul 2, 2024
0ea3ede
unused import
chenkasirer Jul 2, 2024
6a9c286
merge from_frame/plane. only implement abstract properties
chenkasirer Jul 3, 2024
d4f3ee0
fix search and replace accident
chenkasirer Jul 3, 2024
a3d6ecc
removed Surface.__from_data__
chenkasirer Jul 3, 2024
186ba78
instantiating nurbs surface via from_native
chenkasirer Jul 3, 2024
dafabfa
same from_native in nurbs and normal surface
chenkasirer Jul 3, 2024
5d86721
added domain and frame to nurbs params. __from_data__ is generic but …
chenkasirer Jul 3, 2024
33c388f
removed dead code and unused imports
chenkasirer Jul 3, 2024
9f6631d
added nurbssurface_to_compas
chenkasirer Jul 3, 2024
de25d04
native_surface is readonly
chenkasirer Jul 3, 2024
370ec8f
is_closed_u/v instead of is_closed
chenkasirer Jul 3, 2024
af70a6d
removed debug print
chenkasirer Jul 3, 2024
f8db6c9
updated changelog
chenkasirer Jul 3, 2024
5b5e1ac
linting
chenkasirer Jul 3, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
* Added `VolMesh.vertex_edges`.
* Added `VolMesh.from_meshes`.
* Added `VolMesh.from_polyhedrons`.
* Added pluggable `Surface.from_native`.
* Added deprecation warning on `RhinoSurface.from_rhino`.
* Added `Surface.native_surface`.
* Added `RhinoSurface.native_surface`.
* Added plugin `RhinoSurface.from_native`.
* Added parameters `frame`, `domain_u` and `domain_v` to `RhinoNurbs.from_parameters`.
* Added `nurbssurface_to_compas` to `compas_rhino.conversions`.
* Added `is_closed_u` and `is_closed_v` to `compas.geometry.Surface`.

### Changed

Expand All @@ -54,11 +62,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
* Updated `compas.geometry.vector.__truediv__` to allow element-wise division with another vector.
* Fixed bug in registration `shapely` boolean plugins.
* Temporarily restrict `numpy` to versions lower than `2.x`.
* Protected call to `compas.geometry.Surface()`. `Surface` should be instantiated using one of the alternative constructors.
* Protected call to `compas.geometry.NurbsSurface()`. `NurbsSurface` should be instantiated using one of the alternative constructors.

### Removed

* Removed `System` dependency in `compas_ghpython/utilities/drawing.py`.
* Removed GH plugin for `compas.scene.clear` since it clashed with the Rhino version.
* Removed pluggable `new_nurbssurface`.
* Removed pluggable `new_surface`.
* Removed `compas.geometry.Surface.is_closed`.

## [2.1.1] 2024-05-14

Expand Down
93 changes: 67 additions & 26 deletions src/compas/geometry/surfaces/nurbs.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
from __future__ import division
from __future__ import print_function

from compas.geometry import Frame
from compas.geometry import Point
from compas.itertools import linspace
from compas.itertools import meshgrid
Expand All @@ -12,17 +13,17 @@


@pluggable(category="factories")
def new_nurbssurface(cls, *args, **kwargs):
def new_nurbssurface_from_native(cls, *args, **kwargs):
raise PluginNotInstalledError


@pluggable(category="factories")
def new_nurbssurface_from_native(cls, *args, **kwargs):
def new_nurbssurface_from_parameters(cls, *args, **kwargs):
raise PluginNotInstalledError


@pluggable(category="factories")
def new_nurbssurface_from_parameters(cls, *args, **kwargs):
def new_nurbssurface_from_plane(cls, *args, **kwargs):
raise PluginNotInstalledError


Expand Down Expand Up @@ -94,36 +95,66 @@

@property
def __data__(self):
return {
"points": [point.__data__ for point in self.points],
"weights": self.weights,
"knots_u": self.knots_u,
"knots_v": self.knots_v,
"mults_u": self.mults_u,
"mults_v": self.mults_v,
"degree_u": self.degree_u,
"degree_v": self.degree_v,
"is_periodic_u": self.is_periodic_u,
"is_periodic_v": self.is_periodic_v,
}
data = super(NurbsSurface, self).__data__
data["points"] = [[point.__data__ for point in row] for row in self.points] # type: ignore
data["weights"] = self.weights
data["knots_u"] = self.knots_u
data["knots_v"] = self.knots_v
data["mults_u"] = self.mults_u
data["mults_v"] = self.mults_v
data["degree_u"] = self.degree_u
data["degree_v"] = self.degree_v
data["is_periodic_u"] = self.is_periodic_u
data["is_periodic_v"] = self.is_periodic_v
return data

Check warning on line 109 in src/compas/geometry/surfaces/nurbs.py

View check run for this annotation

Codecov / codecov/patch

src/compas/geometry/surfaces/nurbs.py#L98-L109

Added lines #L98 - L109 were not covered by tests

@classmethod
def __from_data__(cls, data):
"""Construct a BSpline surface from its data representation.

Parameters
----------
data : dict
The data dictionary.

Returns
-------
:class:`compas_rhino.geometry.RhinoNurbsSurface`
The constructed surface.

"""
frame = Frame.__from_data__(data["frame"])
domain_u = data["domain_u"]
domain_v = data["domain_v"]
points = [[Point.__from_data__(point) for point in row] for row in data["points"]]
weights = data["weights"]
knots_u = data["knots_u"]
knots_v = data["knots_v"]
mults_u = data["mults_u"]
mults_v = data["mults_v"]
degree_u = data["degree_u"]
degree_v = data["degree_v"]
is_periodic_u = data["is_periodic_u"]
is_periodic_v = data["is_periodic_v"]

Check warning on line 138 in src/compas/geometry/surfaces/nurbs.py

View check run for this annotation

Codecov / codecov/patch

src/compas/geometry/surfaces/nurbs.py#L126-L138

Added lines #L126 - L138 were not covered by tests

return cls.from_parameters(
data["points"],
data["weights"],
data["knots_u"],
data["knots_v"],
data["mults_u"],
data["mults_v"],
data["degree_u"],
data["degree_v"],
data["is_periodic_u"],
data["is_periodic_v"],
frame,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i am not so sure that any of the methods of the surface class take this frame into account. currently, the frame is not settable and always defaults to world XY...

domain_u,
domain_v,
points,
weights,
knots_u,
knots_v,
mults_u,
mults_v,
degree_u,
degree_v,
is_periodic_u,
is_periodic_v,
)

def __new__(cls, *args, **kwargs):
return new_nurbssurface(cls, *args, **kwargs)
raise AssertionError("NurbsSurface() is protected. Use NurbsSurface.from_...() to construct a NurbsSurface object.")

Check warning on line 157 in src/compas/geometry/surfaces/nurbs.py

View check run for this annotation

Codecov / codecov/patch

src/compas/geometry/surfaces/nurbs.py#L157

Added line #L157 was not covered by tests

def __init__(self, name=None):
super(NurbsSurface, self).__init__(name=name)
Expand Down Expand Up @@ -211,6 +242,9 @@
@classmethod
def from_parameters(
cls,
frame,
domain_u,
domain_v,
points,
weights,
knots_u,
Expand Down Expand Up @@ -250,6 +284,9 @@
"""
return new_nurbssurface_from_parameters(
cls,
frame,
domain_u,
domain_v,
points,
weights,
knots_u,
Expand Down Expand Up @@ -350,6 +387,10 @@
"""
return new_nurbssurface_from_fill(cls, curve1, curve2, curve3, curve4, style)

@classmethod
def from_plane(cls, plane, u_degree=1, v_degree=1):
chenkasirer marked this conversation as resolved.
Show resolved Hide resolved
return new_nurbssurface_from_plane(cls, plane, u_degree, v_degree)

Check warning on line 392 in src/compas/geometry/surfaces/nurbs.py

View check run for this annotation

Codecov / codecov/patch

src/compas/geometry/surfaces/nurbs.py#L392

Added line #L392 was not covered by tests

# ==============================================================================
# Conversions
# ==============================================================================
Expand Down
60 changes: 54 additions & 6 deletions src/compas/geometry/surfaces/surface.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,8 @@


@pluggable(category="factories")
def new_surface(cls, *args, **kwargs):
surface = object.__new__(cls)
surface.__init__(*args, **kwargs)
return surface
def new_surface_from_native(cls, *args, **kwargs):
raise NotImplementedError

Check warning on line 17 in src/compas/geometry/surfaces/surface.py

View check run for this annotation

Codecov / codecov/patch

src/compas/geometry/surfaces/surface.py#L17

Added line #L17 was not covered by tests


@pluggable(category="factories")
Expand All @@ -43,15 +41,41 @@
The parameter domain of the surface in the U direction.
domain_v : tuple[float, float], read-only
The parameter domain of the surface in the V direction.
is_closed_u : bool, read-only
Flag indicating if the surface is closed in the U direction.
is_closed_v : bool, read-only
Flag indicating if the surface is closed in the V direction.
is_periodic_u : bool, read-only
Flag indicating if the surface is periodic in the U direction.
is_periodic_v : bool, read-only
Flag indicating if the surface is periodic in the V direction.
native_surface : Any, read-only
The CAD native surface object.

"""

@property
def __data__(self):
return {

Check warning on line 59 in src/compas/geometry/surfaces/surface.py

View check run for this annotation

Codecov / codecov/patch

src/compas/geometry/surfaces/surface.py#L59

Added line #L59 was not covered by tests
"frame": self._frame.__data__,
"domain_u": list(self.domain_u),
"domain_v": list(self.domain_v),
}

@classmethod
def __from_data__(cls, data):
frame = Frame.__from_data__(data["frame"])
domain_u = tuple(data["domain_u"])
domain_v = tuple(data["domain_v"])
return cls.from_plane(frame, domain_u, domain_v)

Check warning on line 70 in src/compas/geometry/surfaces/surface.py

View check run for this annotation

Codecov / codecov/patch

src/compas/geometry/surfaces/surface.py#L67-L70

Added lines #L67 - L70 were not covered by tests
Comment on lines +57 to +70
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since the base surface is abstract and since it should not be possible to create instances of it directly, i don't think it should provide the possibility to create one from data...

Copy link
Member Author

@chenkasirer chenkasirer Jul 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from_plane will invoke the plugin, if available. it just seems to be the canonical surface representation (maybe with exception of frame which it seems I didn't quite get)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but why should we provide functionality for making a "canonical" surface from data, when the surface can't actually by instantiated in the normal way. i though we agreed that only the spacial constructors can be used to create an actual surface and that the base class only provides the shared interface...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for CAD agnostic (de)serialization? maybe what's confusing me is that lack of other kinds of surface types
maybe this should be in a PlanarSurface like in Rhino..

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there are plenty of other surface types (including planar surface). they just don't require a Rhino or OCC plugin...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

compas.geometry.surfaces has CylindricalSurface, ConicalSurface, PlanarSurface, SphericalSurface, ToroidalSurface


@property
def __dtype__(self):
# make serialization CAD agnostic
return "compas.geometry/Surface"

Check warning on line 75 in src/compas/geometry/surfaces/surface.py

View check run for this annotation

Codecov / codecov/patch

src/compas/geometry/surfaces/surface.py#L75

Added line #L75 was not covered by tests

def __new__(cls, *args, **kwargs):
return new_surface(cls, *args, **kwargs)
raise AssertionError("Surface() is protected. Use Surface.from_...() to construct a Surface object.")

Check warning on line 78 in src/compas/geometry/surfaces/surface.py

View check run for this annotation

Codecov / codecov/patch

src/compas/geometry/surfaces/surface.py#L78

Added line #L78 was not covered by tests

def __init__(self, frame=None, name=None):
super(Surface, self).__init__(name=name)
Expand Down Expand Up @@ -95,6 +119,10 @@
self._transformation = Transformation.from_frame(self.frame)
return self._transformation

@property
def native_surface(self):
raise NotImplementedError

Check warning on line 124 in src/compas/geometry/surfaces/surface.py

View check run for this annotation

Codecov / codecov/patch

src/compas/geometry/surfaces/surface.py#L124

Added line #L124 was not covered by tests

@property
def point(self):
if not self._point:
Expand Down Expand Up @@ -134,7 +162,11 @@
return self._domain_v

@property
def is_closed(self):
def is_closed_u(self):
raise NotImplementedError

Check warning on line 166 in src/compas/geometry/surfaces/surface.py

View check run for this annotation

Codecov / codecov/patch

src/compas/geometry/surfaces/surface.py#L166

Added line #L166 was not covered by tests

@property
def is_closed_v(self):
raise NotImplementedError

@property
Expand Down Expand Up @@ -197,6 +229,22 @@
"""
return new_surface_from_plane(cls, plane, *args, **kwargs)

@classmethod
def from_native(cls, surface):
"""Construct a surface from a CAD native surface object.

Parameters
----------
surface : Any
A native surface object.

Returns
-------
:class:`compas.geometry.Surface`

"""
return new_surface_from_native(cls, surface)

Check warning on line 246 in src/compas/geometry/surfaces/surface.py

View check run for this annotation

Codecov / codecov/patch

src/compas/geometry/surfaces/surface.py#L246

Added line #L246 was not covered by tests

# ==============================================================================
# Conversions
# ==============================================================================
Expand Down
2 changes: 2 additions & 0 deletions src/compas_rhino/conversions/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
)
from .surfaces import (
surface_to_rhino,
nurbssurface_to_compas,
data_to_rhino_surface,
surface_to_compas_data,
surface_to_compas,
Expand Down Expand Up @@ -127,6 +128,7 @@
"curve_to_compas",
# surfaces
"surface_to_rhino",
"nurbssurface_to_compas",
"surface_to_compas_data",
"data_to_rhino_surface",
"surface_to_compas",
Expand Down
40 changes: 29 additions & 11 deletions src/compas_rhino/conversions/surfaces.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from compas.datastructures import Mesh
from compas.geometry import NurbsSurface
from compas.geometry import Point
from compas.geometry import Surface
from compas.tolerance import TOL
from compas.utilities import memoize

Expand All @@ -32,12 +33,14 @@ def surface_to_rhino(surface):
Rhino.Geometry.Surface

"""
return surface.rhino_surface
return surface.native_surface


def data_to_rhino_surface(data):
"""Convert a COMPAS surface to a Rhino surface.

TODO: still needed? this is basically RhinoSurface.from_parameters()

Parameters
----------
data: dict
Expand Down Expand Up @@ -85,6 +88,8 @@ def data_to_rhino_surface(data):
def surface_to_compas_data(surface):
"""Convert a Rhino surface to a COMPAS surface.

TODO: still needed? this is basically RhinoSurface.__data__

Parameters
----------
surface: :rhino:`Rhino.Geometry.Surface`
Expand Down Expand Up @@ -147,24 +152,37 @@ def surface_to_compas(surface):
Parameters
----------
surface: :rhino:`Rhino.Geometry.Surface`
A Rhino surface object.

Returns
-------
:class:`compas.geometry.Surface`
:class:`compas_rhino.geometry.RhinoSurface`

"""
if isinstance(surface, Rhino.DocObjects.RhinoObject):
surface = surface.Geometry
if not isinstance(surface, Rhino.Geometry.Surface):
raise ConversionError("Expected a Rhino Surface object but got: {}".format(type(surface)))

return Surface.from_native(surface)

if not isinstance(surface, Rhino.Geometry.Brep):
brep = Rhino.Geometry.Brep.TryConvertBrep(surface)
else:
brep = surface

if brep.Surfaces.Count > 1: # type: ignore
raise ConversionError("Conversion of a Brep with multiple underlying surface is currently not supported.")
def nurbssurface_to_compas(nurbssurface):
"""Convert a Rhino NurbsSurface to a COMPAS NurbsSurface.

Parameters
----------
nurbssurface : :class:`Rhino.Geometry.NurbsSurface`
A Rhino NurbsSurface object.

Returns
-------
:class:`compas_rhino.geometry.RhinoNurbsSurface`
A COMPAS NurbsSurface object.

"""
if not isinstance(nurbssurface, Rhino.Geometry.NurbsSurface):
raise ConversionError("Expected a Rhino NurbsSurface object but got: {}".format(type(nurbssurface)))

return NurbsSurface.from_native(brep.Surfaces[0])
return NurbsSurface.from_native(nurbssurface)


def surface_to_compas_mesh(surface, facefilter=None, cleanup=False, cls=None):
Expand Down
2 changes: 2 additions & 0 deletions src/compas_rhino/geometry/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from __future__ import absolute_import

from .curves.nurbs import RhinoNurbsCurve
from .surfaces import RhinoSurface
from .surfaces.nurbs import RhinoNurbsSurface

from .brep.brep import RhinoBrep
Expand Down Expand Up @@ -34,6 +35,7 @@
"trimesh_principal_curvature",
"trimesh_slice",
"RhinoNurbsCurve",
"RhinoSurface",
"RhinoNurbsSurface",
"RhinoBrep",
"RhinoBrepVertex",
Expand Down
Loading
Loading