VV/BUG: Fill Bad Data Complete VV and minor patches

src/Plugins/SimplnxCore/src/SimplnxCore/Filters/Algorithms/FillBadData.cpp

@@ -52,7 +52,7 @@ namespace
 // @param outputDataStore The data array to update
 // @param neighbors The neighbor assignments (index of the neighbor to copy from)
 template <typename T>
-void FillBadDataUpdateTuples(const Int32AbstractDataStore& featureIds, AbstractDataStore<T>& outputDataStore, const std::vector<int32>& neighbors)
+void FillBadDataUpdateTuples(const Int32AbstractDataStore& featureIds, AbstractDataStore<T>& outputDataStore, const std::vector<int64>& neighbors)
 {
   usize start = 0;
   usize stop = outputDataStore.getNumberOfTuples();
@@ -62,13 +62,7 @@ void FillBadDataUpdateTuples(const Int32AbstractDataStore& featureIds, AbstractD
   for(usize tupleIndex = start; tupleIndex < stop; tupleIndex++)
   {
     const int32 featureName = featureIds[tupleIndex];
-    const int32 neighbor = neighbors[tupleIndex];
-
-    // Skip if no neighbor assignment
-    if(neighbor == tupleIndex)
-    {
-      continue;
-    }
+    const int64 neighbor = neighbors[tupleIndex];
 
     // Copy data from the valid neighbor to bad data voxel
     // Only copy if the current voxel is bad data (-1) and the neighbor is valid (>0)
@@ -91,7 +85,7 @@ void FillBadDataUpdateTuples(const Int32AbstractDataStore& featureIds, AbstractD
 struct FillBadDataUpdateTuplesFunctor
 {
   template <typename T>
-  void operator()(const Int32AbstractDataStore& featureIds, IDataArray* outputIDataArray, const std::vector<int32>& neighbors)
+  void operator()(const Int32AbstractDataStore& featureIds, IDataArray* outputIDataArray, const std::vector<int64>& neighbors)
   {
     auto& outputStore = outputIDataArray->template getIDataStoreRefAs<AbstractDataStore<T>>();
     FillBadDataUpdateTuples(featureIds, outputStore, neighbors);
@@ -339,13 +333,17 @@ void FillBadData::phaseOneCCL(Int32AbstractDataStore& featureIdsStore, ChunkAwar
           // Check already-processed neighbors (scanline order: -Z, -Y, -X)
           // We only check "backward" neighbors because "forward" neighbors
           // haven't been processed yet in the scanline order
+          //
+          // Note the following checks do not need to validate the
+          // featureIdsStore is zero because that is a prerequisite to
+          // insertion of an index value into provisionalLabels
           std::vector<int64> neighborLabels;
 
           // Check -X neighbor
           if(x > 0)
           {
             const usize neighborIdx = index - 1;
-            if(provisionalLabels.contains(neighborIdx) && featureIdsStore[neighborIdx] == 0)
+            if(provisionalLabels.contains(neighborIdx))
             {
               neighborLabels.push_back(provisionalLabels[neighborIdx]);
             }
@@ -355,7 +353,7 @@ void FillBadData::phaseOneCCL(Int32AbstractDataStore& featureIdsStore, ChunkAwar
           if(y > 0)
           {
             const usize neighborIdx = index - dims[0];
-            if(provisionalLabels.contains(neighborIdx) && featureIdsStore[neighborIdx] == 0)
+            if(provisionalLabels.contains(neighborIdx))
             {
               neighborLabels.push_back(provisionalLabels[neighborIdx]);
             }
@@ -365,7 +363,7 @@ void FillBadData::phaseOneCCL(Int32AbstractDataStore& featureIdsStore, ChunkAwar
           if(z > 0)
           {
             const usize neighborIdx = index - dims[0] * dims[1];
-            if(provisionalLabels.contains(neighborIdx) && featureIdsStore[neighborIdx] == 0)
+            if(provisionalLabels.contains(neighborIdx))
             {
               neighborLabels.push_back(provisionalLabels[neighborIdx]);
             }
@@ -473,7 +471,7 @@ void FillBadData::phaseThreeRelabeling(Int32AbstractDataStore& featureIdsStore,
   std::unordered_set<int64> localSmallRegions;
   for(const auto& [root, size] : rootSizes)
   {
-    if(static_cast<int32>(size) < m_InputValues->minAllowedDefectSizeValue)
+    if(size < static_cast<usize>(m_InputValues->minAllowedDefectSizeValue))
     {
       localSmallRegions.insert(root);
     }
@@ -559,7 +557,7 @@ void FillBadData::phaseFourIterativeFill(Int32AbstractDataStore& featureIdsStore
   constexpr std::array<FaceNeighborType, k_NumFaceNeighbors> faceNeighborInternalIdx = initializeFaceNeighborInternalIdx();
 
   // Neighbor assignment array: neighbors[i] = index of the neighbor to copy from
-  std::vector<int32> neighbors(totalPoints, -1);
+  std::vector<int64> neighbors(totalPoints, -1);
 
   // Feature vote counter: tracks how many times each feature appears as the neighbor
   std::vector<int32> featureNumber(numFeatures + 1, 0);
@@ -583,8 +581,18 @@ void FillBadData::phaseFourIterativeFill(Int32AbstractDataStore& featureIdsStore
   // Iteratively fill until no voxels with -1 value remain
   while(count != 0)
   {
+    if(m_ShouldCancel)
+    {
+      return;
+    }
+
     iteration++;
     count = 0; // Reset count of voxels with a -1 value for this iteration
+    // Tracks whether this iteration assigned at least one fill source. If bad voxels remain but none
+    // has a positive neighbor to copy from (e.g. an all-bad volume, or a bad pocket fully enclosed by
+    // other bad voxels), no assignment is ever made and count can never reach 0 — without this guard
+    // the loop would spin forever. When that happens we stop and leave the unfillable voxels as-is.
+    bool madeAssignment = false;
 
     // Pass 1: Determine neighbor assignments for all -1 voxels
     // For each -1 voxel, find the most common positive feature among neighbors
@@ -628,15 +636,15 @@ void FillBadData::phaseFourIterativeFill(Int32AbstractDataStore& featureIdsStore
             if(current > most)
             {
               most = current;
-              neighbors[voxelIndex] = static_cast<int32>(neighborPoint); // Store neighbor to copy from
+              neighbors[voxelIndex] = neighborPoint; // Store neighbor to copy from
+              madeAssignment = true;
             }
           }
         }
 
         // Reset vote counters for next voxel
         // Only reset features that were actually counted to save time
         // Loop over the 6 face neighbors of the voxel
-        isValidFaceNeighbor = computeValidFaceNeighbors(xIdx, yIdx, zIdx, dims);
         for(const auto& faceIndex : faceNeighborInternalIdx)
         {
           if(!isValidFaceNeighbor[faceIndex])
@@ -655,6 +663,15 @@ void FillBadData::phaseFourIterativeFill(Int32AbstractDataStore& featureIdsStore
       }
     }
 
+    // No fill source could be found for any remaining bad voxel: further iterations cannot make
+    // progress, so stop instead of looping forever. Remaining -1 voxels are left unchanged.
+    if(count != 0 && !madeAssignment)
+    {
+      m_MessageHandler(
+          {IFilter::Message::Type::Warning, fmt::format("  {} bad-data voxel(s) could not be filled: they have no adjacent good-data neighbor. Stopping after {} iteration(s).", count, iteration)});
+      break;
+    }
+
     // Pass 2: Update all cell data arrays based on neighbor assignments
     // This propagates all cell data attributes (not just feature IDs) to filled voxels
     for(const auto& cellArrayPath : voxelArrayNames)
@@ -728,14 +745,16 @@ Result<> FillBadData::operator()() const
     }
   }
 
-  // Count the number of existing features for array sizing
+  // Count the number of existing features for array sizing. Only positive feature ids matter;
+  // comparing a negative (bad-data) featureName directly against the unsigned numFeatures would
+  // sign-extend it to a huge value and blow up the featureNumber allocation below.
   usize numFeatures = 0;
   for(usize i = 0; i < totalPoints; i++)
   {
     int32 featureName = featureIdsStore[i];
-    if(featureName > numFeatures)
+    if(featureName > 0 && static_cast<usize>(featureName) > numFeatures)
     {
-      numFeatures = featureName;
+      numFeatures = static_cast<usize>(featureName);
     }
   }
 

src/Plugins/SimplnxCore/src/SimplnxCore/Filters/FillBadDataFilter.cpp

@@ -2,7 +2,6 @@
 
 #include "SimplnxCore/Filters/Algorithms/FillBadData.hpp"
 
-#include "simplnx/DataStructure/AttributeMatrix.hpp"
 #include "simplnx/DataStructure/DataPath.hpp"
 #include "simplnx/Parameters/ArraySelectionParameter.hpp"
 #include "simplnx/Parameters/AttributeMatrixSelectionParameter.hpp"
@@ -68,8 +67,7 @@ Parameters FillBadDataFilter::parameters() const
                                                           ArraySelectionParameter::AllowedTypes{DataType::int32}, ArraySelectionParameter::AllowedComponentShapes{{1}}));
 
   params.insert(std::make_unique<MultiArraySelectionParameter>(k_IgnoredDataArrayPaths_Key, "Attribute Arrays to Ignore", "The list of arrays to ignore when performing the algorithm",
-                                                               MultiArraySelectionParameter::ValueType{}, MultiArraySelectionParameter::AllowedTypes{IArray::ArrayType::DataArray},
-                                                               nx::core::GetAllDataTypes()));
+                                                               MultiArraySelectionParameter::ValueType{}, MultiArraySelectionParameter::AllowedTypes{IArray::ArrayType::DataArray}, GetAllDataTypes()));
   // Associate the Linkable Parameter(s) to the children parameters that they control
   params.linkParameters(k_StoreAsNewPhase_Key, k_CellPhasesArrayPath_Key, true);
 
@@ -96,10 +94,10 @@ IFilter::PreflightResult FillBadDataFilter::preflightImpl(const DataStructure& d
 
   if(pMinAllowedDefectSizeValue < 1)
   {
-    MakeErrorResult(-16500, fmt::format("Minimum Allowed Defect Size must be at least 1. Value given was {}", pMinAllowedDefectSizeValue));
+    return MakePreflightErrorResult(-16500, fmt::format("Minimum Allowed Defect Size must be at least 1. Value given was {}", pMinAllowedDefectSizeValue));
   }
 
-  nx::core::Result<OutputActions> resultOutputActions;
+  Result<OutputActions> resultOutputActions;
 
   std::vector<PreflightValue> preflightUpdatedValues;
 
@@ -112,7 +110,7 @@ IFilter::PreflightResult FillBadDataFilter::preflightImpl(const DataStructure& d
   // Cell Data is going to be modified
   auto featureIdsPath = filterArgs.value<DataPath>(k_CellFeatureIdsArrayPath_Key);
   auto ignoredDataArrayPaths = filterArgs.value<MultiArraySelectionParameter::ValueType>(k_IgnoredDataArrayPaths_Key);
-  nx::core::AppendDataObjectModifications(dataStructure, resultOutputActions.value().modifiedActions, featureIdsPath.getParent(), ignoredDataArrayPaths);
+  AppendDataObjectModifications(dataStructure, resultOutputActions.value().modifiedActions, featureIdsPath.getParent(), ignoredDataArrayPaths);
 
   return {std::move(resultOutputActions), std::move(preflightUpdatedValues)};
 }