From b542d0e1ffa51db97cf373276bf7c085bde586f2 Mon Sep 17 00:00:00 2001 From: Paul Leroy Date: Fri, 16 Feb 2024 08:56:15 +0100 Subject: [PATCH] the use of progressDlg.close instead of progressDlg.hide led to an instability code cleaning --- ContextBasedFeature.cpp | 4 ++-- ContextBasedFeature.h | 4 ++-- FeaturesInterface.cpp | 4 ++-- FeaturesInterface.h | 5 +++++ NeighborhoodFeature.cpp | 1 - NeighborhoodFeature.h | 8 ++------ PointFeature.cpp | 14 +++++++------- PointFeature.h | 8 ++------ ScalarFieldCollector.cpp | 6 +++--- q3DMASC.cpp | 7 ++++--- q3DMASCClassifier.cpp | 3 ++- q3DMASCTools.cpp | 17 ++++++++--------- 12 files changed, 39 insertions(+), 42 deletions(-) diff --git a/ContextBasedFeature.cpp b/ContextBasedFeature.cpp index b34a8cf..b8b4625 100644 --- a/ContextBasedFeature.cpp +++ b/ContextBasedFeature.cpp @@ -123,7 +123,7 @@ bool ContextBasedFeature::prepare( const CorePoints& corePoints, //and the scalar field assert(!sf); - sfWasAlreadyExisting = CheckSFExistence(corePoints.cloud, qPrintable(resultSFName)); + sf1WasAlreadyExisting = CheckSFExistence(corePoints.cloud, qPrintable(resultSFName)); sf = PrepareSF(corePoints.cloud, qPrintable(resultSFName), generatedScalarFields, SFCollector::CAN_REMOVE); if (!sf) { @@ -133,7 +133,7 @@ bool ContextBasedFeature::prepare( const CorePoints& corePoints, source.name = sf->getName(); // NOT NECESSARY IF THE VALUE IS ALREADY COMPUTED - if (!scaled() && !sfWasAlreadyExisting) //with 'kNN' neighbors, we can compute the values right away + if (!scaled() && !sf1WasAlreadyExisting) //with 'kNN' neighbors, we can compute the values right away { unsigned pointCount = corePoints.size(); QString logMessage = "Computing " + typeStr diff --git a/ContextBasedFeature.h b/ContextBasedFeature.h index e6575fa..622dfad 100644 --- a/ContextBasedFeature.h +++ b/ContextBasedFeature.h @@ -75,7 +75,7 @@ namespace masc , kNN(p_kNN) , ctxClassLabel(p_ctxClassLabel) , sf(nullptr) - , sfWasAlreadyExisting(false) + // , sfWasAlreadyExisting(false) { scale = p_scale; } @@ -105,6 +105,6 @@ namespace masc //! The computed scalar CCCoreLib::ScalarField* sf; //! Whether the SF pre-exists - bool sfWasAlreadyExisting; + // bool sfWasAlreadyExisting; }; } diff --git a/FeaturesInterface.cpp b/FeaturesInterface.cpp index d718ab2..cf78fc3 100644 --- a/FeaturesInterface.cpp +++ b/FeaturesInterface.cpp @@ -50,12 +50,12 @@ CCCoreLib::ScalarField* Feature::PrepareSF(ccPointCloud* cloud, const char* resu int sfIdx = cloud->getScalarFieldIndexByName(resultSFName); if (sfIdx >= 0) { -// ccLog::Warning("Existing SF: " + QString(resultSFName) + ", do not store in generatedScalarFields"); + // ccLog::Warning("Existing SF: " + QString(resultSFName) + ", do not store in generatedScalarFields"); resultSF = cloud->getScalarField(sfIdx); } else { -// ccLog::Warning("SF does not exist, create it: " + QString(resultSFName) + ", SFCollector::Behavior " + QString::number(behavior)); + // ccLog::Warning("SF does not exist, create it: " + QString(resultSFName) + ", SFCollector::Behavior " + QString::number(behavior)); ccScalarField* newSF = new ccScalarField(resultSFName); if (!newSF->resizeSafe(cloud->size())) { diff --git a/FeaturesInterface.h b/FeaturesInterface.h index bce022f..6142f6f 100644 --- a/FeaturesInterface.h +++ b/FeaturesInterface.h @@ -162,6 +162,8 @@ namespace masc , source(p_source, p_sourceName) , stat(NO_STAT) , op(NO_OPERATION) + , sf1WasAlreadyExisting(false) + , sf2WasAlreadyExisting(false) {} //! Returns the type (must be reimplemented by child struct) @@ -236,5 +238,8 @@ namespace masc Stat stat; //only considered if a scale is defined Operation op; //only considered if 2 clouds are defined + + bool sf1WasAlreadyExisting; + bool sf2WasAlreadyExisting; // only considered if a second scalar field may be necessary (PointFeature, NeighborhoodFeature) }; } diff --git a/NeighborhoodFeature.cpp b/NeighborhoodFeature.cpp index 33321e9..ede9df4 100644 --- a/NeighborhoodFeature.cpp +++ b/NeighborhoodFeature.cpp @@ -111,7 +111,6 @@ bool NeighborhoodFeature::prepare( const CorePoints& corePoints, if (cloud2 && op != Feature::NO_OPERATION && !sf1WasAlreadyExisting) { QString resultSFName2 = ToString(type) + "_" + cloud2Label + "@" + QString::number(scale); - keepSF2 = (corePoints.cloud->getScalarFieldIndexByName(qPrintable(resultSFName2)) >= 0); //we remember that the scalar field was already existing! assert(!sf2); diff --git a/NeighborhoodFeature.h b/NeighborhoodFeature.h index e088de2..8518f65 100644 --- a/NeighborhoodFeature.h +++ b/NeighborhoodFeature.h @@ -151,9 +151,8 @@ namespace masc : type(p_type) , sf1(nullptr) , sf2(nullptr) - , keepSF2(false) - , sf1WasAlreadyExisting(false) - , sf2WasAlreadyExisting(false) + // , sf1WasAlreadyExisting(false) + // , sf2WasAlreadyExisting(false) { } @@ -177,8 +176,5 @@ namespace masc //! Feature values CCCoreLib::ScalarField *sf1, *sf2; - bool keepSF2; - bool sf1WasAlreadyExisting; - bool sf2WasAlreadyExisting; }; } diff --git a/PointFeature.cpp b/PointFeature.cpp index f9c1e39..7851a02 100644 --- a/PointFeature.cpp +++ b/PointFeature.cpp @@ -422,9 +422,9 @@ static bool ComputeMathOpWithNearestNeighbor( const CorePoints& corePoints, double meanNeighborhoodSize = 0; int tenth = pointCount / 10; error.clear(); + bool cancelled = false; #ifndef _DEBUG #if defined(_OPENMP) - bool cancelled = false; #pragma omp parallel for #endif #endif @@ -584,8 +584,8 @@ bool PointFeature::prepare( const CorePoints& corePoints, resultSF1Name += "@" + QString::number(scale); //prepare the corresponding scalar field - statSF1WasAlreadyExisting = CheckSFExistence(corePoints.cloud, qPrintable(resultSF1Name)); - if (statSF1WasAlreadyExisting) + sf1WasAlreadyExisting = CheckSFExistence(corePoints.cloud, qPrintable(resultSF1Name)); + if (sf1WasAlreadyExisting) { // if the SF exists, it is not added to generatedScalarFields statSF1 = PrepareSF(corePoints.cloud, qPrintable(resultSF1Name), generatedScalarFields, SFCollector::ALWAYS_KEEP); @@ -601,14 +601,14 @@ bool PointFeature::prepare( const CorePoints& corePoints, } source.name = statSF1->getName(); - if (field2 && op != Feature::NO_OPERATION && !statSF1WasAlreadyExisting) // nothing to do if statSF1 was already there + if (field2 && op != Feature::NO_OPERATION && !sf1WasAlreadyExisting) // nothing to do if statSF1 was already there { QString resultSF2Name = field2->getName() + QString("_") + cloud2Label + "_" + Feature::StatToString(stat) + "@" + QString::number(scale); //keepStatSF2 = (corePoints.cloud->getScalarFieldIndexByName(qPrintable(resultSFName2)) >= 0); //we remember that the scalar field was already existing! assert(!statSF2); - statSF2WasAlreadyExisting = CheckSFExistence(corePoints.cloud, qPrintable(resultSF2Name)); - if (statSF2WasAlreadyExisting) + sf2WasAlreadyExisting = CheckSFExistence(corePoints.cloud, qPrintable(resultSF2Name)); + if (sf2WasAlreadyExisting) statSF2 = PrepareSF(corePoints.cloud, qPrintable(resultSF2Name), generatedScalarFields, SFCollector::ALWAYS_KEEP); else statSF2 = PrepareSF(corePoints.cloud, qPrintable(resultSF2Name), generatedScalarFields, SFCollector::ALWAYS_REMOVE); @@ -863,7 +863,7 @@ bool PointFeature::finish(const CorePoints& corePoints, QString& error) } } - if (statSF2 && !statSF1WasAlreadyExisting) + if (statSF2 && !sf1WasAlreadyExisting) { //now perform the math operation if (op != Feature::NO_OPERATION) diff --git a/PointFeature.h b/PointFeature.h index da75c7f..f8cc20b 100644 --- a/PointFeature.h +++ b/PointFeature.h @@ -148,8 +148,8 @@ namespace masc , field2(nullptr) , statSF1(nullptr) , statSF2(nullptr) - , statSF1WasAlreadyExisting(false) - , statSF2WasAlreadyExisting(false) + // , statSF1WasAlreadyExisting(false) + // , statSF2WasAlreadyExisting(false) //, keepStatSF2(false) { //auomatically set the right source for specific features @@ -213,9 +213,5 @@ namespace masc //! For scaled features CCCoreLib::ScalarField *statSF1, *statSF2; - - //bool keepStatSF2; - bool statSF1WasAlreadyExisting; - bool statSF2WasAlreadyExisting; }; } diff --git a/ScalarFieldCollector.cpp b/ScalarFieldCollector.cpp index 5b4b765..b3816f5 100644 --- a/ScalarFieldCollector.cpp +++ b/ScalarFieldCollector.cpp @@ -48,7 +48,7 @@ void SFCollector::releaseSFs(bool keepByDefault) if (desc.behavior == ALWAYS_KEEP || (keepByDefault && desc.behavior == CAN_REMOVE)) { -// ccLog::Warning(QString("[SFCollector] Keep scalar field '%1'").arg(sf->getName())); + // ccLog::Warning(QString("[SFCollector] Keep scalar field '%1' on cloud '%2'").arg(sf->getName()).arg(desc.cloud->getName())); //keep this SF continue; } @@ -56,12 +56,12 @@ void SFCollector::releaseSFs(bool keepByDefault) int sfIdx = desc.cloud->getScalarFieldIndexByName(sf->getName()); if (sfIdx >= 0) { -// ccLog::Warning(QString("[SFCollector] Remove scalar field '%1'").arg(sf->getName())); + // ccLog::Warning(QString("[SFCollector] Remove scalar field '%1' from '%2'").arg(sf->getName()).arg(desc.cloud->getName())); desc.cloud->deleteScalarField(sfIdx); } else { - ccLog::Warning(QString("[SFCollector] Scalar field '%1' can't be found anymore, impossible to remove it").arg(sf->getName())); + // ccLog::Warning(QString("[SFCollector] Scalar field '%1' can't be found anymore on cloud '%2', impossible to remove it").arg(sf->getName()).arg(desc.cloud->getName())); } } diff --git a/q3DMASC.cpp b/q3DMASC.cpp index 9a909de..bf91893 100644 --- a/q3DMASC.cpp +++ b/q3DMASC.cpp @@ -189,7 +189,7 @@ void q3DMASCPlugin::doClassifyAction() generatedScalarFields.releaseSFs(false); return; } - progressDlg.close(); + progressDlg.hide(); QCoreApplication::processEvents(); //apply classifier @@ -425,6 +425,7 @@ void q3DMASCPlugin::doTrainAction() //compute the core points (if necessary) ccProgressDialog progressDlg(true, m_app->getMainWindow()); progressDlg.setAutoClose(false); + ccLog::Error("Qt::WA_DeleteOnClose " + QString::number(progressDlg.testAttribute(Qt::WA_DeleteOnClose))); if (!corePoints.prepare(&progressDlg)) { m_app->dispToConsole("Failed to compute/prepare the core points!", ccMainAppInterface::ERR_CONSOLE_MESSAGE); @@ -517,7 +518,7 @@ void q3DMASCPlugin::doTrainAction() generatedScalarFieldsTest.releaseSFs(false); return; } - progressDlg.close(); + progressDlg.hide(); QCoreApplication::processEvents(); m_app->redrawAll(); @@ -635,7 +636,7 @@ void q3DMASCPlugin::doTrainAction() generatedScalarFieldsTest.releaseSFs(false); return; } - progressDlg.close(); + progressDlg.hide(); QCoreApplication::processEvents(); m_app->redrawAll(); diff --git a/q3DMASCClassifier.cpp b/q3DMASCClassifier.cpp index 91c9eab..3fe0041 100644 --- a/q3DMASCClassifier.cpp +++ b/q3DMASCClassifier.cpp @@ -207,9 +207,10 @@ bool Classifier::classify( const Feature::Source::Set& featureSources, bool success = true; int numberOfTrees = static_cast(m_rtrees->getRoots().size()); + bool cancelled = false; + #ifndef _DEBUG #if defined(_OPENMP) - bool cancelled = false; #pragma omp parallel for #endif #endif diff --git a/q3DMASCTools.cpp b/q3DMASCTools.cpp index 2ad1c0c..e38f1fa 100644 --- a/q3DMASCTools.cpp +++ b/q3DMASCTools.cpp @@ -43,7 +43,6 @@ //system #include -#include #if defined(_OPENMP) #include @@ -1085,7 +1084,7 @@ bool Tools::PrepareFeatures(const CorePoints& corePoints, Feature::Set& features { //build the scaled feature list attached to the first cloud if (feature->cloud1 - && !static_cast(feature.data())->statSF1WasAlreadyExisting) // nothing to compute if the scalar field was already there + && !feature->sf1WasAlreadyExisting) // nothing to compute if the scalar field was already there { FeaturesAndScales& fas = cloudsWithScaledFeatures[feature->cloud1]; fas.pointFeaturesPerScale[feature->scale].push_back(qSharedPointerCast(feature)); @@ -1100,9 +1099,9 @@ bool Tools::PrepareFeatures(const CorePoints& corePoints, Feature::Set& features && feature->cloud2 != feature->cloud1 && feature->op != Feature::NO_OPERATION) { - if(!static_cast(feature.data())->statSF1WasAlreadyExisting) // nothing to compute if the scalar field was already there + if(!feature->sf1WasAlreadyExisting) // nothing to compute if the scalar field was already there { - if (!static_cast(feature.data())->statSF2WasAlreadyExisting) + if (!feature->sf2WasAlreadyExisting) { FeaturesAndScales& fas = cloudsWithScaledFeatures[feature->cloud2]; ++fas.featureCount; @@ -1122,7 +1121,7 @@ bool Tools::PrepareFeatures(const CorePoints& corePoints, Feature::Set& features { //build the scaled feature list attached to the first cloud if (feature->cloud1 - && !static_cast(feature.data())->sf1WasAlreadyExisting) // nothing to compute if the scalar field was already there + && !feature->sf1WasAlreadyExisting) // nothing to compute if the scalar field was already there { FeaturesAndScales& fas = cloudsWithScaledFeatures[feature->cloud1]; fas.neighborhoodFeaturesPerScale[feature->scale].push_back(qSharedPointerCast(feature)); @@ -1138,9 +1137,9 @@ bool Tools::PrepareFeatures(const CorePoints& corePoints, Feature::Set& features && feature->cloud2 != feature->cloud1 && feature->op != Feature::NO_OPERATION) { - if (!static_cast(feature.data())->sf1WasAlreadyExisting) // nothing to compute if the scalar field was already there + if (!feature->sf1WasAlreadyExisting) // nothing to compute if the scalar field was already there { - if (!static_cast(feature.data())->sf2WasAlreadyExisting) + if (!feature->sf2WasAlreadyExisting) { FeaturesAndScales& fas = cloudsWithScaledFeatures[feature->cloud2]; fas.neighborhoodFeaturesPerScale[feature->scale].push_back(qSharedPointerCast(feature)); @@ -1160,7 +1159,7 @@ bool Tools::PrepareFeatures(const CorePoints& corePoints, Feature::Set& features { //build the scaled feature list attached to the context cloud if (feature->cloud1 - && !static_cast(feature.data())->sfWasAlreadyExisting) // nothing to compute if the scalar field was already there + && !feature->sf1WasAlreadyExisting) // nothing to compute if the scalar field was already there { FeaturesAndScales& fas = cloudsWithScaledFeatures[feature->cloud1]; fas.contextBasedFeaturesPerScale[feature->scale].push_back(qSharedPointerCast(feature)); @@ -1233,10 +1232,10 @@ bool Tools::PrepareFeatures(const CorePoints& corePoints, Feature::Set& features CCCoreLib::NormalizedProgress nProgress(progressCb, pointCount); QMutex mutex; + bool cancelled = false; #ifndef _DEBUG #if defined(_OPENMP) - bool cancelled = false; #pragma omp parallel for #endif #endif