BUG: Fix 18 filter defects introduced in the 6.5.171 port by imikejackson · Pull Request #1900 · BlueQuartzSoftware/simplnx

imikejackson · 2026-10-31T14:55:50Z

Summary

These changes fix 18 defects that were introduced when filters were ported from DREAM.3D 6.5.171 to simplnx. Each fix is small and limited to one filter or one shared utility. Where 6.5.171 had the correct behavior, the final code matches it again. Each fix has a regression test that fails on develop, and the expected values come from hand calculations, closed-form values or literal file contents, not from the code under test. 56 files changed, +2449 / −282 lines.

Companion PR: BlueQuartzSoftware/DREAM3DNX#1228. It updates the DREAM3D-NX Read CSV preview so that the preview shows the same columns as the filter reads with the new Consecutive Delimiters behavior.

Breaking and behavior changes

Compute Feature Phases Binary changes its parameters (parametersVersion 2). The output array is now created in a Feature Attribute Matrix that the new key cell_feature_attribute_matrix_path selects. The key cell_data_attribute_matrix_path is removed. Two new parameters, create_cell_ensemble_attribute_matrix and cell_ensemble_attribute_matrix_name, create an optional 2-tuple Cell Ensemble Attribute Matrix. Existing DREAM3D-NX pipelines that use this filter must be edited again. This is intended, because the old output was wrong. DREAM.3D 6.5.171 pipelines convert correctly.
Read CSV File: Consecutive Delimiters now merges runs of delimiters, as the documentation says. Before, the option did the opposite. StringUtilities::split keeps its contract; its parameter is renamed to keepEmptyTokens so that the name tells what it does. The conversion of 6.5.171 ReadASCIIData pipelines always turns Consecutive Delimiters on, because 6.5.171 always dropped empty tokens.
New error codes: -10706 (Convert Color To GrayScale: the number of input paths and output paths is different) and -53802 (Compute Feature Phases Binary: the Cell mask and the Feature Ids have different tuple counts).
Merge conflict: the Triangle Dihedral Angle changes conflict with open PR BUG: Fix OOC Review Findings #1764. This is expected.

1. SimplnxCore: Feature and array filters

Key files:

src/Plugins/SimplnxCore/src/SimplnxCore/Filters/ComputeFeaturePhasesBinaryFilter.cpp — new parameters, preflight tuple check and SIMPL conversion.
src/Plugins/SimplnxCore/src/SimplnxCore/Filters/Algorithms/ScalarSegmentFeatures.cpp — exact 64-bit integer comparison.

Fixes:

ComputeFeaturePhasesBinaryFilter: The output was created in the Cell Attribute Matrix but filled by Feature Id, so only the first tuples got values. The filter now writes one value per Feature. With a Cell mask, the last Cell of each Feature sets its phase, as in 6.5.171. With a mask in the selected Feature Attribute Matrix, each mask value sets the phase of its Feature. The cell-level exemplar comparison is replaced by hand-computed tests.
ScalarSegmentFeaturesFilter: int64 and uint64 values were converted to float64, so values above 2^53 that differ by 1 merged at tolerance 0. The filter now compares 64-bit integers in their own type, without overflow.
ArrayCalculatorFilter: A binary operator checked only the component count of the left operand, so A + (B)[0] read past the 1-component operand. The filter now returns the existing error -4024 (InconsistentCompDims) when two array operands have different component counts. Numbers still broadcast.
ComputeVectorColorsFilter: With Apply to Good Voxels Only off, preflight created a stray root-level mask array, so a second instance failed with an "already exists" error. Preflight no longer creates this array.
ConvertColorToGrayScaleFilter: The output path iterator did not advance, so every input was written to the first output array. Each input now goes to its own output array.

2. SimplnxCore: Geometry and surface mesh filters

Key files:

src/Plugins/SimplnxCore/src/SimplnxCore/Filters/Algorithms/TupleTransfer.hpp — tuple transfer for Point Sample Triangle Geometry.
src/Plugins/SimplnxCore/src/SimplnxCore/Filters/Algorithms/CalculateTriangleGroupCurvatures.cpp — NaN triangle removal.
src/simplnx/Utilities/SampleSurfaceMesh.cpp — inside-test ray length.

