From c8106982de865075e44dd9a7e8a58713755ae4f0 Mon Sep 17 00:00:00 2001 From: PaddleStroke Date: Sun, 4 Jan 2026 21:57:13 +0100 Subject: [PATCH] Sketcher: Use DenseQR for small sketches to avoid SparseQR rank issues (#26559) --- src/Mod/Sketcher/App/Sketch.h | 8 ++ src/Mod/Sketcher/App/planegcs/GCS.cpp | 11 ++ src/Mod/Sketcher/App/planegcs/GCS.h | 2 + .../Gui/TaskSketcherSolverAdvanced.cpp | 102 +++++++++++++----- .../Sketcher/Gui/TaskSketcherSolverAdvanced.h | 2 + .../Gui/TaskSketcherSolverAdvanced.ui | 81 ++++++++++++++ 6 files changed, 182 insertions(+), 24 deletions(-) diff --git a/src/Mod/Sketcher/App/Sketch.h b/src/Mod/Sketcher/App/Sketch.h index fa847a658b..7d659e548a 100644 --- a/src/Mod/Sketcher/App/Sketch.h +++ b/src/Mod/Sketcher/App/Sketch.h @@ -641,6 +641,14 @@ public: { return debugMode; } + inline void setAutoQRThreshold(int val) + { + GCSsys.autoQRThreshold = val; + } + inline void setSketchAutoAlgo(bool val) + { + GCSsys.autoChooseAlgorithm = val; + } inline void setMaxIter(int maxiter) { GCSsys.maxIter = maxiter; diff --git a/src/Mod/Sketcher/App/planegcs/GCS.cpp b/src/Mod/Sketcher/App/planegcs/GCS.cpp index b0ef5a7711..eb9ebe668b 100644 --- a/src/Mod/Sketcher/App/planegcs/GCS.cpp +++ b/src/Mod/Sketcher/App/planegcs/GCS.cpp @@ -486,6 +486,8 @@ System::System() , convergence(1e-10) , convergenceRedundant(1e-10) , qrAlgorithm(EigenSparseQR) + , autoChooseAlgorithm(true) + , autoQRThreshold(1000) , dogLegGaussStep(FullPivLU) , qrpivotThreshold(1E-13) , debugMode(Minimal) @@ -4808,6 +4810,15 @@ int System::diagnose(Algorithm alg) hasDiagnosis = true; dofs = pdiagnoselist.size(); + // Use DenseQR for small to medium systems to avoid SparseQR rank issues. + // SparseQR is known to fail rank detection on specific geometric structures (e.g. aligned slots). + // 200 parameters roughly corresponds to ~100 points/curves, covering most complex sketches + // where stability is preferred over pure O(N) performance. + // See: https://github.com/FreeCAD/FreeCAD/issues/10903 + if (autoChooseAlgorithm) { + qrAlgorithm = dofs < autoQRThreshold ? EigenDenseQR : EigenSparseQR; + } + // There is a legacy decision to use QR decomposition. I (abdullah) do not know all the // consideration taken in that decisions. I see that: // - QR decomposition is able to provide information about the rank and diff --git a/src/Mod/Sketcher/App/planegcs/GCS.h b/src/Mod/Sketcher/App/planegcs/GCS.h index eae8a1a31a..6ce451f212 100644 --- a/src/Mod/Sketcher/App/planegcs/GCS.h +++ b/src/Mod/Sketcher/App/planegcs/GCS.h @@ -239,6 +239,8 @@ public: double convergence; double convergenceRedundant; QRAlgorithm qrAlgorithm; + bool autoChooseAlgorithm; + int autoQRThreshold; DogLegGaussStep dogLegGaussStep; double qrpivotThreshold; DebugMode debugMode; diff --git a/src/Mod/Sketcher/Gui/TaskSketcherSolverAdvanced.cpp b/src/Mod/Sketcher/Gui/TaskSketcherSolverAdvanced.cpp index 755b1f0f69..c9be668b4c 100644 --- a/src/Mod/Sketcher/Gui/TaskSketcherSolverAdvanced.cpp +++ b/src/Mod/Sketcher/Gui/TaskSketcherSolverAdvanced.cpp @@ -71,6 +71,8 @@ TaskSketcherSolverAdvanced::TaskSketcherSolverAdvanced(ViewProviderSketch* sketc ui->checkBoxSketchSizeMultiplier->onRestore(); ui->lineEditConvergence->onRestore(); ui->comboBoxQRMethod->onRestore(); + ui->spinBoxAutoQRThreshold->onRestore(); + ui->checkBoxAutoChooseAlgo->onRestore(); ui->lineEditQRPivotThreshold->onRestore(); ui->comboBoxRedundantDefaultSolver->onRestore(); ui->spinBoxRedundantSolverMaxIterations->onRestore(); @@ -78,6 +80,12 @@ TaskSketcherSolverAdvanced::TaskSketcherSolverAdvanced(ViewProviderSketch* sketc ui->lineEditRedundantConvergence->onRestore(); ui->comboBoxDebugMode->onRestore(); + bool autoAlgo = ui->checkBoxAutoChooseAlgo->isChecked(); + ui->labelAutoQRThreshold->setVisible(autoAlgo); + ui->spinBoxAutoQRThreshold->setVisible(autoAlgo); + ui->labelQRAlgorithm->setVisible(!autoAlgo); + ui->comboBoxQRMethod->setVisible(!autoAlgo); + updateSketchObject(); } @@ -104,6 +112,27 @@ void TaskSketcherSolverAdvanced::setupConnections() this, &TaskSketcherSolverAdvanced::onSpinBoxMaxIterValueChanged ); + connect( + ui->spinBoxAutoQRThreshold, + qOverload(&QSpinBox::valueChanged), + this, + &TaskSketcherSolverAdvanced::onSpinBoxAutoQRAlgoChanged + ); +#if QT_VERSION >= QT_VERSION_CHECK(6, 7, 0) + connect( + ui->checkBoxAutoChooseAlgo, + &QCheckBox::checkStateChanged, + this, + &TaskSketcherSolverAdvanced::onCheckBoxAutoQRAlgoStateChanged + ); +#else + connect( + ui->checkBoxAutoChooseAlgo, + &QCheckBox::stateChanged, + this, + &TaskSketcherSolverAdvanced::onCheckBoxAutoQRAlgoStateChanged + ); +#endif #if QT_VERSION >= QT_VERSION_CHECK(6, 7, 0) connect( ui->checkBoxSketchSizeMultiplier, @@ -637,6 +666,35 @@ void TaskSketcherSolverAdvanced::onSpinBoxMaxIterValueChanged(int i) const_cast(sketchView->getSketchObject()->getSolvedSketch()).setMaxIter(i); } +void TaskSketcherSolverAdvanced::onSpinBoxAutoQRAlgoChanged(int i) +{ + ui->spinBoxAutoQRThreshold->onSave(); + const_cast(sketchView->getSketchObject()->getSolvedSketch()) + .setAutoQRThreshold(i); +} + +void TaskSketcherSolverAdvanced::onCheckBoxAutoQRAlgoStateChanged(int state) +{ + if (state == Qt::Checked) { + ui->spinBoxAutoQRThreshold->show(); + ui->comboBoxQRMethod->hide(); + ui->labelAutoQRThreshold->show(); + ui->labelQRAlgorithm->hide(); + ui->checkBoxAutoChooseAlgo->onSave(); + const_cast(sketchView->getSketchObject()->getSolvedSketch()) + .setSketchAutoAlgo(true); + } + else if (state == Qt::Unchecked) { + ui->spinBoxAutoQRThreshold->hide(); + ui->comboBoxQRMethod->show(); + ui->labelAutoQRThreshold->hide(); + ui->labelQRAlgorithm->show(); + ui->checkBoxAutoChooseAlgo->onSave(); + const_cast(sketchView->getSketchObject()->getSolvedSketch()) + .setSketchAutoAlgo(false); + } +} + void TaskSketcherSolverAdvanced::onCheckBoxSketchSizeMultiplierStateChanged(int state) { if (state == Qt::Checked) { @@ -784,6 +842,8 @@ void TaskSketcherSolverAdvanced::onPushButtonDefaultsClicked(bool checked /* = f ui->checkBoxSketchSizeMultiplier->onRestore(); ui->lineEditConvergence->onRestore(); ui->comboBoxQRMethod->onRestore(); + ui->spinBoxAutoQRThreshold->onRestore(); + ui->checkBoxAutoChooseAlgo->onRestore(); ui->lineEditQRPivotThreshold->onRestore(); ui->comboBoxRedundantDefaultSolver->onRestore(); ui->spinBoxRedundantSolverMaxIterations->onRestore(); @@ -796,30 +856,24 @@ void TaskSketcherSolverAdvanced::onPushButtonDefaultsClicked(bool checked /* = f void TaskSketcherSolverAdvanced::updateSketchObject() { - const_cast(sketchView->getSketchObject()->getSolvedSketch()) - .setDebugMode((GCS::DebugMode)ui->comboBoxDebugMode->currentIndex()); - const_cast(sketchView->getSketchObject()->getSolvedSketch()) - .setSketchSizeMultiplierRedundant(ui->checkBoxRedundantSketchSizeMultiplier->isChecked()); - const_cast(sketchView->getSketchObject()->getSolvedSketch()) - .setMaxIterRedundant(ui->spinBoxRedundantSolverMaxIterations->value()); - const_cast(sketchView->getSketchObject()->getSolvedSketch()).defaultSolverRedundant - = static_cast(ui->comboBoxRedundantDefaultSolver->currentIndex()); - const_cast(sketchView->getSketchObject()->getSolvedSketch()) - .setQRAlgorithm((GCS::QRAlgorithm)ui->comboBoxQRMethod->currentIndex()); - const_cast(sketchView->getSketchObject()->getSolvedSketch()) - .setQRPivotThreshold(ui->lineEditQRPivotThreshold->text().toDouble()); - const_cast(sketchView->getSketchObject()->getSolvedSketch()) - .setConvergenceRedundant(ui->lineEditRedundantConvergence->text().toDouble()); - const_cast(sketchView->getSketchObject()->getSolvedSketch()) - .setConvergence(ui->lineEditConvergence->text().toDouble()); - const_cast(sketchView->getSketchObject()->getSolvedSketch()) - .setSketchSizeMultiplier(ui->checkBoxSketchSizeMultiplier->isChecked()); - const_cast(sketchView->getSketchObject()->getSolvedSketch()) - .setMaxIter(ui->spinBoxMaxIter->value()); - const_cast(sketchView->getSketchObject()->getSolvedSketch()).defaultSolver - = static_cast(ui->comboBoxDefaultSolver->currentIndex()); - const_cast(sketchView->getSketchObject()->getSolvedSketch()) - .setDogLegGaussStep((GCS::DogLegGaussStep)ui->comboBoxDogLegGaussStep->currentIndex()); + auto& sketch = const_cast(sketchView->getSketchObject()->getSolvedSketch()); + + sketch.setDebugMode((GCS::DebugMode)ui->comboBoxDebugMode->currentIndex()); + sketch.setSketchSizeMultiplierRedundant(ui->checkBoxRedundantSketchSizeMultiplier->isChecked()); + sketch.setMaxIterRedundant(ui->spinBoxRedundantSolverMaxIterations->value()); + sketch.defaultSolverRedundant = static_cast( + ui->comboBoxRedundantDefaultSolver->currentIndex() + ); + sketch.setQRAlgorithm((GCS::QRAlgorithm)ui->comboBoxQRMethod->currentIndex()); + sketch.setAutoQRThreshold(ui->spinBoxAutoQRThreshold->value()); + sketch.setSketchAutoAlgo(ui->checkBoxAutoChooseAlgo->isChecked()); + sketch.setQRPivotThreshold(ui->lineEditQRPivotThreshold->text().toDouble()); + sketch.setConvergenceRedundant(ui->lineEditRedundantConvergence->text().toDouble()); + sketch.setConvergence(ui->lineEditConvergence->text().toDouble()); + sketch.setSketchSizeMultiplier(ui->checkBoxSketchSizeMultiplier->isChecked()); + sketch.setMaxIter(ui->spinBoxMaxIter->value()); + sketch.defaultSolver = static_cast(ui->comboBoxDefaultSolver->currentIndex()); + sketch.setDogLegGaussStep((GCS::DogLegGaussStep)ui->comboBoxDogLegGaussStep->currentIndex()); updateDefaultMethodParameters(); updateRedundantMethodParameters(); diff --git a/src/Mod/Sketcher/Gui/TaskSketcherSolverAdvanced.h b/src/Mod/Sketcher/Gui/TaskSketcherSolverAdvanced.h index b8bd326608..08c90b80b5 100644 --- a/src/Mod/Sketcher/Gui/TaskSketcherSolverAdvanced.h +++ b/src/Mod/Sketcher/Gui/TaskSketcherSolverAdvanced.h @@ -53,6 +53,8 @@ private: void onComboBoxDefaultSolverCurrentIndexChanged(int index); void onComboBoxDogLegGaussStepCurrentIndexChanged(int index); void onSpinBoxMaxIterValueChanged(int i); + void onSpinBoxAutoQRAlgoChanged(int i); + void onCheckBoxAutoQRAlgoStateChanged(int state); void onCheckBoxSketchSizeMultiplierStateChanged(int state); void onLineEditConvergenceEditingFinished(); void onComboBoxQRMethodCurrentIndexChanged(int index); diff --git a/src/Mod/Sketcher/Gui/TaskSketcherSolverAdvanced.ui b/src/Mod/Sketcher/Gui/TaskSketcherSolverAdvanced.ui index b69fa653bb..c0a6fc4fe6 100644 --- a/src/Mod/Sketcher/Gui/TaskSketcherSolverAdvanced.ui +++ b/src/Mod/Sketcher/Gui/TaskSketcherSolverAdvanced.ui @@ -290,6 +290,87 @@ to determine whether a solution converges or not + + + + + + Automatically select the QR algorithm based on number of dofs + + + Automatic QR algorithm + + + + + + + + 0 + 0 + + + + true + + + Automatically select the QR algorithm based on number of dofs + + + Qt::RightToLeft + + + + + + AutoChooseAlgo + + + Mod/Sketcher/SolverAdvanced + + + + + + + + + + + Maximum number of parameters before switching to sparse QR algorithm + + + Auto QR threshold + + + + + + + Maximum number of parameters before switching to sparse QR algorithm + + + Qt::AlignRight|Qt::AlignTrailing|Qt::AlignVCenter + + + 0 + + + 10000000 + + + 1000 + + + AutoQRThreshold + + + Mod/Sketcher/SolverAdvanced + + + + +