From eb52dd4a9a238fd70b1e9b8cfe8a59ac2727750e Mon Sep 17 00:00:00 2001 From: wmayer Date: Mon, 27 Jan 2025 22:08:11 +0100 Subject: [PATCH 1/3] PD: Fix regression about single-solid check For several PD features the single-solid check fails. The regression is caused by PR 13960 and reported as issue 19002. The reason for the failure is that the first solid of the output shape is retrieved and then checked for a single solid. This test will always pass, of course. The single-solid is fixed for these features: * Pad * Pocket (never worked there) * Fillet * Chamfer * Groove (never worked there) * Revolution (never worked there) * Loft Fixes: 935bdf9a0f1a ("PartDesign: Refactor single-solid rule enforcement") --- src/Mod/PartDesign/App/FeatureChamfer.cpp | 16 +++++----------- src/Mod/PartDesign/App/FeatureExtrude.cpp | 4 ++-- src/Mod/PartDesign/App/FeatureFillet.cpp | 16 +++++----------- src/Mod/PartDesign/App/FeatureGroove.cpp | 6 +++--- src/Mod/PartDesign/App/FeatureLoft.cpp | 2 +- src/Mod/PartDesign/App/FeatureRevolution.cpp | 6 +++++- 6 files changed, 21 insertions(+), 29 deletions(-) diff --git a/src/Mod/PartDesign/App/FeatureChamfer.cpp b/src/Mod/PartDesign/App/FeatureChamfer.cpp index e154eb2411..dd289e160c 100644 --- a/src/Mod/PartDesign/App/FeatureChamfer.cpp +++ b/src/Mod/PartDesign/App/FeatureChamfer.cpp @@ -159,7 +159,6 @@ App::DocumentObjectExecReturn *Chamfer::execute() TopTools_ListOfShape aLarg; aLarg.Append(TopShape.getShape()); - bool failed = false; if (!BRepAlgo::IsValid(aLarg, shape.getShape(), Standard_False, Standard_False)) { ShapeFix_ShapeTolerance aSFT; aSFT.LimitTolerance(shape.getShape(), @@ -167,21 +166,16 @@ App::DocumentObjectExecReturn *Chamfer::execute() Precision::Confusion(), TopAbs_SHAPE); } - if (!failed) { - // store shape before refinement - this->rawShape = shape; - shape = refineShapeIfActive(shape); - shape = getSolid(shape); - } + // store shape before refinement + this->rawShape = shape; + shape = refineShapeIfActive(shape); if (!isSingleSolidRuleSatisfied(shape.getShape())) { return new App::DocumentObjectExecReturn(QT_TRANSLATE_NOOP("Exception", "Result has multiple solids: that is not currently supported.")); } + + shape = getSolid(shape); this->Shape.setValue(shape); - if (failed) { - return new App::DocumentObjectExecReturn( - QT_TRANSLATE_NOOP("Exception", "Resulting shape is invalid")); - } return App::DocumentObject::StdReturn; } catch (Standard_Failure& e) { diff --git a/src/Mod/PartDesign/App/FeatureExtrude.cpp b/src/Mod/PartDesign/App/FeatureExtrude.cpp index 0a911f1b45..c009ca6be4 100644 --- a/src/Mod/PartDesign/App/FeatureExtrude.cpp +++ b/src/Mod/PartDesign/App/FeatureExtrude.cpp @@ -809,7 +809,7 @@ App::DocumentObjectExecReturn* FeatureExtrude::buildExtrusion(ExtrudeOptions opt // store shape before refinement this->rawShape = result; - solRes = refineShapeIfActive(solRes); + solRes = refineShapeIfActive(result); if (!isSingleSolidRuleSatisfied(solRes.getShape())) { return new App::DocumentObjectExecReturn(QT_TRANSLATE_NOOP("Exception", "Result has multiple solids: that is not currently supported.")); @@ -824,10 +824,10 @@ App::DocumentObjectExecReturn* FeatureExtrude::buildExtrusion(ExtrudeOptions opt // store shape before refinement this->rawShape = prism; prism = refineShapeIfActive(prism); - prism = getSolid(prism); if (!isSingleSolidRuleSatisfied(prism.getShape())) { return new App::DocumentObjectExecReturn(QT_TRANSLATE_NOOP("Exception", "Result has multiple solids: that is not currently supported.")); } + prism = getSolid(prism); this->Shape.setValue(prism); } else { diff --git a/src/Mod/PartDesign/App/FeatureFillet.cpp b/src/Mod/PartDesign/App/FeatureFillet.cpp index 35e7250330..c26c5486bc 100644 --- a/src/Mod/PartDesign/App/FeatureFillet.cpp +++ b/src/Mod/PartDesign/App/FeatureFillet.cpp @@ -104,7 +104,6 @@ App::DocumentObjectExecReturn *Fillet::execute() TopTools_ListOfShape aLarg; aLarg.Append(baseShape.getShape()); - bool failed = false; if (!BRepAlgo::IsValid(aLarg, shape.getShape(), Standard_False, Standard_False)) { ShapeFix_ShapeTolerance aSFT; aSFT.LimitTolerance(shape.getShape(), @@ -113,20 +112,15 @@ App::DocumentObjectExecReturn *Fillet::execute() TopAbs_SHAPE); } - if (!failed) { - // store shape before refinement - this->rawShape = shape; - shape = refineShapeIfActive(shape); - shape = getSolid(shape); - } + // store shape before refinement + this->rawShape = shape; + shape = refineShapeIfActive(shape); if (!isSingleSolidRuleSatisfied(shape.getShape())) { return new App::DocumentObjectExecReturn(QT_TRANSLATE_NOOP("Exception", "Result has multiple solids: that is not currently supported.")); } - this->Shape.setValue(shape); - if (failed) { - return new App::DocumentObjectExecReturn("Resulting shape is invalid"); - } + shape = getSolid(shape); + this->Shape.setValue(shape); return App::DocumentObject::StdReturn; } catch (Standard_Failure& e) { diff --git a/src/Mod/PartDesign/App/FeatureGroove.cpp b/src/Mod/PartDesign/App/FeatureGroove.cpp index 3df6e019eb..72d7d35076 100644 --- a/src/Mod/PartDesign/App/FeatureGroove.cpp +++ b/src/Mod/PartDesign/App/FeatureGroove.cpp @@ -185,17 +185,17 @@ App::DocumentObjectExecReturn *Groove::execute() }catch(Standard_Failure &) { return new App::DocumentObjectExecReturn("Failed to cut base feature"); } - boolOp = this->getSolid(boolOp); - if (boolOp.isNull()) + TopoShape solid = this->getSolid(boolOp); + if (solid.isNull()) return new App::DocumentObjectExecReturn("Resulting shape is not a solid"); // store shape before refinement this->rawShape = boolOp; boolOp = refineShapeIfActive(boolOp); - boolOp = getSolid(boolOp); if (!isSingleSolidRuleSatisfied(boolOp.getShape())) { return new App::DocumentObjectExecReturn(QT_TRANSLATE_NOOP("Exception", "Result has multiple solids: that is not currently supported.")); } + boolOp = getSolid(boolOp); Shape.setValue(boolOp); return App::DocumentObject::StdReturn; } diff --git a/src/Mod/PartDesign/App/FeatureLoft.cpp b/src/Mod/PartDesign/App/FeatureLoft.cpp index 1bfe3cc696..ef7b3426af 100644 --- a/src/Mod/PartDesign/App/FeatureLoft.cpp +++ b/src/Mod/PartDesign/App/FeatureLoft.cpp @@ -266,10 +266,10 @@ App::DocumentObjectExecReturn *Loft::execute() // store shape before refinement this->rawShape = boolOp; boolOp = refineShapeIfActive(boolOp); - boolOp = getSolid(boolOp); if (!isSingleSolidRuleSatisfied(boolOp.getShape())) { return new App::DocumentObjectExecReturn(QT_TRANSLATE_NOOP("Exception", "Result has multiple solids: that is not currently supported.")); } + boolOp = getSolid(boolOp); Shape.setValue(boolOp); return App::DocumentObject::StdReturn; } diff --git a/src/Mod/PartDesign/App/FeatureRevolution.cpp b/src/Mod/PartDesign/App/FeatureRevolution.cpp index d60b1f6558..e3fdf31575 100644 --- a/src/Mod/PartDesign/App/FeatureRevolution.cpp +++ b/src/Mod/PartDesign/App/FeatureRevolution.cpp @@ -228,7 +228,11 @@ App::DocumentObjectExecReturn* Revolution::execute() this->rawShape = result; result = refineShapeIfActive(result); } - this->Shape.setValue(getSolid(result)); + if (!isSingleSolidRuleSatisfied(result.getShape())) { + return new App::DocumentObjectExecReturn(QT_TRANSLATE_NOOP("Exception", "Result has multiple solids: that is not currently supported.")); + } + result = getSolid(result); + this->Shape.setValue(result); } else { return new App::DocumentObjectExecReturn( From 4ac7f57ac47d17a21cabe484732860cd3d49fb8d Mon Sep 17 00:00:00 2001 From: wmayer Date: Tue, 28 Jan 2025 01:46:38 +0100 Subject: [PATCH 2/3] PD: Use isSingleSolidRuleSatisfied() for pipe feature This fixes issue 18977 Fixes: 935bdf9a0f1a ("PartDesign: Refactor single-solid rule enforcement") --- src/Mod/PartDesign/App/FeaturePipe.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/Mod/PartDesign/App/FeaturePipe.cpp b/src/Mod/PartDesign/App/FeaturePipe.cpp index 08c4077948..f2358cd0e0 100644 --- a/src/Mod/PartDesign/App/FeaturePipe.cpp +++ b/src/Mod/PartDesign/App/FeaturePipe.cpp @@ -400,8 +400,7 @@ App::DocumentObjectExecReturn *Pipe::execute() if (boolOp.isNull()) return new App::DocumentObjectExecReturn(QT_TRANSLATE_NOOP("Exception", "Resulting shape is not a solid")); - int solidCount = countSolids(boolOp.getShape()); - if (solidCount > 1) { + if (!isSingleSolidRuleSatisfied(boolOp.getShape())) { return new App::DocumentObjectExecReturn(QT_TRANSLATE_NOOP("Exception", "Result has multiple solids: that is not currently supported.")); } @@ -422,8 +421,7 @@ App::DocumentObjectExecReturn *Pipe::execute() if (boolOp.isNull()) return new App::DocumentObjectExecReturn(QT_TRANSLATE_NOOP("Exception", "Resulting shape is not a solid")); - int solidCount = countSolids(boolOp.getShape()); - if (solidCount > 1) { + if (!isSingleSolidRuleSatisfied(boolOp.getShape())) { return new App::DocumentObjectExecReturn(QT_TRANSLATE_NOOP("Exception", "Result has multiple solids: that is not currently supported.")); } From 23d93e45ffce3598a6346a344ecaa450a42c3392 Mon Sep 17 00:00:00 2001 From: wmayer Date: Thu, 30 Jan 2025 16:47:56 +0100 Subject: [PATCH 3/3] PD: Correctly handle single solid rule for loft with and without base --- src/Mod/PartDesign/App/FeatureLoft.cpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/Mod/PartDesign/App/FeatureLoft.cpp b/src/Mod/PartDesign/App/FeatureLoft.cpp index ef7b3426af..becc62d406 100644 --- a/src/Mod/PartDesign/App/FeatureLoft.cpp +++ b/src/Mod/PartDesign/App/FeatureLoft.cpp @@ -234,6 +234,9 @@ App::DocumentObjectExecReturn *Loft::execute() result = shapes.front(); if(base.isNull()) { + if (!isSingleSolidRuleSatisfied(result.getShape())) { + return new App::DocumentObjectExecReturn(QT_TRANSLATE_NOOP("Exception", "Result has multiple solids: that is not currently supported.")); + } Shape.setValue(getSolid(result)); return App::DocumentObject::StdReturn; } @@ -258,11 +261,11 @@ App::DocumentObjectExecReturn *Loft::execute() catch(Standard_Failure&) { return new App::DocumentObjectExecReturn(QT_TRANSLATE_NOOP("Exception", "Failed to perform boolean operation")); } - boolOp = this->getSolid(boolOp); + TopoShape solid = getSolid(boolOp); // lets check if the result is a solid - if (boolOp.isNull()) + if (solid.isNull()) { return new App::DocumentObjectExecReturn(QT_TRANSLATE_NOOP("Exception", "Resulting shape is not a solid")); - + } // store shape before refinement this->rawShape = boolOp; boolOp = refineShapeIfActive(boolOp);