DGtal-team / DGtal

Digital Geometry Tools and Algorithm Library
https://dgtal.org
GNU Lesser General Public License v3.0
370 stars 115 forks source link

Incertitude on the chosen basis in Point2DEmbedderIn3D #1348

Closed rolanddenis closed 4 years ago

rolanddenis commented 6 years ago

Point2DEmbedderIn3D has two constructors:

  1. the first one (https://github.com/DGtal-team/DGtal/blob/master/src/DGtal/kernel/BasicPointFunctors.h#L388) takes an origin and two other points on the 2D plane in order to build the corresponding basis.
  2. the second one (https://github.com/DGtal-team/DGtal/blob/master/src/DGtal/kernel/BasicPointFunctors.h#L420) takes only an origin and a vector normal to the plane.

For this last constructor, the calculated 2D -> 3D embeddings will depend on the chosen basis but this choice is not documented. So, is the way this basis is actually chosen a kind of canonical way ?

In addition, the fix applied when the pRefOrigin point is equal to anOriginPoint (see https://github.com/DGtal-team/DGtal/blob/master/src/DGtal/kernel/BasicPointFunctors.h#L432) may lead to a basis that is not orthogonal to the given normal (eg with an origin at (1, 0, 0) and with the normal (sqrt(2)/2, sqrt(2)/2, 0)).

rolanddenis commented 6 years ago

Another question: if we choose std::round as the rounding mode when calculating the origin point (https://github.com/DGtal-team/DGtal/blob/master/src/DGtal/kernel/BasicPointFunctors.h#L459), shouldn't we do the same when calculating the embeddings in operator() (https://github.com/DGtal-team/DGtal/blob/master/src/DGtal/kernel/BasicPointFunctors.h#L482) ? Maybe also store the origin point as a RealPoint ?

rolanddenis commented 5 years ago

A temporary fix (but completely wrong) has been merged through https://github.com/DGtal-team/DGtal/pull/1345/commits/2abba3d6e18a0327c59dc7b14b3c295fa0e806f0 so that testBasicFunctors passes.

dcoeurjo commented 4 years ago

Could this issue be closed?

kerautret commented 4 years ago

perhaps not I will have a look to remake the test correctly and complete some add in the doc.