Skip to content

Registrations wraps/components + downsample components#18

Merged
9and3 merged 13 commits intomainfrom
glb_lcl_registration_wrap
May 28, 2024
Merged

Registrations wraps/components + downsample components#18
9and3 merged 13 commits intomainfrom
glb_lcl_registration_wrap

Conversation

@9and3
Copy link
Contributor

@9and3 9and3 commented May 17, 2024

This PR introduces:

image (6)
image (7)
image (10)
image (9)
image (8)
image (11)

downsampling example:
downsample_test

icp registration example:
icp_test

@9and3 9and3 added the dev label May 17, 2024
@9and3 9and3 self-assigned this May 17, 2024
@9and3 9and3 changed the title Registrations wraps + components Registrations wraps/components + downsample components May 20, 2024
@9and3
Copy link
Contributor Author

9and3 commented May 20, 2024

Hello! This PR is ready! Happy to integrate your suggestions if you have!
Just few side notes:

  • tall registration components: I tried to integrate the settings in a separate component to lighten up the registration components (they are very tall) but I didn't manage since each one has its own set. So I decided to expose the majority of values with default values so they do not have to be set.
  • decoupling registration prep: (esp. @DamienGilliard ) In c++ we offer the possibility to downsample the cloud in the function registration but I think it's better to separate everything, that's wh I added 3 downsampling methods/components.. the user will decide.
  • hidden parameters: parameters for registrations are completly exposed except for the ICP where the thershold fitness and RMSE is hidden and set by default to 1e-6 since it will always get the best value by doing so.
  • ICPs together: the ICPs functions are coupled together in one component and user can switch with a toggle

Cheers!

Copy link
Collaborator

Choose a reason for hiding this comment

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

Not intentional, yes ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

what do you mean?

*
* @param source the source diffCheck point cloud
* @param target the target diffCheck point cloud
* @param voxelize whether to voxelize the point clouds before computing the FPFHFeatures. A higher value will result in a more coarse point cloud (less resulting points).
Copy link
Collaborator

Choose a reason for hiding this comment

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

Oups, thanks for spotting that !

@@ -20,7 +20,7 @@ PYBIND11_MODULE(diffcheck_bindings, m) {
submodule_test.def("test", &test, "Simple function testing a vanilla python bindings.");
Copy link
Collaborator

@DamienGilliard DamienGilliard May 24, 2024

Choose a reason for hiding this comment

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

Simple question, why do you keep a "test" submodule ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

just to have a simple test component that we can import to see if everythin is alright. It's more of an utility than anything else.

Comment on lines +156 to +161
elif RhinoDoc.ModelUnitSystem == Rhino.UnitSystem.Inches:
unit_scale = 0.0254
elif RhinoDoc.ModelUnitSystem == Rhino.UnitSystem.Feet:
unit_scale = 0.3048
elif RhinoDoc.ModelUnitSystem == Rhino.UnitSystem.Yards:
unit_scale = 0.9144
Copy link
Collaborator

Choose a reason for hiding this comment

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

bien vu


# rotation
rotation = df_xform.rotation_matrix
rh_xform = rh_xform * rg.Transform.Rotation(rotation[0], rg.Vector3d(rotation[1], rotation[2], rotation[3]), rg.Point3d(0, 0, 0))
Copy link
Collaborator

@DamienGilliard DamienGilliard May 26, 2024

Choose a reason for hiding this comment

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

Are you sure this works ? The DFTransformation.RotationMatrix is a 3x3 matrix, so I don't understand what happens here. From the Rhino Common API i suppose you use this constructor : https://developer.rhino3d.com/api/rhinocommon/rhino.geometry.transform/rotation#(vector3d,vector3d,point3d) but rotation[1], rotation[2] and rotation[3] are not doubles or int, they should be lines of the matrix and rotation[3] should be out of range, so as I read it, it will not work as intended, but I am probably wrong.

Copy link
Collaborator

@DamienGilliard DamienGilliard left a comment

Choose a reason for hiding this comment

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

Amazing work, thanks for letting me the time to go through this @9and3 . Very happy to dive again into python bindings. I still have a lot of work to fully understand the ghcomponentizer and pyperizer. I'll reserve a few hours for that during my vacations ;)

@9and3
Copy link
Contributor Author

9and3 commented May 27, 2024

Amazing work, thanks for letting me the time to go through this @9and3 . Very happy to dive again into python bindings. I still have a lot of work to fully understand the ghcomponentizer and pyperizer. I'll reserve a few hours for that during my vacations ;)

thank you for going through it! It's always better to have a pair of eyes more :) thanks !👐

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants