REV: PR #1631 review round-2 — revert IPF Phase2 fix + cleanup pass

Round-2 PR-review feedback on PR #1631 of `topic/ebsdlib_v3_updates`.

ComputeFaceIPFColoring Phase 2 bug revert.
- Revert the Phase 2 IPF coloring fix (`phase1`→`phase2` at the validity
  guard + IPF operator lookup) that was applied earlier in this PR. The
  fix is provably correct, but the existing test exemplar in
  `6_6_Small_IN100_GBCD.tar.gz` encodes pre-EbsdLib-3.0 colors and now
  fails for a broader reason than just the phase2 bug (EbsdLib 2.4.1→3.0.0
  ripple through `_calcRodNearestOrigin` + HexConvention defaults).
  Tracked at #1635;
  the V&V cycle on this filter will regenerate the exemplar AND re-apply
  the phase2 fix. The known-bug location is now flagged with an inline
  block comment.

Dead-code removal: `getCancel()` API.
- All 5 of the PR's algorithm classes (BadDataNeighborOrientationCheck,
  ComputeFaceIPFColoring, ComputeFeatureFaceMisorientation,
  ComputeFeatureNeighborMisorientations, WritePoleFigure) declared a
  `const std::atomic_bool& getCancel()` member that returned
  `m_ShouldCancel`. None had any callers — algorithm bodies use
  `m_ShouldCancel` directly. Removed declarations from headers and
  definitions from `.cpp` files.

LLM-style boilerplate scrub.
- ComputeAvgCAxes.cpp: 6-line "2 possibilities that could have happened"
  multi-bullet hedge comment block in the per-feature finalize loop
  collapsed to a one-line sentence stating the actual reason for NaN.
- ComputeFaceIPFColoring.cpp + ComputeFeatureFaceMisorientation.cpp:
  removed boilerplate Doxygen blocks above the parallel-impl classes
  that just restated the class name in English.

Readability polish.
- ComputeAvgCAxes.cpp: replaced the `temp` scratch float used 3× to
  narrow Eigen-double → float32 with inline `static_cast<float32>`.
- BadDataNeighborOrientationCheck.cpp: error message at the mask-load
  catch now includes the underlying `std::out_of_range::what()` and uses
  positive phrasing ("could not be loaded; expected Bool or UInt8").
- BadDataNeighborOrientationCheck.cpp: 3 of 4 `if(!maskCompare->isTrue(...))`
  sites now resolve the double negative via a named local
  (`voxelIsBad` / `neighborIsBad`); the one positive-form
  `if(maskCompare->isTrue(...))` site is unchanged.
- ComputeFeatureNeighborCAxisMisalignments.cpp: added a one-line inline
  comment on the `w` variable's first use to disambiguate the
  cos(angle)→angle reassignment chain.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Author: imikejackson

src/Plugins/OrientationAnalysis/src/OrientationAnalysis/Filters/Algorithms/BadDataNeighborOrientationCheck.cpp