Fixes:

TriangleDihedralAngleFilter: The corner angles used absolute dot products, so the minimum angle depended on the vertex order. The filter now uses signed dot products, as 6.5.171 did, and clamps the acos arguments to [-1, 1].
PointSampleTriangleGeometryFilter: The face transfer swapped the source and destination indexes and used tuple indexes as value indexes. The filter now copies the complete tuple of the sampled triangle to the new vertex, for face arrays and feature arrays.
FeatureFaceCurvatureFilter: Removing a NaN triangle at the end of a patch dereferenced end(), and a NaN seed triangle was replaced by a different triangle. The filter now removes NaN triangles safely. When the seed has NaN data or fewer than two valid triangles remain, every requested output of the seed is NaN, as in 6.5.171.
CropVertexGeometryFilter: Cropped arrays got a flat component shape (for example {3,3} became {9}). They now keep the component shape of the input array.
UncertainRegularGridSampleSurfaceMeshFilter: The point-in-polyhedron ray was half the bounding-box diagonal, so points near the end of a long feature could get Feature Id 0. The ray is now the full diagonal plus a 1% margin, as in 6.5.171.

3. SimplnxCore: File readers and writers

Key files:

src/simplnx/Utilities/FileUtilities.cpp and src/Plugins/SimplnxCore/src/SimplnxCore/Filters/ReadCSVFileFilter.cpp — CSV header and row splitting.
src/simplnx/Parameters/ReadCSVFileParameter.cpp — SIMPL conversion of the CSV options.
src/Plugins/SimplnxCore/src/SimplnxCore/Filters/Algorithms/ReadVtkStructuredPoints.cpp — section order.
src/Plugins/SimplnxCore/src/SimplnxCore/utils/VtkUtilities.hpp — ASCII float format.

Fixes:

ReadCSVFileFilter: The option was passed directly to the "keep empty tokens" flag of StringUtilities::split, so space-aligned files failed with error -104. Header and row parsing now pass the inverse of the option. Other callers of split do not change.
ReadVtkStructuredPointsFilter: When the unselected section (POINT_DATA or CELL_DATA) came first, the reader stopped before the selected section and gave an empty Attribute Matrix with no error. Preflight and execute now skip the first section and read the selected one, as in 6.5.171.
WriteVtkRectilinearGridFilter: ASCII floats were written with 6 decimals, so 1e-7 became 0.000000. Floats are now written with the shortest text that round-trips. Integers and binary output do not change. The existing ASCII exemplar test compares values at 6 decimals.
WriteLAMMPSFileFilter: The progress interval was 0 for fewer than 1000 vertices, which caused a division by zero. The interval is now at least 1.

4. ImageProcessing

Key files:

src/simplnx/Utilities/ImageProcessing/MorphologyEngine.hpp — 2D kernel offsets for the binary routes.
src/simplnx/Utilities/ImageProcessing/ProjectionReducers.hpp — variance update.

Fixes:

Binary morphology (MorphologyEngine): On a 2D image (Z = 1), out-of-plane kernel offsets counted as outside the image, so Binary Dilate with Boundary To Foreground on made every cell foreground, and Binary Erode with it off made every cell background. Every binary route now uses only the in-plane offsets, as the ITK filters in 6.5.171 did. When no in-plane offset remains, the route copies the input.
StandardDeviationProjectionImageFilter: The variance sumSq - sum*sum/n cancelled for large values, so {1e9, 1e9+1, 1e9+2} gave 0, not 1. The reducer now uses Welford's online update with the same per-column state size and the sample (n-1) definition.

5. OrientationAnalysis

Key files:

src/Plugins/OrientationAnalysis/src/OrientationAnalysis/Filters/Algorithms/ReadH5Ebsd.cpp — HKL hexagonal correction.

Fixes:

ReadH5EbsdFilter: For an HKL file with a Hexagonal_High phase, the +30 degree phi2 correction ran only when Phases was also selected. The reader now reads the phase channel whenever it reads EulerAngles from an HKL file, as 6.5.171 did. The Phases array is still created only when the user selects it. This also removes a volume-sized phase cache.

6. Core utilities

Key files:

