diff --git a/packages/adapters/src/adapters/Cornerstone3D/Segmentation/compactMergeSegData.ts b/packages/adapters/src/adapters/Cornerstone3D/Segmentation/compactMergeSegData.ts index 163cc3cf02..7862930224 100644 --- a/packages/adapters/src/adapters/Cornerstone3D/Segmentation/compactMergeSegData.ts +++ b/packages/adapters/src/adapters/Cornerstone3D/Segmentation/compactMergeSegData.ts @@ -46,20 +46,34 @@ export const compactMergeSegmentDataWithoutInformationLoss = ({ continue; } - largerArray.forEach((_, currentImageIndex) => { + // Iterate the index RANGE rather than one array's populated entries. + // + // getSegmentData() builds its result as a SPARSE array — segmentData[i] is assigned + // only for images that actually hold voxels — and Array.prototype.forEach SKIPS HOLES. + // Iterating `largerArray` therefore visited only the indices where that array had an + // element. When the existing layer is the longer one, `largerArray` IS the existing + // layer, so every image where only the INCOMING segment had data was never visited, + // and was silently dropped. + const mergeLength = Math.max( + currentTestedArray.length, + newSegmentData.length + ); + + for ( + let currentImageIndex = 0; + currentImageIndex < mergeLength; + currentImageIndex++ + ) { const originalImagePixelData = currentTestedArray[currentImageIndex]; const newImagePixelData = newSegmentData[currentImageIndex]; - if ( - (!originalImagePixelData && !newImagePixelData) || - !newImagePixelData - ) { - return; + if (!newImagePixelData) { + continue; } if (!originalImagePixelData) { currentTestedArray[currentImageIndex] = newImagePixelData; - return; + continue; } const mergedPixelData = originalImagePixelData.map( @@ -70,7 +84,7 @@ export const compactMergeSegmentDataWithoutInformationLoss = ({ ); currentTestedArray[currentImageIndex] = mergedPixelData; - }); + } return; } diff --git a/packages/adapters/test/compactMergeSegData.jest.js b/packages/adapters/test/compactMergeSegData.jest.js index 887ed480b9..cc5043c919 100644 --- a/packages/adapters/test/compactMergeSegData.jest.js +++ b/packages/adapters/test/compactMergeSegData.jest.js @@ -161,4 +161,37 @@ describe('compactMergeSegmentDataWithoutInformationLoss', () => { ], ]); }); + + // getSegmentData() assigns only the indices that hold voxels, so its result is a SPARSE + // array (real holes), not a dense one containing `undefined`. That distinction matters: + // Array.prototype.forEach visits an `undefined` ELEMENT but skips a HOLE. Every test above + // uses dense arrays, so none of them exercise this. + it('should keep new data at indices the existing layer does not span (sparse arrays)', () => { + // Existing layer holds images 3..4 — length 5, holes at 0..2. + const existingLayer = []; + existingLayer[3] = [1, 0]; + existingLayer[4] = [1, 0]; + + // New segment holds images 1..3 — length 4, so the EXISTING layer is the longer one + // and becomes `largerArray`. Indices 1 and 2 are outside the existing layer's + // populated set and used to be dropped. + const newSegmentData = []; + newSegmentData[1] = [0, 2]; + newSegmentData[2] = [0, 2]; + newSegmentData[3] = [0, 2]; + + const arrayOfSegmentData = [existingLayer]; + + compactMergeSegmentDataWithoutInformationLoss({ + arrayOfSegmentData, + newSegmentData, + }); + + // Merged into the single existing layer, keeping every image from both. + expect(arrayOfSegmentData).toHaveLength(1); + expect(arrayOfSegmentData[0][1]).toEqual([0, 2]); + expect(arrayOfSegmentData[0][2]).toEqual([0, 2]); + expect(arrayOfSegmentData[0][3]).toEqual([1, 2]); + expect(arrayOfSegmentData[0][4]).toEqual([1, 0]); + }); });