@@ -24,12 +24,6 @@ BadDataNeighborOrientationCheck::BadDataNeighborOrientationCheck(DataStructure&
 // -----------------------------------------------------------------------------
 BadDataNeighborOrientationCheck::~BadDataNeighborOrientationCheck() noexcept = default;
 
-// -----------------------------------------------------------------------------
-const std::atomic_bool& BadDataNeighborOrientationCheck::getCancel()
-{
-  return m_ShouldCancel;
-}
-
 // -----------------------------------------------------------------------------
 Result<> BadDataNeighborOrientationCheck::operator()()
 {
@@ -53,9 +47,10 @@ Result<> BadDataNeighborOrientationCheck::operator()()
     maskCompare = MaskCompareUtilities::InstantiateMaskCompare(m_DataStructure, m_InputValues->MaskArrayPath);
   } catch(const std::out_of_range& exception)
   {
-    // This really should NOT be happening as the path was verified during preflight, BUT we may be calling this from
-    // somewhere else that is NOT going through the normal nx::core::IFilter API of Preflight and Execute
-    return MakeErrorResult(-54900, fmt::format("Mask Array DataPath does not exist or is not of the correct type (Bool | UInt8) {}", m_InputValues->MaskArrayPath.toString()));
+    // Defensive: the path was verified during preflight, but this algorithm may be called outside the standard
+    // IFilter Preflight/Execute path.
+    return MakeErrorResult(-54900,
+                           fmt::format("Mask Array at '{}' could not be loaded; expected Bool or UInt8 backing. Underlying error: {}", m_InputValues->MaskArrayPath.toString(), exception.what()));
   }
 
   std::array<int64, 3> dims = {
@@ -106,7 +101,9 @@ Result<> BadDataNeighborOrientationCheck::operator()()
     }
     throttledMessenger.sendThrottledMessage([&] { return fmt::format("Processing Data {:.2f}% completed", CalculatePercentComplete(voxelIndex, totalPoints)); });
     // If the mask was set to false, then we check this voxel
-    if(!maskCompare->isTrue(voxelIndex))
+    // "Bad" voxels are those whose mask value is false; only these get processed.
+    const bool voxelIsBad = !maskCompare->isTrue(voxelIndex);
+    if(voxelIsBad)
     {
       // We precalculate the positive voxel quaternion and laue class here to prevent reading and recalculating it for each face below
       ebsdlib::QuatD quat1(quats[voxelIndex * 4], quats[voxelIndex * 4 + 1], quats[voxelIndex * 4 + 2], quats[voxelIndex * 4 + 3]);
@@ -196,7 +193,8 @@ Result<> BadDataNeighborOrientationCheck::operator()()
 
         // If the current voxel's neighbor count is >= the current level and the mask is FALSE,
         // we flip the voxel to TRUE and recompute its (still-bad) neighbors' counts below.
-        if(neighborCount[voxelIndex] >= currentLevel && !maskCompare->isTrue(voxelIndex))
+        const bool voxelIsBad = !maskCompare->isTrue(voxelIndex);
+        if(neighborCount[voxelIndex] >= currentLevel && voxelIsBad)
         {
           maskCompare->setValue(voxelIndex, true);
           counter++; // Increment the `counter` to force the loop to iterate again
@@ -232,7 +230,8 @@ Result<> BadDataNeighborOrientationCheck::operator()()
             const int64 neighborPoint = voxelIndexI64 + neighborVoxelIndexOffsets[faceIndex];
 
             // If the neighbor voxel's mask is false, then compute misorientation angle
-            if(!maskCompare->isTrue(neighborPoint))
+            const bool neighborIsBad = !maskCompare->isTrue(neighborPoint);
+            if(neighborIsBad)
             {
               // Make sure both cells phase values are identical and valid
               if(cellPhases[voxelIndex] == cellPhases[neighborPoint] && cellPhases[voxelIndex] > 0)

src/Plugins/OrientationAnalysis/src/OrientationAnalysis/Filters/Algorithms/BadDataNeighborOrientationCheck.hpp

@@ -37,8 +37,6 @@ class ORIENTATIONANALYSIS_EXPORT BadDataNeighborOrientationCheck
 
   Result<> operator()();
 
-  const std::atomic_bool& getCancel();
-
 private:
   DataStructure& m_DataStructure;
   const BadDataNeighborOrientationCheckInputValues* m_InputValues = nullptr;

src/Plugins/OrientationAnalysis/src/OrientationAnalysis/Filters/Algorithms/ComputeAvgCAxes.cpp

@@ -121,16 +121,10 @@ Result<> ComputeAvgCAxes::operator()()
         cellCAxis *= -1.0f;
       }
 
-      // Continue summing up the rotations
-      // Eigen math is double; output array is float32
-      float temp = avgCAxes[cAxesIndex] + cellCAxis[0];
-      avgCAxes[cAxesIndex] = temp;
-
-      temp = avgCAxes[cAxesIndex + 1] + cellCAxis[1];
-      avgCAxes[cAxesIndex + 1] = temp;
-
-      temp = avgCAxes[cAxesIndex + 2] + cellCAxis[2];
-      avgCAxes[cAxesIndex + 2] = temp;
+      // Accumulate per-component into the float32 output (Eigen math is double; narrow on store).
+      avgCAxes[cAxesIndex] = static_cast<float32>(avgCAxes[cAxesIndex] + cellCAxis[0]);
+      avgCAxes[cAxesIndex + 1] = static_cast<float32>(avgCAxes[cAxesIndex + 1] + cellCAxis[1]);
+      avgCAxes[cAxesIndex + 2] = static_cast<float32>(avgCAxes[cAxesIndex + 2] + cellCAxis[2]);
     }
   }
 
@@ -144,16 +138,10 @@ Result<> ComputeAvgCAxes::operator()()
       return result;
     }
 
