GSoC 2017 Dev Log: jnxd

Here's the place for discussion related to coding in FreeCAD, C++ or Python. Design, interfaces and structures.
jnxd
Posts: 162
Joined: Mon Mar 30, 2015 2:30 pm

Re: GSoC 2017 Dev Log: jnxd

Postby jnxd » Fri May 26, 2017 3:11 am

Update 26th May 2017

This was quite an inactive week with me visiting my native place and then my visa interview. I did however manage to read through OCC's OCAF documentation and sgrogan's links.

In a talk with ickby, it has become apparent that I should now be exploring ezzieyguywuf's repository and thread on toponaming to see the merits and demerits of his approach.
jnxd
Posts: 162
Joined: Mon Mar 30, 2015 2:30 pm

Re: GSoC 2017 Dev Log: jnxd

Postby jnxd » Fri Jun 02, 2017 3:29 pm

Update 2nd June 2017

This week was spent getting a simple example up and running using OCC's TNaming framework. The exercise was basically just following @ezzieyguywuf's footsteps, and was made much easier thanks to his efforts. However, it still needed quite a bit of trial and error due to the poor documentation.

The model itself is the classic example of a box with a slot, such that if the slot is made longer to the full length of the box, one particular edge splits into two, making filleting it (them) difficult. Using TNaming, I was able to track this change. The file is attached.
fillet_with_cut_testing.zip
(3.36 KiB) Downloaded 20 times
I also penned down a draft of how I plan to use TNaming in FreeCAD, but that's a very recent development, and DeepSOIC and I are still discussing about it.
design_notes.txt
(3.57 KiB) Downloaded 53 times
User avatar
fosselius
Posts: 344
Joined: Sat Apr 23, 2016 10:03 am

Re: GSoC 2017 Dev Log: jnxd

Postby fosselius » Thu Jun 08, 2017 11:24 am

I just went through your design notes and have a question.
When you reach this state:

Code: Select all

  (a)--(b)--(c)--(d)--(e)...
        |    |    |
        |    |    |
       (B)--(C)--(D)
Will you save this structure indefinitely or will you then remove b,c, d an keep a -- B -- C -- D -- e ?

I am mostly concerned about the build up of part complexity vs preserving the "undo capability"/history .

I would not like to have structure that expands like this after N changes:

Code: Select all

(a1)  -- (b1)  - (..) - (N1)
          |       |      |
         (b2) - (..)  - (N2)
          |       |      |
         (..) -  (..) - (..)
          |       |      |        
         (Bn) - (..)  - (Nn)
 
jnxd
Posts: 162
Joined: Mon Mar 30, 2015 2:30 pm

Re: GSoC 2017 Dev Log: jnxd

Postby jnxd » Thu Jun 08, 2017 2:10 pm

I had planned on removing the old ones as we progress. However, I had not considered undo-redo functionality. Without that, I believe I could go so far as even deleting (c) when generating (E).
jnxd
Posts: 162
Joined: Mon Mar 30, 2015 2:30 pm

Re: GSoC 2017 Dev Log: jnxd

Postby jnxd » Fri Jun 09, 2017 5:30 am

wmayer wrote: ...
So, we were trying to expose the history framework of OCC, i.e. the methods that tell which elements of the old shape generated/were modified to which elements of the new shape, or which were deleted. However, we have hit a roadblock since w e have to make some potentially radical changes. Thus the call to @wmayer.

We plan to expose the methods by storing within TopoShape an instance of BRepBuilderAPI_MakeShape, which has the relevant methods (viz. Modified, IsDeleted and Generated). Now the problem with that comes up that the methods that use BRepBuilderAPI_MakeShape to make shapes just return the resultant TopoDS_Shape, and so not give us the opportunity to store the BRepBuilderAPI_MakeShape. See, for example, TopoShapePy::fuse to see why this would be a problem. So now, we might have to modify TopoShape::fuse to return TopoShape rather than TopoDS_Shape. However, @DeepSOIC is worried this might disturb third party libraries.

Any inputs?
Jee-Bee
Posts: 1959
Joined: Tue Jun 16, 2015 10:32 am
Location: Netherlands

Re: GSoC 2017 Dev Log: jnxd

Postby Jee-Bee » Fri Jun 09, 2017 8:08 am

