Code review of merged Link3 branch

Here's the place for discussion related to coding in FreeCAD, C++ or Python. Design, interfaces and structures.
Forum rules
Be nice to others! Respect the FreeCAD code of conduct!
User avatar
DeepSOIC
Veteran
Posts: 7896
Joined: Fri Aug 29, 2014 12:45 am
Location: used to be Saint-Petersburg, Russia

Re: Code review of merged Link3 branch

Post by DeepSOIC »

Zolko wrote: Tue Oct 22, 2019 6:39 am
In SolidWorks they're called "configurations":

https://www.cati.com/blog/2018/01/solid ... gurations/
https://grabcad.com/tutorials/using-sol ... mbly-level

In T-Flex they're called "variables":

https://www.youtube.com/watch?v=WndPlY8aQoY

Lattice2's ParaSeries and TopoSeries kinda do it. But in a limited and specialized way, and slowly (because it creates a temporary project to recompute the variation, and often the slowest part is copying objects to the tmp doc (involves writing shapes to files and reading them back under the hood)).

So, to properly support it, we'll need an optimized mechanism of copying objects, and a mechanism of storing these varied objects.
realthunder
Veteran
Posts: 2190
Joined: Tue Jan 03, 2017 10:55 am

Re: Code review of merged Link3 branch

Post by realthunder »

Zolko wrote: Tue Oct 22, 2019 6:39 am In SolidWorks they're called "configurations":
In T-Flex they're called "variables":
I think FC Spreadsheet can achieve something similar. I'll make some changes to make it more friendly with this type of use case, like allow one sheet's column/row bindable to another sheet, with optional override. The idea is that, say you create a body, and put everything configurable into a spreadsheet. Once your initial design is finished, move the configuration to a master sheet, and change the local sheet to bind to the master sheet. To create variant parts, just copy the body and local sheet around. Change the local sheet to override some variables, or change the binding to a different row in the master sheet.

Forgot to mention, even without my proposed changes, you can already do this manually. It's just that you have to bind individual cells one by one, that is, for each configuration cell in the local sheet, manually type expression refer to the corresponding cell in the master sheet. You just need to do it once to the initial part. Variant parts just need copy and paste, and override selected cells.
Last edited by realthunder on Wed Oct 23, 2019 2:18 am, edited 1 time in total.
Try Assembly3 with my custom build of FreeCAD at here.
And if you'd like to show your support, you can donate through patreon, liberapay, or paypal
realthunder
Veteran
Posts: 2190
Joined: Tue Jan 03, 2017 10:55 am

Re: Code review of merged Link3 branch

Post by realthunder »

ezzieyguywuf wrote: Tue Oct 22, 2019 1:46 am What if you change the height of the cylinder, so that its top surface is no longer co-planar to the two cubes?
Same. I have already identified the cause. It is due to missing color state preservation when using VBO.
Try Assembly3 with my custom build of FreeCAD at here.
And if you'd like to show your support, you can donate through patreon, liberapay, or paypal
User avatar
Zolko
Veteran
Posts: 2213
Joined: Mon Dec 17, 2018 10:02 am

Re: Code review of merged Link3 branch

Post by Zolko »

vocx wrote: Mon Oct 21, 2019 10:36 pm
Zolko wrote: Mon Oct 21, 2019 9:56 pm ... so now would be the best time to think about such feature, before the feature freeze in preparation of the v0.19 release...
You cannot expect a completely new feature like what you want to be included in 0.19. That's a very short time to develop and test.
I disagree: it's not a new feature but a particular way to use App::Link, and once App::Link is merged released, it will be very difficult to change the fundamental underlyings. So if this feature-request needs some minimal but core changes to App::Link, then it should be done during the initial merge phase. That's why I post it under this thread.

Now, if this new feature needs a complete rewrite of App::Link, or an entirely new interface, then you're right, it should be postponed for later. I'm not able to tell how far-reaching impact it would have/need.
Last edited by Zolko on Wed Oct 23, 2019 10:06 am, edited 1 time in total.
try the Assembly4 workbench for FreCAD — tutorials here and here
C_h_o_p_i_n
Posts: 225
Joined: Fri Apr 26, 2019 3:14 pm

Re: Code review of merged Link3 branch

Post by C_h_o_p_i_n »

I think Zolko has the right Idea.

