From 40a1ac8c8b0f228be9b5c7d0b565f5fc1692b0d4 Mon Sep 17 00:00:00 2001 From: wandererfan Date: Mon, 20 May 2024 12:40:41 -0400 Subject: [PATCH] [TD]fix autocorrect handling of older documents --- src/Mod/TechDraw/App/DimensionAutoCorrect.cpp | 5 ++- src/Mod/TechDraw/App/DrawViewDimension.cpp | 21 ++++++---- src/Mod/TechDraw/App/GeometryMatcher.cpp | 42 ++++--------------- 3 files changed, 23 insertions(+), 45 deletions(-) diff --git a/src/Mod/TechDraw/App/DimensionAutoCorrect.cpp b/src/Mod/TechDraw/App/DimensionAutoCorrect.cpp index 457817a898..02ff03fa31 100644 --- a/src/Mod/TechDraw/App/DimensionAutoCorrect.cpp +++ b/src/Mod/TechDraw/App/DimensionAutoCorrect.cpp @@ -134,8 +134,9 @@ bool DimensionAutoCorrect::autocorrectReferences(std::vector& referenceSta const std::vector savedGeometry = getDimension()->SavedGeometry.getValues(); if (savedGeometry.empty() || savedGeometry.size() != refsAll.size()) { // this must be an old document without savedGeometry property. We can not - // validate the references in this case. - return false; + // validate the references in this case so we have to assume the references are + // correct. + return true; } size_t iRef {0}; diff --git a/src/Mod/TechDraw/App/DrawViewDimension.cpp b/src/Mod/TechDraw/App/DrawViewDimension.cpp index f5d0480edd..47341114a1 100644 --- a/src/Mod/TechDraw/App/DrawViewDimension.cpp +++ b/src/Mod/TechDraw/App/DrawViewDimension.cpp @@ -453,9 +453,10 @@ short DrawViewDimension::mustExecute() const App::DocumentObjectExecReturn* DrawViewDimension::execute() { - // Base::Console().Message("DVD::execute() - %s\n", getNameInDocument()); if (!okToProceed()) { - return new App::DocumentObjectExecReturn("Dimension could not execute"); + // if we set an error here, it will be triggering many times during + // document load. + return DrawView::execute(); } m_referencesCorrect = true; @@ -463,6 +464,7 @@ App::DocumentObjectExecReturn* DrawViewDimension::execute() m_referencesCorrect = autocorrectReferences(); } if (!m_referencesCorrect) { + m_referencesCorrect = true; new App::DocumentObjectExecReturn("Autocorrect failed to fix broken references", this); } @@ -529,7 +531,8 @@ bool DrawViewDimension::okToProceed() } DrawViewPart* dvp = getViewPart(); if (!dvp) { - // TODO: translate these messages + // TODO: translate these messages and figure out how to present them to + // the user since we can't pop up a message box here. // this case is probably temporary during restore // Base::Console().Message("DVD::okToProceed - no view for dimension\n"); return false; @@ -537,20 +540,20 @@ bool DrawViewDimension::okToProceed() if (!(has2DReferences() || has3DReferences())) { // no references, can't do anything - Base::Console().Warning("DVD::okToProceed - Dimension object has no valid references\n"); + // Base::Console().Message("DVD::okToProceed - Dimension object has no valid references\n"); return false; } if (!getViewPart()->hasGeometry()) { // can't do anything until Source has geometry - Base::Console().Warning("DVD::okToProceed - Dimension object has no geometry\n"); + // Base::Console().Message("DVD::okToProceed - Dimension object has no geometry\n"); return false; } // is this check still relevant or is it replaced by the autocorrect and // validate methods? if (References3D.getValues().empty() && !checkReferences2D()) { - Base::Console().Warning("%s has invalid 2D References\n", getNameInDocument()); + // Base::Console().Warning("%s has invalid 2D References\n", getNameInDocument()); return false; } return validateReferenceForm(); @@ -560,7 +563,9 @@ bool DrawViewDimension::okToProceed() //! everything matches, we don't need to correct anything. bool DrawViewDimension::autocorrectReferences() { - // Base::Console().Message("DVD::autocorrectReferences()\n"); + // TODO: check for saved geometry here. If we don't have saved geometry, we can't + // successfully auto correct in phase 1. This check is currently in + // referencesHaveValidGeometry. std::vector referenceState; bool refsAreValid = m_corrector->referencesHaveValidGeometry(referenceState); if (!refsAreValid) { @@ -569,8 +574,6 @@ bool DrawViewDimension::autocorrectReferences() refsAreValid = m_corrector->autocorrectReferences(referenceState, repairedRefs); if (!refsAreValid) { // references are broken and we can not fix them - Base::Console().Warning("Autocorrect failed to fix references for %s\n", - getNameInDocument()); return false; } diff --git a/src/Mod/TechDraw/App/GeometryMatcher.cpp b/src/Mod/TechDraw/App/GeometryMatcher.cpp index e7ba33dffa..5792081e49 100644 --- a/src/Mod/TechDraw/App/GeometryMatcher.cpp +++ b/src/Mod/TechDraw/App/GeometryMatcher.cpp @@ -44,6 +44,7 @@ #include #include +#include #include "GeometryMatcher.h" #include "DrawUtil.h" @@ -51,23 +52,21 @@ using namespace TechDraw; using DU = DrawUtil; +using SU = ShapeUtils; // a set of routines for comparing geometry for equality. bool GeometryMatcher::compareGeometry(const Part::TopoShape &shape1, const Part::TopoShape &shape2) { - // Base::Console().Message("GM::compareGeometry()\n"); if (!Preferences::useExactMatchOnDims()) { return false; } if (shape1.isNull() || shape2.isNull()) { - Base::Console().Message("GM::compareGeometry - at least 1 input shape is null (1)\n"); return false; } const TopoDS_Shape& geom1 = shape1.getShape(); const TopoDS_Shape& geom2 = shape2.getShape(); if (geom1.IsNull() || geom2.IsNull()) { - Base::Console().Message("GM::compareGeometry - at least 1 input shape is null (2)\n"); return false; } @@ -86,7 +85,6 @@ bool GeometryMatcher::compareGeometry(const Part::TopoShape &shape1, const Part: bool GeometryMatcher::comparePoints(const TopoDS_Shape& shape1, const TopoDS_Shape& shape2) { - // Base::Console().Message("GM::comparePoints()\n"); if (shape1.ShapeType() != TopAbs_VERTEX || shape2.ShapeType() != TopAbs_VERTEX) { // can not compare these shapes return false; @@ -100,8 +98,6 @@ bool GeometryMatcher::comparePoints(const TopoDS_Shape& shape1, const TopoDS_Sha bool GeometryMatcher::compareFaces(const TopoDS_Shape& shape1, const TopoDS_Shape& shape2) { - // Base::Console().Message("GM::compareFaces()\n"); - if (shape1.ShapeType() != TopAbs_FACE || shape2.ShapeType() != TopAbs_FACE) { // can not compare these shapes return false; @@ -121,7 +117,6 @@ bool GeometryMatcher::compareFaces(const TopoDS_Shape& shape1, const TopoDS_Shap bool GeometryMatcher::compareEdges(const TopoDS_Shape& shape1, const TopoDS_Shape& shape2) { - // Base::Console().Message("GM::compareEdges()\n"); if (shape1.ShapeType() != TopAbs_EDGE || shape2.ShapeType() != TopAbs_EDGE) { return false; } @@ -162,23 +157,17 @@ bool GeometryMatcher::compareEdges(const TopoDS_Shape& shape1, const TopoDS_Shap bool GeometryMatcher::compareLines(const TopoDS_Edge& edge1, const TopoDS_Edge& edge2) { - // Base::Console().Message("GM::compareLines()\n"); // how does the edge that was NOT null in compareEdges become null here? // should not happen, but does! if (edge1.IsNull() || edge2.IsNull()) { // Base::Console().Message("GM::compareLine - an input edge is null\n"); return false; } - auto start1 = DU::toVector3d(BRep_Tool::Pnt(TopExp::FirstVertex(edge1))); - auto end1 = DU::toVector3d(BRep_Tool::Pnt(TopExp::LastVertex(edge1))); - auto start2 = DU::toVector3d(BRep_Tool::Pnt(TopExp::FirstVertex(edge2))); - auto end2 = DU::toVector3d(BRep_Tool::Pnt(TopExp::LastVertex(edge2))); - return start1.IsEqual(start2, EWTOLERANCE) && end1.IsEqual(end2, EWTOLERANCE); + return compareEndPoints(edge1, edge2); } bool GeometryMatcher::compareCircles(const TopoDS_Edge& edge1, const TopoDS_Edge& edge2) { - // Base::Console().Message("GM::compareCircles()\n"); // how does the edge that was NOT null in compareEdges become null here? if (edge1.IsNull() || edge2.IsNull()) { return false; @@ -220,7 +209,6 @@ bool GeometryMatcher::compareEllipses(const TopoDS_Edge& edge1, const TopoDS_Edg // for our purposes, only lines or circles masquerading as bsplines are of interest bool GeometryMatcher::compareBSplines(const TopoDS_Edge& edge1, const TopoDS_Edge& edge2) { - // Base::Console().Message("GM::compareBSplines()\n"); // how does the edge that was NOT null in compareEdges become null here? if (edge1.IsNull() || edge2.IsNull()) { return false; @@ -260,7 +248,7 @@ bool GeometryMatcher::compareBSplines(const TopoDS_Edge& edge1, const TopoDS_Edg // this is a weak comparison. we should also check center & radius? bool GeometryMatcher::compareCircleArcs(const TopoDS_Edge& edge1, const TopoDS_Edge& edge2) { - return compareEndPoints(edge1, edge2); + return compareCircles(edge1, edge2) && compareEndPoints(edge1, edge2); } bool GeometryMatcher::compareEllipseArcs(const TopoDS_Edge& edge1, const TopoDS_Edge& edge2) @@ -272,7 +260,6 @@ bool GeometryMatcher::compareEllipseArcs(const TopoDS_Edge& edge1, const TopoDS_ // not sure how successful this would be. For now, we just say it doesn't match bool GeometryMatcher::compareDifferent(const TopoDS_Edge& edge1, const TopoDS_Edge& edge2) { - // Base::Console().Message("GM::compareDifferent()\n"); BRepAdaptor_Curve adapt1(edge1); BRepAdaptor_Curve adapt2(edge2); return false; @@ -285,22 +272,9 @@ bool GeometryMatcher::compareEndPoints(const TopoDS_Edge& edge1, const TopoDS_Ed return false; } - BRepAdaptor_Curve adapt1(edge1); - BRepAdaptor_Curve adapt2(edge2); - double pFirst1 = adapt1.FirstParameter(); - double pLast1 = adapt1.LastParameter(); - BRepLProp_CLProps props1(adapt1, pFirst1, 0, Precision::Confusion()); - auto begin1 = DU::toVector3d(props1.Value()); - props1.SetParameter(pLast1); - auto end1 = DU::toVector3d(props1.Value()); - double pFirst2 = adapt2.FirstParameter(); - double pLast2 = adapt2.LastParameter(); - BRepLProp_CLProps props2(adapt2, pFirst2, 0, Precision::Confusion()); - auto begin2 = DU::toVector3d(props2.Value()); - props2.SetParameter(pLast2); - auto end2 = DU::toVector3d(props2.Value()); - - return (begin1.IsEqual(begin2, EWTOLERANCE) && - end1.IsEqual(end2, EWTOLERANCE)); + auto ends1 = SU::getEdgeEnds(edge1); + auto ends2 = SU::getEdgeEnds(edge2); + return (ends1.first.IsEqual(ends2.first, EWTOLERANCE) && + ends1.second.IsEqual(ends2.second, EWTOLERANCE)); }