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 » Sun Jul 09, 2017 6:52 pm

Update 9th July 2017

Just made the PR as described in my previous post.

Supported methods (all booleans, extrude, mirror, makeFillet2, makeChamfer2) now come with an optional withHistory parameter that can be set to True if you want to store history. The development of the sub-shapes from the sub-shapes of the base(s) can be studied by using shapeName.History.modified, shapeName.History.generated, shapeName.History.isDeleted.

Some testing is required, however. Works well enough for my test cases, but they are fairly naive. Some stress-testing would be greatly appreciated.
User avatar
Kunda1
Posts: 5783
Joined: Thu Jan 05, 2017 9:03 pm

Re: GSoC 2017 Dev Log: jnxd

Postby Kunda1 » Thu Jul 13, 2017 3:58 pm

Thanks for the PR, jnxd. I hope it get merged soon. What else have you been working on ?
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 » Thu Jul 13, 2017 6:42 pm

Kunda1 wrote:
Thu Jul 13, 2017 3:58 pm
Thanks for the PR, jnxd. I hope it get merged soon. What else have you been working on ?
Hi, @Kunda1. Glad to see your interest in the development.

Unfortunately, most of my time went in development towards the PR. It doesn't really look like much, but the commits are a result of squashing around 35 commits, and are the result of the GSoC work done so far. My next efforts are towards getting rid of the need of storing the entire BRepBuilderAPI_MakeShape.

I was tending towards using OCC's TNaming framework, but it seems I might have to change TopoHistory's methods for using it. Strictly going by experiments using the framework as a black-box, it seems we will also need a `context` parameter, which is the shape that the sub-shape we're searching for is a part of, for the solution to work. I imagine this won't be much of a problem, but I am wondering why this is the case.

I did read @ezzieguywuf's post, but I haven't yet read through his code. As of now, I think I will continue using OCC's tools, but @DeepSOIC and I have been regularly arguing whether developing something of our own would be a better idea. From @ezzieguywuf's post, it does seem that OCC has some poorly documented code. Also, since FreeCAD itself is a framework, I can imagine stacking it on top of OCC's OCAF (that TNaming is a part of) would be less efficient. Still, I think it should turn out relatively straightforward to change from OCC's to our own implementation when it is ready.
Jee-Bee
Posts: 1959
Joined: Tue Jun 16, 2015 10:32 am
Location: Netherlands

Re: GSoC 2017 Dev Log: jnxd

Postby Jee-Bee » Thu Jul 13, 2017 7:45 pm

Is there not a way to ask by OpenCascade if they have some more/ better info around there to fix?
Or else Salome use the same kernel is there not a way to see how they fix some things??
ezzieyguywuf
Posts: 558
Joined: Tue May 19, 2015 1:11 am

Re: GSoC 2017 Dev Log: jnxd

Postby ezzieyguywuf » Wed Jul 19, 2017 12:22 pm

jnxd wrote:
Sun Jul 09, 2017 6:52 pm

Just made the PR as described in my previous post.

<snip>

Some testing is required, however. Works well enough for my test cases, but they are fairly naive. Some stress-testing would be greatly appreciated.
@jnxd, I was messing with this a bit today and noticed an issue. Upon further investigation, it seems that you have a type on Line 72 of TopoHistory.cpp. I believe this should call `shapeMaker->Generated` not `shapeMaker->Modified`. I've made this change to my local copy and everything works as expected.

Great work on this by the way!
jnxd
Posts: 162
Joined: Mon Mar 30, 2015 2:30 pm

Re: GSoC 2017 Dev Log: jnxd

Postby jnxd » Thu Jul 27, 2017 3:38 am

Update 27th July 2017

With a PR complete, my efforts since the last update had been towards developing a simple implementation that gives sensible toponaming for a small set of Part::Features that would be useful. I chose Box, Boolean and Fillet because they had a way to define all faces (Box has Top, Bottom, Left, Right, Front, and Back, the others have it based on which faces/edges of the previous shapes(s) they were modified/generated from). For some shapes, like Cone, it appears OCC simply does not have a method to identify all sub shapes (for cone, only the lateral face)

