diff --git a/src/Mod/PartDesign/App/FeatureAddSub.cpp b/src/Mod/PartDesign/App/FeatureAddSub.cpp index 8744d24e4d..3faea289fd 100644 --- a/src/Mod/PartDesign/App/FeatureAddSub.cpp +++ b/src/Mod/PartDesign/App/FeatureAddSub.cpp @@ -81,6 +81,14 @@ TopoDS_Shape FeatureAddSub::refineShapeIfActive(const TopoDS_Shape& oldShape) co return oldShape; } +void FeatureAddSub::getAddSubShape(Part::TopoShape &addShape, Part::TopoShape &subShape) +{ + if (addSubType == Additive) + addShape = AddSubShape.getShape(); + else if (addSubType == Subtractive) + subShape = AddSubShape.getShape(); +} + } namespace App { diff --git a/src/Mod/PartDesign/App/FeatureAddSub.h b/src/Mod/PartDesign/App/FeatureAddSub.h index c257c911e9..c662a0df15 100644 --- a/src/Mod/PartDesign/App/FeatureAddSub.h +++ b/src/Mod/PartDesign/App/FeatureAddSub.h @@ -49,6 +49,8 @@ public: virtual short mustExecute() const override; + virtual void getAddSubShape(Part::TopoShape &addShape, Part::TopoShape &subShape); + Part::PropertyPartShape AddSubShape; App::PropertyBool Refine; diff --git a/src/Mod/PartDesign/App/FeatureDressUp.cpp b/src/Mod/PartDesign/App/FeatureDressUp.cpp index 84f30cbb78..7c24ba95d8 100644 --- a/src/Mod/PartDesign/App/FeatureDressUp.cpp +++ b/src/Mod/PartDesign/App/FeatureDressUp.cpp @@ -51,9 +51,10 @@ DressUp::DressUp() Placement.setStatus(App::Property::ReadOnly, true); ADD_PROPERTY_TYPE(SupportTransform,(false),"Base", App::Prop_None, - "Enable support for transformed patterns"); + "Include the base additive/subtractive shape when used in pattern features.\n" + "If disabled, only the dressed part of the shape is used for patterning."); - addSubType = Additive; + AddSubShape.setStatus(App::Property::Output, true); } short DressUp::mustExecute() const @@ -63,12 +64,6 @@ short DressUp::mustExecute() const return PartDesign::Feature::mustExecute(); } -void DressUp::setupObject() -{ - SupportTransform.setValue(true); - Feature::setupObject(); -} - void DressUp::positionByBaseFeature(void) { Part::Feature *base = static_cast(BaseFeature.getValue()); @@ -182,43 +177,94 @@ void DressUp::onChanged(const App::Property* prop) BaseFeature.setValue (Base.getValue()); } } else if (prop == &Shape || prop == &SupportTransform) { - // This is an expensive operation and to avoid to perform it unnecessarily it's not sufficient - // to check for the 'Restore' flag of the dress-up feature because at that time it's already reset. - // Instead the 'Restore' flag of the document must be checked. - // For more details see: https://forum.freecadweb.org/viewtopic.php?f=3&t=43799 (and issue 4276) if (!getDocument()->testStatus(App::Document::Restoring) && - !getDocument()->isPerformingTransaction()) { - Part::TopoShape s; - auto base = Base::freecad_dynamic_cast(getBaseObject(true)); - if(!base) { - addSubType = Additive; - if(!SupportTransform.getValue()) - s = getBaseShape(); - else - s = Shape.getShape(); - s.setPlacement(Base::Placement()); - } else if (!SupportTransform.getValue()) { - addSubType = base->getAddSubType(); - s = base->AddSubShape.getShape(); - } else { - addSubType = base->getAddSubType(); - Part::TopoShape baseShape = base->getBaseTopoShape(true); - baseShape.setPlacement(Base::Placement()); - Part::TopoShape shape = Shape.getShape(); - shape.setPlacement(Base::Placement()); - if (baseShape.isNull() || !baseShape.hasSubShape(TopAbs_SOLID)) { - s = shape; - addSubType = Additive; - } else if (addSubType == Additive) - s = shape.cut(baseShape.getShape()); - else - s = baseShape.cut(shape.getShape()); - } - AddSubShape.setValue(s); + !getDocument()->isPerformingTransaction()) + { + // AddSubShape in DressUp acts as a shape cache. And here we shall + // invalidate the cache upon changes in Shape. Other features + // (currently only feature Transformed) shall call getAddSubShape() + // to rebuild the cache. This allow us to perform expensive + // calculation of AddSubShape only when necessary. + AddSubShape.setValue(Part::TopoShape()); } } Feature::onChanged(prop); } +void DressUp::getAddSubShape(Part::TopoShape &addShape, Part::TopoShape &subShape) +{ + Part::TopoShape res = AddSubShape.getShape(); + + if(res.isNull()) { + try { + std::vector shapes; + Part::TopoShape shape = Shape.getShape(); + shape.setPlacement(Base::Placement()); + + FeatureAddSub *base = nullptr; + if(SupportTransform.getValue()) + base = Base::freecad_dynamic_cast(getBaseObject(true)); + + Part::TopoShape baseShape; + if(base) { + baseShape = base->getBaseTopoShape(true); + baseShape.setPlacement(Base::Placement()); + if (base->getAddSubType() == Additive) { + if(!baseShape.isNull() && baseShape.hasSubShape(TopAbs_SOLID)) + shapes.push_back(shape.cut(baseShape.getShape())); + else + shapes.push_back(shape); + } else { + BRep_Builder builder; + TopoDS_Compound comp; + builder.MakeCompound(comp); + // push an empty compound to indicate null additive shape + shapes.emplace_back(comp); + if(!baseShape.isNull() && baseShape.hasSubShape(TopAbs_SOLID)) + shapes.push_back(baseShape.cut(shape.getShape())); + else + shapes.push_back(shape); + } + } else { + baseShape = getBaseTopoShape(); + baseShape.setPlacement(Base::Placement()); + shapes.push_back(shape.cut(baseShape.getShape())); + shapes.push_back(baseShape.cut(shape.getShape())); + } + + // Make a compound to contain both additive and subtractive shape, + // bceause a dressing (e.g. a fillet) can either be additive or + // subtractive. And the dressup feature can contain mixture of both. + AddSubShape.setValue(Part::TopoShape().makECompound(shapes)); + + } catch (Standard_Failure &e) { + FC_THROWM(Base::CADKernelError, "Failed to calculate AddSub shape: " + << e.GetMessageString()); + } + res = AddSubShape.getShape(); + } + + if(res.isNull()) + throw Part::NullShapeException("Null AddSub shape"); + + if(res.getShape().ShapeType() != TopAbs_COMPOUND) { + addShape = res; + } else { + int count = res.countSubShapes(TopAbs_SHAPE); + if(!count) + throw Part::NullShapeException("Null AddSub shape"); + if(count) { + Part::TopoShape s = res.getSubShape(TopAbs_SHAPE, 1); + if(!s.isNull() && s.hasSubShape(TopAbs_SOLID)) + addShape = s; + } + if(count > 1) { + Part::TopoShape s = res.getSubShape(TopAbs_SHAPE, 2); + if(!s.isNull() && s.hasSubShape(TopAbs_SOLID)) + subShape = s; + } + } +} + } diff --git a/src/Mod/PartDesign/App/FeatureDressUp.h b/src/Mod/PartDesign/App/FeatureDressUp.h index 3fcc4f15ba..be22b3399e 100644 --- a/src/Mod/PartDesign/App/FeatureDressUp.h +++ b/src/Mod/PartDesign/App/FeatureDressUp.h @@ -60,9 +60,10 @@ public: /// all C0 continuous edges to the vector void getContiniusEdges(Part::TopoShape, std::vector< std::string >&); + virtual void getAddSubShape(Part::TopoShape &addShape, Part::TopoShape &subShape); + protected: virtual void onChanged(const App::Property* prop); - virtual void setupObject(); }; } //namespace PartDesign diff --git a/src/Mod/PartDesign/App/FeatureTransformed.cpp b/src/Mod/PartDesign/App/FeatureTransformed.cpp index e3a734313a..b6204a6c76 100644 --- a/src/Mod/PartDesign/App/FeatureTransformed.cpp +++ b/src/Mod/PartDesign/App/FeatureTransformed.cpp @@ -256,15 +256,14 @@ App::DocumentObjectExecReturn *Transformed::execute(void) { // Extract the original shape and determine whether to cut or to fuse TopoDS_Shape shape; - bool fuse; + Part::TopoShape fuseShape; + Part::TopoShape cutShape; if ((*o)->getTypeId().isDerivedFrom(PartDesign::FeatureAddSub::getClassTypeId())) { PartDesign::FeatureAddSub* feature = static_cast(*o); - shape = feature->AddSubShape.getShape().getShape(); - if (shape.IsNull()) - return new App::DocumentObjectExecReturn("Shape of additive feature is empty"); - - fuse = (feature->getAddSubType() == FeatureAddSub::Additive) ? true : false; + feature->getAddSubShape(fuseShape, cutShape); + if (fuseShape.isNull() && cutShape.isNull()) + return new App::DocumentObjectExecReturn("Shape of addsub feature is empty"); } else { return new App::DocumentObjectExecReturn("Only additive and subtractive features can be transformed"); @@ -280,7 +279,7 @@ App::DocumentObjectExecReturn *Transformed::execute(void) for (; t != transformations.end(); ++t) { // Make an explicit copy of the shape because the "true" parameter to BRepBuilderAPI_Transform // seems to be pretty broken - BRepBuilderAPI_Copy copy(shape); + BRepBuilderAPI_Copy copy(fuseShape.isNull()?cutShape.getShape():fuseShape.getShape()); shape = copy.Shape(); if (shape.IsNull()) return new App::DocumentObjectExecReturn("Transformed: Linked shape object is empty"); @@ -289,15 +288,24 @@ App::DocumentObjectExecReturn *Transformed::execute(void) if (!mkTrf.IsDone()) return new App::DocumentObjectExecReturn("Transformation failed", (*o)); - // Check for intersection with support + shape = mkTrf.Shape(); try { + // Intersection checking for additive shape is redundant. + // Because according to CheckIntersection() source code, it is + // implemented using fusion and counting of the resulting + // solid, which will be done in the following modeling step + // anyway. + // + // There is little reason for doing intersection checking on + // subtractive shape either, because it does not produce + // multiple solids. + // + // if (!Part::checkIntersection(support, mkTrf.Shape(), false, true)) { - if (!Part::checkIntersection(support, mkTrf.Shape(), false, true)) { -#ifdef FC_DEBUG // do not write this in release mode because a message appears already in the task view - Base::Console().Warning("Transformed shape does not intersect support %s: Removed\n", (*o)->getNameInDocument()); -#endif - nointersect_trsfms[*o].insert(t); - } else { + + TopoDS_Shape current = support; + + if (!fuseShape.isNull()) { // We cannot wait to fuse a transformation with the support until all the transformations are done, // because the "support" potentially changes with every transformation, basically when checking intersection // above you need: @@ -310,7 +318,7 @@ App::DocumentObjectExecReturn *Transformed::execute(void) /*v_transformations.push_back(t); v_transformedShapes.push_back(mkTrf.Shape());*/ - + // Note: Transformations that do not intersect the support are ignored in the overlap tests //insert scheme here. @@ -320,48 +328,45 @@ App::DocumentObjectExecReturn *Transformed::execute(void) // Fuse/Cut the compounded transformed shapes with the support //TopoDS_Shape result; - TopoDS_Shape current = support; - - if (fuse) { - BRepAlgoAPI_Fuse mkFuse(current, mkTrf.Shape()); - if (!mkFuse.IsDone()) - return new App::DocumentObjectExecReturn("Fusion with support failed", *o); - // we have to get the solids (fuse sometimes creates compounds) - current = this->getSolid(mkFuse.Shape()); - // lets check if the result is a solid - if (current.IsNull()) - return new App::DocumentObjectExecReturn("Resulting shape is not a solid", *o); - /*std::vector::const_iterator individualIt; - for (individualIt = individualTools.begin(); individualIt != individualTools.end(); ++individualIt) - { - BRepAlgoAPI_Fuse mkFuse2(current, *individualIt); - if (!mkFuse2.IsDone()) - return new App::DocumentObjectExecReturn("Fusion with support failed", *o); - // we have to get the solids (fuse sometimes creates compounds) - current = this->getSolid(mkFuse2.Shape()); - // lets check if the result is a solid - if (current.IsNull()) - return new App::DocumentObjectExecReturn("Resulting shape is not a solid", *o); - }*/ - } else { - BRepAlgoAPI_Cut mkCut(current, mkTrf.Shape()); - if (!mkCut.IsDone()) - return new App::DocumentObjectExecReturn("Cut out of support failed", *o); - current = mkCut.Shape(); - /*std::vector::const_iterator individualIt; - for (individualIt = individualTools.begin(); individualIt != individualTools.end(); ++individualIt) - { - BRepAlgoAPI_Cut mkCut2(current, *individualIt); - if (!mkCut2.IsDone()) - return new App::DocumentObjectExecReturn("Cut out of support failed", *o); - current = this->getSolid(mkCut2.Shape()); - if (current.IsNull()) - return new App::DocumentObjectExecReturn("Resulting shape is not a solid", *o); - }*/ + BRepAlgoAPI_Fuse mkFuse(current, shape); + if (!mkFuse.IsDone()) + return new App::DocumentObjectExecReturn("Fusion with support failed", *o); + + if(Part::TopoShape(current).countSubShapes(TopAbs_SOLID) + != Part::TopoShape(mkFuse.Shape()).countSubShapes(TopAbs_SOLID)) + { +#ifdef FC_DEBUG // do not write this in release mode because a message appears already in the task view + Base::Console().Warning("Transformed shape does not intersect support %s: Removed\n", (*o)->getNameInDocument()); +#endif + nointersect_trsfms[*o].insert(t); + continue; + } + // we have to get the solids (fuse sometimes creates compounds) + current = this->getSolid(mkFuse.Shape()); + // lets check if the result is a solid + if (current.IsNull()) + return new App::DocumentObjectExecReturn("Resulting shape is not a solid", *o); + + if (!cutShape.isNull()) { + BRepBuilderAPI_Copy copy(cutShape.getShape()); + shape = copy.Shape(); + if (shape.IsNull()) + return new App::DocumentObjectExecReturn("Transformed: Linked shape object is empty"); + + BRepBuilderAPI_Transform mkTrf(shape, *t, false); // No need to copy, now + if (!mkTrf.IsDone()) + return new App::DocumentObjectExecReturn("Transformation failed", (*o)); + shape = mkTrf.Shape(); } - support = current; // Use result of this operation for fuse/cut of next original } + if (!cutShape.isNull()) { + BRepAlgoAPI_Cut mkCut(current, shape); + if (!mkCut.IsDone()) + return new App::DocumentObjectExecReturn("Cut out of support failed", *o); + current = mkCut.Shape(); + } + support = current; // Use result of this operation for fuse/cut of next original } catch (Standard_Failure& e) { // Note: Ignoring this failure is probably pointless because if the intersection check fails, the later // fuse operation of the transformation result will also fail diff --git a/src/Mod/PartDesign/Gui/ViewProviderTransformed.cpp b/src/Mod/PartDesign/Gui/ViewProviderTransformed.cpp index ed16e0a974..1f83738f16 100644 --- a/src/Mod/PartDesign/Gui/ViewProviderTransformed.cpp +++ b/src/Mod/PartDesign/Gui/ViewProviderTransformed.cpp @@ -195,16 +195,17 @@ void ViewProviderTransformed::recomputeFeature(bool recompute) for (PartDesign::Transformed::rejectedMap::const_iterator o = rejected_trsf.begin(); o != rejected_trsf.end(); o++) { if (o->second.empty()) continue; - TopoDS_Shape shape; + Part::TopoShape fuseShape; + Part::TopoShape cutShape; if ((o->first)->getTypeId().isDerivedFrom(PartDesign::FeatureAddSub::getClassTypeId())) { PartDesign::FeatureAddSub* feature = static_cast(o->first); - shape = feature->AddSubShape.getShape().getShape(); + feature->getAddSubShape(fuseShape, cutShape); } - if (shape.IsNull()) continue; + if (fuseShape.isNull()) continue; // Display the rejected transformations in red - TopoDS_Shape cShape(shape); + TopoDS_Shape cShape(fuseShape.getShape()); try { // calculating the deflection value