App::Part question

Discussion about the development of the Assembly workbench.
Forum rules
Be nice to others! Respect the FreeCAD code of conduct!
ickby
Veteran
Posts: 3116
Joined: Wed Oct 05, 2011 7:36 am

Re: App::Part question

Post by ickby »

return false from onDelete(), haven't tried yet, but should work fine
Are you sure? I'm not entirly sure from where onDelete is called. But the object deletion is a App::document thing and the relevant functions issue signals that something gets deleted, but they do never use any return value. Of course you can change the functions, the relevant signals (including all connecting slots) and than create a signal return type combiner to anylse if any registered slot returnes false, but well, that is a large undertaking. Don't know if it is worth the result.
just check if plane/axis/origin is part of origin/origin/part and delete if not...
But this restricts the origin to known classes it can be in, so it would become impossible for any workbench to reuse the origin in a different context.
Fat-Zer
Posts: 176
Joined: Thu Oct 30, 2014 10:38 pm

Re: App::Part question

Post by Fat-Zer »

ickby wrote:Are you sure?
not absolutely... it definitely should work when deleting from the Gui with delete press, but I'm not sure about other ways... will think about it one more time...
ickby wrote:But this restricts the origin to known classes it can be in, so it would become impossible for any workbench to reuse the origin in a different context.
yes, just thought about that too... "refuse to delete if somebody links to it" would be better...
ickby
Veteran
Posts: 3116
Joined: Wed Oct 05, 2011 7:36 am

Re: App::Part question

Post by ickby »

not absolutely... it definitely should work when deleting from the Gui with delete press, but I'm not sure about other ways... will think about it one more time...
ah, I found a call to onDelete in the delete command class. There it realy checks the return type. So this should work, yes, and is actually much simpler. One would just need to check if all delete operations really issue the command an do not use the document function directly.
yes, just thought about that too... "refuse to delete if somebody links to it" would be better...
most likely yes
Fat-Zer
Posts: 176
Joined: Thu Oct 30, 2014 10:38 pm

Re: App::Part question

Post by Fat-Zer »

Ok, about the delete issue: forbiding the deletion works great in the view providers level as expected...
But the cascade deletion of objects does not... I strongly don't like the Stefan's implementation I reverted, so I'll better add a yet another callback like I've done for setupObject()...

Now I'm a bit stuck on a relatively easy thing... I need a nice small value to indicate the bounding box (and the origin) is too small to be displayed... previously it was with hardcoded "1e-7", but I want something non-magick.
I don't want to use opencascade's Precision::Confusion(), due to Gui module doesn't depends on it directly... Also some user provided value would be nice here... Any ideas?
Fat-Zer
Posts: 176
Joined: Thu Oct 30, 2014 10:38 pm

Re: App::Part question

Post by Fat-Zer »

Finished App::Origin's refactoring... If somebody wants to look through: https://github.com/Fat-Zer/FreeCAD_sf_m ... partRework
It took a bit more time when I suspected...
Next tasks: Gui::ViewProvider{Plane,Line} and App::Part

PS: I want to rename App::Line->Axis any agrees/disagrees?
PPS: DeepSOIC, what's this assert for? src/Mod/Part/App/Attacher.cpp: 617
ickby
Veteran
Posts: 3116
Joined: Wed Oct 05, 2011 7:36 am

Re: App::Part question

Post by ickby »

Hello Fat-Zer,

unfortunatly I don't realy have time the next weeks to check your changes. However, as we discussed quite a lot about this I'm confident that this works out well.

One thought I had recently: you may want to pull in the changes I've done in my branch, as it will get only more complicated with more large changes in your branch.
User avatar
DeepSOIC
Veteran
Posts: 7896
Joined: Fri Aug 29, 2014 12:45 am
Location: used to be Saint-Petersburg, Russia

Re: App::Part question

Post by DeepSOIC »

Fat-Zer wrote:PPS: DeepSOIC, what's this assert for? src/Mod/Part/App/Attacher.cpp: 617
Some time back, there were two options to attach sketches to planes. The selection was done by adding "front"/"back" subelement into the reference's subelement field. At some point I decided to remove that, because I've added a reversal property. That's what the assert is about - when attaching to planes, subelement is expected to be empty. You can kill that assert if you want to.
Fat-Zer
Posts: 176
Joined: Thu Oct 30, 2014 10:38 pm

Re: App::Part question

Post by Fat-Zer »

ickby wrote:unfortunatly I don't realy have time the next weeks to check your changes.
sure, no hurry here... anyway it's not finished yet...
ickby wrote:One thought I had recently: you may want to pull in the changes I've done in my branch, as it will get only more complicated with more large changes in your branch.
I was gonna to propose to merge after Part changes... except by the Plane's view provider's selection stuff, which I was gonna to cherry-pick sooner...
DeepSOIC wrote:Some time back, there were two options to attach sketches to planes. The selection was done by adding "front"/"back" subelement into the reference's subelement field. At some point I decided to remove that, because I've added a reversal property. That's what the assert is about - when attaching to planes, subelement is expected to be empty. You can kill that assert if you want to.
Thanks, I've run into those several times because Plain's view provider returns "Main" sometimes, so I'll just remove them...
Fat-Zer
Posts: 176
Joined: Thu Oct 30, 2014 10:38 pm

Re: App::Part question

Post by Fat-Zer »

Finished Part rework. Now the structure of Part classes is next:
  • DocumentObjectGroup — a generic set of objects.
  • GeoFeatureGroup — placeable set of objects.
  • OriginGroup — placeable set of objects with an origin.
  • Part — General abstraction created by the user. Got it's License, Id and other properties which doesn't work now...
The code should be rewritten to work with one of three first objects (with which one depends on the context), but it's not supposed to refer the Part class in general anymore... Assembly abstractions should be based on either OriginGroup or GeoFeatureGroup...

Current code contains several bugs (mostly around PartDesign workbench) e.g. Datums attachment is heavily broken... but I haven't a lot focused on them, becouse it will likely be broken again soon...
Shor/middle term plans are:
  • Relatively small enhancements/code unifications to Plane/Line view providers.
  • Fix drag & drop API: currently for drop there are two type of API: "old one" with canDropObjects ()/dropObject () and the "new" (unfinished and/or broken) with allowDrop/drop, the second one is more flexible, but unfinished and used only in the assembly...
  • Add Origin to the Body (And nearly complete PartDesign review which follows)
  • Merge/rebase
  • Stability/functionality fixes
  • Rename App::Line → App::Axis ( may be I won't do that )
  • Refactoring of PartDesign/Command.cpp : it's barely readable now because of insane use of lambdas and function pointers and contain lots of stuff in the global namespace... I've got a nice Ideas how to get rid of them... will try if it will workout...
Last edited by Fat-Zer on Sat Sep 05, 2015 11:07 pm, edited 2 times in total.
ickby
Veteran
Posts: 3116
Joined: Wed Oct 05, 2011 7:36 am

Re: App::Part question

Post by ickby »

it's barely readable now because of insane use of lambdas and function pointers and contain lots of stuff in the global namespace... I've got a nice Ideas how to get rid of them... will try if it will workout...
Hehe, remember: just because it is not your style of programming does not automatically make it worse. I find the lambda stuff to be way clearer as any possible object orientated approach, as the code is located where it normally really is, this makes it way simpler to understand what happens! No need for extra functions! Bu twell, I'm one of the few template metaprogramming lovers :)
Post Reply