In my previous post (on 14th), I had said I plan to use an implementation of TopoHistory that avoids storing BRepBuilderAPI_MakeShape, and I was using TNaming for that. Unfortunately, the approach suffers from 2 problems:
  1. We can only track modified subshapes. For example, in a prism P generated from a face F, if I feed in an edge E of F into the selector, Tnaming_Selector::Solve succeeds but only returns E as a part of P. Since there is a method to store generation data into the framework, I suspect there might be some way to retrieve it too, but so far I don't know much about it.
  2. We lose all BRepBuilderAPI_MakeShape specific data, like front, back, etc., which is useful in comparing subshapes between the shapes of a feature before and after modification (thanks @ickby for pointing it out). For this a per-feature reimplementation of BRepBuilderAPI_MakeShape might be necessary to both keep these data, and avoid the huge memory penalty of BRepBuilderAPI_MakeShape.
However, TNaming has a major advantage over BRepBuilderAPI_MakeShape in that it can be created between shapes not necessarily sharing a input-output relationship. We can, thus, store a translated cube as a modification of the original cube. Thus, I now plan to postpone the plan of re-implementing TopoHistory, but create a separate class TopoParaHistory that uses TNaming to relate modifications of the same feature.

Once that is decided, next is how to do the toponaming proper. For that, I had shared an implementation for an idealized model, and I'm sharing it again:
design_notes.txt
(4.75 KiB) Downloaded 31 times
The horizontal (and diagonal) arrows within the plan would be stored as TopoHistory and the vertical ones will be stored as TopoParaHistory. I planned to complete the TopoNaming solely using the method Part::Feature::execute and it's redefinitions in the subclasses, as so:

Code: Select all

App::DocumentObjectExecReturn *Feature::execute(void)
{
// 1. From para-history of dependencies modify parameters if needed.

// 2. See if something wrong has happened after modification

// 3. Perform the main execute

// 4. Create para-history for this feature
}
Unfortunately this makes an incorrect assumption that execute will only be called once, and that a recompute will call execute of all the shapes when initiated. However, this is not the case. For instance, when editing a sketch, the method has to be called multiple times as we add, remove and modify the various elements of the sketch. Or, it is called multiple times when we modify the parameters of any feature, like the length of a cube. Thus, something like this fails:

Code: Select all

>>> # Make Cube (using Button)
>>> oB = App.ActiveDocument.Box.Shape
>>> # Edit length of box from combo-view (say 10 mm -> 5 mm)
>>> nB = App.ActiveDocument.Box.Shape
>>> nB.ParaHistory.modified(oB.Faces[0], oB)
Efforts are underway to figure out how to make a para-history only when the editing between the shape before the editing starts, and that after it finishes. Any inputs as to how this can be achieved will be greatly appreciated.

The developments discussed here can be found on branch tnaming-3 of my fork.
ezzieyguywuf
Posts: 558
Joined: Tue May 19, 2015 1:11 am

Re: GSoC 2017 Dev Log: jnxd

Postby ezzieyguywuf » Thu Jul 27, 2017 4:40 am

jnxd wrote:
Thu Jul 27, 2017 3:38 am

Efforts are underway to figure out how to make a para-history only when the editing between the shape before the editing starts, and that after it finishes. Any inputs as to how this can be achieved will be greatly appreciated.

The developments discussed here can be found on branch tnaming-3 of my fork.
I ran into this when I was working on my tnaming proof of concept last year. It has been a while, but I believe I found a workaround. When I have a few mi items I'll look through my code to see how I dealt with it.
User avatar
yorik
Site Admin
Posts: 11565
Joined: Tue Feb 17, 2009 9:16 pm
Location: São Paulo, Brazil
Contact:

Re: GSoC 2017 Dev Log: jnxd

Postby yorik » Thu Jul 27, 2017 2:22 pm

