From 44f15d586848c4e17152f10e2b1afaec166169b0 Mon Sep 17 00:00:00 2001 From: jrheinlaender Date: Mon, 5 Nov 2012 18:28:17 +0430 Subject: [PATCH 1/6] Pad/Pocket: Fixed bug that led to failed UpToFace when finishing the feature (thanks to wmayer for pointing this out) --- src/Mod/PartDesign/Gui/TaskPadParameters.cpp | 24 ++++++------- .../PartDesign/Gui/TaskPocketParameters.cpp | 34 +++++++++---------- 2 files changed, 29 insertions(+), 29 deletions(-) diff --git a/src/Mod/PartDesign/Gui/TaskPadParameters.cpp b/src/Mod/PartDesign/Gui/TaskPadParameters.cpp index 7d181de8c6..6cea2cd49b 100644 --- a/src/Mod/PartDesign/Gui/TaskPadParameters.cpp +++ b/src/Mod/PartDesign/Gui/TaskPadParameters.cpp @@ -97,10 +97,10 @@ TaskPadParameters::TaskPadParameters(ViewProviderPad *PadView,QWidget *parent) double l2 = pcPad->Length2.getValue(); int index = pcPad->Type.getValue(); // must extract value here, clear() kills it! std::vector subStrings = pcPad->UpToFace.getSubValues(); - std::string upToFace; + std::string upToFace; int faceId = -1; - if (!subStrings.empty()) { - upToFace = subStrings.front(); + if (!subStrings.empty()) { + upToFace = subStrings.front(); if (upToFace.substr(0,4) == "Face") faceId = std::atoi(&upToFace[4]); } @@ -116,8 +116,8 @@ TaskPadParameters::TaskPadParameters(ViewProviderPad *PadView,QWidget *parent) // According to bug #0000521 the reversed option // shouldn't be de-activated if the pad has a support face ui->checkBoxReversed->setChecked(reversed); - ui->lineFaceName->setText(faceId >= 0 ? - tr("Face") + QString::number(faceId) : + ui->lineFaceName->setText(faceId >= 0 ? + tr("Face") + QString::number(faceId) : tr("No face selected")); ui->lineFaceName->setProperty("FaceName", QByteArray(upToFace.c_str())); ui->changeMode->clear(); @@ -212,7 +212,7 @@ void TaskPadParameters::onSelectionChanged(const Gui::SelectionChanges& msg) } if (strcmp(msg.pObjectName, support->getNameInDocument()) != 0) return; - + std::vector upToFaces(1,subName); pcPad->UpToFace.setValue(support, upToFaces); if (updateView()) @@ -228,7 +228,7 @@ void TaskPadParameters::onSelectionChanged(const Gui::SelectionChanges& msg) ui->lineFaceName->blockSignals(true); ui->lineFaceName->setText(tr("No face selected")); ui->lineFaceName->setProperty("FaceName", QByteArray()); - ui->lineFaceName->blockSignals(false); + ui->lineFaceName->blockSignals(false); } } @@ -303,7 +303,7 @@ void TaskPadParameters::onButtonFace(const bool pressed) { doc->setShow(support->getNameInDocument()); } Gui::Selection().clearSelection(); - Gui::Selection().addSelectionGate + Gui::Selection().addSelectionGate (new ReferenceSelection(support, false, true, false)); } else { Gui::Selection().rmvSelectionGate(); @@ -351,7 +351,7 @@ void TaskPadParameters::onUpdateView(bool on) { if (on) { PartDesign::Pad* pcPad = static_cast(PadView->getObject()); - pcPad->getDocument()->recomputeFeature(pcPad); + pcPad->getDocument()->recomputeFeature(pcPad); } } @@ -472,14 +472,14 @@ bool TaskDlgPadParameters::accept() Gui::Command::doCommand(Gui::Command::Doc,"App.ActiveDocument.%s.Midplane = %i",name.c_str(),parameter->getMidplane()?1:0); Gui::Command::doCommand(Gui::Command::Doc,"App.ActiveDocument.%s.Length2 = %f",name.c_str(),parameter->getLength2()); Gui::Command::doCommand(Gui::Command::Doc,"App.ActiveDocument.%s.Type = %u",name.c_str(),parameter->getMode()); - QByteArray facename = parameter->getFaceName(); + std::string facename = parameter->getFaceName().data(); PartDesign::Pad* pcPad = static_cast(PadView->getObject()); Part::Feature* support = pcPad->getSupport(); - if (support != NULL && !facename.isEmpty()) { + if (support != NULL && !facename.empty()) { QString buf = QString::fromUtf8("(App.ActiveDocument.%1,[\"%2\"])"); buf = buf.arg(QString::fromUtf8(support->getNameInDocument())); - buf = buf.arg(QString::fromUtf8(facename.data())); + buf = buf.arg(QString::fromStdString(facename)); Gui::Command::doCommand(Gui::Command::Doc,"App.ActiveDocument.%s.UpToFace = %s", name.c_str(), buf.toStdString().c_str()); } else Gui::Command::doCommand(Gui::Command::Doc,"App.ActiveDocument.%s.UpToFace = None", name.c_str()); diff --git a/src/Mod/PartDesign/Gui/TaskPocketParameters.cpp b/src/Mod/PartDesign/Gui/TaskPocketParameters.cpp index e465f2aaf5..152943a5a0 100644 --- a/src/Mod/PartDesign/Gui/TaskPocketParameters.cpp +++ b/src/Mod/PartDesign/Gui/TaskPocketParameters.cpp @@ -89,10 +89,10 @@ TaskPocketParameters::TaskPocketParameters(ViewProviderPocket *PocketView,QWidge bool midplane = pcPocket->Midplane.getValue(); int index = pcPocket->Type.getValue(); // must extract value here, clear() kills it! std::vector subStrings = pcPocket->UpToFace.getSubValues(); - std::string upToFace; + std::string upToFace; int faceId = -1; - if (!subStrings.empty()) { - upToFace = subStrings.front(); + if (!subStrings.empty()) { + upToFace = subStrings.front(); if (upToFace.substr(0,4) == "Face") faceId = std::atoi(&upToFace[4]); } @@ -102,8 +102,8 @@ TaskPocketParameters::TaskPocketParameters(ViewProviderPocket *PocketView,QWidge ui->doubleSpinBox->setMaximum(INT_MAX); ui->doubleSpinBox->setValue(l); ui->checkBoxMidplane->setChecked(midplane); - ui->lineFaceName->setText(faceId >= 0 ? - tr("Face") + QString::number(faceId) : + ui->lineFaceName->setText(faceId >= 0 ? + tr("Face") + QString::number(faceId) : tr("No face selected")); ui->lineFaceName->setProperty("FaceName", QByteArray(upToFace.c_str())); ui->changeMode->clear(); @@ -192,7 +192,7 @@ void TaskPocketParameters::onSelectionChanged(const Gui::SelectionChanges& msg) } if (strcmp(msg.pObjectName, support->getNameInDocument()) != 0) return; - + std::vector upToFaces(1,subName); pcPocket->UpToFace.setValue(support, upToFaces); if (updateView()) @@ -241,13 +241,13 @@ void TaskPocketParameters::onModeChanged(int index) pcPocket->Length.setValue(oldLength); ui->doubleSpinBox->setValue(oldLength); break; - case 1: - oldLength = pcPocket->Length.getValue(); - pcPocket->Type.setValue("ThroughAll"); + case 1: + oldLength = pcPocket->Length.getValue(); + pcPocket->Type.setValue("ThroughAll"); break; - case 2: - oldLength = pcPocket->Length.getValue(); - pcPocket->Type.setValue("UpToFirst"); + case 2: + oldLength = pcPocket->Length.getValue(); + pcPocket->Type.setValue("UpToFirst"); break; case 3: // Because of the code at the begining of Pocket::execute() which is used to detect @@ -257,7 +257,7 @@ void TaskPocketParameters::onModeChanged(int index) pcPocket->Length.setValue(0.0); ui->doubleSpinBox->setValue(0.0); break; - default: + default: pcPocket->Type.setValue("Length"); } @@ -282,7 +282,7 @@ void TaskPocketParameters::onButtonFace(const bool pressed) { doc->setShow(support->getNameInDocument()); } Gui::Selection().clearSelection(); - Gui::Selection().addSelectionGate + Gui::Selection().addSelectionGate (new ReferenceSelection(support, false, true, false)); } else { Gui::Selection().rmvSelectionGate(); @@ -430,13 +430,13 @@ bool TaskDlgPocketParameters::accept() //Gui::Command::openCommand("Pocket changed"); Gui::Command::doCommand(Gui::Command::Doc,"App.ActiveDocument.%s.Length = %f",name.c_str(),parameter->getLength()); Gui::Command::doCommand(Gui::Command::Doc,"App.ActiveDocument.%s.Type = %u",name.c_str(),parameter->getMode()); - QByteArray facename = parameter->getFaceName(); + std::string facename = parameter->getFaceName().data(); PartDesign::Pocket* pcPocket = static_cast(PocketView->getObject()); Part::Feature* support = pcPocket->getSupport(); - if (support != NULL && !facename.isEmpty()) { + if (support != NULL && !facename.empty()) { QString buf = QString::fromUtf8("(App.ActiveDocument.%1,[\"%2\"])"); buf = buf.arg(QString::fromUtf8(support->getNameInDocument())); - buf = buf.arg(QString::fromUtf8(facename.data())); + buf = buf.arg(QString::fromStdString(facename)); Gui::Command::doCommand(Gui::Command::Doc,"App.ActiveDocument.%s.UpToFace = %s", name.c_str(), buf.toStdString().c_str()); } else Gui::Command::doCommand(Gui::Command::Doc,"App.ActiveDocument.%s.UpToFace = None", name.c_str()); From 6cdd265ca17ca55b976970940862c02d021a8454 Mon Sep 17 00:00:00 2001 From: jrheinlaender Date: Mon, 5 Nov 2012 18:42:45 +0430 Subject: [PATCH 2/6] Enabled "Reversed" option for Pad up to first/last --- src/Mod/PartDesign/App/FeaturePad.cpp | 11 +++++++---- src/Mod/PartDesign/Gui/TaskPadParameters.cpp | 2 +- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/Mod/PartDesign/App/FeaturePad.cpp b/src/Mod/PartDesign/App/FeaturePad.cpp index ca02bf850f..1b97da8fe6 100644 --- a/src/Mod/PartDesign/App/FeaturePad.cpp +++ b/src/Mod/PartDesign/App/FeaturePad.cpp @@ -112,18 +112,21 @@ App::DocumentObjectExecReturn *Pad::execute(void) gp_Dir dir(SketchVector.x,SketchVector.y,SketchVector.z); dir.Transform(invObjLoc.Transformation()); - + TopoDS_Shape sketchshape = makeFace(wires); if (sketchshape.IsNull()) return new App::DocumentObjectExecReturn("Pad: Creating a face from sketch failed"); sketchshape.Move(invObjLoc); - + TopoDS_Shape prism; std::string method(Type.getValueAsString()); if (method == "UpToFirst" || method == "UpToLast" || method == "UpToFace") { TopoDS_Face supportface = getSupportFace(); supportface.Move(invObjLoc); + if (Reversed.getValue()) + dir.Reverse(); + // Find a valid face to extrude up to TopoDS_Face upToFace; if (method == "UpToFace") { @@ -154,7 +157,7 @@ App::DocumentObjectExecReturn *Pad::execute(void) // set the additive shape property for later usage in e.g. pattern this->AddShape.setValue(prism); - + // if the sketch has a support fuse them to get one result object if (!support.IsNull()) { // Let's call algorithm computing a fuse operation: @@ -178,7 +181,7 @@ App::DocumentObjectExecReturn *Pad::execute(void) catch (Standard_Failure) { Handle_Standard_Failure e = Standard_Failure::Caught(); if (std::string(e->GetMessageString()) == "TopoDS::Face") - return new App::DocumentObjectExecReturn("Could not create face from sketch.\n" + return new App::DocumentObjectExecReturn("Could not create face from sketch.\n" "Intersecting sketch entities or multiple faces in a sketch are not allowed."); else return new App::DocumentObjectExecReturn(e->GetMessageString()); diff --git a/src/Mod/PartDesign/Gui/TaskPadParameters.cpp b/src/Mod/PartDesign/Gui/TaskPadParameters.cpp index 6cea2cd49b..e47ae2d78d 100644 --- a/src/Mod/PartDesign/Gui/TaskPadParameters.cpp +++ b/src/Mod/PartDesign/Gui/TaskPadParameters.cpp @@ -158,7 +158,7 @@ void TaskPadParameters::updateUI(int index) } else if (index == 1 || index == 2) { // up to first/last ui->doubleSpinBox->setEnabled(false); ui->checkBoxMidplane->setEnabled(false); - ui->checkBoxReversed->setEnabled(false); + ui->checkBoxReversed->setEnabled(true); ui->doubleSpinBox2->setEnabled(false); ui->buttonFace->setEnabled(false); ui->lineFaceName->setEnabled(false); From b796b3af78b61788c7831083ef5670469cd7b077 Mon Sep 17 00:00:00 2001 From: jrheinlaender Date: Mon, 5 Nov 2012 19:03:35 +0430 Subject: [PATCH 3/6] Fixed bug in Pad where option TwoLengths was broken if Symmetric had been selected before --- src/Mod/PartDesign/App/FeatureSketchBased.cpp | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/Mod/PartDesign/App/FeatureSketchBased.cpp b/src/Mod/PartDesign/App/FeatureSketchBased.cpp index 4904471845..a3e354d8c9 100644 --- a/src/Mod/PartDesign/App/FeatureSketchBased.cpp +++ b/src/Mod/PartDesign/App/FeatureSketchBased.cpp @@ -140,7 +140,7 @@ std::vector SketchBased::getSketchWires() const { return result; } - + // TODO: This code is taken from and duplicates code in Part2DObject::positionBySupport() // Note: We cannot return a reference, because it will become Null. // Not clear where, because we check for IsNull() here, but as soon as it is passed out of @@ -172,14 +172,14 @@ const TopoDS_Face SketchBased::getSupportFace() const { return face; } - + Part::Feature* SketchBased::getSupport() const { // get the support of the Sketch if any App::DocumentObject* SupportLink = static_cast(Sketch.getValue())->Support.getValue(); Part::Feature* SupportObject = NULL; if (SupportLink && SupportLink->getTypeId().isDerivedFrom(Part::Feature::getClassTypeId())) SupportObject = static_cast(SupportLink); - + return SupportObject; } @@ -197,7 +197,7 @@ const TopoDS_Shape& SketchBased::getSupportShape() const { return result; } - + int SketchBased::getSketchAxisCount(void) const { Part::Part2DObject *sketch = static_cast(Sketch.getValue()); @@ -484,12 +484,13 @@ void SketchBased::generatePrism(TopoDS_Shape& prism, // Note: 1E6 created problems once... Ltotal = 1E4; - if (midplane) - Loffset = -Ltotal/2; - else if (method == "TwoLengths") { + + if (method == "TwoLengths") { + // midplane makes no sense here Loffset = -L2; Ltotal += L2; - } + } else if (midplane) + Loffset = -Ltotal/2; TopoDS_Shape from = sketchshape; if (method == "TwoLengths" || midplane) { From ac2f2dc20ee66da4df6b381819f6c202ad356438 Mon Sep 17 00:00:00 2001 From: jrheinlaender Date: Mon, 5 Nov 2012 20:36:59 +0430 Subject: [PATCH 4/6] Fixed bug where sketch support face was used for distance measuring instead of sketch face itself (doesn't resolve problem of false positives, though) --- src/Mod/PartDesign/App/FeatureSketchBased.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Mod/PartDesign/App/FeatureSketchBased.cpp b/src/Mod/PartDesign/App/FeatureSketchBased.cpp index a3e354d8c9..ce96157de2 100644 --- a/src/Mod/PartDesign/App/FeatureSketchBased.cpp +++ b/src/Mod/PartDesign/App/FeatureSketchBased.cpp @@ -452,7 +452,7 @@ void SketchBased::getUpToFace(TopoDS_Face& upToFace, } // Check that the upToFace does not intersect the sketch face and - // is not parallel to the extrusion direction + // is not parallel to the extrusion direction (for simplicity, supportface is used instead of sketchshape) BRepAdaptor_Surface adapt1(TopoDS::Face(supportface)); BRepAdaptor_Surface adapt2(TopoDS::Face(upToFace)); @@ -461,7 +461,9 @@ void SketchBased::getUpToFace(TopoDS_Face& upToFace, throw Base::Exception("SketchBased: Up to face: Must not be parallel to extrusion direction!"); } - BRepExtrema_DistShapeShape distSS(supportface, upToFace); + // We must measure from sketchshape, not supportface, here + // TODO: distSS() sometimes gives false positives for unlimited upToFaces! + BRepExtrema_DistShapeShape distSS(sketchshape, upToFace); if (distSS.Value() < Precision::Confusion()) throw Base::Exception("SketchBased: Up to face: Must not intersect sketch!"); From c30c06cababe9f3b3f099fead790261166f8416f Mon Sep 17 00:00:00 2001 From: wmayer Date: Tue, 6 Nov 2012 20:44:31 +0100 Subject: [PATCH 5/6] Fix error in up to face option --- src/Mod/PartDesign/App/FeatureSketchBased.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Mod/PartDesign/App/FeatureSketchBased.cpp b/src/Mod/PartDesign/App/FeatureSketchBased.cpp index ce96157de2..ec46989d6f 100644 --- a/src/Mod/PartDesign/App/FeatureSketchBased.cpp +++ b/src/Mod/PartDesign/App/FeatureSketchBased.cpp @@ -444,11 +444,13 @@ void SketchBased::getUpToFace(TopoDS_Face& upToFace, if (remove_limits) { // Note: Using an unlimited face every time gives unnecessary failures for concave faces + TopLoc_Location loc = upToFace.Location(); BRepAdaptor_Surface adapt(upToFace, Standard_False); BRepBuilderAPI_MakeFace mkFace(adapt.Surface().Surface()); if (!mkFace.IsDone()) throw Base::Exception("SketchBased: Up To Face: Failed to create unlimited face"); upToFace = TopoDS::Face(mkFace.Shape()); + upToFace.Location(loc); } // Check that the upToFace does not intersect the sketch face and @@ -462,7 +464,6 @@ void SketchBased::getUpToFace(TopoDS_Face& upToFace, } // We must measure from sketchshape, not supportface, here - // TODO: distSS() sometimes gives false positives for unlimited upToFaces! BRepExtrema_DistShapeShape distSS(sketchshape, upToFace); if (distSS.Value() < Precision::Confusion()) throw Base::Exception("SketchBased: Up to face: Must not intersect sketch!"); From 1fb178db628f8f335ce456ad592906614a9b9024 Mon Sep 17 00:00:00 2001 From: wmayer Date: Tue, 6 Nov 2012 21:12:39 +0100 Subject: [PATCH 6/6] Reset modified flag after document is loaded, touch pad/pocket if its Type has changed --- src/Gui/Document.cpp | 5 ++++- src/Mod/PartDesign/App/FeaturePad.cpp | 1 + src/Mod/PartDesign/App/FeaturePocket.cpp | 1 + 3 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/Gui/Document.cpp b/src/Gui/Document.cpp index a3d4390039..4bab9e58b6 100644 --- a/src/Gui/Document.cpp +++ b/src/Gui/Document.cpp @@ -668,7 +668,7 @@ void Document::RestoreDocFile(Base::Reader &reader) if (!xmlReader.getFilenames().empty()) xmlReader.readFiles(static_cast(reader)); - // reset modifeid flag + // reset modified flag setModified(false); } @@ -697,6 +697,9 @@ void Document::slotFinishRestoreDocument(const App::Document& doc) for (it = d->_ViewProviderMap.begin(); it != d->_ViewProviderMap.end(); ++it) { it->second->finishRestoring(); } + + // reset modified flag + setModified(false); } /** diff --git a/src/Mod/PartDesign/App/FeaturePad.cpp b/src/Mod/PartDesign/App/FeaturePad.cpp index 1b97da8fe6..47e459204c 100644 --- a/src/Mod/PartDesign/App/FeaturePad.cpp +++ b/src/Mod/PartDesign/App/FeaturePad.cpp @@ -64,6 +64,7 @@ Pad::Pad() short Pad::mustExecute() const { if (Placement.isTouched() || + Type.isTouched() || Length.isTouched() || Length2.isTouched() || UpToFace.isTouched()) diff --git a/src/Mod/PartDesign/App/FeaturePocket.cpp b/src/Mod/PartDesign/App/FeaturePocket.cpp index 351fb512d3..f32d4ae22c 100644 --- a/src/Mod/PartDesign/App/FeaturePocket.cpp +++ b/src/Mod/PartDesign/App/FeaturePocket.cpp @@ -66,6 +66,7 @@ Pocket::Pocket() short Pocket::mustExecute() const { if (Placement.isTouched() || + Type.isTouched() || Length.isTouched() || UpToFace.isTouched()) return 1;