src/simplnx/Utilities/DataArrayUtilities.cpp — Feature Id validation.
src/simplnx/Utilities/StringUtilities.hpp — split parameter name.

Fixes:

ValidateFeatureIdsToFeatureAttributeMatrixIndexing: The scan used getNumberOfTuples(), so for a multi-component Feature Ids array (for example 2-component Face Labels) the second half of the values was not checked. The scan now uses getSize().
StringUtilities::split: The parameter is renamed to keepEmptyTokens. The behavior does not change, and a new test pins the contract.

Test plan

Each fix has a regression test that fails on develop and passes on this branch.
Expected values in the new tests come from hand calculations, closed-form values or literal file contents.
The full build passes.
The full test suite passes serially (3483 of 3483).
CI passes on Windows, macOS and Linux.

Related Issue

Closes #1818
Closes #1759
Closes #1799
Closes #1790
Closes #1886
Related to #1875
Related to #1864
Related to #1899

…ices The progress interval was numVerts / 1000, which is 0 for a Vertex Geometry with fewer than 1000 vertices. The loop then computed i % 0. The interval is now at least 1. Add a test that exports eight vertices and compares the complete file with hand-written expected text. Closes BlueQuartzSoftware#1818 Signed-off-by: Michael Jackson <mike.jackson@bluequartz.net>

The output path iterator was never advanced, so every selected input was converted into the first output array. The other outputs kept their initial values, and a larger later input wrote past the end of the first output. Advance the iterator once per input, and return error -10706 when a caller gives a different number of input and output paths. Add tests with two inputs of different tuple counts (Direct and Scanline) and for mismatched path counts. The expected values are calculated by hand from the luminosity weights. Closes BlueQuartzSoftware#1759 Signed-off-by: Michael Jackson <mike.jackson@bluequartz.net>

…ertices The face-attribute transfer used the sampled triangle index as the destination and the new vertex index as the source. It also used tuple indexes as flat value indexes, so arrays with more than one component got wrong values and could be read or written outside their range. Copy the complete tuple of the sampled triangle to the tuple of the new vertex, for face arrays and for feature arrays. Add tests for face arrays of every supported type and for feature arrays. Each sampled vertex must hold the complete tuple of a triangle that contains it; the test finds that triangle with barycentric coordinates. Closes BlueQuartzSoftware#1799 Signed-off-by: Michael Jackson <mike.jackson@bluequartz.net>

…ngles The corner angles used the absolute value of each dot product. For an acute angle at the second vertex, 180 - acos(|AB.BC|) gave 180 - B, so the minimum angle depended on the vertex order of the face. Use signed dot products, as DREAM.3D 6.5.171 did, and clamp the acos arguments to [-1, 1]. The clamp also fixes TriangleDihedralAngleFilter#2 in BlueQuartzSoftware#1875 (NaN or 180 degrees for nearly collinear float32 triangles). Add a test with four triangles (the BlueQuartzSoftware#1790 triangle, a very acute triangle, an obtuse triangle and a nearly collinear triangle) in all six vertex orders. The expected minimum angles are closed-form values. Closes BlueQuartzSoftware#1790 Signed-off-by: Michael Jackson <mike.jackson@bluequartz.net>

The shape check for a binary operator used the component count of the left operand only. For A + (B)[0] with 3-component arrays, preflight accepted the expression, and execute read three values per tuple from the 1-component right operand. Return error -4024 (InconsistentCompDims) when both operands are arrays and their component counts differ. Numbers still broadcast. Add tests for both operand orders and for valid same-shape, extracted component and scalar expressions, with hand-calculated results. Signed-off-by: Michael Jackson <mike.jackson@bluequartz.net>

On an Image Geometry with Z = 1, the binary morphology engine counted kernel offsets with dz != 0 as outside the image. Binary Dilate with Boundary To Foreground on made every cell foreground, and Binary Erode with it off made every cell background. A single-slice image has no Z neighbors, so every binary route (2D block body, moving window and the rolling Opening/Closing pipeline) now uses only the in-plane offsets, as the ITK filters in DREAM.3D 6.5.171 did with a 2D image. When no in-plane offset is left (an Annulus with X and Y radius 0), every route copies the input. Add MorphologyEngine tests on small 2D images with hand-drawn expected results for Direct, Scanline and the composite route, and document the 2D behavior in the Binary Dilate and Binary Erode pages. Signed-off-by: Michael Jackson <mike.jackson@bluequartz.net>

