Shapebinder fusion: remove it?

About the development of the Part Design module/workbench. PLEASE DO NOT POST HELP REQUESTS HERE!
User avatar
DeepSOIC
Posts: 6536
Joined: Fri Aug 29, 2014 12:45 am
Location: Saint-Petersburg, Russia

Shapebinder fusion: remove it?

Postby DeepSOIC » Thu Jan 31, 2019 2:24 pm

Hi
I want to attempt fixing shapebinder placement bug, reported here: https://forum.freecadweb.org/viewtopic.php?f=20&t=33794

I noticed, there is fusion going on under the hood, and it doesn't seem right to me. Shapebinder is only used for linking, and fusion is a great way to mess up the indexing. So I'd like to change it to a compound, but that might break some older projects (probably, very few).

The code:
https://github.com/FreeCAD/FreeCAD/blob ... r.cpp#L160

Code: Select all

Part::TopoShape ShapeBinder::buildShapeFromReferences( Part::Feature* obj, std::vector< std::string > subs) {

    if (!obj)
        return TopoDS_Shape();

    if (subs.empty())
        return obj->Shape.getShape();

    //if we use multiple subshapes we build a shape from them by fusing them together
    Part::TopoShape base;
    std::vector<TopoDS_Shape> operators;
    for (std::string sub : subs) {
        if (base.isNull())
            base = obj->Shape.getShape().getSubShape(sub.c_str());
        else
            operators.push_back(obj->Shape.getShape().getSubShape(sub.c_str()));
    }

    try {
        if (!operators.empty() && !base.isNull())
            return base.fuse(operators); //                <----    !!!!!!!!!!
    }
    catch (...) {
        return base;
    }
    return base;
}
Fusion might be not that bad if shapes share topology - I think, OCC detects that, and doesn't search for intersections. But if they don't, fusion will probably do nothing except wasting cpu cycles (and if the subs actually intersect, which is pretty unlikely but is still possible, new edges are not at all helpful IMO).

Any objections for changing fusion to compounding?
User avatar
NormandC
Posts: 18534
Joined: Sat Feb 06, 2010 9:52 pm
Location: Québec, Canada

Re: Shapebinder fusion: remove it?

Postby NormandC » Thu Jan 31, 2019 6:00 pm

Something to take into consideration, before doing this change. At one point, ickby and I discussed of expanding the ShapeBinder functionality to transform it into a base feature through a parameter. My lunch break is nearly over so I don't have time to locate the discussion or Mantis ticket.