Skip to content

Conversation

@DamienGilliard
Copy link
Collaborator

@DamienGilliard DamienGilliard commented Aug 17, 2024

This PR contains changes to make diffCheck work with cylindrical beams.

image

The main changes target:

  • the joint detector df_joint_detector.py
  • the segmentation backend to deal with faces that are not flat :
  1. the method IsPointOnFace is skipped for the logs because it doesn't make sense in the case of round wood construction: > we assume we will never have 2 logs of exactly the same diameter, perfectly aligned along the same axis.

Change log:

  • DFMesh now has a GetCenterAndAxis method that is useful in the case of cylindrical meshes and is used in the DFSegmentation
  • DFMesh also had the IsPointOnFace method adapted to make it more robust
  • DFSegmentation had quite a lot of changes to accommodate two cases: standard case and log case. The two are differentiated by a flag IsCylinder as mathod parameter
  • The changes hereabove are reflected in the code.py of the segmentation components
  • df_joint_detector.py has had a few changes to: i) detect if we have a log or not, and ii) improve the joint detector itself that sometimes had some misses in complicated joints
  • df_util.py has a merge_shared_index that helps the joint detector with the joint_ids
  • DFBuildAssembiles now has a i_is_cylinder boolean parameter so signal is the assemply we are building is made out of logs or not
  • DFMergeAssemblies component created if you want to make an assembly by merging other assemblies. This is useful if you have a case where some beams are logs and others are squared. In that case you would make 2 assemblies, on with i_is_cylinder = True and the other False, then merge the two.
  • a small change was made to the MeshToCloud component to deal with "None" point clouds

No additional test was made, the changes are mainly reliant on rhino and difficult to test. If you see something to test during the review, let me know !!

@DamienGilliard DamienGilliard self-assigned this Aug 17, 2024
@DamienGilliard DamienGilliard marked this pull request as draft August 17, 2024 17:24
@9and3 9and3 linked an issue Aug 17, 2024 that may be closed by this pull request
@DamienGilliard
Copy link
Collaborator Author

DamienGilliard commented Aug 17, 2024

Joint detector working with test data:
image

image

@9and3 9and3 added enhancement New feature or request PythonAPI labels Aug 17, 2024
@9and3
Copy link
Contributor

9and3 commented Aug 17, 2024

Joint detector working with test data:

Nice! @DamienGilliard have you already tried also with square sections for compatibility by any chance?

@9and3
Copy link
Contributor

9and3 commented Sep 10, 2024

Hello @DamienGilliard , following up our meeting I marked the issue about not taking in acoount the holes in the df_joint_detector.py, can you have a look to implement it plis? #96

@9and3 9and3 linked an issue Sep 12, 2024 that may be closed by this pull request
@DamienGilliard DamienGilliard marked this pull request as draft September 12, 2024 12:48
@DamienGilliard
Copy link
Collaborator Author

DamienGilliard commented Sep 12, 2024

joint_detector now excludes cylindrical holes

image

NB: if a real joint face was created with a cylinder (by boolean difference, for example), it will also be excluded (corner case).

image
Here 1 was created from a circle arc, 2 from a cylinder, and 3 from a nurbs

@9and3
Copy link
Contributor

9and3 commented Sep 12, 2024

NB: if a real joint face was created with a cylinder (by boolean difference, for example), it will also be excluded (corner case).

indeed correct. At least we keep the same limitation everywhere for curved elements. I really want to take the time to review this PR properly since it introduces some nice feats and changes, hold on please @DamienGilliard :)

@DamienGilliard
Copy link
Collaborator Author

DamienGilliard commented Sep 21, 2024

New component added: DFMergeAssemblies:
image
Component modified: DFBuilAssembly now takes a "i_is_cylinder"
With small tweekings, the results are quite satisfactory:
image

@DamienGilliard DamienGilliard marked this pull request as ready for review September 22, 2024 14:04
@DamienGilliard
Copy link
Collaborator Author

DamienGilliard commented Sep 22, 2024

@9and3 , @mariklad , @eleniv3d I believe this PR is (again ;) ) ready for review. It's a big one, sorry for that \(*.*)/

Copy link
Contributor

@9and3 9and3 left a comment

Choose a reason for hiding this comment

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

Hello @DamienGilliard ! I left my comments and feedback here and there. The biggest remark I have is on the choice of the name cylinder, I thought that it would make more sense to have "roundwood" since for the other we used beam, something more related to the type of timber rather than the shape.