… component The validation scanned getNumberOfTuples() values. For a Feature Ids array with more than one component (for example 2-component Face Labels), the values in the second half of the array were never checked. Scan getSize() values (tuples x components). Add DataArrayUtilities tests with an invalid id in component 1 of the last tuple, for both target kinds and both negative-id policies, and for empty arrays. Signed-off-by: Michael Jackson <mike.jackson@bluequartz.net>

…s off With Apply to Good Voxels Only off, preflight created a root-level DataArray named 'mask array' that was never deleted. The stray array stayed in the output, and a second instance of the filter failed with an 'already exists' error. The algorithm already handles a missing mask when the mask is off, so remove the stray array. Add a test that runs the filter twice without a mask and checks the complete object list and hand-calculated colors. Signed-off-by: Michael Jackson <mike.jackson@bluequartz.net>

Preflight created each cropped array with a one-dimensional component shape equal to the number of components, so an array with component shape {3,3} became {9}. Create the cropped array with the component shape of the input array. Add a test that crops int32 {3,3} and float32 {2} vertex arrays and checks the shapes and every kept value against hand-written values. Signed-off-by: Michael Jackson <mike.jackson@bluequartz.net>

When the last triangle of a patch had NaN normal or centroid data, erase() returned end() and the code dereferenced it. A NaN seed triangle was also replaced by another triangle, so the output of the seed came from a different triangle. Remove NaN triangles without dereferencing the end iterator. When the seed triangle has NaN data, or fewer than two valid triangles are left, write NaN to every requested output of the seed, as DREAM.3D 6.5.171 did for NaN input. Add a test with a planar nine-triangle patch (expected curvature 0) that puts NaN in each component of the first, middle and last triangle, for both fitting modes. Document the neighborhood and NaN behavior. Signed-off-by: Michael Jackson <mike.jackson@bluequartz.net>

StringUtilities::split takes a flag that keeps empty tokens when true. ReadCSV passed its Consecutive Delimiters option straight to that flag, so the option did the opposite of its documentation: on kept empty fields, and off merged delimiters. Space-aligned files failed with error -104. Invert the option where ReadCSV calls split (header and row parsing, and the cached header path in the filter). The behavior of split for its other callers does not change. Rename the split parameter to keepEmptyTokens so that its name matches what it does. DREAM.3D 6.5.171 always dropped empty tokens, so the SIMPL conversion of ReadASCIIData now always turns Consecutive Delimiters on. Add ReadCSV tests for a space-aligned file with the option on and off, and for header tokens, with hand-written expected values, a test that pins the split contract, and a check of the converted option. Signed-off-by: Michael Jackson <mike.jackson@bluequartz.net>

…elected For an HKL (.ctf based) file with a Hexagonal_High phase, the +30 degree phi2 correction ran only when the Phases array was also selected for import. With only EulerAngles selected, the angles were not corrected. Read the phase channel from the file whenever EulerAngles is read from an HKL file, and use the reader's phase buffer for the correction, as DREAM.3D 6.5.171 did. The phase array is still created only when the user selects it. This also removes a volume-sized phase cache. Add a test with a generated 3-cell HKL H5Ebsd file (hexagonal, cubic, unindexed). The expected angles are calculated by hand, and the result with only EulerAngles selected must equal the result with Phases selected. Signed-off-by: Michael Jackson <mike.jackson@bluequartz.net>

…s second When the unselected data section (POINT_DATA or CELL_DATA) came first in the file, the reader stopped before the selected section. The geometry got an empty Attribute Matrix and there was no error. Skip the unselected first section with the existing ASCII and binary skip routines, in preflight and in execute, and then read the selected section, as DREAM.3D 6.5.171 did. Add a test with generated ASCII and binary files in both section orders and all four selections. The expected values are the literal values written into each file. Signed-off-by: Michael Jackson <mike.jackson@bluequartz.net>

