Draft: Fix projectPoint method

[marioalexis84]

see https://forum.freecadweb.org/viewtopic.php?f=23&t=59903&sid=b6fcc8f0fae011760ddf1cd858ba354a

Thank you for creating a pull request to contribute to FreeCAD! To ease integration, we ask you to conform to the following items. Pull requests which don't satisfy all the items below might be rejected. If you are in doubt with any of the items below, don't hesitate to ask for help in the FreeCAD forum!

• Your pull request is confined strictly to a single module. That is, all the files changed by your pull request are either in App, Base, Gui or one of the Mod subfolders. If you need to make changes in several locations, make several pull requests and wait for the first one to be merged before submitting the next ones
• In case your pull request does more than just fixing small bugs, make sure you discussed your ideas with other developers on the FreeCAD forum
• Your branch is rebased on latest master git pull --rebase upstream master
• All FreeCAD unit tests are confirmed to pass by running ./bin/FreeCAD --run-test 0
• All commit messages are well-written ex: Fixes typo in Draft Move command text
• Your pull request is well written and has a good description, and its title starts with the module name, ex: Draft: Fixed typos
• Commit messages include issue #<id> or fixes #<id> where <id> is the FreeCAD bug tracker issue number in case a particular commit solves or is related to an existing issue on the tracker. Ex: Draft: fix typos - fixes #0004805

And please remember to update the Wiki with the features added or changed once this PR is merged.
Note: If you don't have wiki access, then please mention your contribution on the 0.20 Changelog Forum Thread.

---

[freecadci]

for feature branch PR_5307. Pipeline 439265240 was triggered at fcd4386. All CI branches and pipelines.

[Roy-043]

I can confirm that this code fixes the issue in the forum topic. Thanks for that.

Repeating this test: https://forum.freecadweb.org/viewtopic.php?p=557743#p557743 with the new code shows the improved accuracy. The deviation of the local Z from zero goes from ca. 1e-5 to ca. 1e-15!

I am not that experienced with Python, but why do you use:
if direction is not None:
Instead of just:
if direction:
?

Two small remarks:

1. If direction is orthogonal to axis the function should return None IMO. You should not use the axis in this case.
2. If direction is a null vector there is a division by zero error. I am not sure if, or how, that should be handled.

[marioalexis84]

I am not that experienced with Python, but why do you use: if direction is not None: Instead of just: if direction: ?

Although effectively in the case of Base.Vector we can use if direction: without inconvenience, the code style guide proposes avoiding the use of booleans in a situation like this : https://www.python.org/dev/peps/pep-0008/#programming-recommendations

1. If `direction` is orthogonal to `axis` the function should return `None` IMO. You should not use the `axis` in this case.

Using None generates a segmentation fault in Coin. Another option is to use a Vector made up of nan, but this generates some unexpected behavior in the 3D view. Therefore, to avoid these problems, I decided to use axis as the direction of the projection in this specific case since it allows a good workflow when the direction of the view is changed.

2. If `direction` is a null vector there is a division by zero error. I am not sure if, or how, that should be handled.

It should be the responsibility of the programmer not to pass a null vector as the direction of the projection.

[Roy-043]

Thanks for your answers.

Using None generates a segmentation fault in Coin.

Yes, the calling code would have to catch the None return. Implementing that would be more involved. But it is perhaps possible to create an independent project_point function in DraftVecUtils.py (?) and use that function here?

[marioalexis84]

There are only two functions (in gui_selectplane.py and gui_snapper.py) that use the direction argument of projectPoint, so we can use None as return value when direction is orthogonal to axis, and catch it in those functions.
I will do these changes and force push again.

[marioalexis84]

@Roy-043 can you test again?
I modified the method to take into account the case of orthogonality. A new mode force_projection is added. If false the return value is None when the direction is orthogonal to axis. If True, the direction is modified to point towards a distant point on the plane. In this way it is not necessary to capture this particular case in the other functions that make use of projectPoint.

[freecadci]

for feature branch PR_5307. Pipeline 444168608 was triggered at 1cd0124. All CI branches and pipelines.

[yorikvanhavre]

Thanks for the work on this guys! Also very high-level math for me 😅

[yorikvanhavre]

BTW @Roy-043 I have invited you to the "FreeCAD Reviewers" group here on github (you should see an invite somewhere) so you can be asked a review, if you don't mind of course.

[Roy-043]

@marioalexis84 Thanks. The code works fine.

[Roy-043]

@yorikvanhavre My Python knowledge is still limited. But I do not mind testing code.

[yorikvanhavre]

Thanks for testing! Ok to merge @marioalexis84 ?

[Roy-043]

I am not sure how important this is, but I notice that the epsilon value is very small (2.2e-16). It is much smaller than the tolerance used elsewhere. F.e. App.Rotation() uses a tolerance of 1e-7. Shorter vectors are ignored.

[marioalexis84]

I am not sure how important this is, but I notice that the epsilon value is very small (2.2e-16)

The class Base::Vector also use this precision value. No need to use OCCT precision.

Thanks for testing! Ok to merge @marioalexis84 ?

It's ok to me.

[marioalexis84]

It is worth noting that this particular case (orthogonality) occurs only if we project in parallel to WorkingPlane, which is not a useful case.

[yorikvanhavre]

In any case indeed it would deserve to have a better discussion over overall Draft precision... Okay, let's merge already!