Let's take the time to close this properly, we can also discuss tomorrow at the lab if needed ! 👐

Comment on lines +38 to +47
siffed_df_cloud_source_list = []
siffed_rh_mesh_target_list = []
for i in range(len(i_cloud_source)):
if i_cloud_source[i] is not None:
siffed_df_cloud_source_list.append(df_cvt_bindings.cvt_rhcloud_2_dfcloud(i_cloud_source[i]))
siffed_rh_mesh_target_list.append(rh_mesh_target_list[i])


# calculate distances
o_result = df_error_estimation.df_cloud_2_rh_mesh_comparison(df_cloud_source_list, rh_mesh_target_list, i_signed_flag, i_swap)
o_result = df_error_estimation.df_cloud_2_rh_mesh_comparison(siffed_df_cloud_source_list, siffed_rh_mesh_target_list, i_signed_flag, i_swap)
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe to be cordinate with @eleniv3d ?

face = DFFace.from_brep_face(data[0], data[1])
faces.append(face)
beam = cls("Beam", faces)
beam.is_cylinder = is_cylinder
Copy link
Contributor

Choose a reason for hiding this comment

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

see above

Comment on lines 118 to 154
'''
the structure of the disctionnary is as follows:
{
face_id: (face, is_inside)
...
}
face_id is int
face is Rhino.Geometry.BrepFace
is_inside is bool
'''
faces = {}
if is_cylinder_beam:
for idx, face in enumerate(self.brep.Faces):
faces[idx] = (face, face.IsPlanar(1000 * sc.doc.ModelAbsoluteTolerance))
else:
for idx, face in enumerate(self.brep.Faces):
face_centroid = rg.AreaMassProperties.Compute(face).Centroid
coord = face.ClosestPoint(face_centroid)
projected_centroid = face.PointAt(coord[1], coord[2])
faces[idx] = (face, Bounding_geometry.IsPointInside(projected_centroid, sc.doc.ModelAbsoluteTolerance, True))

# compute the adjacency list of each face
adjacency_of_faces = {}
'''
the structure of the dictionnary is as follows:
{
face_id: (face, [adj_face_id_1, adj_face_id_2, ...])
...
}
face_id is int
face is Rhino.Geometry.BrepFace
adj_face_id_1, adj_face_id_2, ... are int
'''
for idx, face in faces.items():
if not face[1]:
continue
adjacency_of_faces[idx] = (face[0], [adj_face for adj_face in face[0].AdjacentFaces() if faces[adj_face][1] and faces[adj_face][0].IsPlanar(1000 * sc.doc.ModelAbsoluteTolerance) and adj_face != idx]) # used to be not faces[adj_face][0].IsPlanar(1000 * sc.doc.ModelAbsoluteTolerance)
Copy link
Contributor

Choose a reason for hiding this comment

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

I propose two things here:

  • first it would be great if you can contain what you added in a function encapsualted inside the DFJointDetector (_my_cylinder_func()) with some docstrings explaining why you need this because it's a bit cumbersome like this.
  • second I propose to do these to state clearly that what you added is needed only for cylinder beams right?
joint_face_ids = []
if is_cylinder_beam:
    my_cylinder_treatement_func()  # change name
else:
    faces = {idx: (face, Bounding_geometry.IsPointInside(rg.AreaMassProperties.Compute(face).Centroid, sc.doc.ModelAbsoluteTolerance, True)) for idx, face in enumerate(self.brep.Faces)}
    joint_face_ids = [[key] + [adj_face for adj_face in value[0].AdjacentFaces() if faces[adj_face][1] and adj_face != key] for key, value in faces.items() if value[1]]

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done: I created two functions, one for each dictionnary:

faces = _find_joint_faces(self, bounding_geometry)
adjacency_of_faces = _compute_adjacency_of_faces(self, faces)
adjacency_of_faces = diffCheck.df_util.merge_shared_indexes(adjacency_of_faces)
joint_face_ids = [[key] + value[1] for key, value in adjacency_of_faces.items()]

Copy link
Contributor

@9and3 9and3 left a comment

Choose a reason for hiding this comment

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

lookin good! Thanks @DamienGilliard ! 👐

@9and3 9and3 merged commit 03df9c6 into main Sep 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request PythonAPI

Projects

None yet

3 participants