-
Notifications
You must be signed in to change notification settings - Fork 108
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
Surface from rhino #1350
Changes from all commits
c41b638
1d0b2c6
27a20d9
b2a18e3
fdd97a9
4dd4294
4354d35
605a95e
7249f1e
4242ee3
b78432a
5ed8791
3e5017c
5566f95
de97233
0d72fe5
0ea3ede
6a9c286
d4f3ee0
a3d6ecc
186ba78
dafabfa
5d86721
33c388f
9f6631d
de25d04
370ec8f
af70a6d
f8db6c9
5b5e1ac
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
|
||
|
||
@pluggable(category="factories") | ||
|
@@ -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 { | ||
"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) | ||
Comment on lines
+57
to
+70
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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... There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
||
@property | ||
def __dtype__(self): | ||
# make serialization CAD agnostic | ||
return "compas.geometry/Surface" | ||
|
||
def __new__(cls, *args, **kwargs): | ||
return new_surface(cls, *args, **kwargs) | ||
raise AssertionError("Surface() is protected. Use Surface.from_...() to construct a Surface object.") | ||
|
||
def __init__(self, frame=None, name=None): | ||
super(Surface, self).__init__(name=name) | ||
|
@@ -95,6 +119,10 @@ | |
self._transformation = Transformation.from_frame(self.frame) | ||
return self._transformation | ||
|
||
@property | ||
def native_surface(self): | ||
raise NotImplementedError | ||
|
||
@property | ||
def point(self): | ||
if not self._point: | ||
|
@@ -134,7 +162,11 @@ | |
return self._domain_v | ||
|
||
@property | ||
def is_closed(self): | ||
def is_closed_u(self): | ||
raise NotImplementedError | ||
|
||
@property | ||
def is_closed_v(self): | ||
raise NotImplementedError | ||
|
||
@property | ||
|
@@ -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) | ||
|
||
# ============================================================================== | ||
# Conversions | ||
# ============================================================================== | ||
|
There was a problem hiding this comment.
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...