From a6b89251a4be451e2f919b64d3828518f453f554 Mon Sep 17 00:00:00 2001 From: wandererfan Date: Thu, 6 Mar 2025 19:07:13 -0500 Subject: [PATCH 1/4] [TD]prevent crash when no event loop - threaded hlr operations do not return if qApplication is not available --- src/Mod/TechDraw/App/AppTechDrawPy.cpp | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/src/Mod/TechDraw/App/AppTechDrawPy.cpp b/src/Mod/TechDraw/App/AppTechDrawPy.cpp index a41bb697c5..34839381d4 100644 --- a/src/Mod/TechDraw/App/AppTechDrawPy.cpp +++ b/src/Mod/TechDraw/App/AppTechDrawPy.cpp @@ -43,6 +43,9 @@ #include #include #include + +#include + #include #include #include @@ -434,6 +437,10 @@ private: obj = static_cast(viewObj)->getDocumentObjectPtr(); dvp = static_cast(obj); TechDraw::GeometryObjectPtr gObj = dvp->getGeometryObject(); + if (!gObj) { + Base::Console().Message("TechDraw: %s has no geometry object!\n", dvp->Label.getValue()); + return Py::String(); + } TopoDS_Shape shape = ShapeUtils::mirrorShape(gObj->getVisHard()); ss << dxfOut.exportEdges(shape); shape = ShapeUtils::mirrorShape(gObj->getVisOutline()); @@ -472,6 +479,7 @@ private: return dxfReturn; } + Py::Object viewPartAsSvg(const Py::Tuple& args) { PyObject *viewObj(nullptr); @@ -482,6 +490,7 @@ private: std::string grpHead1 = "\n"; std::string grpTail = "\n"; + Base::Console().Message("TechDraw:: gui is up? %d\n", (int)DrawUtil::isGuiUp()); try { App::DocumentObject* obj = nullptr; TechDraw::DrawViewPart* dvp = nullptr; @@ -492,9 +501,13 @@ private: obj = static_cast(viewObj)->getDocumentObjectPtr(); dvp = static_cast(obj); TechDraw::GeometryObjectPtr gObj = dvp->getGeometryObject(); + if (!gObj) { + Base::Console().Message("TechDraw: %s has no geometry object!\n", dvp->Label.getValue()); + return Py::String(); + } + //visible group begin "" ss << grpHead1; -// double thick = dvp->LineWidth.getValue(); double thick = DrawUtil::getDefaultLineWeight("Thick"); ss << thick; ss << grpHead2; @@ -518,7 +531,6 @@ private: dvp->SeamHidden.getValue() ) { //hidden group begin ss << grpHead1; -// thick = dvp->HiddenWidth.getValue(); thick = DrawUtil::getDefaultLineWeight("Thin"); ss << thick; ss << grpHead2; @@ -553,9 +565,16 @@ private: void write1ViewDxf( ImpExpDxfWrite& writer, TechDraw::DrawViewPart* dvp, bool alignPage) { - if(!dvp->hasGeometry()) + if(!dvp->hasGeometry()) { return; + } + TechDraw::GeometryObjectPtr gObj = dvp->getGeometryObject(); + if (!gObj) { + // this test might be redundent here since we already checked hasGeometry. + Base::Console().Message("TechDraw: %s has no geometry object!\n", dvp->Label.getValue()); + return; + } TopoDS_Shape shape = ShapeUtils::mirrorShape(gObj->getVisHard()); double offX = 0.0; double offY = 0.0; From 1fc53e5f2791ba73b38a863fb980657a54e24b58 Mon Sep 17 00:00:00 2001 From: wandererfan Date: Fri, 7 Mar 2025 13:19:01 -0500 Subject: [PATCH 2/4] [TD]add check for Gui up --- src/Mod/TechDraw/App/DrawUtil.cpp | 8 ++++++++ src/Mod/TechDraw/App/DrawUtil.h | 1 + 2 files changed, 9 insertions(+) diff --git a/src/Mod/TechDraw/App/DrawUtil.cpp b/src/Mod/TechDraw/App/DrawUtil.cpp index e6e58c846c..60a7bd92d2 100644 --- a/src/Mod/TechDraw/App/DrawUtil.cpp +++ b/src/Mod/TechDraw/App/DrawUtil.cpp @@ -1887,6 +1887,14 @@ std::string DrawUtil::cleanFilespecBackslash(const std::string& filespec) return noBackslash; } +//! returns true if the Gui module and its event loop are active. +bool DrawUtil::isGuiUp() +{ + std::string pyCommand{"import FreeCAD\nguiState = FreeCAD.GuiUp"}; + std::string result{Base::Interpreter().runStringWithKey(pyCommand.c_str(), "guiState", "None")}; + return (result == "1"); +} + //============================ // various debugging routines. diff --git a/src/Mod/TechDraw/App/DrawUtil.h b/src/Mod/TechDraw/App/DrawUtil.h index 3eec9bed80..9e4a9edcee 100644 --- a/src/Mod/TechDraw/App/DrawUtil.h +++ b/src/Mod/TechDraw/App/DrawUtil.h @@ -271,6 +271,7 @@ public: static std::string cleanFilespecBackslash(const std::string& filespec); + static bool isGuiUp(); //debugging routines static void dumpVertexes(const char* text, const TopoDS_Shape& s); From ac4ba822b409c4d1f7eae20b3d502508f3c8df7f Mon Sep 17 00:00:00 2001 From: wandererfan Date: Fri, 7 Mar 2025 13:19:36 -0500 Subject: [PATCH 3/4] [TD]allow view creation in main thread if gui no available --- src/Mod/TechDraw/App/AppTechDrawPy.cpp | 1 - src/Mod/TechDraw/App/DrawViewDetail.cpp | 10 +++- src/Mod/TechDraw/App/DrawViewPart.cpp | 61 +++++++++++++----------- src/Mod/TechDraw/App/DrawViewSection.cpp | 37 +++++++------- 4 files changed, 57 insertions(+), 52 deletions(-) diff --git a/src/Mod/TechDraw/App/AppTechDrawPy.cpp b/src/Mod/TechDraw/App/AppTechDrawPy.cpp index 34839381d4..c2d574f4b5 100644 --- a/src/Mod/TechDraw/App/AppTechDrawPy.cpp +++ b/src/Mod/TechDraw/App/AppTechDrawPy.cpp @@ -490,7 +490,6 @@ private: std::string grpHead1 = "\n"; std::string grpTail = "\n"; - Base::Console().Message("TechDraw:: gui is up? %d\n", (int)DrawUtil::isGuiUp()); try { App::DocumentObject* obj = nullptr; TechDraw::DrawViewPart* dvp = nullptr; diff --git a/src/Mod/TechDraw/App/DrawViewDetail.cpp b/src/Mod/TechDraw/App/DrawViewDetail.cpp index 6a31f6efd9..34d78fa7e2 100644 --- a/src/Mod/TechDraw/App/DrawViewDetail.cpp +++ b/src/Mod/TechDraw/App/DrawViewDetail.cpp @@ -189,6 +189,12 @@ void DrawViewDetail::detailExec(TopoDS_Shape& shape, DrawViewPart* dvp, DrawView return; } + if (!DU::isGuiUp()) { + makeDetailShape(shape, dvp, dvs); + onMakeDetailFinished(); + waitingForDetail(false); + } + //note that &m_detailWatcher in the third parameter is not strictly required, but using the //4 parameter signature instead of the 3 parameter signature prevents clazy warning: //https://github.com/KDE/clazy/blob/1.11/docs/checks/README-connect-3arg-lambda.md @@ -404,8 +410,10 @@ void DrawViewDetail::onMakeDetailFinished(void) waitingForDetail(false); QObject::disconnect(connectDetailWatcher); - //ancestor's buildGeometryObject will run HLR and face finding in a separate thread m_tempGeometryObject = buildGeometryObject(m_scaledShape, m_viewAxis); + if (!DU::isGuiUp()) { + onHlrFinished(); + } } bool DrawViewDetail::waitingForResult() const diff --git a/src/Mod/TechDraw/App/DrawViewPart.cpp b/src/Mod/TechDraw/App/DrawViewPart.cpp index 1dfc2af7ed..f6fbb1d889 100644 --- a/src/Mod/TechDraw/App/DrawViewPart.cpp +++ b/src/Mod/TechDraw/App/DrawViewPart.cpp @@ -284,7 +284,6 @@ void DrawViewPart::onChanged(const App::Property* prop) void DrawViewPart::partExec(TopoDS_Shape& shape) { - // Base::Console().Message("DVP::partExec() - %s\n", getNameInDocument()); if (waitingForHlr()) { //finish what we are already doing before starting a new cycle return; @@ -292,8 +291,9 @@ void DrawViewPart::partExec(TopoDS_Shape& shape) //we need to keep using the old geometryObject until the new one is fully populated m_tempGeometryObject = makeGeometryForShape(shape); - if (CoarseView.getValue()) { - onHlrFinished();//poly algo does not run in separate thread, so we need to invoke + if (CoarseView.getValue() || + !DU::isGuiUp()) { + onHlrFinished();//poly algo and console mode do not run in separate thread, so we need to invoke //the post hlr processing manually } } @@ -301,8 +301,6 @@ void DrawViewPart::partExec(TopoDS_Shape& shape) //! prepare the shape for HLR processing by centering, scaling and rotating it GeometryObjectPtr DrawViewPart::makeGeometryForShape(TopoDS_Shape& shape) { -// Base::Console().Message("DVP::makeGeometryForShape() - %s\n", getNameInDocument()); - // if we use the passed reference directly, the centering doesn't work. Maybe the underlying OCC TShape // isn't modified? using a copy works and the referenced shape (from getSourceShape in execute()) // isn't used for anything anyway. @@ -322,7 +320,6 @@ GeometryObjectPtr DrawViewPart::makeGeometryForShape(TopoDS_Shape& shape) TopoDS_Shape DrawViewPart::centerScaleRotate(const DrawViewPart *dvp, TopoDS_Shape& inOutShape, Base::Vector3d centroid) { -// Base::Console().Message("DVP::centerScaleRotate() - %s\n", dvp->getNameInDocument()); gp_Ax2 viewAxis = dvp->getProjectionCS(); //center shape on origin @@ -341,9 +338,6 @@ TopoDS_Shape DrawViewPart::centerScaleRotate(const DrawViewPart *dvp, TopoDS_Sha TechDraw::GeometryObjectPtr DrawViewPart::buildGeometryObject(TopoDS_Shape& shape, const gp_Ax2& viewAxis) { -// Base::Console().Message("DVP::buildGeometryObject() - %s\n", getNameInDocument()); - showProgressMessage(getNameInDocument(), "is finding hidden lines"); - TechDraw::GeometryObjectPtr go( std::make_shared(getNameInDocument(), this)); go->setIsoCount(IsoCount.getValue()); @@ -356,31 +350,37 @@ TechDraw::GeometryObjectPtr DrawViewPart::buildGeometryObject(TopoDS_Shape& shap //the polygon approximation HLR process runs quickly, so doesn't need to be in a //separate thread go->projectShapeWithPolygonAlgo(shape, viewAxis); + return go; } - else { - //projectShape (the HLR process) runs in a separate thread since it can take a long time - //note that &m_hlrWatcher in the third parameter is not strictly required, but using the - //4 parameter signature instead of the 3 parameter signature prevents clazy warning: - //https://github.com/KDE/clazy/blob/1.11/docs/checks/README-connect-3arg-lambda.md - connectHlrWatcher = QObject::connect(&m_hlrWatcher, &QFutureWatcherBase::finished, - &m_hlrWatcher, [this] { this->onHlrFinished(); }); - // We create a lambda closure to hold a copy of go, shape and viewAxis. - // This is important because those variables might be local to the calling - // function and might get destructed before the parallel processing finishes. - auto lambda = [go, shape, viewAxis]{go->projectShape(shape, viewAxis);}; - m_hlrFuture = QtConcurrent::run(std::move(lambda)); - m_hlrWatcher.setFuture(m_hlrFuture); - waitingForHlr(true); + if (!DU::isGuiUp()) { + // if the Gui is not running (actual the event loop), we cannot use the separate thread, + // since we will never be notified of thread completion. + go->projectShape(shape, viewAxis); + return go; } + + //projectShape (the HLR process) runs in a separate thread since it can take a long time + //note that &m_hlrWatcher in the third parameter is not strictly required, but using the + //4 parameter signature instead of the 3 parameter signature prevents clazy warning: + //https://github.com/KDE/clazy/blob/1.11/docs/checks/README-connect-3arg-lambda.md + connectHlrWatcher = QObject::connect(&m_hlrWatcher, &QFutureWatcherBase::finished, + &m_hlrWatcher, [this] { this->onHlrFinished(); }); + + // We create a lambda closure to hold a copy of go, shape and viewAxis. + // This is important because those variables might be local to the calling + // function and might get destructed before the parallel processing finishes. + auto lambda = [go, shape, viewAxis]{go->projectShape(shape, viewAxis);}; + m_hlrFuture = QtConcurrent::run(std::move(lambda)); + m_hlrWatcher.setFuture(m_hlrFuture); + waitingForHlr(true); + return go; } //! continue processing after hlr thread completes void DrawViewPart::onHlrFinished() { - // Base::Console().Message("DVP::onHlrFinished() - %s\n", getNameInDocument()); - //now that the new GeometryObject is fully populated, we can replace the old one if (m_tempGeometryObject) { geometryObject = m_tempGeometryObject;//replace with new @@ -406,6 +406,13 @@ void DrawViewPart::onHlrFinished() //start face finding in a separate thread. We don't find faces when using the polygon //HLR method. + + if (handleFaces() && !DU::isGuiUp()) { + extractFaces(); + onFacesFinished(); + return; + } + if (handleFaces() && !CoarseView.getValue()) { try { //note that &m_faceWatcher in the third parameter is not strictly required, but using the @@ -432,7 +439,6 @@ void DrawViewPart::onHlrFinished() //! run any tasks that need to been done after geometry is available void DrawViewPart::postHlrTasks() { - // Base::Console().Message("DVP::postHlrTasks() - %s\n", getNameInDocument()); //add geometry that doesn't come from HLR addCosmeticVertexesToGeom(); addCosmeticEdgesToGeom(); @@ -469,7 +475,6 @@ void DrawViewPart::postHlrTasks() // Run any tasks that need to be done after faces are available void DrawViewPart::postFaceExtractionTasks() { - // Base::Console().Message("DVP::postFaceExtractionTasks() - %s\n", getNameInDocument()); // Some centerlines depend on faces so we could not add CL geometry before now addCenterLinesToGeom(); @@ -487,8 +492,6 @@ void DrawViewPart::postFaceExtractionTasks() //! make faces from the edge geometry void DrawViewPart::extractFaces() { - // Base::Console().Message("DVP::extractFaces() - %s waitingForHlr: %d waitingForFaces: %d\n", - // getNameInDocument(), waitingForHlr(), waitingForFaces()); if (!geometryObject) { //geometry is in flux, can not make faces right now return; diff --git a/src/Mod/TechDraw/App/DrawViewSection.cpp b/src/Mod/TechDraw/App/DrawViewSection.cpp index 615a350e91..a97d45b08b 100644 --- a/src/Mod/TechDraw/App/DrawViewSection.cpp +++ b/src/Mod/TechDraw/App/DrawViewSection.cpp @@ -446,8 +446,6 @@ bool DrawViewSection::isBaseValid() const void DrawViewSection::sectionExec(TopoDS_Shape& baseShape) { - // Base::Console().Message("DVS::sectionExec() - %s baseShape.IsNull: %d\n", Label.getValue(), baseShape.IsNull()); - if (waitingForHlr() || waitingForCut()) { return; } @@ -459,6 +457,14 @@ void DrawViewSection::sectionExec(TopoDS_Shape& baseShape) m_cuttingTool = makeCuttingTool(m_shapeSize); + if (!DU::isGuiUp()) { + // without the gui we will never be informed of completion of the separate thread + makeSectionCut(baseShape); + waitingForCut(false); + onSectionCutFinished(); + return; + } + try { // note that &m_cutWatcher in the third parameter is not strictly required, // but using the 4 parameter signature instead of the 3 parameter signature @@ -485,8 +491,6 @@ void DrawViewSection::sectionExec(TopoDS_Shape& baseShape) void DrawViewSection::makeSectionCut(const TopoDS_Shape& baseShape) { - // Base::Console().Message("DVS::makeSectionCut() - %s - baseShape.IsNull:%d\n", Label.getValue(), baseShape.IsNull()); - showProgressMessage(getNameInDocument(), "is making section cut"); // We need to copy the shape to not modify the BRepstructure @@ -550,14 +554,10 @@ void DrawViewSection::makeSectionCut(const TopoDS_Shape& baseShape) waitingForCut(false); } -//! position, scale and rotate shape for buildGeometryObject +//! position, scale and rotate shape for buildGeometryObject //! save the cut shape for further processing TopoDS_Shape DrawViewSection::prepareShape(const TopoDS_Shape& rawShape, double shapeSize) { - // Base::Console().Message("DVS::prepareShape - %s - rawShape.IsNull: %d - // shapeSize: %.3f\n", - // getNameInDocument(), rawShape.IsNull(), - // shapeSize); (void)shapeSize;// shapeSize is not used in this base class, but is // interesting for derived classes // build display geometry as in DVP, with minor mods @@ -596,8 +596,6 @@ TopoDS_Shape DrawViewSection::prepareShape(const TopoDS_Shape& rawShape, double TopoDS_Shape DrawViewSection::makeCuttingTool(double shapeSize) { - // Base::Console().Message("DVS::makeCuttingTool(%.3f) - %s\n", shapeSize, - // getNameInDocument()); // Make the extrusion face gp_Pln pln = getSectionPlane(); gp_Dir gpNormal = pln.Axis().Direction(); @@ -615,11 +613,10 @@ TopoDS_Shape DrawViewSection::makeCuttingTool(double shapeSize) void DrawViewSection::onSectionCutFinished() { - // Base::Console().Message("DVS::onSectionCutFinished() - %s\n", - // getNameInDocument()); - QObject::disconnect(connectCutWatcher); - - showProgressMessage(getNameInDocument(), "has finished making section cut"); + if (DU::isGuiUp()) { + QObject::disconnect(connectCutWatcher); + showProgressMessage(getNameInDocument(), "has finished making section cut"); + } m_preparedShape = prepareShape(getShapeToPrepare(), m_shapeSize); if (debugSection()) { @@ -630,13 +627,14 @@ void DrawViewSection::onSectionCutFinished() // display geometry for cut shape is in geometryObject as in DVP m_tempGeometryObject = buildGeometryObject(m_preparedShape, getProjectionCS()); + if (!DU::isGuiUp()) { + onHlrFinished(); + } } // activities that depend on updated geometry object void DrawViewSection::postHlrTasks(void) { - // Base::Console().Message("DVS::postHlrTasks() - %s\n", Label.getValue()); - DrawViewPart::postHlrTasks(); // second pass if required @@ -683,7 +681,6 @@ void DrawViewSection::postHlrTasks(void) // activities that depend on a valid section cut void DrawViewSection::postSectionCutTasks() { - // Base::Console().Message("DVS::postSectionCutTasks()\n"); std::vector children = getInList(); for (auto& c : children) { if (c->isDerivedFrom()) { @@ -717,8 +714,6 @@ gp_Pln DrawViewSection::getSectionPlane() const //! section plane. TopoDS_Compound DrawViewSection::findSectionPlaneIntersections(const TopoDS_Shape& shape) { - // Base::Console().Message("DVS::findSectionPlaneIntersections() - %s\n", - // getNameInDocument()); if (shape.IsNull()) { // this shouldn't happen Base::Console().Warning( From 1bbeb21eec95dbce5fe1e5e80296fc425adc4ac9 Mon Sep 17 00:00:00 2001 From: wandererfan Date: Fri, 21 Mar 2025 09:48:55 -0400 Subject: [PATCH 4/4] [TD]apply review comments --- src/Mod/TechDraw/App/AppTechDrawPy.cpp | 3 --- src/Mod/TechDraw/App/DrawUtil.cpp | 5 ++--- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/src/Mod/TechDraw/App/AppTechDrawPy.cpp b/src/Mod/TechDraw/App/AppTechDrawPy.cpp index c2d574f4b5..15529bba1d 100644 --- a/src/Mod/TechDraw/App/AppTechDrawPy.cpp +++ b/src/Mod/TechDraw/App/AppTechDrawPy.cpp @@ -44,8 +44,6 @@ #include #include -#include - #include #include #include @@ -74,7 +72,6 @@ #include "GeometryObject.h" #include "ProjectionAlgos.h" #include "TechDrawExport.h" -#include "CosmeticVertexPy.h" #include "DrawLeaderLinePy.h" namespace TechDraw { diff --git a/src/Mod/TechDraw/App/DrawUtil.cpp b/src/Mod/TechDraw/App/DrawUtil.cpp index 60a7bd92d2..09d98c7b12 100644 --- a/src/Mod/TechDraw/App/DrawUtil.cpp +++ b/src/Mod/TechDraw/App/DrawUtil.cpp @@ -1890,9 +1890,8 @@ std::string DrawUtil::cleanFilespecBackslash(const std::string& filespec) //! returns true if the Gui module and its event loop are active. bool DrawUtil::isGuiUp() { - std::string pyCommand{"import FreeCAD\nguiState = FreeCAD.GuiUp"}; - std::string result{Base::Interpreter().runStringWithKey(pyCommand.c_str(), "guiState", "None")}; - return (result == "1"); + auto* app = QCoreApplication::instance(); + return (app != nullptr) && app->inherits("QApplication"); }