From 8f8bdd5c93d0e085e4326f988942545904ad7214 Mon Sep 17 00:00:00 2001 From: wandererfan Date: Thu, 21 Nov 2024 16:37:00 -0500 Subject: [PATCH] [TD]add shape validation tool for debugging --- src/Mod/TechDraw/App/Preferences.cpp | 15 + src/Mod/TechDraw/App/Preferences.h | 4 + src/Mod/TechDraw/App/ShapeExtractor.cpp | 86 +-- src/Mod/TechDraw/App/ShapeExtractor.h | 2 + .../TechDraw/Gui/DlgPrefsTechDrawAdvanced.ui | 553 ++++++++++-------- .../Gui/DlgPrefsTechDrawAdvancedImp.cpp | 6 + 6 files changed, 372 insertions(+), 294 deletions(-) diff --git a/src/Mod/TechDraw/App/Preferences.cpp b/src/Mod/TechDraw/App/Preferences.cpp index 8db68f852b..7962467cd0 100644 --- a/src/Mod/TechDraw/App/Preferences.cpp +++ b/src/Mod/TechDraw/App/Preferences.cpp @@ -650,6 +650,21 @@ void Preferences::setBalloonDragModifiers(Qt::KeyboardModifiers newModifiers) getPreferenceGroup("General")->SetUnsigned("BalloonDragModifier", (uint)newModifiers); } +//! if true, shapes are validated before use and problematic ones are skipped. +//! validating shape takes time, but can prevent crashes/bad results in occt. +//! this would normally be set to false and set to true to aid in debugging/support. +bool Preferences::checkShapesBeforeUse() +{ + return getPreferenceGroup("General")->GetBool("CheckShapesBeforeUse", false); +} + + +//! if true, shapes which fail validation are saved as brep files +bool Preferences::debugBadShape() +{ + return getPreferenceGroup("debug")->GetBool("debugBadShape", false); +} + //! if true, automatically switch to TD workbench when a Page is set in edit (double click) bool Preferences::switchOnClick() diff --git a/src/Mod/TechDraw/App/Preferences.h b/src/Mod/TechDraw/App/Preferences.h index 03558f0522..b93889f1bd 100644 --- a/src/Mod/TechDraw/App/Preferences.h +++ b/src/Mod/TechDraw/App/Preferences.h @@ -152,6 +152,10 @@ public: static void setBalloonDragModifiers(Qt::KeyboardModifiers newModifiers); static bool switchOnClick(); + + static bool checkShapesBeforeUse(); + static bool debugBadShape(); + }; diff --git a/src/Mod/TechDraw/App/ShapeExtractor.cpp b/src/Mod/TechDraw/App/ShapeExtractor.cpp index 99b74df521..8acb890674 100644 --- a/src/Mod/TechDraw/App/ShapeExtractor.cpp +++ b/src/Mod/TechDraw/App/ShapeExtractor.cpp @@ -31,6 +31,7 @@ # include # include # include +#include #endif #include @@ -51,7 +52,7 @@ #include "ShapeExtractor.h" #include "DrawUtil.h" #include "ShapeUtils.h" - +#include "Preferences.h" using namespace TechDraw; using DU = DrawUtil; @@ -62,14 +63,13 @@ using SU = ShapeUtils; //! Note that point objects will not make it through the hlr/projection process. std::vector ShapeExtractor::getShapes2d(const std::vector links) { -// Base::Console().Message("SE::getShapes2d() - links: %d\n", links.size()); - std::vector shapes2d; for (auto& l:links) { if (is2dObject(l)) { if (l->getTypeId().isDerivedFrom(Part::Feature::getClassTypeId())) { TopoDS_Shape temp = getLocatedShape(l); + // checkShape on 2d objs? if (!temp.IsNull()) { shapes2d.push_back(temp); } @@ -83,12 +83,10 @@ std::vector ShapeExtractor::getShapes2d(const std::vector links, bool include2d) { - // Base::Console().Message("SE::getShapes() - links in: %d\n", links.size()); std::vector sourceShapes; for (auto& l:links) { if (is2dObject(l) && !include2d) { - // Base::Console().Message("SE::getShapes - skipping 2d link: %s\n", l->Label.getValue()); continue; } @@ -136,9 +134,11 @@ TopoDS_Shape ShapeExtractor::getShapes(const std::vector l } else { auto shape = Part::Feature::getShape(obj); - // if link obj has a shape, we use that shape. + // if source obj has a shape, we use that shape. if(!SU::isShapeReallyNull(shape) && !isExplodedView) { - sourceShapes.push_back(getLocatedShape(obj)); + if (checkShape(obj, shape)) { + sourceShapes.push_back(getLocatedShape(obj)); + } } else { std::vector shapeList = getShapesFromObject(obj); @@ -179,17 +179,14 @@ TopoDS_Shape ShapeExtractor::getShapes(const std::vector l } //it appears that an empty compound is !IsNull(), so we need to check a different way if (!SU::isShapeReallyNull(comp)) { -// BRepTools::Write(comp, "SEResult.brep"); //debug return comp; } -// Base::Console().Error("DEVEL: ShapeExtractor failed to get any shape.\n"); return TopoDS_Shape(); } std::vector ShapeExtractor::getXShapes(const App::Link* xLink) { - // Base::Console().Message("SE::getXShapes() - %s\n", xLink->getNameInDocument()); std::vector xSourceShapes; if (!xLink) { return xSourceShapes; @@ -231,6 +228,9 @@ std::vector ShapeExtractor::getXShapes(const App::Link* xLink) if (ts.isInfinite()) { shape = stripInfiniteShapes(shape); } + if (!checkShape(l, shape)) { + continue; + } // copying the shape prevents "non-orthogonal GTrsf" errors in some versions // of OCC. Something to do with triangulation of shape?? // it may be that incremental mesh would work here too. @@ -252,7 +252,8 @@ std::vector ShapeExtractor::getXShapes(const App::Link* xLink) } else { // link points to a regular object, not another link? no sublinks? TopoDS_Shape xLinkShape = getShapeFromXLink(xLink); - if (!xLinkShape.IsNull()) { + if (!xLinkShape.IsNull() && + checkShape(xLink, xLinkShape)) { // copying the shape prevents "non-orthogonal GTrsf" errors in some versions // of OCC. Something to do with triangulation of shape?? BRepBuilderAPI_Copy copier(xLinkShape); @@ -265,7 +266,6 @@ std::vector ShapeExtractor::getXShapes(const App::Link* xLink) // get the located shape for a single childless App::Link TopoDS_Shape ShapeExtractor::getShapeFromXLink(const App::Link* xLink) { - // Base::Console().Message("SE::getShapeFromXLink()\n"); Base::Placement xLinkPlacement; if (xLink->hasPlacement()) { xLinkPlacement = xLink->getLinkPlacementProperty()->getValue(); @@ -298,21 +298,24 @@ TopoDS_Shape ShapeExtractor::getShapeFromXLink(const App::Link* xLink) Base::Console().Error("ShapeExtractor failed to retrieve shape from %s\n", xLink->getNameInDocument()); return TopoDS_Shape(); } - return ts.getShape(); + if (checkShape(linkedObject, ts.getShape())) { + return ts.getShape(); + } } return TopoDS_Shape(); } std::vector ShapeExtractor::getShapesFromObject(const App::DocumentObject* docObj) { -// Base::Console().Message("SE::getShapesFromObject(%s)\n", docObj->getNameInDocument()); std::vector result; const App::GroupExtension* gex = dynamic_cast(docObj); App::Property* gProp = docObj->getPropertyByName("Group"); App::Property* sProp = docObj->getPropertyByName("Shape"); if (docObj->isDerivedFrom()) { - result.push_back(getLocatedShape(docObj)); + if (checkShape(docObj, getLocatedShape(docObj))) { + result.push_back(getLocatedShape(docObj)); + } } else if (gex) { //is a group extension std::vector objs = gex->Group.getValues(); std::vector shapes; @@ -326,18 +329,17 @@ std::vector ShapeExtractor::getShapesFromObject(const App::Documen } else if (gProp) { //has a Group property App::PropertyLinkList* list = dynamic_cast(gProp); if (list) { - std::vector objs = list->getValues(); - std::vector shapes; - for (auto& d: objs) { - shapes = getShapesFromObject(d); - if (!shapes.empty()) { - result.insert(result.end(), shapes.begin(), shapes.end()); - } + std::vector objsAll = list->getValues(); + std::vector shapesAll; + for (auto& obj : objsAll) { + shapesAll = getShapesFromObject(obj); + result.insert(result.end(), shapesAll.begin(), shapesAll.end()); } } } else if (sProp) { //has a Shape property - Part::PropertyPartShape* shape = dynamic_cast(sProp); - if (shape) { + Part::PropertyPartShape* shapeProperty = dynamic_cast(sProp); + if (shapeProperty && + checkShape(docObj, getLocatedShape(docObj))) { result.push_back(getLocatedShape(docObj)); } } @@ -346,7 +348,6 @@ std::vector ShapeExtractor::getShapesFromObject(const App::Documen TopoDS_Shape ShapeExtractor::getShapesFused(const std::vector links) { -// Base::Console().Message("SE::getShapesFused()\n"); // get only the 3d shapes and fuse them TopoDS_Shape baseShape = getShapes(links, false); if (!baseShape.IsNull()) { @@ -365,12 +366,10 @@ TopoDS_Shape ShapeExtractor::getShapesFused(const std::vector shapes2d = getShapes2d(links); - BRepTools::Write(DrawUtil::shapeVectorToCompound(shapes2d, false), "SEshapes2d.brep"); if (!shapes2d.empty()) { shapes2d.push_back(baseShape); @@ -385,7 +384,6 @@ TopoDS_Shape ShapeExtractor::getShapesFused(const std::vectorgetNameInDocument()); if (obj) { Base::Type t = obj->getTypeId(); if (t.isDerivedFrom(Part::Vertex::getClassTypeId())) { @@ -452,12 +449,10 @@ bool ShapeExtractor::isPointType(const App::DocumentObject* obj) bool ShapeExtractor::isDraftPoint(const App::DocumentObject* obj) { -// Base::Console().Message("SE::isDraftPoint()\n"); //if the docObj doesn't have a Proxy property, it definitely isn't a Draft point App::PropertyPythonObject* proxy = dynamic_cast(obj->getPropertyByName("Proxy")); if (proxy) { std::string pp = proxy->toString(); -// Base::Console().Message("SE::isDraftPoint - pp: %s\n", pp.c_str()); if (pp.find("Point") != std::string::npos) { return true; } @@ -479,7 +474,6 @@ bool ShapeExtractor::isDatumPoint(const App::DocumentObject* obj) //! get the location of a point object Base::Vector3d ShapeExtractor::getLocation3dFromFeat(const App::DocumentObject* obj) { - // Base::Console().Message("SE::getLocation3dFromFeat()\n"); if (!isPointType(obj)) { return Base::Vector3d(0.0, 0.0, 0.0); } @@ -498,8 +492,6 @@ Base::Vector3d ShapeExtractor::getLocation3dFromFeat(const App::DocumentObject* } } -// Base::Console().Message("SE::getLocation3dFromFeat - returns: %s\n", -// DrawUtil::formatVector(result).c_str()); return Base::Vector3d(0.0, 0.0, 0.0); } @@ -529,3 +521,29 @@ bool ShapeExtractor::isSketchObject(const App::DocumentObject* obj) return false; } + +//! true if shape fails validity check. A fail here is not a guarantee of later +//! problems, but invalid shapes are known to cause issues with HLR_Algo and boolean ops. +bool ShapeExtractor::checkShape(const App::DocumentObject* shapeObj, TopoDS_Shape shape) +{ + if (!Preferences::checkShapesBeforeUse()) { + return true; + } + + if (!BRepCheck_Analyzer(shape).IsValid()) { + if (Preferences::debugBadShape()) { + std::stringstream ssFileName; + ssFileName << "BadShape" << shapeObj->Label.getValue() << ".brep"; + BRepTools::Write(shape, ssFileName.str().c_str()); + } + // this is ok for devs, but there must be a better way to inform the user from somewhere deep in the + // call stack. notification area from App? + Base::Console().Warning( + "ShapeExtractor found a problem shape in %s. Results may be incorrect.\n", + shapeObj->getNameInDocument()); + return false; + } + return true; +} + + diff --git a/src/Mod/TechDraw/App/ShapeExtractor.h b/src/Mod/TechDraw/App/ShapeExtractor.h index 972d37a33b..c644bd6ad7 100644 --- a/src/Mod/TechDraw/App/ShapeExtractor.h +++ b/src/Mod/TechDraw/App/ShapeExtractor.h @@ -58,6 +58,8 @@ public: static TopoDS_Shape getLocatedShape(const App::DocumentObject* docObj); + static bool checkShape(const App::DocumentObject* shapeObj, TopoDS_Shape shape); + protected: private: diff --git a/src/Mod/TechDraw/Gui/DlgPrefsTechDrawAdvanced.ui b/src/Mod/TechDraw/Gui/DlgPrefsTechDrawAdvanced.ui index 2594f96f31..e775476392 100644 --- a/src/Mod/TechDraw/Gui/DlgPrefsTechDrawAdvanced.ui +++ b/src/Mod/TechDraw/Gui/DlgPrefsTechDrawAdvanced.ui @@ -28,19 +28,163 @@ - - - - - true - + + + + If this box is checked, double clicking on a page in the tree will automatically switch to TechDraw and the page will be made visible. - Overlap Edges Scrub Passes + Switch Workbench on Click + + + true + + + SwitchToWB + + + Mod/TechDraw/General - + + + + + 0 + 0 + + + + Dump intermediate results during Section view processing + + + Debug Section + + + debugSection + + + Mod/TechDraw/debug + + + + + + + Edge Fuzz + + + + + + + If checked, FreeCAD will use the new face finder algorithm. If not checked, FreeCAD will use the original face finder. + + + Use New Face Finder Algorithm + + + true + + + NewFaceFinder + + + Mod/TechDraw/General + + + + + + + + 0 + 0 + + + + Dump intermediate results during Detail view processing + + + Debug Detail + + + debugDetail + + + Mod/TechDraw/debug + + + + + + + + 0 + 0 + + + + If checked, TechDraw will attempt to build faces using the +line segments returned by the hidden line removal algorithm. +Faces must be detected in order to use hatching, but there +can be a performance penalty in complex models. + + + Detect Faces + + + true + + + HandleFaces + + + /Mod/TechDraw/General + + + + + + + If checked, system will attempt to automatically correct dimension references when the model changes. + + + + + + Auto Correct Dimension Refs + + + true + + + AutoCorrectRefs + + + Mod/TechDraw/Dimensions + + + + + + + If checked, input shapes will be checked for errors before use.and invalid shapes will be skipped by the shape extractor. Checking for errors is slower, but can prevent crashes from some geometry problems. + + + + Validate Shapes + + + CheckShapesBeforeUse + + + Mod/TechDraw/General + + + + @@ -62,12 +206,71 @@ - - + + - Limit of 64x64 pixel SVG tiles used to hatch a single face. -For large scalings you might get an error about too many SVG tiles. -Then you need to increase the tile limit. + Issue progress messages while building View geometry + + + Report Progress + + + ReportProgress + + + /Mod/TechDraw/General + + + + + + + The number of times FreeCAD should try to remove overlapping edges returned by the Hidden Line Removal algorithm. A value of 0 indicates no scrubbing, 1 indicates a single pass and 2 indicates a second pass should be performed. Values above 2 are generally not productive. Each pass adds to the time required to produce the drawing. + + + Qt::AlignmentFlag::AlignRight|Qt::AlignmentFlag::AlignTrailing|Qt::AlignmentFlag::AlignVCenter + + + + + + + + + 1 + + + ScrubCount + + + Mod/TechDraw/General + + + + + + + + true + + + + Overlap Edges Scrub Passes + + + + + + + Mark Fuzz + + + + + + + Maximum hatch line segments to use +when hatching a face with a PAT pattern Qt::AlignmentFlag::AlignRight @@ -85,10 +288,60 @@ Then you need to increase the tile limit. 10000 - MaxSVGTile + MaxSeg - Mod/TechDraw/Decorations + Mod/TechDraw/PAT + + + + + + + Max SVG Hatch Tiles + + + + + + + If checked, shapes which fail validation will be saved as brep files for later analysis. + + + Debug Bad Shape + + + debugBadShape + + + Mod/TechDraw/debug + + + + + + + + 0 + 0 + + + + + true + + + + Perform a fuse operation on input shape(s) before Section view processing + + + Fuse Before Section + + + SectionFuseFirst + + + Mod/TechDraw/General @@ -127,47 +380,6 @@ Each unit is approx. 0.1 mm wide - - - - - 0 - 0 - - - - Dump intermediate results during Section view processing - - - Debug Section - - - debugSection - - - Mod/TechDraw/debug - - - - - - - If checked, FreeCAD will use the new face finder algorithm. If not checked, FreeCAD will use the original face finder. - - - Use New Face Finder Algorithm - - - true - - - NewFaceFinder - - - Mod/TechDraw/General - - - @@ -181,119 +393,6 @@ Each unit is approx. 0.1 mm wide - - - - Max SVG Hatch Tiles - - - - - - - Issue progress messages while building View geometry - - - Report Progress - - - ReportProgress - - - /Mod/TechDraw/General - - - - - - - Edge Fuzz - - - - - - - Mark Fuzz - - - - - - - - 0 - 0 - - - - Highlights border of section cut in section views - - - Show Section Edges - - - true - - - ShowSectionEdges - - - /Mod/TechDraw/General - - - - - - - Maximum hatch line segments to use -when hatching a face with a PAT pattern - - - Qt::AlignmentFlag::AlignRight - - - 1 - - - 1000000 - - - 100 - - - 10000 - - - MaxSeg - - - Mod/TechDraw/PAT - - - - - - - If checked, system will attempt to automatically correct dimension references when the model changes. - - - - - - Auto Correct Dimension Refs - - - true - - - AutoCorrectRefs - - - Mod/TechDraw/Dimensions - - - @@ -329,60 +428,8 @@ Each unit is approx. 0.1 mm wide - - - - - 0 - 0 - - - - - true - - - - Perform a fuse operation on input shape(s) before Section view processing - - - Fuse Before Section - - - SectionFuseFirst - - - Mod/TechDraw/General - - - - - - - The number of times FreeCAD should try to remove overlapping edges returned by the Hidden Line Removal algorithm. A value of 0 indicates no scrubbing, 1 indicates a single pass and 2 indicates a second pass should be performed. Values above 2 are generally not productive. Each pass adds to the time required to produce the drawing. - - - Qt::AlignmentFlag::AlignRight|Qt::AlignmentFlag::AlignTrailing|Qt::AlignmentFlag::AlignVCenter - - - - - - - - - 1 - - - ScrubCount - - - Mod/TechDraw/General - - - - - + + 0 @@ -390,47 +437,22 @@ Each unit is approx. 0.1 mm wide - If checked, TechDraw will attempt to build faces using the -line segments returned by the hidden line removal algorithm. -Faces must be detected in order to use hatching, but there -can be a performance penalty in complex models. + Highlights border of section cut in section views - Detect Faces + Show Section Edges true - HandleFaces + ShowSectionEdges /Mod/TechDraw/General - - - - - 0 - 0 - - - - Dump intermediate results during Detail view processing - - - Debug Detail - - - debugDetail - - - Mod/TechDraw/debug - - - @@ -438,22 +460,33 @@ can be a performance penalty in complex models. - - + + - If this box is checked, double clicking on a page in the tree will automatically switch to TechDraw and the page will be made visible. + Limit of 64x64 pixel SVG tiles used to hatch a single face. +For large scalings you might get an error about too many SVG tiles. +Then you need to increase the tile limit. - - Switch Workbench on Click + + Qt::AlignmentFlag::AlignRight - - true + + 1 + + + 1000000 + + + 100 + + + 10000 - SwitchToWB + MaxSVGTile - Mod/TechDraw/General + Mod/TechDraw/Decorations diff --git a/src/Mod/TechDraw/Gui/DlgPrefsTechDrawAdvancedImp.cpp b/src/Mod/TechDraw/Gui/DlgPrefsTechDrawAdvancedImp.cpp index ef987f8e8f..557ee38868 100644 --- a/src/Mod/TechDraw/Gui/DlgPrefsTechDrawAdvancedImp.cpp +++ b/src/Mod/TechDraw/Gui/DlgPrefsTechDrawAdvancedImp.cpp @@ -63,6 +63,9 @@ void DlgPrefsTechDrawAdvancedImp::saveSettings() ui->cbNewFaceFinder->onSave(); ui->sbScrubCount->onSave(); + ui->cbDebugBadShape->onSave(); + ui->cbValidateShapes->onSave(); + saveBalloonOverride(); ui->cbSwitchWB->onSave(); @@ -115,6 +118,9 @@ void DlgPrefsTechDrawAdvancedImp::loadSettings() ui->cbNewFaceFinder->onRestore(); ui->sbScrubCount->onRestore(); + ui->cbDebugBadShape->onRestore(); + ui->cbValidateShapes->onRestore(); + loadBalloonOverride(); ui->cbSwitchWB->onRestore();