[PR#3757] [Part Design] allow preselect additional sections/spines

Merged, abandoned or rejected pull requests are moved here to clear the main Pull Requests forum.
TheMarkster
Veteran
Posts: 5505
Joined: Thu Apr 05, 2018 1:53 am

[PR#3757] [Part Design] allow preselect additional sections/spines

Post by TheMarkster »

https://github.com/FreeCAD/FreeCAD/pull/3757

Currently, in order to do a loft in Part Design you must first open the dialog and then select the profile and additional sections. You can preselect the first profile, but not the additional selections. Those must be selected in the task dialog, and you get an annoying and unavoidable error message about missing sections. Similarly with the sweeps you must select the spine in the dialog.

This PR allows you to preselect (in the tree or in the 3d view) the additional sections or spines prior to executing the command and opening the dialog. For additive and subtractive lofts you select the sketches to use. For additive and subtractive pipes you select the sketches and/or paths. The first object in the selection is the profile, the others are treated as sections or spines depending on the tool. The order of selection is important. The dialog still opens, but no more error message and the additional elements are already in the appropriate places.
chrisb
Veteran
Posts: 53920
Joined: Tue Mar 17, 2015 9:14 am

Re: [PR#3757] [Part Design] allow preselect additional sections/spines

Post by chrisb »

This sounds good. I wait until it is in master to test.
It's not clear to me yet how the sequence exactly is, but it may be sensible to have the path selected last with a sweep.
A Sketcher Lecture with in-depth information is available in English, auf Deutsch, en français, en español.
TheMarkster
Veteran
Posts: 5505
Joined: Thu Apr 05, 2018 1:53 am

Re: [PR#3757] [Part Design] allow preselect additional sections/spines

Post by TheMarkster »

chrisb wrote: Thu Jul 30, 2020 7:01 am This sounds good. I wait until it is in master to test.
It's not clear to me yet how the sequence exactly is, but it may be sensible to have the path selected last with a sweep.
Yes, the profile is the first selection, the sweep path is the second. If subobjects are selected they are applied in the order of selection, but only subobjects of a single object can be used.

After some additional testing I've discovered a bug that I need to solve. With subtractive pipes the preselected paths are appearing in the dialog as expected, but the preview isn't being displayed. The operation works, but then the previous tip is still visible, too. Clearly the dialog isn't expecting the spine to be preselected, so there's something additional it does when the user selects the path manually that it isn't doing on initialization.
TheMarkster
Veteran
Posts: 5505
Joined: Thu Apr 05, 2018 1:53 am

Re: [PR#3757] [Part Design] allow preselect additional sections/spines

Post by TheMarkster »

Preselection of 2d object edges works for building pipes, but not preselecting edges 3d objects, so these are no longer added to the Spine. Instead, a warning is given to the user that this is not supported at this time and that these can be selected inside the dialog.

As an example, if you have a Pad and you'd like to sweep a profile along the edge of the Pad you will need to select the Pad edges in the dialog rather than preselect them. If on the other hand you create a sketch containing the path to sweep along then you can preselect one or more edges of that sketch to be used as the path.

Perhaps the dialog could be modified to support this, but I'm not sure at this time what changes would be needed. For now, this is at least an improvement for lofts and for pipes with 2d objects containing the paths/spines to be used for the sweeps.
TheMarkster
Veteran
Posts: 5505
Joined: Thu Apr 05, 2018 1:53 am

Re: [PR#3757] [Part Design] allow preselect additional sections/spines

Post by TheMarkster »

I think I have the bugs worked out. Paths can be preselected that are edges of 2d and 3d objects.
User avatar
uwestoehr
Veteran
Posts: 4961
Joined: Sun Jan 27, 2019 3:21 am
Location: Germany
Contact:

Re: [PR#3757] [Part Design] allow preselect additional sections/spines

Post by uwestoehr »

TheMarkster wrote: Fri Jul 31, 2020 2:22 am I think I have the bugs worked out. Paths can be preselected that are edges of 2d and 3d objects.
I would like to test the feature. Do you have a screencast or example file? Also the PR needs a merge commit in order to compile it.

Is there a reason why the PR was not discussed nor touched for so long?
TheMarkster
Veteran
Posts: 5505
Joined: Thu Apr 05, 2018 1:53 am

Re: [PR#3757] [Part Design] allow preselect additional sections/spines

Post by TheMarkster »

uwestoehr wrote: Mon Sep 14, 2020 9:02 pm
TheMarkster wrote: Fri Jul 31, 2020 2:22 am I think I have the bugs worked out. Paths can be preselected that are edges of 2d and 3d objects.
I would like to test the feature. Do you have a screencast or example file? Also the PR needs a merge commit in order to compile it.

Is there a reason why the PR was not discussed nor touched for so long?
I'm not sure what the delay is. You can compile easily enough.

From your source folder master branch in git:

git branch pd_loft
git checkout pd_loft
git pull https://github.com/mwganson/FreeCAD pd_loft

Then compile as you normally would from the bin folder.
User avatar
uwestoehr
Veteran
Posts: 4961
Joined: Sun Jan 27, 2019 3:21 am
Location: Germany
Contact:

Re: [PR#3757] [Part Design] allow preselect additional sections/spines

Post by uwestoehr »

I could compile your PR (despite of a merge conflict). Here is the testfile I used:
Loft-play.FCStd
testfile
(30.79 KiB) Downloaded 141 times
At least from the UI perspective I cannot see a difference. What do I have to do to see the feature?
wmayer
Founder
Posts: 20243
Joined: Thu Feb 19, 2009 10:32 am
Contact:

Re: [PR#3757] [Part Design] allow preselect additional sections/spines

Post by wmayer »

Review:
the feature works nicely as soon as you follow the supposed workflow.

But in the TaskPipeParameters you access the document via "ActiveDocument" and this is considered bad practice. This is because there is no guarantee that the document that opened the task panel is the active document at that time, e.g. when the user activates another document after the task dialog has been opened.

The consequence usually are side-effects because the further operations are applied to the wrong document and in this special case it even causes the application to crash. The crash happens because doCommand() raises an exception inside the destructor of TaskPipeParameters and because it's not declared with a throw() specification the OS terminates the application with SIGABRT.
User avatar
uwestoehr
Veteran
Posts: 4961
Joined: Sun Jan 27, 2019 3:21 am
Location: Germany
Contact:

Re: [PR#3757] [Part Design] allow preselect additional sections/spines

Post by uwestoehr »

Now I got the workflow:

1. select several sketches
2. start the lofting

result: the sketch that was first selected becomes the base object, the others go to the sections in the order they have been selected.
So the PR works fine. It was just not clear to me at the first try that the selection order becomes also the order in the dialog. So everything is fine (apart of the coding as Werner reviewed).

That one can also/later easily change the section order in the dialog is possible with my according PR: https://github.com/FreeCAD/FreeCAD/pull/3871
Post Reply