-    // If the value of this feature's counter == 0, then there are 2 possibilities
-    // that could have happened:
-    // [1] The feature's phase was not hexagonal and therefore in the "totalPoints" loop above the counter never got incremented.
-    // [2] There is a featureId that does not have any voxels assigned to it. We need more information here to determine what to do.
-    //     - If we had the "Feature-Phases array then we could look up the phase and then we would know. This would require another input from the user
-    //     - If the featureId does not have any voxels assigned, then how would you generate an average anyway, so applying NaN to the value is the right move here
-
     const usize cAxesIndex = 3 * currentFeatureId;
     if(cellCount[currentFeatureId] == 0)
     {
+      // Feature is either non-hexagonal or has no assigned voxels; either way, no meaningful average exists.
       avgCAxes[cAxesIndex] = NAN;
       avgCAxes[cAxesIndex + 1] = NAN;
       avgCAxes[cAxesIndex + 2] = NAN;

src/Plugins/OrientationAnalysis/src/OrientationAnalysis/Filters/Algorithms/ComputeFaceIPFColoring.cpp

@@ -21,10 +21,6 @@
 
 using namespace nx::core;
 
-/**
- * @brief The CalculateNormalsImpl class implements a threaded algorithm that computes the IPF colors for the given list of
- * surface mesh labels
- */
 class CalculateFaceIPFColorsImpl
 {
   const Int32Array& m_Labels;
@@ -110,8 +106,12 @@ class CalculateFaceIPFColorsImpl
       // Now compute for Phase 2
       if(phase2 > 0)
       {
-        // Make sure we are using a valid Euler Angles with valid crystal symmetry
-        if(m_CrystalStructures[phase2] < ebsdlib::CrystalStructure::LaueGroupEnd)
+        // KNOWN BUG — tracked at https://github.com/BlueQuartzSoftware/simplnx/issues/1635.
+        // Both the validity guard and the IPF operator lookup below use phase1 instead of phase2,
+        // assigning Phase 1's symmetry operator to Phase 2's Euler angles on mixed-phase faces.
+        // The 2-line fix (phase1→phase2) is held pending a V&V cycle that will regenerate the
+        // test exemplar (which currently encodes the bug — a circular oracle).
+        if(m_CrystalStructures[phase1] < ebsdlib::CrystalStructure::LaueGroupEnd)
         {
           dEuler[0] = m_Eulers[3 * feature2 + 0];
           dEuler[1] = m_Eulers[3 * feature2 + 1];
@@ -120,7 +120,7 @@ class CalculateFaceIPFColorsImpl
           refDir[1] = -m_Normals[3 * i + 1];
           refDir[2] = -m_Normals[3 * i + 2];
 
-          argb = ops[m_CrystalStructures[phase2]]->generateIPFColor(dEuler, refDir, false, m_ColorKey);
+          argb = ops[m_CrystalStructures[phase1]]->generateIPFColor(dEuler, refDir, false, m_ColorKey);
           m_SecondColors[3 * i + 0] = RgbColor::dRed(argb);
           m_SecondColors[3 * i + 1] = RgbColor::dGreen(argb);
           m_SecondColors[3 * i + 2] = RgbColor::dBlue(argb);
@@ -158,12 +158,6 @@ ComputeFaceIPFColoring::ComputeFaceIPFColoring(DataStructure& dataStructure, con
 // -----------------------------------------------------------------------------
 ComputeFaceIPFColoring::~ComputeFaceIPFColoring() noexcept = default;
 
-// -----------------------------------------------------------------------------
-const std::atomic_bool& ComputeFaceIPFColoring::getCancel()
-{
-  return m_ShouldCancel;
-}
-
 // -----------------------------------------------------------------------------
 Result<> ComputeFaceIPFColoring::operator()()
 {

src/Plugins/OrientationAnalysis/src/OrientationAnalysis/Filters/Algorithms/ComputeFaceIPFColoring.hpp

@@ -41,8 +41,6 @@ class ORIENTATIONANALYSIS_EXPORT ComputeFaceIPFColoring
 
   Result<> operator()();
 
-  const std::atomic_bool& getCancel();
-
 private:
   DataStructure& m_DataStructure;
   const ComputeFaceIPFColoringInputValues* m_InputValues = nullptr;

src/Plugins/OrientationAnalysis/src/OrientationAnalysis/Filters/Algorithms/ComputeFeatureFaceMisorientation.cpp

@@ -12,11 +12,6 @@ using LaueOpsContainer = std::vector<LaueOpsShPtrType>;
 
 using namespace nx::core;
 
-/**
- * @brief The CalculateFaceMisorientationColorsImpl class implements a threaded algorithm that computes the misorientation
- * colors for the given list of surface mesh labels
- */
-
 class ComputeFeatureMisorientationPerTriangleImpl
 {
   const Int32Array& m_FaceLabels;
@@ -121,12 +116,6 @@ ComputeFeatureFaceMisorientation::ComputeFeatureFaceMisorientation(DataStructure
 // -----------------------------------------------------------------------------
 ComputeFeatureFaceMisorientation::~ComputeFeatureFaceMisorientation() noexcept = default;
 
-// -----------------------------------------------------------------------------
-const std::atomic_bool& ComputeFeatureFaceMisorientation::getCancel()
-{
-  return m_ShouldCancel;
-}
-
 // -----------------------------------------------------------------------------
 Result<> ComputeFeatureFaceMisorientation::operator()()
 {

src/Plugins/OrientationAnalysis/src/OrientationAnalysis/Filters/Algorithms/ComputeFeatureFaceMisorientation.hpp

@@ -35,8 +35,6 @@ class ORIENTATIONANALYSIS_EXPORT ComputeFeatureFaceMisorientation
 
   Result<> operator()();
 
-  const std::atomic_bool& getCancel();
-
 private:
   DataStructure& m_DataStructure;
   const ComputeFeatureFaceMisorientationInputValues* m_InputValues = nullptr;

src/Plugins/OrientationAnalysis/src/OrientationAnalysis/Filters/Algorithms/ComputeFeatureNeighborCAxisMisalignments.cpp

@@ -128,6 +128,7 @@ Result<> ComputeFeatureNeighborCAxisMisalignments::operator()()
         // dividing by the magnitudes (they would be 1)
         c2.normalize();
 
+        // w holds cos(angle) here; converted to the angle (radians) on the next assignment.
         float64 w = ImageRotationUtilities::CosBetweenVectors(c1, c2);
         w = std::clamp(w, -1.0, 1.0);
         w = std::acos(w);

src/Plugins/OrientationAnalysis/src/OrientationAnalysis/Filters/Algorithms/ComputeFeatureNeighborMisorientations.cpp

@@ -22,12 +22,6 @@ ComputeFeatureNeighborMisorientations::ComputeFeatureNeighborMisorientations(Dat
 // -----------------------------------------------------------------------------
 ComputeFeatureNeighborMisorientations::~ComputeFeatureNeighborMisorientations() noexcept = default;
 
-// -----------------------------------------------------------------------------
-const std::atomic_bool& ComputeFeatureNeighborMisorientations::getCancel()
-{
-  return m_ShouldCancel;
-}
-
 // -----------------------------------------------------------------------------
 Result<> ComputeFeatureNeighborMisorientations::operator()()
 {

src/Plugins/OrientationAnalysis/src/OrientationAnalysis/Filters/Algorithms/ComputeFeatureNeighborMisorientations.hpp

@@ -37,8 +37,6 @@ class ORIENTATIONANALYSIS_EXPORT ComputeFeatureNeighborMisorientations
 
   Result<> operator()();
 
-  const std::atomic_bool& getCancel();
-
 private:
   DataStructure& m_DataStructure;
   const ComputeFeatureNeighborMisorientationsInputValues* m_InputValues = nullptr;