int64 and uint64 scalar values were converted to float64 before the tolerance test. Values above 2^53 that differ by 1 became equal and merged into one feature at Scalar Tolerance 0. Keep 64-bit integer values in their own type and compare the unsigned absolute difference without overflow, as DREAM.3D 6.5.171 compared values in their native type. The unbuffered int64 comparator, used by executeCCL() after the slice buffers are released, now also computes the difference without overflow. The slice buffers do not grow. Add int64 and uint64 tests (values near 2^53 and near the type limits, tolerance 0 and 1) for both neighbor schemes and both algorithm selections, and for the unbuffered route, with hand-written expected Feature Ids, and document the exact comparison. Signed-off-by: Michael Jackson <mike.jackson@bluequartz.net>

The variance was calculated as sumSq - sum*sum/n. For large values that differ by a small amount, for example {1e9, 1e9+1, 1e9+2}, the two terms cancel and the standard deviation was 0, not 1. Use Welford's online update, which keeps the same per-column state size (two float64 values and a count) and the sample (n-1) definition. DREAM.3D 6.5.171 used the two-pass ITK accumulator. Add float64, float32 and int32 tests with large offsets on every axis, and tests of the staged and bounded routes. The expected values are closed-form ({a, a+d, a+2d} gives |d|). Describe the update in the filter documentation. Signed-off-by: Michael Jackson <mike.jackson@bluequartz.net>

The point-in-polyhedron ray was half as long as the diagonal of the feature bounding box. From a point near one end of a long, thin feature, the ray could stop inside the feature, count 0 crossings, and give the point Feature Id 0. Use the full diagonal plus a 1% margin, so the ray always leaves the bounding box, as DREAM.3D 6.5.171 did. Add an UncertainRegularGridSampleSurfaceMesh test with a closed, sheared 1 x 1 x 20 box. The expected Feature Ids of points near both ends come from the box inequalities. Signed-off-by: Michael Jackson <mike.jackson@bluequartz.net>

…trip In ASCII mode, float values were written with ' {:f}' (6 decimals), so 1e-7 was written as 0.000000. Write floats with the shortest text that round-trips ('{}'). Integers and binary output do not change. Add float32 and float64 tests that write 1e-7, 123456.789 and -2.5e-12 and parse the text back to the exact input values. The existing ASCII exemplar was written with 6 decimals, so the exemplar test now parses each value as its declared VTK type, formats it with 6 decimals and compares the text exactly. A test checks that this comparison still rejects real differences. Signed-off-by: Michael Jackson <mike.jackson@bluequartz.net>

…5.171 The output array was created in the Cell Attribute Matrix but filled by Feature Id, so only the first tuples were written. The filter now writes a Feature-level array in a selected Cell Feature Attribute Matrix: - Cell mask: phases[featureIds[i]] = mask[i] in cell order (last cell wins), as in DREAM.3D 6.5.171. Mask and Feature Ids must have the same number of tuples (new error -53802). - Feature mask (the Mask lives in the selected Feature AM): phases[f] = mask[f]. Feature Ids are not read. - An optional 2-tuple Cell Ensemble Attribute Matrix can be created next to the Feature AM (6.5.171 always created it). Parameters changed (parametersVersion 2). The SIMPL conversion maps FeaturePhasesArrayPath and CellEnsembleAttributeMatrixName. The cell-level exemplar comparison is replaced by hand-computed tests. Signed-off-by: Michael Jackson <mike.jackson@bluequartz.net>

imikejackson added 18 commits October 30, 2026 15:09

imikejackson self-assigned this Oct 31, 2026

Navigation Menu

Repository navigation

BUG: Fix 18 filter defects introduced in the 6.5.171 port - #1900

BUG: Fix 18 filter defects introduced in the 6.5.171 port#1900
imikejackson wants to merge 18 commits into
BlueQuartzSoftware:developfrom
imikejackson:topic/port_regression_fixes

imikejackson commented Oct 31, 2026 •

edited

Loading

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

imikejackson commented Oct 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Breaking and behavior changes

1. SimplnxCore: Feature and array filters

2. SimplnxCore: Geometry and surface mesh filters

3. SimplnxCore: File readers and writers

4. ImageProcessing

5. OrientationAnalysis

6. Core utilities

Test plan

Related Issue

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

imikejackson commented Oct 31, 2026 •

edited

Loading