I prefer (as user not as programmer) that if things need te be broken to make it better it is fine...
at least it is better than having 4 or more generations of software in a single package like Pro-E Creo have at this moment :( .
jnxd
Posts: 162
Joined: Mon Mar 30, 2015 2:30 pm

Re: GSoC 2017 Dev Log: jnxd

Postby jnxd » Wed Jun 21, 2017 6:11 pm

Update 21st June 2017

I created a tnaming branch in my own fork of FreeCAD for this project's purposes (long time coming :mrgreen:).

As for my previous post, I went with a "ask for forgiveness rather than permission" policy and created an overloaded function TopoShape fuse(TopoShape) const to support the history methods. With that, two proof of concept implementations are created.

First, Part.Shape.fuse() in python is modified such that it returns a shape injected with the BRepBuilderAPI_MakeShape. So you could try something like:

Code: Select all

import FreeCAD
import Part

App.newDocument("Unnamed")
App.setActiveDocument("Unnamed")
App.ActiveDocument=App.getDocument("Unnamed")
Gui.ActiveDocument=Gui.getDocument("Unnamed")
App.ActiveDocument.addObject("Part::Box","Box")
App.ActiveDocument.ActiveObject.Label = "Cube"
App.ActiveDocument.recompute()
#Gui.SendMsgToActiveView("ViewFit")
App.ActiveDocument.addObject("Part::Cone","Cone")
App.ActiveDocument.ActiveObject.Label = "Cone"
App.ActiveDocument.recompute()

fusion = App.ActiveDocument.Box.Shape.fuse(App.ActiveDocument.Cone.Shape)
fusion.modified(App.ActiveDocument.Box.Shape.Edges[3])
Note that the last two lines will have to be entered manually (as opposed to using a macro) to see any results.

Secondly, Part::Boolean is modified such that execute() command always creates a TopoShape with injected BRepBuilderAPI_MakeShape. So, you could also try:

Code: Select all

import FreeCAD
import Part

App.newDocument("Unnamed")
App.setActiveDocument("Unnamed")
App.ActiveDocument=App.getDocument("Unnamed")
Gui.ActiveDocument=Gui.getDocument("Unnamed")
App.ActiveDocument.addObject("Part::Box","Box")
App.ActiveDocument.ActiveObject.Label = "Cube"
App.ActiveDocument.recompute()
#Gui.SendMsgToActiveView("ViewFit")
App.ActiveDocument.addObject("Part::Cone","Cone")
App.ActiveDocument.ActiveObject.Label = "Cone"
App.ActiveDocument.recompute()

App.activeDocument().addObject("Part::Cut","Cut")
App.activeDocument().Cut.Base = App.activeDocument().Box
App.activeDocument().Cut.Tool = App.activeDocument().Cone
Gui.activeDocument().Box.Visibility=False
Gui.activeDocument().Cone.Visibility=False
Gui.ActiveDocument.Cut.ShapeColor=Gui.ActiveDocument.Box.ShapeColor
Gui.ActiveDocument.Cut.DisplayMode=Gui.ActiveDocument.Box.DisplayMode
App.ActiveDocument.recompute()
and then App.ActiveDocument.Cut.Shape.modified(App.ActiveDocument.Box.Shape.Edges[3]). The results of this command will change accordingly if either the cube or the cone are moved using the appropriate tool.

Moving forward, there is a need to replace the use of BRepBuilderAPI_MakeShape with a more general implementation, since many features use more than one BRepBuilderAPI_MakeShape objects to build their final shapes. One likely contender is the Part::ShapeHistory Struct, but it may have to be extended into a full class before using it.
ezzieyguywuf
Posts: 558
Joined: Tue May 19, 2015 1:11 am

Re: GSoC 2017 Dev Log: jnxd

Postby ezzieyguywuf » Wed Jun 21, 2017 6:56 pm

Hello all. I am late to the party and have yet to read through this thread. I did receive a PM from jnxd asking for some input regarding the Topo Naming work I did last year, so I imagine this year's GSoC is relating to Topo Naming? That is great! I have PMed jnxd my personal email so that he can access me more readily, as I'm not on these boards as often as I was last year.

I will say I've been working 'offline' (from here) a bit on a prototype Topological Naming solution written purely in python. The idea was that it is easier and quicker to develop-test-refine using python and its built-in testing framework rather than trying to do all the development within the larger FreeCAD project. If I could develop a solution using just python, the idea was to later write the FreeCAD->python code that is necessary to use that solution in the FreeCAD project.

I was hesitant to post anything about this work on the boards here due to the strong opinions that any Topo Naming solution must by definition be written in C++ at the lower level - I wanted to wait until I had a better working solution before floating it out for further discussion. But, all that being said, I'm also open to discussing and sharing the work I've done in that arena as well. It hasn't come all that far, but I do think it may be a more sustainable solution rather than using the OCC built-in topo naming stuff.

Edit: after reading through the thread a bit I have an additional comment.
Moving forward, there is a need to replace the use of BRepBuilderAPI_MakeShape with a more general implementation
For what it's worth, if you look at my tnaming_ezziey_01 branch, you'll notice I've created a TopoNamingHelper class. This class is subsequently used in various helper methods that I've added to the TopoShape class. One of these methods is trackFuse.

This trackFuse method is then used in FeaturePartFuse, which is the existing legacy FreeCAD code that gets called whenever two shapes are fused together.

So why did I go with this approach? Well, there are various analogs to FeaturePartFuse already that do many of the common tasks that we want in FreeCAD: there's FeatureFillet, FeaturePartBox, FeaturePartBoolean, etc... Traditionally, each of these creates the requisite OCC objects needed to perform the needed operation, i.e. BrepAlgoAPI_Fuse for the fuse operation. You'll notice in my FeaturePartFuse implementation, I've removed that code and moved it instead to the trackFuse method in TopoShape. The reason for this was in order to retain access to the BrepAlgoAPI_Fuse instance without having to pass it around - also, philosophically, I don't believe that FeaturePartFuse should itself modify the TopoShape, but rather call access methods located in TopoShape that themselves modify the stored TopoDS _Shape variable.

Finally, if you look at the inherited methods that BrepAlgoAPI_Fuse has from BrepAlgoAPI_BooleanOperation, you'll notice there are methods such as IsDeleted, HasModified, etc...

So that was the strategy - move all the actual "change an OCC TopoDS_Shape" logic out of periphery classes such as FeaturePartFuse and move it into TopoShape. Subsequently, tap into the *API_whatever OCC classes to track the modified/deleted/created topology using OCC's builtin tnaming classes.
User avatar
Kunda1
Posts: 5783
Joined: Thu Jan 05, 2017 9:03 pm

Re: GSoC 2017 Dev Log: jnxd

Postby Kunda1 » Mon Jul 03, 2017 12:12 pm

jnxd wrote:ping
Any updates?
Want to contribute back to FC? Checkout:
#lowhangingfruit | Use the Source, Luke. | How to Help FreeCAD | How to report FC bugs and features
jnxd
Posts: 162
Joined: Mon Mar 30, 2015 2:30 pm

Re: GSoC 2017 Dev Log: jnxd

Postby jnxd » Mon Jul 03, 2017 5:45 pm

Kunda1 wrote:
Mon Jul 03, 2017 12:12 pm
jnxd wrote:ping
Any updates?
Oh! So sorry for not posting for so long.

Update: 3rd July 2017

We're working on making a pull request that stores the history algorithm for others to experiment with. When complete, methods in Part.Shape shall have an optional parameter withHistory that will lead to the shape being returned have a History attribute attached that can be used to query the developments that went into making this shape. Some technical details follow.

First of all, why are we making it optional? In some very basic experiments, we found that the memory footprint of history is really high:
as much as 367 MB for 1000 shapes as compared to 41.5 MB without. We're working on using better implementations to bring this difference down, but I think it'll still make substantial difference with and without history.

Secondly, why are we going for a new class rather than the current implementation? That is simply so that we can encapsulate any alternative implementation neatly away from TopoShape. Apart from the previously mentioned memory footprint issue, we plan to make the new implementation such that it can support features that use more than one BRepBuilderAPI_MakeShapes from OCC.

Finally, DeepSOIC wanted to have the return such with withHistory, the python methods return a tuple (shape, history). While that is possible, I went for having history as an attribute to TopoShape, since I felt that kept the shape generation methods cleaner.

It'll take about 7-10 days to implement this for all the methods in TopoShape, after which I'll make the PR. As such TopoNaming is too much work if targeted for the already delayed 0.17 release, but these optional parameters should not be too much of a hassle.