jnxd wrote:
Thu Jul 27, 2017 3:38 am
I chose Box, Boolean and Fillet because they had a way to define all faces (Box has Top, Bottom, Left, Right, Front, and Back
That is an interesting approach... Actually a big number of parametric objects in FreeCAD could do that: define faces when they create their shape, instead of letting OCC do it automatically...
jnxd
Posts: 162
Joined: Mon Mar 30, 2015 2:30 pm

Re: GSoC 2017 Dev Log: jnxd

Postby jnxd » Thu Jul 27, 2017 4:03 pm

yorik wrote:
Thu Jul 27, 2017 2:22 pm
jnxd wrote:
Thu Jul 27, 2017 3:38 am
I chose Box, Boolean and Fillet because they had a way to define all faces (Box has Top, Bottom, Left, Right, Front, and Back
That is an interesting approach... Actually a big number of parametric objects in FreeCAD could do that: define faces when they create their shape, instead of letting OCC do it automatically...
I wonder how that would go about. For one, the algorithms of OCC will define these faces, we can only choose to ignore it. But as for having our own implementation, we are kind of in the dark. DeepSOIC suggested we use the Face numbers, but it's going to be hell for cases where there are multiple faces. A cone in OCC and FC, for example, is a term used to span cones, frustums (frusta?) and slices of cones or frustums. Thus, a cone object can have 2, 3, 4 or 5 faces! Adding to that that we'll have to code the para-histories: 16 combinations!
ezzieyguywuf
Posts: 558
Joined: Tue May 19, 2015 1:11 am

Re: GSoC 2017 Dev Log: jnxd

Postby ezzieyguywuf » Thu Jul 27, 2017 5:28 pm

jnxd wrote:
Thu Jul 27, 2017 4:03 pm
I wonder how that would go about. For one, the algorithms of OCC will define these faces, we can only choose to ignore it. But as for having our own implementation, we are kind of in the dark. DeepSOIC suggested we use the Face numbers, but it's going to be hell for cases where there are multiple faces. A cone in OCC and FC, for example, is a term used to span cones, frustums (frusta?) and slices of cones or frustums. Thus, a cone object can have 2, 3, 4 or 5 faces! Adding to that that we'll have to code the para-histories: 16 combinations!
I agree that relying on hard-coding which face is which in a given Shape will become difficult. I used your same approach, @jnxd, for my TNaming proof-of-concept, but I'm not sure how easily it will really extend.

My current working concept is the following (and can be seen in my PyTopoNamer proof-of-concept): rather than relying on knowing which face is which, the topological namer (I'll refer to this as TopoNamer in the rest of this post) must only know about when it is created and when its faces are modified. I believe that this information gives TopoNamer enough information to fully track what's going on. I'll give an example:

Code: Select all

class SolidTopoNamer{
    public:
        SolidTopoNamer(TopoDS_Solid aSolid);
        void addFace(TopoDS_Face aFace);
        void modifyFace(TopoDS_Face oldFace, TopoDS_Face newFace);
        void deleteFace(std::string faceName);
        
    private:
        // This method will create a unique name
        std::string makeName() const;
        TopoDS_Solid mySolid;
        std::map<std::string, TopoDS_Face> myFaces;
        int i;
};
And the implementation might look something like this (this is untested, it probable won't compile. Just for conversation):

Code: Select all

SolidTopoNamer::SolidTopoNamer(TopodDS_Solid aSolid){
    i = 0;
    std::vector<TopoDS_Face> faces = HelperClass.getFaces(aSolid);
    for (auto&& face : faces){
        this->addFace(face);
    }

void Solid::addFace(TopoDS_Face aFace){
    // check to make sure this Face hasn't already been added. throw an error if it has
    std::string name = this->makeName();
    myFaces.insert(std::pair<std::string, TopoDS_Face>(name, aFace));
}

void Solid::modifyFace(TopodS_Face oldFace, TopoDS_Face newFace){
    // loop through myFaces to find the faceName of oldFace. Note that since we throw an error in 
    // addFace if a face is added more than once, this is guaranteed to result in a single key
    oldFaceName = FindOldFaceName();
    myFaces[oldFaceName] = newFace;
}

std::string getName(){
    std::ostream stream;
    stream << "Face" << i;
    i++;
}
}
And finally, a use-case for this might look something like this:

Code: Select all

int main(){
    TopoShape box = GetBoxFromSomewhere();
    TopoShape cyl  = GetCylinderFromSomewhere();
    
    SolidTopoNamer namedBox(box);
    
    TopoShape fusedBox = box.Fuse(cyl, withHistory=true)
    
    // excuse this pseudocode
    for (face in box.getFaces()){
        TopoDS_Face newFace = fusedBox.modified(face);
        namedBox.modifyFace(face, newFace);
    }
   // also do namedBox.addFace() for new faces, etc...
}
So you can see in this example, it doesn't really matter which TopoDS_Face on the Box is the front, back, etc... all that matters is that we keep track of what happens to each Face. I think this approach is much more flexible and should allow for a better long-term solution within FreeCAD.