PartDesign Loft

Discussion about the development of the Assembly workbench.
Forum rules
Be nice to others! Respect the FreeCAD code of conduct!
Post Reply
Fat-Zer
Posts: 176
Joined: Thu Oct 30, 2014 10:38 pm

PartDesign Loft

Post by Fat-Zer »

Ok, I've got some questions about PartDesign's loft...

Why TaskLoftParameters.ui have two additional hidden widgets under stackedWidget? it's accidentally or on some purpose?
ickby, why you haven't used TopoShape::makeLoft()? I remember I've asked but don't remember the answer...
Also there are some malfunctions/unimplemented features:
1. Closed property is not implemented.
2. Loft constructs set of lofts rather one full loft...
3. Because of 2. ruled property is useless (it takes visual effect only if there are 3 or more wires in a single loft).

The GUI is quite crappy yet, but I believe it will be changed later...

Also I've got a proposal to make the loft non-sketchBased (not to derive from PartDesign::SketchBased), because unlike others it require more than one equivalent sketches.
ickby
Veteran
Posts: 3116
Joined: Wed Oct 05, 2011 7:36 am

Re: PartDesign Loft

Post by ickby »

Hello,

first of all you need toconsider that this tool, like most of my work on the assermbly branch, was done under heavy time constraints. So it is most likely not the best implementation ever. The idea was to throw out somehing and fix upcoming problems when merged.

The thing was that I started with the sweep tool, and for this I needed to create new code to get all the functionality I needed. Once this was done I copied that code over to loft and only sligthly changed it. TopoShape::makeLoft() works with faces, but a quick test back then showed tha tit does not work with faces which have inner wires, e.g. holes. This is a needed feature for the tool, as it will be natural to loft things like pipes in PartDesign. Maybe it would have been a better idea to extend the TopoShape functionality, but even then testing it in the partDesign tool to fix bugs will be helpful, the transition can be made later.
Why TaskLoftParameters.ui have two additional hidden widgets under stackedWidget? it's accidentally or on some purpose
mos likely a copy paste error.
2. Loft constructs set of lofts rather one full loft...
Yes, due to the inner wire problem the idea was to construct lofts for every wire combination and then sew everything together. This approach was needed as tests showed that making multiple lofts and subtracting them is way too slow. The speed issue is especially important for sweeps, as there the booleans were highly annoying.
Because of 2. ruled property is useless (it takes visual effect only if there are 3 or more wires in a single loft).
Yeah, this one is a open question
The GUI is quite crappy yet, but I believe it will be changed later...
Yeah, tha tis also a time issue. I think it is important to have the same ui as in the sweep tool for profile selection, but the one used is most likly not the perfect one :)
Also I've got a proposal to make the loft non-sketchBased (not to derive from PartDesign::SketchBased), because unlike others it require more than one equivalent sketches.
I don't think this is a good idea. A Loft (and sweep) are based on sketches. You have more than one, this is true, but you start of with a sketch too. This sketch needs to be processed like every other sketch for pad or pocket. So it is highly useful to reuse the code of SketchBased for this. Also I always thought of extending SketchBased to allow arbitrary faces as base instead of only sketches. And having only one place to change such things is highly useful in reducing work. In general there are multiple tools which need a base sketch and additional ones, right now for example sweep (for path, multiprofile or auxillery orientation), but also a possible "path transform" tool. So maybe making a MultiSketchBased thing based on SketchBased will be helpful, which extends the sketch processing to mutliple sketches. But I don't see any real use in making loft non-Sketch based
Fat-Zer
Posts: 176
Joined: Thu Oct 30, 2014 10:38 pm

Re: PartDesign Loft

Post by Fat-Zer »