IIRC you could already overwrite attributes like colors using app:link - So why don't make an "overwrite" avaliable vor "any" named attribute ... e.g. a certain length blablub.length=xyz ... (one could think on marking certain values as "exportable" where it make sense to modify them in a linked objekt (without modifying the linked objekt itself) ... the lenght / size of a screw, the lenght of an exgtruded aluminium profile, length/width of steel rod. Linking once designed primitives, adjust/adapt them, assembley them.

Just my 2 cents.

Stefan
User avatar
Zolko
Veteran
Posts: 2213
Joined: Mon Dec 17, 2018 10:02 am

Re: Code review of merged Link3 branch

Post by Zolko »

realthunder wrote: Tue Oct 22, 2019 12:36 am what you are asking defeats the purpose of Link, because you want new geometry, while Link shares them. You can achieve similar effect by simply copying the objects.
OK, so if I understand then the "variant" would need a copy, and thus cannot be a pure App::Link. I did try to toy with this idea, and tried to copy() or deepcopy() FreeCAD objects. As you can guess, it doesn't work:

Code: Select all

>>> import copy
>>> part = App.activeDocument().addObject( 'App::Part', 'PartName')
>>> part
<Part object>
>>> part.TypeId
'App::Part'
>>> part_copy = copy.copy(part)
Traceback (most recent call last):
  File "<input>", line 1, in <module>
  File "/tmp/.mount_FreeCAqhOZPF/usr/lib/python3.7/copy.py", line 96, in copy
    rv = reductor(4)
TypeError: can't pickle App.GroupExtension objects
>>> 
and the code around line 96 is:

Code: Select all

        reductor = getattr(x, "__reduce_ex__", None)
        if reductor:
            rv = reductor(4)
duh !

realthunder wrote: Wed Oct 23, 2019 1:57 am To create variant parts, just copy the body and local sheet around.
DeepSOIC wrote: Tue Oct 22, 2019 9:31 am copying objects to the tmp doc (involves writing shapes to files and reading them back under the hood). So, to properly support it, we'll need an optimized mechanism of copying objects, and a mechanism of storing these varied objects.
So it's this just copy part that is a trouble. Could the App::Link interface take care of the just copy ? Or is there really a missing command/interface in FreeCAD for that ?
try the Assembly4 workbench for FreCAD — tutorials here and here
User avatar
adrianinsaval
Veteran
Posts: 5541
Joined: Thu Apr 05, 2018 5:15 pm

Re: Code review of merged Link3 branch

Post by adrianinsaval »

C_h_o_p_i_n wrote: Wed Oct 23, 2019 7:03 am I think Zolko has the right Idea.

IIRC you could already overwrite attributes like colors using app:link - So why don't make an "overwrite" avaliable vor "any" named attribute ... e.g. a certain length blablub.length=xyz ... (one could think on marking certain values as "exportable" where it make sense to modify them in a linked objekt (without modifying the linked objekt itself) ... the lenght / size of a screw, the lenght of an exgtruded aluminium profile, length/width of steel rod. Linking once designed primitives, adjust/adapt them, assembley them.

Just my 2 cents.

Stefan
That would certainly be useful, I guess brp files for the geometry would have to be saved within the asseembly file... another alternative would be to have an special type of file that exposes different configurations that are selected through a property of the link, this file would include the brp files for every configuration... just a tought... if brp files for linked geometry are already saved within the assembly file this option is pointless :lol: first option is more flexible anyway ;)
realthunder
Veteran
Posts: 2190
Joined: Tue Jan 03, 2017 10:55 am

Re: Code review of merged Link3 branch

Post by realthunder »

Zolko wrote: Wed Oct 23, 2019 7:52 am So it's this just copy part that is a trouble. Could the App::Link interface take care of the just copy ? Or is there really a missing command/interface in FreeCAD for that ?
You can use App.ActiveDocument.copyObject() API. Or just start exploring its capability through GUI with CTRL+C and CTRL+V. The default setting of CTRL+C copy is a deep copy excluding external linked object, but it lets you customize.

DeepSOIC wrote: Tue Oct 22, 2019 9:31 am Lattice2's ParaSeries and TopoSeries kinda do it. But in a limited and specialized way, and slowly (because it creates a temporary project to recompute the variation, and often the slowest part is copying objects to the tmp doc (involves writing shapes to files and reading them back under the hood)).
Any particular reason you need to create a temp project for that?
Try Assembly3 with my custom build of FreeCAD at here.
And if you'd like to show your support, you can donate through patreon, liberapay, or paypal
realthunder
Veteran
Posts: 2190
Joined: Tue Jan 03, 2017 10:55 am

