Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1443 +/- ##
==========================================
- Coverage 61.88% 61.86% -0.03%
==========================================
Files 208 208
Lines 22333 22327 -6
==========================================
- Hits 13821 13812 -9
- Misses 8512 8515 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
tomvanmele
left a comment
There was a problem hiding this comment.
i think we have to add a bit of documentation
| frame_stack = [self.frame] if self.frame else [] | ||
| parent = self.parent | ||
| transformations = [self.transformation] if self.transformation else [] | ||
| parent: SceneObject = self.parent |
There was a problem hiding this comment.
i like your enthusiasm, but this annotation can't be introduced yet in the current version of COMPAS...
There was a problem hiding this comment.
removed, i got so used to it 😂
| assert sceneobj1.frame == Frame([10.0, 0.0, 0.0], [1.0, 0.0, 0.0], [0.0, 1.0, 0.0]) | ||
|
|
||
| sceneobj2 = scene.add(Box(), parent=sceneobj1) | ||
| sceneobj2.frame = Frame([1.0, 1.0, 0.0], xaxis=[1.0, 0.0, 0.0], yaxis=[0.0, 1.0, 0.0]) |
There was a problem hiding this comment.
by simply removing the frame, you're not reproducing the same result as before...
There was a problem hiding this comment.
Yep, but if you see in next few lines, the assert condition was updated according to the new computing scheme, or perhaps I misunderstood something here?
Hey folks, I'd like propose to update the relation between
frameandtransformationinSceneObject, to align better with how they are currently defined incompas_model(https://github.com/BlockResearchGroup/compas_model/blob/b8d7a992956abcfe96d41eb11fecd5a064a12674/src/compas_model/elements/element.py#L271), which I found makes more sense and easier to understand.worldtransformationto the simple multiplication of all transformations along the scene tree.frameproperty as a read-only computed property which is derived fromFrame.from_transformation(self.worldtransformation), and it can be think of as the reference frame of the local coordinate system of this object after all the transformations applied.This relation also aligns better with other CAD systems and libraries like Threejs. And it will give a lot of advantage for
compas_viewerwhere I will be able to push the transformation and scene tree into GPU and calculate the multiplication there.