Thanks for explanations... can I assume you are onto this and I may not to bother myself to fix it more than I need?
ickby wrote: A Loft (and sweep) are based on sketches.
The key difference of Loft from others (including the sweep) is that it must have several sketches with equivalent meaning, and it doesn't make a lot of sense to make one of them anyway special (at least because they should be reorderable)...
ickby wrote:This sketch needs to be processed like every other sketch for pad or pocket.
All sketches should be processed that way, not only the first one... (Again, unlike other SketchBased's). Sure the processing code must be shared, but not by inheriting IMHO... I suppose some property-unaware code may be just moved to AddSub feature...
ickby wrote: I think it is important to have the same ui as in the sweep tool for profile selection, but the one used is most likly not the perfect one
hm... I don't really see any reasons why they should match...

By the word, I've just now crashed it:

Code: Select all

#0  0x00007fffd134c198 in BRepFill_CompatibleWires::SameNumberByPolarMethod(unsigned int) () from /usr/lib64/opencascade-6.9.0/ros/lin/lib64/libTKBool.so.0
#1  0x00007fffd134f607 in BRepFill_CompatibleWires::Perform(unsigned int) () from /usr/lib64/opencascade-6.9.0/ros/lin/lib64/libTKBool.so.0
#2  0x00007fffcf7db913 in BRepOffsetAPI_ThruSections::Build() () from /usr/lib64/opencascade-6.9.0/ros/lin/lib64/libTKOffset.so.0
#3  0x00007fffccf2142f in PartDesign::Loft::execute (this=0x53c5b50) at /home/alexander/projects/free-cad_code/src/Mod/PartDesign/App/FeatureLoft.cpp:141
#4  0x00007ffff697bf73 in App::DocumentObject::recompute (this=0x53c5b50) at /home/alexander/projects/free-cad_code/src/App/DocumentObject.cpp:86
#5  0x00007fffd3eb1c95 in Part::Feature::recompute (this=0x53c5b50) at /home/alexander/projects/free-cad_code/src/Mod/Part/App/PartFeature.cpp:83
#6  0x00007ffff693bb26 in App::Document::_recomputeFeature (this=0x4cfd250, Feat=0x53c5b50) at /home/alexander/projects/free-cad_code/src/App/Document.cpp:1484
#7  0x00007ffff693bfcd in App::Document::recomputeFeature (this=0x4cfd250, Feat=0x53c5b50) at /home/alexander/projects/free-cad_code/src/App/Document.cpp:1541
#8  0x00007fffcd2bcfce in PartDesignGui::TaskSketchBasedParameters::recomputeFeature (this=0x53af630) at /home/alexander/projects/free-cad_code/src/Mod/PartDesign/Gui/TaskSketchBasedParameters.cpp:197
#9  0x00007fffcd31a7e9 in PartDesignGui::TaskLoftParameters::onClosed (this=0x53af630, val=false) at /home/alexander/projects/free-cad_code/src/Mod/PartDesign/Gui/TaskLoftParameters.cpp:229
#10 0x00007fffcd31aae8 in PartDesignGui::TaskLoftParameters::qt_static_metacall (_o=0x53af630, _c=QMetaObject::InvokeMetaMethod, _id=2, _a=0x7fffffffb2f0)
    at /tmp/FreeCAD_trunk-build/src/Mod/PartDesign/Gui/moc_TaskLoftParameters.cpp:55
#11 0x00007ffff13264aa in QMetaObject::activate(QObject*, QMetaObject const*, int, void**) () from /usr/lib64/qt4/libQtCore.so.4
#12 0x00007ffff2423562 in QAbstractButton::toggled(bool) () from /usr/lib64/qt4/libQtGui.so.4
#13 0x00007ffff21818c8 in QAbstractButton::setChecked(bool) () from /usr/lib64/qt4/libQtGui.so.4
#14 0x00007ffff21937e6 in QCheckBox::nextCheckState() () from /usr/lib64/qt4/libQtGui.so.4
#15 0x00007ffff21814e2 in ?? () from /usr/lib64/qt4/libQtGui.so.4
#16 0x00007ffff218160c in QAbstractButton::mouseReleaseEvent(QMouseEvent*) () from /usr/lib64/qt4/libQtGui.so.4
#17 0x00007ffff1e1d9a2 in QWidget::event(QEvent*) () from /usr/lib64/qt4/libQtGui.so.4
#18 0x00007ffff1dcd36c in QApplicationPrivate::notify_helper(QObject*, QEvent*) () from /usr/lib64/qt4/libQtGui.so.4
#19 0x00007ffff1dd3c1a in QApplication::notify(QObject*, QEvent*) () from /usr/lib64/qt4/libQtGui.so.4
#20 0x00007ffff72fbb95 in Gui::GUIApplication::notify (this=0x7fffffffc530, receiver=0x4a3aa40, event=0x7fffffffb9b0) at /home/alexander/projects/free-cad_code/src/Gui/Application.cpp:1572
#21 0x00007ffff13122dd in QCoreApplication::notifyInternal(QObject*, QEvent*) () from /usr/lib64/qt4/libQtCore.so.4
#22 0x00007ffff1dd33f3 in QApplicationPrivate::sendMouseEvent(QWidget*, QMouseEvent*, QWidget*, QWidget*, QWidget**, QPointer<QWidget>&, bool) () from /usr/lib64/qt4/libQtGui.so.4
#23 0x00007ffff1e45d4b in ?? () from /usr/lib64/qt4/libQtGui.so.4
#24 0x00007ffff1e4479c in QApplication::x11ProcessEvent(_XEvent*) () from /usr/lib64/qt4/libQtGui.so.4
#25 0x00007ffff1e6b8f2 in ?? () from /usr/lib64/qt4/libQtGui.so.4
#26 0x00007fffea5d1754 in g_main_context_dispatch () from /usr/lib64/libglib-2.0.so.0
#27 0x00007fffea5d1998 in ?? () from /usr/lib64/libglib-2.0.so.0
#28 0x00007fffea5d1a3c in g_main_context_iteration () from /usr/lib64/libglib-2.0.so.0
#29 0x00007ffff133f9ae in QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) () from /usr/lib64/qt4/libQtCore.so.4
#30 0x00007ffff1e6b9a6 in ?? () from /usr/lib64/qt4/libQtGui.so.4
#31 0x00007ffff1310e7f in QEventLoop::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) () from /usr/lib64/qt4/libQtCore.so.4
#32 0x00007ffff1311175 in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) () from /usr/lib64/qt4/libQtCore.so.4
#33 0x00007ffff13165c9 in QCoreApplication::exec() () from /usr/lib64/qt4/libQtCore.so.4
#34 0x00007ffff72f7316 in Gui::Application::runApplication () at /home/alexander/projects/free-cad_code/src/Gui/Application.cpp:1865
#35 0x0000000000407429 in main (argc=1, argv=0x7fffffffcec8) at /home/alexander/projects/free-cad_code/src/Main/MainGui.cpp:23
ickby
Veteran
Posts: 3116
Joined: Wed Oct 05, 2011 7:36 am

Re: PartDesign Loft

Post by ickby »

Thanks for explanations... can I assume you are onto this and I may not to bother myself to fix it more than I need?
Well not realy, currently I have no real time to work on this.
The key difference of Loft from others (including the sweep) is that it must have several sketches with equivalent meaning, and it doesn't make a lot of sense to make one of them anyway special (at least because they should be reorderable)...
There is no use case for reordering the first sketch, only the other ones you select additional. Also it is the same for sweep if you make a multi-profile sweep. The only difference you get from chaning all this is that the user must not select the first sketch when starting the tool. This is very little gain for a large change. And you can't do this anyway for multi-profile sweeps, so you would get different behaviour for tools that should work the same form user-point of view.
All sketches should be processed that way, not only the first one... (Again, unlike other SketchBased's). Sure the processing code must be shared, but not by inheriting IMHO... I suppose some property-unaware code may be just moved to AddSub feature...
Again, the gain seems neglectable.
hm... I don't really see any reasons why they should match...
Because they do exactly the same, the user selects additional profiles the tool should consider. Having different UI layouts for exact the same action is stupid.

Thanks for the crash report, seems it happens inside of occ. Can you share the profiles you used?
Fat-Zer
Posts: 176
Joined: Thu Oct 30, 2014 10:38 pm

Re: PartDesign Loft

Post by Fat-Zer »

ickby wrote:Well not realy, currently I have no real time to work on this.
hmm... ok...
ickby wrote: There is no use case for reordering the first sketch, only the other ones you select additional. Also it is the same for sweep if you make a multi-profile sweep.
Hmm... I wasn't aware of multisection sweeps and how they work... now I see why do you tell they have something common... I'still think that loft shouldn't be a sketch based, but I'm more open now to reconsider my position... Now I'm more about introducing some multiSketchFeature abstraction for both of them...
ickby wrote: The only difference you get from chaning all this is that the user must not select the first sketch when starting the tool. This is very little gain for a large change. And you can't do this anyway for multi-profile sweeps, so you would get different behaviour for tools that should work the same form user-point of view.
The second difference is that loft will be distinguished from other sketchbased features, so e.g sketch axes won't be referred in the transformations list, which doesn't make so much sense for a loft, also I don't suppose there will be a lot of work over there, most of it is just moving some code...
Fat-Zer wrote:3. Because of 2. ruled property is useless (it takes visual effect only if there are 3 or more wires in a single loft).
seems I was wrong about that one...
ickby wrote:Thanks for the crash report, seems it happens inside of occ. Can you share the profiles you used?
AFAIR I've done exactly the same (see attachment), but I can't reproduce it anymore =(...
Attachments
loft-crash.fcstd
(20.47 KiB) Downloaded 79 times
User avatar
DeepSOIC
Veteran
Posts: 7896
Joined: Fri Aug 29, 2014 12:45 am
Location: used to be Saint-Petersburg, Russia

Re: PartDesign Loft

Post by DeepSOIC »

ickby wrote:There is no use case for reordering the first sketch, only the other ones you select additional.
Eem, what?? what if the sketch that I wanted in the middle has accidentally ended up being the first? (please excuse me if this is irrelevant, I am not even familiar with partdesign lofts and sweeps UI)
ickby
Veteran
Posts: 3116
Joined: Wed Oct 05, 2011 7:36 am

Re: PartDesign Loft

Post by ickby »

ickby wrote:
There is no use case for reordering the first sketch, only the other ones you select additional.
Eem, what?? what if the sketch that I wanted in the middle has accidentally ended up being the first? (please excuse me if this is irrelevant, I am not even familiar with partdesign lofts and sweeps UI)
I'm saying this because currently one selects the first sketch, then uses the tool and then adds the additional sketches. So the first sketch is picked delibertly by the user. You are of course right when the gui gets changed so that one can select all sketches before starting the tool, I did not yet consider this case.
Post Reply