Re: Code review of merged Link3 branch

Post by realthunder »

Zolko wrote: Wed Oct 23, 2019 7:52 am OK, so if I understand then the "variant" would need a copy, and thus cannot be a pure App::Link.
It is not impossible through App::Link. Well, nothing is impossible. But it's too much trouble to achieve this, when we have other better simpler alternatives. Maybe you can create a new thread about this. I'll try to mock up some possible ways to achieve this, and post the screencast.
Try Assembly3 with my custom build of FreeCAD at here.
And if you'd like to show your support, you can donate through patreon, liberapay, or paypal
User avatar
Zolko
Veteran
Posts: 2213
Joined: Mon Dec 17, 2018 10:02 am

Re: Code review of merged Link3 branch

Post by Zolko »

vocx wrote: Mon Oct 21, 2019 10:36 pm You cannot expect a completely new feature like what you want to be included in 0.19. [...] As soon as you develop a prototype, the faster it can be tested and included into FreeCAD.
realthunder wrote: Thu Oct 24, 2019 12:36 am It is not impossible through App::Link. Well, nothing is impossible. But it's too much trouble to achieve this
Well, as it happens, this link to a "variant" part is not a new feature and o trouble at all because it is already possible to do it with FreeCAD_v019. I'll show here the prototype. It does some workarounds because of the way App::Link behaves, and this should be fixed IMO, and the sooner the better. That's the reason I continue to post under this thread.

DeepSOIC wrote: Tue Oct 22, 2019 9:31 am So, to properly support it, we'll need an optimized mechanism of copying objects, and a mechanism of storing these varied objects.
This is the crux of the matter, and there is a solution: create a hidden document, copy the part there, and link to that part. Except that App::Link doesn't allow that, so you have to "cheat" it by giving a fake file-name to the hidden document. This produces an error <PropertyLinks> PropertyLinks.cpp(2566): document not found /home/hubertz/Documents/3Dcad/FreeCAD/Data/Forum/toto but the links still work. This means that it's a self-imposed limitation and the error checking is useless.

Zolko wrote: Wed Oct 23, 2019 7:52 am OK, so if I understand then the "variant" would need a copy, and thus cannot be a pure App::Link.
In the attached file, there is a part Beam with an extrusion, and an assembly with 4 links to that same part. If you insert into the Python console the following lines, it will create the hidden document, copy the Beam part there, link back 3 links to this Part, and change the variables of the copied part. This affects only the 3 links to that copied part, but not the original part and not the first link that is linked to the original part:

Code: Select all

asmDoc = App.ActiveDocument
tmpDoc = App.newDocument('tmpDoc', label='tmpDoc', hidden='True')
App.setActiveDocument(asmDoc.Name)
tmpDoc.FileName ='toto' 
beamCopy = tmpDoc.copyObject( asmDoc.getObject( 'Beam'), 'True' )
asmDoc.getObject('Beam_2').LinkedObject = beamCopy
asmDoc.getObject('Beam_3').LinkedObject = beamCopy
asmDoc.getObject('Beam_4').LinkedObject = beamCopy
copyVars = beamCopy.getObject('Variables')
copyVars.Length=50
copyVars.Size=50
asmDoc.recompute()

See:
LinkCopy.png
LinkCopy.png (368.17 KiB) Viewed 3384 times


Unfortunately, I cannot post here the resulting assembly because it cannot be saved: since the variant parts point to a document in memory App::Link in its current form cannot deal with it.

This means that the functionality of "variant" parts cannot be represented on disk with the current App::Link implementation, although it's a perfectly valid — and extremely useful — functionality. So it's not a question of App::Link "functionality" but a question of App::Link "data structure". I haven't seen here an analysis of the data structure in the fcstd files used by App::Link, and I think that this also should be reviewed. There are 17 properties for each App::Link instance, and I think this should be adapted/extended.

As I said in another thread, the user's data structure is what matters most: once users start producing data with this new tool (App::Link) it will be very difficult to change the way the data is represented on disk. As long as v0.19-pre is identified as a development version this is still adaptable, but once it reaches -beta status it's over, then all developers will have to support whatever has been agreed-on (or left-in by lazynes). And I think that the current App::Link data structure as stored in the *.fcstd file needs refinement, one of them being able to represent "variant" links as described here.
Attachments
linkCopy.FCStd
(19.1 KiB) Downloaded 94 times
try the Assembly4 workbench for FreCAD — tutorials here and here
Post Reply