From f0a55fdfd90bc56415dad809ef2625ef05e60ec8 Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Sun, 1 Nov 2026 22:08:37 -0500 Subject: [PATCH 1/4] fix: isolate segment definitions by image Each image now owns its segment definitions, order and selection. A new image copies the previous image definitions once, and a restored set or companion segmentation replaces a copy nobody has edited yet. --- docs/configuration_file.md | 47 +- docs/quick_start_guide.md | 2 +- docs/toolbar.md | 16 +- .../__tests__/loadImageSegmentSets.spec.ts | 470 ++++++++++++++++++ src/actions/loadUserFiles.ts | 3 +- src/components/EditableItemList.vue | 43 +- src/components/LayoutGridItem.vue | 2 +- src/components/MeasurementsToolList.vue | 1 + .../__tests__/EditableItemList.spec.ts | 36 ++ .../__tests__/MeasurementsToolList.spec.ts | 49 +- src/components/tools/paint/PaintWidget2D.vue | 19 +- src/components/tools/polygon/PolygonTool.vue | 2 +- .../polygon/__tests__/PolygonTool.spec.ts | 4 +- src/components/tools/ruler/RulerTool.vue | 2 + src/components/tools/ruler/RulerWidget2D.vue | 8 +- .../__tests__/annotationTool.spec.ts | 28 +- src/composables/actions.ts | 14 +- src/composables/annotationTool.ts | 2 +- src/enface/__tests__/controls.spec.ts | 4 +- src/enface/__tests__/inputCalibration.spec.ts | 2 +- src/enface/__tests__/restoreSelection.spec.ts | 5 +- .../__tests__/thinRegionAppearance.spec.ts | 8 +- src/io/import/__tests__/configJson.spec.ts | 22 +- .../crossImageAnnotationNotice.spec.ts | 87 ++++ .../__tests__/postStateSegmentConfig.spec.ts | 47 +- .../__tests__/repeatedSegmentIds.spec.ts | 48 ++ .../__tests__/restoreStateIdCollision.spec.ts | 3 +- src/io/import/configJson.ts | 10 +- src/io/import/importDataSources.ts | 77 ++- src/io/import/processors/handleConfig.ts | 7 +- src/io/import/processors/restoreStateFile.ts | 27 +- .../__tests__/annotationSaveSnapshot.spec.ts | 6 +- .../__tests__/annotationToolSource.spec.ts | 3 +- .../__tests__/boundedMaskRoundTrip.spec.ts | 24 +- .../__tests__/droppedMaskReport.spec.ts | 21 +- .../imageSegmentSetsRoundTrip.spec.ts | 263 ++++++++++ .../manyToOneImageSegmentRestore.spec.ts | 269 ++++++++++ .../__tests__/maskIoConcurrency.spec.ts | 11 +- .../__tests__/migrate640To700.spec.ts | 33 +- .../__tests__/migrate700To800.spec.ts | 227 +++++++++ .../segmentationArtifactSource.spec.ts | 6 +- .../segmentationDisplayRoundTrip.spec.ts | 26 +- .../__tests__/segmentationRoundTrip.spec.ts | 23 +- .../__tests__/serializeResilience.spec.ts | 29 +- src/io/state-file/migrations.ts | 132 ++++- src/io/state-file/schema.ts | 18 +- src/io/state-file/serialize.ts | 5 +- .../applyResults.annotations.spec.ts | 67 ++- .../__tests__/applyResults.segments.spec.ts | 36 +- .../__tests__/declaredEmptySegments.spec.ts | 4 +- src/processing/__tests__/job-adoption.spec.ts | 6 +- .../__tests__/segmentationPacking.spec.ts | 7 +- src/processing/__tests__/serializedScene.ts | 17 +- src/processing/applyResults.ts | 10 +- src/processing/components/JobsModule.vue | 5 +- .../components/__tests__/JobsModule.spec.ts | 8 +- src/processing/composables/useInputStaging.ts | 2 +- src/processing/segmentationInput.ts | 7 +- src/segmentation/README.md | 63 ++- .../__tests__/annotationToolSegments.spec.ts | 57 ++- .../__tests__/artifactParentRestore.spec.ts | 10 +- .../__tests__/bulkImageRemoval.spec.ts | 205 ++++++++ .../__tests__/configSegments.spec.ts | 157 +++++- .../convertParentRemovedMidImport.spec.ts | 4 +- .../__tests__/deleteSegment.spec.ts | 4 +- .../descriptorlessActiveSegment.spec.ts | 9 +- src/segmentation/__tests__/fillHoles.spec.ts | 4 +- .../__tests__/findMaskVoxels.spec.ts | 2 +- .../__tests__/imageSetInitialization.spec.ts | 278 +++++++++++ .../__tests__/lastImageRemoval.spec.ts | 82 +-- .../lockedEditCreatesNothing.spec.ts | 12 +- .../__tests__/maskAndNameLookups.spec.ts | 6 +- .../__tests__/paintProcess.spec.ts | 17 +- .../__tests__/paintProcessStorage.spec.ts | 2 +- .../__tests__/paintTarget.spec.ts | 12 +- .../__tests__/restoreSegmentLifetime.spec.ts | 21 +- .../__tests__/segmentDisplayState.spec.ts | 30 +- .../__tests__/segmentImportPath.spec.ts | 15 +- .../__tests__/segmentMaskFixtures.ts | 29 +- .../__tests__/segmentRegistry.spec.ts | 450 +++++++++++++---- .../segmentSetAppearanceRestore.spec.ts | 167 +++++++ .../segmentationRestoreResilience.spec.ts | 3 +- .../__tests__/segmentations.spec.ts | 103 ++-- .../sharedSegmentAcrossImages.spec.ts | 137 +++-- .../components/SegmentAssignmentList.vue | 4 +- src/segmentation/components/SegmentEditor.vue | 4 +- src/segmentation/components/SegmentList.vue | 82 ++- .../components/SegmentListActions.vue | 4 +- .../__tests__/ProcessWorkflow.spec.ts | 2 +- .../__tests__/SaveSegmentationDialog.spec.ts | 2 +- .../__tests__/SegmentAssignmentList.spec.ts | 16 +- .../__tests__/SegmentEditor.spec.ts | 6 +- .../components/__tests__/SegmentList.spec.ts | 182 ++++--- .../__tests__/useSegmentEditing.spec.ts | 26 +- .../__tests__/useSegmentShapes.spec.ts | 10 +- .../composables/useSegmentEditing.ts | 30 +- .../composables/useSegmentShapes.ts | 1 + .../__tests__/rasterizePolygon.spec.ts | 24 +- src/segmentation/editing/paintProcess.ts | 4 +- src/segmentation/editing/rasterizePolygon.ts | 9 +- src/segmentation/io/stateFile.ts | 20 +- src/segmentation/segment.ts | 4 +- src/segmentation/segmentRegistry.ts | 463 ++++++++++++----- src/segmentation/segments.ts | 197 +++++++- src/segmentation/store.ts | 53 +- .../__tests__/datasetRemoveCascade.spec.ts | 17 +- .../legacyManifestSegmentGroups.spec.ts | 37 +- src/store/__tests__/rulers.spec.ts | 4 +- src/store/tools/index.ts | 22 +- src/store/tools/paint.ts | 2 +- src/store/tools/polygons.ts | 6 +- src/store/tools/rectangles.ts | 2 +- src/store/tools/rulers.ts | 2 +- src/store/tools/types.ts | 7 +- src/store/tools/useAnnotationTool.ts | 76 ++- tests/specs/annotations-sidebar.e2e.ts | 13 +- tests/specs/clear-scene-segments.e2e.ts | 3 +- tests/specs/image-segmentation-loading.e2e.ts | 321 ++++++++++++ tests/specs/image-segmentation-sets.e2e.ts | 378 ++++++++++++++ tests/specs/paint-brush-size.e2e.ts | 6 + tests/specs/paint-preview-color.e2e.ts | 103 ++++ tests/specs/polygon-rasterize-segment.e2e.ts | 12 +- .../specs/restore-binds-segment-names.e2e.ts | 22 +- tests/specs/segment-shared-type.e2e.ts | 117 ----- tests/specs/segment-shortcuts.e2e.ts | 68 ++- tests/specs/segment-tooltips.e2e.ts | 36 ++ tests/specs/segmentationTestUtils.ts | 29 ++ tests/specs/session-state-lifecycle.e2e.ts | 2 +- tests/specs/utils.ts | 14 + 129 files changed, 5509 insertions(+), 1172 deletions(-) create mode 100644 src/actions/__tests__/loadImageSegmentSets.spec.ts create mode 100644 src/components/__tests__/EditableItemList.spec.ts create mode 100644 src/io/import/__tests__/crossImageAnnotationNotice.spec.ts create mode 100644 src/io/state-file/__tests__/imageSegmentSetsRoundTrip.spec.ts create mode 100644 src/io/state-file/__tests__/manyToOneImageSegmentRestore.spec.ts create mode 100644 src/io/state-file/__tests__/migrate700To800.spec.ts create mode 100644 src/segmentation/__tests__/bulkImageRemoval.spec.ts create mode 100644 src/segmentation/__tests__/imageSetInitialization.spec.ts create mode 100644 src/segmentation/__tests__/segmentSetAppearanceRestore.spec.ts create mode 100644 tests/specs/image-segmentation-loading.e2e.ts create mode 100644 tests/specs/image-segmentation-sets.e2e.ts create mode 100644 tests/specs/paint-preview-color.e2e.ts delete mode 100644 tests/specs/segment-shared-type.e2e.ts diff --git a/docs/configuration_file.md b/docs/configuration_file.md index 2f574e207..992ff0906 100644 --- a/docs/configuration_file.md +++ b/docs/configuration_file.md @@ -152,12 +152,8 @@ This removes the specified view types from the dropdown menu and replaces them i ## Segments -Paint, rectangles, polygons and rulers share one registry of segments, configured under -`segments`. Each entry is keyed by name, and every appearance field is optional: an -omitted one means the app default for a new segment. For an existing session segment, -omitted fields keep the appearance it had before configuration. Replacing a config entry -removes its previous appearance overrides, including color, while keeping the segment id, -visibility and lock state. +Use `segments` to define segments by name. Paint, rectangles, polygons and rulers on +an image share its segments. ```json { @@ -168,40 +164,35 @@ visibility and lock state. } ``` -Fields: +- `color`: a 3, 4, 6 or 8 digit hex value (4 and 8 include alpha), or a CSS color + keyword. Invalid colors are reported and ignored. +- `fillOpacity` and `outlineOpacity`: mask fill and outline opacity, from 0 to 1. + The Display sliders in "Annotations" scale these per image. +- `strokeWidth`: line width for rectangles, polygons and rulers. -- `color` takes a hex value of 3, 4, 6 or 8 digits such as `#ff0000` (the 4 and 8 digit - forms carry alpha) or a CSS color keyword such as `green`. Any other value is reported - as an error and ignored. -- `fillOpacity` and `outlineOpacity`, from 0 to 1, set how strongly the segment's masks - are filled and outlined. The Display sliders in "Annotations" scale them per image. -- `strokeWidth` sets the line width of the segment's rectangles, polygons and rulers. +All fields are optional. Omitted fields use the app defaults. -Omitting the key leaves the registry alone. An empty record (`{}`) or `null` drops the -config's entries: a segment the config created is deleted unless a mask or shape -references it, and any other segment stays with its last configured appearance. +A configuration loaded with images applies to those images. A configuration loaded on +its own applies to the image that was active when loading started. Either way, the +configured segments also become the starting set for new images with no earlier image +to copy from. -Entries are keyed by name. A configured segment keeps its id while its key stays the -same, so recoloring it never detaches its masks and shapes. Changing a key moves the -entry to the segment of the new name, adding one if none exists, and drops the old entry -as above. Applying a config again restores each key's name and configured appearance, -undoing a rename made in the app. - -Configured segments outlive the images. Removing the last image deletes every other -segment and keeps these. +Applying a configuration again restores each configured name and appearance. Omitting +`segments` changes nothing. `{}` or `null` removes the configured segments, except any +that hold masks or shapes. ### Legacy `labels` A legacy `labels` section is converted into `segments` at configuration ingestion, with a deprecation warning. Runtime configuration contains only `segments`. Its -`defaultLabels`, `rulerLabels`, `rectangleLabels` and `polygonLabels` all describe the one -registry now, so they read as `segments` entries. A name that appears in more than one +`defaultLabels`, `rulerLabels`, `rectangleLabels` and `polygonLabels` become `segments` +entries shared by the tools on each image. A name that appears in more than one becomes a single segment: the first record to declare it sets its appearance, reading `rulerLabels`, `rectangleLabels` and `polygonLabels` in that order and `defaultLabels` last. `defaultLabels` stood in for the tools that declared no record of their own, so it is read only when at least one of the three tool records is omitted. A section whose -records are all omitted or `null` leaves the registry alone. A rectangle label's -`fillColor` is dropped with a warning, since fill color is a property of the rectangle +records are all omitted or `null` leaves segment definitions unchanged. A rectangle +label's `fillColor` is dropped with a warning, since fill color is a property of the rectangle rather than of the segment. A config carrying both `segments` and `labels` has been converted already, so `segments` is read, `labels` is ignored, and the warning says so. diff --git a/docs/quick_start_guide.md b/docs/quick_start_guide.md index c4baa90de..6c5c17a72 100644 --- a/docs/quick_start_guide.md +++ b/docs/quick_start_guide.md @@ -35,7 +35,7 @@ The three main radiological controls are as following: - 2D Left mouse button: Window / Level, Pan, Zoom, or Crosshairs: Select these options to control the function of the left mouse button in the 2D windows. ![Window-Level, Pan, Zoom, Crosshairs](./assets/10-volview-wl-pan-zoom-notes.jpg) -- 2D Annotations: Paint, rectangles, polygons and rulers all draw into the segments listed in the "Annotations" tab. Segments are shared by every tool and image: select one in the list before drawing, and use the Paint controls below the list to set the brush size, erase, or set an intensity threshold. When the ruler tool is selected, the left mouse button places and adjusts ruler end-markers, and right clicking an end-marker displays a menu for deleting that ruler. The "Measurements" section of the "Annotations" tab lists the rectangles, polygons and rulers on the current image, with buttons to jump to their slice or delete them. See ["Toolbar controls"](toolbar.html#2d-annotations) for more. ![Paint and Ruler](./assets/11-volview-paint-notes.jpg) +- 2D Annotations: Paint, rectangles, polygons and rulers all draw into the current image's segments listed in the "Annotations" tab. Each image keeps its own segments and selection, shared by all four tools. Select a segment in the list before drawing, and use the Paint controls below the list to set the brush size, erase, or set an intensity threshold. When the ruler tool is selected, the left mouse button places and adjusts ruler end-markers, and right clicking an end-marker displays a menu for deleting that ruler. The "Measurements" section of the "Annotations" tab lists the rectangles, polygons and rulers on the current image, with buttons to jump to their slice or delete them. See ["Toolbar controls"](toolbar.html#2d-annotations) for more. ![Paint and Ruler](./assets/11-volview-paint-notes.jpg) - 3D Crop: Select this tool to adjust the extent of data shown in the 3D rendering. In the 3D window you can pick and move the corner, edge, and side markers to make adjustments. In the 2D windows, grab and move the edges of the bounding box overlaid on the data. ![Crop](./assets/13-volview-crop.jpg) diff --git a/docs/toolbar.md b/docs/toolbar.md index 5d9837135..a3b9faa19 100644 --- a/docs/toolbar.md +++ b/docs/toolbar.md @@ -14,19 +14,22 @@ Window / Level, Pan, Zoom, or Crosshairs: Select these options to control the fu ## 2D Annotations -The "Annotations" tab lists segments shared by paint, rectangles, polygons and rulers. +The "Annotations" tab lists the current image's segments, shared by paint, rectangles, +polygons and rulers. Select a segment in the list, use `q` and `w` to cycle through segments, or press `1` through `0` to select one of the first ten (each row shows its key). Use "New segment" to add one, and its color dot or edit button to change its name and appearance. Drag a row, or press Alt+Up or Alt+Down, to reorder segments and their keys. The selection -applies across all four tools and images. +applies across all four tools on that image. Each image keeps its own definitions, +order and selection. A new image copies the previous image's definitions once, +without copying its masks or shapes. Each segment row has Reveal, Edit, Lock, Hide and Delete buttons. Reveal jumps to the segment's mask or shapes on the current image and stays disabled when it has none there. A locked segment keeps its voxels when another segment paints over it, and cannot be edited or deleted until it is unlocked. Delete removes the segment with its -masks and shapes from every image. The buttons beside the "Segments" heading lock or -hide every segment at once and save the current image's segments. Saving writes one +masks and shapes from the current image. The buttons beside the "Segments" heading +lock or hide every segment on that image at once and save its segments. Saving writes one file, or several bundled into a zip when segments overlap or number more than 65535. The "Display" section sets Fill Opacity, Outline Opacity and Outline Thickness for the @@ -95,8 +98,9 @@ New rulers use the selected segment from "Annotations". ### Segment configuration -If VolView loads a JSON file matching the schema below, segments are added to the -registry. Paint, rectangles, polygons and rulers all share `segments`. Appearance +If VolView loads a JSON file matching the schema below, it configures segment +definitions for the destination images and defaults for future images. Paint, +rectangles, polygons and rulers share `segments` on each image. Appearance fields are optional. See [segment configuration](./configuration_file.md#segments) for replacement behavior and how omitted fields use session appearance or defaults. diff --git a/src/actions/__tests__/loadImageSegmentSets.spec.ts b/src/actions/__tests__/loadImageSegmentSets.spec.ts new file mode 100644 index 000000000..77a5e0bd9 --- /dev/null +++ b/src/actions/__tests__/loadImageSegmentSets.spec.ts @@ -0,0 +1,470 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { enableAutoUnmount, flushPromises, mount } from '@vue/test-utils'; +import { nextTick } from 'vue'; +import JSZip from 'jszip'; +import vtkXMLImageDataReader from '@kitware/vtk.js/IO/XML/XMLImageDataReader'; +import vtkSTLReader from '@kitware/vtk.js/IO/Geometry/STLReader'; +import { z } from 'zod'; + +import { loadDataSources, loadFiles } from '@/src/actions/loadUserFiles'; +import { FILE_READERS } from '@/src/io'; +import { FILE_EXT_TO_MIME } from '@/src/io/mimeTypes'; +import { readFile } from '@/src/io/vtk/common'; +import { fileToDataSource } from '@/src/io/import/dataSource'; +import { registerConfigSection } from '@/src/io/import/configJson'; +import { buildSegNrrdMetadata } from '@/src/io/segNrrdMetadata'; +import { useDatasetStore } from '@/src/store/datasets'; +import { useImageCacheStore } from '@/src/store/image-cache'; +import { useImageStore } from '@/src/store/datasets-images'; +import { useViewStore } from '@/src/store/views'; +import { useSegmentStore } from '@/src/segmentation/segments'; +import { useSegmentationStore } from '@/src/segmentation/store'; +import useLoadDataStore from '@/src/store/load-data'; +import SegmentAssignmentList from '@/src/segmentation/components/SegmentAssignmentList.vue'; +import { + activateAppPinia, + markedVoxels, + serializeAnnotations, + strokeAt, +} from '@/src/segmentation/__tests__/segmentMaskFixtures'; +import { defer } from '@/src/utils'; + +enableAutoUnmount(afterEach); + +const imageFile = (name: string, mask = false) => + new File( + [ + ` + + + +${Array.from({ length: 32 }, (_, i) => (mask ? Number(i === 5) : i)).join(' ')} +`, + ], + name + ); + +const configFile = (name: string, extra = {}) => + new File( + [JSON.stringify({ segments: { [name]: { color: '#ff0000' } }, ...extra })], + 'config.json' + ); +const gatedConfig = (name: string) => { + const file = configFile(name); + const readText = file.text.bind(file); + const entered = defer(); + const gate = defer(); + vi.spyOn(file, 'text').mockImplementation(async () => { + entered.resolve(); + await gate.promise; + return readText(); + }); + return { file, entered: entered.promise, release: () => gate.resolve() }; +}; +const badImage = () => new File(['not VTI'], 'broken.vti'); +const vtiMime = FILE_EXT_TO_MIME.vti; +const stlMime = FILE_EXT_TO_MIME.stl; +const originalReaders = new Map([ + [vtiMime, FILE_READERS.get(vtiMime)!], + [stlMime, FILE_READERS.get(stlMime)!], +]); +const conversionListeners: Array<() => void> = []; +const nextLabelmapConversion = () => { + const completed = defer(); + conversionListeners.push( + useSegmentationStore().$onAction(({ name, after, onError }) => { + if (name !== 'startLabelmapConversion') return; + after(() => completed.resolve()); + onError((error) => completed.reject(error)); + }) + ); + void completed.promise.catch(() => {}); + return completed.promise; +}; + +let slowEntered = defer(); +let slowGate = defer(); + +beforeEach(() => { + activateAppPinia(); + slowEntered = defer(); + slowGate = defer(); + // Run the production codecs in process; only their Worker transport differs. + FILE_READERS.set(vtiMime, async (file) => { + if (file.name === 'pending.vti') { + slowEntered.resolve(); + await slowGate.promise; + } + const dataObject = await readFile(file, vtkXMLImageDataReader); + const headerMetadata = file.name.endsWith('.seg.vti') + ? buildSegNrrdMetadata( + [{ value: 1, name: 'Tumor', color: [0, 0, 255, 255], visible: true }], + [4, 4, 2] + ) + : undefined; + return { dataObject, headerMetadata }; + }); + FILE_READERS.set(stlMime, async (file) => ({ + dataObject: await readFile(file, vtkSTLReader, false), + })); +}); + +afterEach(() => { + conversionListeners.splice(0).forEach((unsubscribe) => unsubscribe()); + originalReaders.forEach((reader, mime) => FILE_READERS.set(mime, reader)); + vi.restoreAllMocks(); +}); + +const openImage = async (name: string) => { + const ids = await loadFiles([imageFile(name)]); + expect(ids).toHaveLength(1); + return ids![0]; +}; +const picker = (imageId: string) => { + const wrapper = mount(SegmentAssignmentList, { + props: { imageId }, + global: { + stubs: { + VList: { template: '
' }, + VListItem: { template: '
    • ' }, + VListItemTitle: { template: '' }, + }, + }, + }); + return () => wrapper.findAll('.title').map((title) => title.text()); +}; +const paintedName = (imageId: string) => { + strokeAt(imageId, [1, 1, 0]); + const store = useSegmentationStore(); + const [maskId] = store.getSegmentationForImage(imageId)!.order; + expect(markedVoxels(maskId)!.length).toBeGreaterThan(0); + return useSegmentStore().segments.appearanceOf( + store.getMask(maskId).segmentId + ).name; +}; +const duringPendingImage = async (check: () => Promise) => { + const pending = loadFiles([imageFile('pending.vti')]); + await slowEntered.promise; + try { + await check(); + } finally { + slowGate.resolve(); + await pending; + } +}; +const loadContainedFiles = async ( + container: 'files' | 'collection' | 'archive', + files: File[] +) => { + if (container === 'collection') + return loadDataSources([ + { type: 'collection', sources: files.map(fileToDataSource) }, + ]); + if (container === 'archive') { + const zip = new JSZip(); + for (const file of files) zip.file(file.name, await file.text()); + return loadFiles([ + new File([await zip.generateAsync({ type: 'arraybuffer' })], 'batch.zip'), + ]); + } + return loadFiles(files); +}; + +describe('image segment initialization through public loading', () => { + it.each(['configured', 'borrowed'] as const)( + 'keeps %s definitions usable while an unrelated image is unfinished', + async (source) => { + if (source === 'configured') await loadFiles([configFile('Liver')]); + else { + const first = await openImage('first.vti'); + useSegmentStore().segments.addSegment(first, { name: 'Liver' }); + } + await duringPendingImage(async () => { + const completed = await openImage('completed.vti'); + expect(useLoadDataStore().isLoading).toBe(true); + useViewStore().setDataForAllViews(completed); + await nextTick(); + expect(picker(completed)()).toEqual(['Liver']); + expect(paintedName(completed)).toBe('Liver'); + }); + } + ); + + it('seeds configured defaults with no other operation pending', async () => { + await loadFiles([configFile('Liver')]); + const completed = await openImage('completed.vti'); + expect(picker(completed)()).toEqual(['Liver']); + expect(paintedName(completed)).toBe('Liver'); + }); + + it('merges its accompanying segmentation into the configured defaults', async () => { + await loadFiles([configFile('Liver')]); + await duringPendingImage(async () => { + const converted = nextLabelmapConversion(); + const ids = await loadFiles([ + imageFile('scan.vti'), + imageFile('scan.seg.vti', true), + ]); + const views = useViewStore(); + const imageId = views.getView(views.activeView)!.dataID!; + expect(ids).toContain(imageId); + const names = picker(imageId); + await converted; + await flushPromises(); + expect(names()).toEqual(['Liver', 'Tumor']); + expect(paintedName(imageId)).toBe('Liver'); + }); + }); + + it('applies a companion config to an image shown before the config is read', async () => { + await loadFiles([configFile('Liver')]); + const { + file: config, + entered: configEntered, + release: releaseConfig, + } = gatedConfig('Tumor'); + const published = defer(); + const unsubscribe = useImageStore().$onAction(({ name, args, after }) => { + if (name === 'addVTKImageData' && args[0] === 'owned.vti') + after((id) => published.resolve(id)); + }); + const unrelated = loadFiles([imageFile('pending.vti')]); + await slowEntered.promise; + const own = loadFiles([imageFile('owned.vti'), config]); + try { + const imageId = await published.promise; + await configEntered; + useViewStore().setDataForAllViews(imageId); + await nextTick(); + const names = picker(imageId); + expect(names()).toEqual(['Liver']); + slowGate.resolve(); + await unrelated; + await nextTick(); + expect(names()).toEqual(['Liver']); + releaseConfig(); + await own; + await nextTick(); + expect(names()).toEqual(['Tumor']); + expect(paintedName(imageId)).toBe('Tumor'); + } finally { + unsubscribe(); + releaseConfig(); + slowGate.resolve(); + await Promise.all([own, unrelated]); + } + }); +}); + +describe('standalone config destination during asynchronous loading', () => { + it.each([ + 'focus changed', + 'no initial image', + 'initial image removed', + ] as const)( + 'keeps the destination captured when loading began: %s', + async (transition) => { + const initial = + transition === 'no initial image' + ? undefined + : (await loadFiles([ + imageFile('initial.vti'), + configFile('Liver'), + ]))![0]; + let unrelated: string | undefined; + if (transition === 'focus changed') { + [unrelated] = (await loadFiles([ + imageFile('unrelated.vti'), + configFile('Kidney'), + ]))!; + useViewStore().setDataForAllViews(initial!); + } + + const { + file: config, + entered: configEntered, + release: releaseConfig, + } = gatedConfig('Tumor'); + const loading = loadFiles([config]); + try { + await configEntered; + if (transition === 'initial image removed') + useDatasetStore().remove(initial!); + if (transition !== 'focus changed') + unrelated = await openImage('unrelated.vti'); + useViewStore().setDataForAllViews(unrelated!); + await nextTick(); + const names = picker(unrelated!); + const expected = + transition === 'focus changed' + ? ['Kidney'] + : transition === 'initial image removed' + ? ['Liver'] + : []; + expect(names()).toEqual(expected); + + releaseConfig(); + await loading; + await nextTick(); + expect(names()).toEqual(expected); + if (transition === 'focus changed') + expect(picker(initial!)()).toEqual(['Tumor']); + if (transition === 'initial image removed') + expect( + serializeAnnotations().imageSegmentSets?.find( + ({ imageId }) => imageId === initial + ) + ).toBeUndefined(); + expect( + serializeAnnotations().seedSegmentDefaults?.map(({ name }) => name) + ).toEqual(['Tumor']); + } finally { + releaseConfig(); + await loading; + } + } + ); + + it.each( + (['files', 'collection', 'archive'] as const).flatMap((container) => [ + { + container, + failure: 'recognized config validation', + contents: JSON.stringify({ segments: 17 }), + expected: ['Tumor'], + }, + { + container, + failure: 'unrecognized JSON syntax', + contents: '{', + expected: ['Liver'], + }, + ]) + )( + 'keeps standalone config scope with a $failure failure through $container', + async ({ container, contents, expected }) => { + const [active] = (await loadFiles([ + imageFile('active.vti'), + configFile('Liver'), + ]))!; + const setError = vi.spyOn(useLoadDataStore(), 'setError'); + const ids = await loadContainedFiles(container, [ + configFile('Tumor'), + new File([contents], 'invalid-config.json'), + ]); + await nextTick(); + expect(ids).toEqual([]); + expect(picker(active)()).toEqual(expected); + expect( + serializeAnnotations().seedSegmentDefaults?.map(({ name }) => name) + ).toEqual(['Tumor']); + expect(setError).toHaveBeenCalledExactlyOnceWith( + expect.objectContaining({ + message: expect.stringContaining('invalid-config.json'), + }) + ); + } + ); +}); + +describe('background labelmap conversion', () => { + it('finishes the file load while its segmentation is still streaming', async () => { + await loadFiles([configFile('Liver')]); + const cache = useImageCacheStore(); + let streamingId: string | undefined; + const unsubscribe = cache.$onAction(({ name, args, after }) => { + if (name === 'addVTKImageData' && args[1] === 'stream.seg.vti') + after((id) => { + streamingId = id; + cache.imageById[id].status.value = 'incomplete'; + }); + }); + try { + const converted = nextLabelmapConversion(); + const ids = await loadFiles([ + imageFile('stream.vti'), + imageFile('stream.seg.vti', true), + ]); + expect(ids).toHaveLength(2); + expect(useLoadDataStore().isLoading).toBe(false); + const views = useViewStore(); + const imageId = views.getView(views.activeView)!.dataID!; + const names = picker(imageId); + expect(names()).toEqual(['Liver']); + cache.imageById[streamingId!].status.value = 'complete'; + await converted; + await flushPromises(); + expect(names()).toEqual(['Liver', 'Tumor']); + expect(paintedName(imageId)).toBe('Liver'); + } finally { + unsubscribe(); + if (streamingId) cache.imageById[streamingId].status.value = 'complete'; + } + }); +}); + +describe('destination scope when companion data imports fail', () => { + it.each(['files', 'collection', 'archive'] as const)( + 'does not apply the companion config to the active image through %s', + async (container) => { + const [active] = (await loadFiles([ + imageFile('active.vti'), + configFile('Liver'), + ]))!; + const ids = await loadContainedFiles(container, [ + badImage(), + configFile('Tumor'), + ]); + await nextTick(); + expect(ids).toEqual([]); + expect(picker(active)()).toEqual(['Liver']); + expect( + serializeAnnotations().seedSegmentDefaults?.map(({ name }) => name) + ).toEqual(['Tumor']); + } + ); + + it('keeps model-only companion config off the active image', async () => { + const [active] = (await loadFiles([ + imageFile('active.vti'), + configFile('Liver'), + ]))!; + const model = new File( + [ + `solid triangle +facet normal 0 0 1 +outer loop +vertex 0 0 0 +vertex 1 0 0 +vertex 0 1 0 +endloop +endfacet +endsolid triangle`, + ], + 'triangle.stl' + ); + await loadFiles([model, configFile('Tumor')]); + expect(picker(active)()).toEqual(['Liver']); + }); + + it.each([false, true])( + 'still applies standalone config with a pre-state failure of %s', + async (failPreState) => { + const [active] = (await loadFiles([ + imageFile('active.vti'), + configFile('Liver'), + ]))!; + registerConfigSection({ + key: 'loadingBoundaryFailure', + schema: z.boolean().optional(), + apply: (value) => { + if (value) throw new Error('Unavailable config section'); + }, + }); + await loadFiles([ + configFile('Tumor', { loadingBoundaryFailure: failPreState }), + ]); + expect(picker(active)()).toEqual(['Tumor']); + } + ); +}); diff --git a/src/actions/loadUserFiles.ts b/src/actions/loadUserFiles.ts index f0fa45f68..3efbe78b2 100644 --- a/src/actions/loadUserFiles.ts +++ b/src/actions/loadUserFiles.ts @@ -265,7 +265,8 @@ function loadSegmentations( const loadable = toDataSelection(ds); segmentationStore.startLabelmapConversion( loadable, - toDataSelection(primaryDataSource) + toDataSelection(primaryDataSource), + { seedDefaults: true } ); }); } diff --git a/src/components/EditableItemList.vue b/src/components/EditableItemList.vue index 60fb9bdfd..a578de825 100644 --- a/src/components/EditableItemList.vue +++ b/src/components/EditableItemList.vue @@ -3,6 +3,7 @@ import { nextTick, ref, watch } from 'vue'; import { Maybe } from '@/src/types'; +import ReasonedAction from '@/src/components/ReasonedAction.vue'; const emit = defineEmits<{ create: []; @@ -13,6 +14,7 @@ const emit = defineEmits<{ const props = defineProps<{ items: Array; createText: string; + createReason?: string; reorderHint: string; modelValue: Maybe; selectionRevision?: number; @@ -171,18 +173,21 @@ const moveBy = async (id: string, offset: number, event: KeyboardEvent) => {
    - -
    - mdi-plus - {{ createText }} -
    -     + + +
    @@ -205,7 +210,7 @@ const moveBy = async (id: string, offset: number, event: KeyboardEvent) => { } .v-list-item.item-row, -.v-list-item.create-row { +.v-list-item.create-row-content { border: 1px solid transparent; border-radius: 4px; /* The default inset would dwarf the gaps between the row's parts. */ @@ -231,6 +236,18 @@ const moveBy = async (id: string, offset: number, event: KeyboardEvent) => { .create-row { opacity: var(--v-medium-emphasis-opacity); + text-align: left; + border-radius: 4px; +} + +.create-row:hover:enabled, +.create-row:focus-visible { + background-color: rgba(var(--v-theme-on-surface), var(--v-hover-opacity)); +} + +.create-row:disabled { + opacity: var(--v-disabled-opacity); + pointer-events: none; } .create-icon { diff --git a/src/components/LayoutGridItem.vue b/src/components/LayoutGridItem.vue index f8adb33c4..87e0e8d5f 100644 --- a/src/components/LayoutGridItem.vue +++ b/src/components/LayoutGridItem.vue @@ -54,8 +54,8 @@ function onDrop(event: DragEvent) { const droppedImageID = event.dataTransfer?.getData(IMAGE_DRAG_MEDIA_TYPE); if (droppedImageID) { - viewStore.setActiveView(props.viewId); viewStore.setDataForView(props.viewId, droppedImageID); + viewStore.setActiveView(props.viewId); } } diff --git a/src/components/MeasurementsToolList.vue b/src/components/MeasurementsToolList.vue index f27a2f23e..b5c5980ed 100644 --- a/src/components/MeasurementsToolList.vue +++ b/src/components/MeasurementsToolList.vue @@ -169,6 +169,7 @@ function toggleSelectedHidden() { diff --git a/src/components/__tests__/EditableItemList.spec.ts b/src/components/__tests__/EditableItemList.spec.ts new file mode 100644 index 000000000..50c85c444 --- /dev/null +++ b/src/components/__tests__/EditableItemList.spec.ts @@ -0,0 +1,36 @@ +import { afterEach, describe, expect, it } from 'vitest'; +import { enableAutoUnmount, mount } from '@vue/test-utils'; +import { nextTick } from 'vue'; +import { createVuetify } from 'vuetify'; + +import EditableItemList from '@/src/components/EditableItemList.vue'; + +enableAutoUnmount(afterEach); + +describe('creating an item', () => { + it('uses a disabled native button until the blocking reason clears', async () => { + const wrapper = mount(EditableItemList, { + props: { + items: [], + modelValue: undefined, + createText: 'New segment', + createReason: 'Select an image to edit segments', + reorderHint: 'Drag to reorder', + }, + global: { plugins: [createVuetify()] }, + }); + const button = wrapper.get('.create-row'); + + expect(button.element.tagName).toBe('BUTTON'); + expect(button.element.disabled).toBe(true); + button.element.click(); + button.element.dispatchEvent(new MouseEvent('click', { bubbles: true })); + await nextTick(); + expect(wrapper.emitted('create')).toBeUndefined(); + + await wrapper.setProps({ createReason: '' }); + expect(button.element.disabled).toBe(false); + button.element.click(); + expect(wrapper.emitted('create')).toEqual([[]]); + }); +}); diff --git a/src/components/__tests__/MeasurementsToolList.spec.ts b/src/components/__tests__/MeasurementsToolList.spec.ts index 3e71b3309..d85e5ff4b 100644 --- a/src/components/__tests__/MeasurementsToolList.spec.ts +++ b/src/components/__tests__/MeasurementsToolList.spec.ts @@ -86,7 +86,7 @@ describe('measurement list', () => { it('shows a shape of a hidden segment as hidden and says why its eye is off', () => { const { segments } = useSegmentStore(); - const segmentId = segments.mintSegment({ name: 'Tumor' }); + const segmentId = segments.mintSegment('img-1', { name: 'Tumor' }); drawRuler(segmentId); segments.updateSegment(segmentId, { visible: false }); @@ -100,7 +100,7 @@ describe('measurement list', () => { }); it('names the shape of an unnamed segment', () => { - drawRuler(useSegmentStore().segments.mintSegment({ name: '' })); + drawRuler(useSegmentStore().segments.mintSegment('img-1', { name: '' })); expect(mountList().get('.row-title').text()).toBe('(no name)'); }); @@ -123,7 +123,9 @@ describe('measurement list', () => { }); it('says why the selection actions wait for a selection', () => { - drawRuler(useSegmentStore().segments.mintSegment({ name: 'Tumor' })); + drawRuler( + useSegmentStore().segments.mintSegment('img-1', { name: 'Tumor' }) + ); const wrapper = mountList(); const hide = actionLabelled(wrapper, /^Hide selected/); @@ -141,8 +143,8 @@ describe('measurement list', () => { it('selects all rows from a partial selection and clears them on a second click', async () => { const { segments } = useSegmentStore(); - drawRuler(segments.mintSegment({ name: 'Tumor' })); - drawRuler(segments.mintSegment({ name: 'Node' })); + drawRuler(segments.mintSegment('img-1', { name: 'Tumor' })); + drawRuler(segments.mintSegment('img-1', { name: 'Node' })); const wrapper = mountList(); const selectAll = wrapper.get( 'input[aria-label="Select all measurements"]' @@ -185,9 +187,12 @@ describe('measurement list', () => { it('hides and shows selected measurements without changing hidden-segment children or unselected rows', async () => { const { segments } = useSegmentStore(); - const tumor = segments.mintSegment({ name: 'Tumor' }); - const node = segments.mintSegment({ name: 'Node', visible: false }); - const other = segments.mintSegment({ name: 'Other' }); + const tumor = segments.mintSegment('img-1', { name: 'Tumor' }); + const node = segments.mintSegment('img-1', { + name: 'Node', + visible: false, + }); + const other = segments.mintSegment('img-1', { name: 'Other' }); drawRuler(tumor); drawRuler(tumor, { slice: 1, hidden: true }); drawRuler(node, { slice: 2 }); @@ -261,7 +266,10 @@ describe('measurement list', () => { it('explains why a selection under hidden segments cannot be shown or hidden', async () => { drawRuler( - useSegmentStore().segments.mintSegment({ name: 'Tumor', visible: false }) + useSegmentStore().segments.mintSegment('img-1', { + name: 'Tumor', + visible: false, + }) ); const wrapper = mountList(); await wrapper @@ -283,11 +291,11 @@ describe('measurement list', () => { it('deletes selected rulers and rectangles while keeping an unselected measurement', async () => { const { segments } = useSegmentStore(); - drawRuler(segments.mintSegment({ name: 'Tumor' })); - drawRuler(segments.mintSegment({ name: 'Other' })); + drawRuler(segments.mintSegment('img-1', { name: 'Tumor' })); + drawRuler(segments.mintSegment('img-1', { name: 'Other' })); useRectangleStore().addTool({ imageID: IMAGE_ID, - segmentId: segments.mintSegment({ name: 'Node' }), + segmentId: segments.mintSegment('img-1', { name: 'Node' }), slice: 0, frameOfReference: AXIAL_FRAME_OF_REFERENCE, }); @@ -318,11 +326,14 @@ describe('measurement list', () => { }); it('shows a cine ruler with a one-based frame caption and its measured length', () => { - drawRuler(useSegmentStore().segments.mintSegment({ name: 'Tumor' }), { - frame: 7, - firstPoint: [0, 0, 0], - secondPoint: [3, 4, 0], - }); + drawRuler( + useSegmentStore().segments.mintSegment('img-1', { name: 'Tumor' }), + { + frame: 7, + firstPoint: [0, 0, 0], + secondPoint: [3, 4, 0], + } + ); const row = mountList().get('[data-testid="segment-shape-row"]'); expect(row.get('.row-subtitle').text()).toBe('Frame 8 5.00mm'); @@ -333,8 +344,8 @@ describe('measurement list', () => { it('reassigns only the measurement whose segment picker is used', async () => { const registry = useSegmentStore().segments; - const first = registry.mintSegment({ name: 'Tumor' }); - registry.mintSegment({ name: 'Node' }); + const first = registry.mintSegment('img-1', { name: 'Tumor' }); + registry.mintSegment('img-1', { name: 'Node' }); drawRuler(first); drawRuler(first); const wrapper = mountList(); diff --git a/src/components/tools/paint/PaintWidget2D.vue b/src/components/tools/paint/PaintWidget2D.vue index 8f5b1d0d3..694560f21 100644 --- a/src/components/tools/paint/PaintWidget2D.vue +++ b/src/components/tools/paint/PaintWidget2D.vue @@ -71,10 +71,13 @@ export default defineComponent({ // --- widget representation config --- // const { segments } = useSegmentStore(); + const pointerInView = ref(false); watchEffect(() => { - const [r, g, b] = segments.appearanceOf( - segments.selectedSegmentId.value - ).color; + if (!pointerInView.value) return; + const segmentId = segments.selectedSegmentId(imageId.value); + const [r, g, b] = segmentId + ? segments.appearanceOf(segmentId).color + : segments.nextSegmentColor(imageId.value); widgetState.getBrush().setColor3([r, g, b]); }); @@ -151,7 +154,6 @@ export default defineComponent({ // --- visibility --- // let checkIfPointerInView = false; - const pointerInView = ref(false); watchEffect(() => { if (pointerInView.value && imageId.value) { paintStore.setSliceAxis(viewAxisIndex.value, imageId.value); @@ -173,16 +175,17 @@ export default defineComponent({ }); // Turn on widget visibility and update stencil if mouse starts within view + const enterView = () => { + pointerInView.value = true; + }; const showPreviewOnFirstMove = () => { if (!checkIfPointerInView) return; checkIfPointerInView = false; - pointerInView.value = true; + enterView(); }; onVTKEvent(view.interactor, 'onMouseMove', showPreviewOnFirstMove); - onVTKEvent(view.interactor, 'onMouseEnter', () => { - pointerInView.value = true; - }); + onVTKEvent(view.interactor, 'onMouseEnter', enterView); onVTKEvent(view.interactor, 'onMouseLeave', () => { pointerInView.value = false; diff --git a/src/components/tools/polygon/PolygonTool.vue b/src/components/tools/polygon/PolygonTool.vue index 964680e8f..69f56c473 100644 --- a/src/components/tools/polygon/PolygonTool.vue +++ b/src/components/tools/polygon/PolygonTool.vue @@ -213,7 +213,7 @@ export default defineComponent({ return 'Rasterization is not supported for cine images'; if (!rasterizeFrame(toolId)) return 'The polygon must align with a loaded image slice'; - return rasterizeTargetDisabledReason(tool.segmentId); + return rasterizeTargetDisabledReason(tool.imageID, tool.segmentId); } function rasterize(toolId: ToolID) { diff --git a/src/components/tools/polygon/__tests__/PolygonTool.spec.ts b/src/components/tools/polygon/__tests__/PolygonTool.spec.ts index ab81a7ccd..37266b6d9 100644 --- a/src/components/tools/polygon/__tests__/PolygonTool.spec.ts +++ b/src/components/tools/polygon/__tests__/PolygonTool.spec.ts @@ -55,7 +55,7 @@ const rasterizeButton = (wrapper: ReturnType) => .find((button) => button.text().includes('Rasterize'))!; const makePolygon = (imageID: string, slice = 2) => { - const segmentId = useSegmentStore().segments.addSegment({ + const segmentId = useSegmentStore().segments.addSegment(imageID, { name: 'Tumor', }); return usePolygonStore().addTool({ @@ -86,7 +86,7 @@ describe('PolygonTool rasterize context', () => { else await seatImage('b', { dimensions: [6, 6, 4] }); await showImage('a'); const polygonId = makePolygon('a'); - useSegmentStore().segments.addSegment({ name: 'Kidney' }); + useSegmentStore().segments.addSegment('b', { name: 'Kidney' }); const wrapper = mountMenu(polygonId, 'a'); await showImage('b'); diff --git a/src/components/tools/ruler/RulerTool.vue b/src/components/tools/ruler/RulerTool.vue index b38ff8b89..8e0a3dc75 100644 --- a/src/components/tools/ruler/RulerTool.vue +++ b/src/components/tools/ruler/RulerTool.vue @@ -10,6 +10,7 @@ :view-id="viewId" :view-direction="viewDirection" @contextmenu="openContextMenu(ruler.id, $event)" + @placing="onPlacementStarted" @placed="onRulerPlaced" @widgetHover="onHover(ruler.id, $event)" /> @@ -129,6 +130,7 @@ export default defineComponent({ return { rulers: currentRulers, placingRulerID: placingTool.id, + onPlacementStarted: placingTool.beginPlacement, onRulerPlaced, contextMenu, openContextMenu, diff --git a/src/components/tools/ruler/RulerWidget2D.vue b/src/components/tools/ruler/RulerWidget2D.vue index 8e06ba3e2..59356bb96 100644 --- a/src/components/tools/ruler/RulerWidget2D.vue +++ b/src/components/tools/ruler/RulerWidget2D.vue @@ -37,7 +37,7 @@ import { toolRenderSlice } from '@/src/core/annotations/locator'; export default defineComponent({ name: 'RulerWidget2D', - emits: ['placed', 'contextmenu', 'widgetHover'], + emits: ['placing', 'placed', 'contextmenu', 'widgetHover'], props: { toolId: { type: String as unknown as PropType, @@ -103,7 +103,11 @@ export default defineComponent({ } }); - // --- placed event --- // + // --- placement events --- // + + onVTKEvent(widget, 'onStartInteractionEvent', () => { + if (isPlacing.value) emit('placing'); + }); onVTKEvent(widget, 'onPlacedEvent', () => { emit('placed'); diff --git a/src/composables/__tests__/annotationTool.spec.ts b/src/composables/__tests__/annotationTool.spec.ts index 50420eb3e..c7b8d41e8 100644 --- a/src/composables/__tests__/annotationTool.spec.ts +++ b/src/composables/__tests__/annotationTool.spec.ts @@ -30,7 +30,7 @@ describe.each([ it('keeps the active placement alive through hiding, committing and starting again', () => { const tools = useStore(); const segments = useSegmentStore().segments; - const segmentId = segments.addSegment(); + const segmentId = segments.addSegment('img-1'); const metadata = ref({ imageID: 'img-1', segmentId, @@ -64,6 +64,15 @@ describe.each([ expect(ids()).toEqual([first, placing.id.value]); placing.remove(); expect(ids()).toEqual([first]); + + placing.add(); + const next = placing.id.value!; + whitelist.value = [next]; + segments.deleteSegment(segmentId); + tools.updateTool(next, { slice: 1 }); + placing.beginPlacement(); + placing.commit(); + expect(ids()).toEqual([next]); }); }); @@ -71,7 +80,7 @@ describe.each([ ['ruler', useRulerStore], ['rectangle', useRectangleStore], ['polygon', usePolygonStore], -] as const)('shared segment visibility for a %s', (_name, useStore) => { +] as const)('image-owned segment visibility for a %s', (_name, useStore) => { beforeEach(async () => { setActivePinia(createPinia()); await viewImage('img-1'); @@ -80,11 +89,12 @@ describe.each([ it('filters hidden segments and shapes across images and cine frames', async () => { const tools = useStore(); const segments = useSegmentStore().segments; - const segmentId = segments.addSegment(); + const segmentId = segments.addSegment('img-1'); + const cineSegmentId = segments.addSegment('cine-1'); const addShape = (imageID: string, hidden = false, frame?: number) => tools.addTool({ imageID, - segmentId, + segmentId: imageID === 'cine-1' ? cineSegmentId : segmentId, slice: 0, frameOfReference: AXIAL_FRAME_OF_REFERENCE, hidden, @@ -104,16 +114,20 @@ describe.each([ expect(ids()).toEqual([]); await showImage('cine-1'); viewFrame.value = 0; + expect(ids()).toEqual([cineFirst]); + segments.updateSegment(cineSegmentId, { visible: false }); expect(ids()).toEqual([]); - segments.updateSegment(segmentId, { visible: true }); + segments.updateSegment(cineSegmentId, { visible: true }); expect(ids()).toEqual([cineFirst]); viewFrame.value = 1; expect(ids()).toEqual([cineSecond]); - segments.updateSegment(segmentId, { visible: false }); + segments.updateSegment(cineSegmentId, { visible: false }); expect(ids()).toEqual([]); - segments.updateSegment(segmentId, { visible: true }); + segments.updateSegment(cineSegmentId, { visible: true }); await showImage('img-1'); viewFrame.value = undefined; + expect(ids()).toEqual([]); + segments.updateSegment(segmentId, { visible: true }); expect(ids()).toEqual([shown]); }); }); diff --git a/src/composables/actions.ts b/src/composables/actions.ts index d515b0460..915c6ef4c 100644 --- a/src/composables/actions.ts +++ b/src/composables/actions.ts @@ -12,12 +12,15 @@ import { usePaintToolStore } from '../store/tools/paint'; import { PaintMode } from '../core/tools/paint'; import { computeEffectiveView } from '../core/views/effectiveView'; -// One registry holds the segments every tool draws into, so cycling it is not -// scoped to a tool: paint takes the selection the same way a polygon does. +// Each image shares its selection across tools, so cycling applies equally to +// paint and shape placement on the active image. const applySegmentOffset = (offset: number) => () => { const { segments } = useSegmentStore(); - const ids = segments.segmentList.value.map(({ id }) => id); - const selectedIndex = ids.indexOf(segments.selectedSegmentId.value ?? ''); + const { currentImageID } = useCurrentImage('global'); + const ids = segments.segmentList(currentImageID.value).map(({ id }) => id); + const selectedIndex = ids.indexOf( + segments.selectedSegmentId(currentImageID.value) ?? '' + ); if (selectedIndex === -1) return; // A negative index wraps, so cycling back from the first lands on the last. const next = ids.at((selectedIndex + offset) % ids.length); @@ -26,7 +29,8 @@ const applySegmentOffset = (offset: number) => () => { const selectSegmentByIndex = (index: number) => () => { const { segments } = useSegmentStore(); - const segment = segments.segmentList.value[index]; + const { currentImageID } = useCurrentImage('global'); + const segment = segments.segmentList(currentImageID.value)[index]; if (segment) segments.selectSegment(segment.id); }; diff --git a/src/composables/annotationTool.ts b/src/composables/annotationTool.ts index 598c0d439..fc00621d4 100644 --- a/src/composables/annotationTool.ts +++ b/src/composables/annotationTool.ts @@ -235,7 +235,7 @@ export const usePlacingAnnotationTool = ( // The stub follows the selection, so it is drawn in the segment it will join. const patch = computed(() => ({ ...metadata.value, - segmentId: selectedSegmentId.value ?? '', + segmentId: selectedSegmentId(metadata.value.imageID) ?? '', })); const commit = () => { diff --git a/src/enface/__tests__/controls.spec.ts b/src/enface/__tests__/controls.spec.ts index e96663ded..9af82397d 100644 --- a/src/enface/__tests__/controls.spec.ts +++ b/src/enface/__tests__/controls.spec.ts @@ -444,7 +444,9 @@ function mountImageControls(spacing = 0.01) { const cache = useImageCacheStore(); const imageId = cache.addVTKImageData(source, 'relative-spacing.vti'); const segmentation = useSegmentationStore(); - const segmentId = useSegmentStore().segments.addSegment({ name: 'Retina' }); + const segmentId = useSegmentStore().segments.addSegment(imageId, { + name: 'Retina', + }); const mask = segmentation.createMask( segmentation.ensureSegmentationForImage(imageId).id, segmentId diff --git a/src/enface/__tests__/inputCalibration.spec.ts b/src/enface/__tests__/inputCalibration.spec.ts index 83d907b2a..b95d07a4f 100644 --- a/src/enface/__tests__/inputCalibration.spec.ts +++ b/src/enface/__tests__/inputCalibration.spec.ts @@ -48,7 +48,7 @@ async function inputFor(pixelSpacing: string, oblique = false) { async function mountInput(id: string) { const image = useImageCacheStore().getVtkImageData(id)!; const segmentation = useSegmentationStore(); - const segment = useSegmentStore().segments.addSegment({ name: 'Layer' }); + const segment = useSegmentStore().segments.addSegment(id, { name: 'Layer' }); const mask = segmentation.createMask( segmentation.ensureSegmentationForImage(id).id, segment diff --git a/src/enface/__tests__/restoreSelection.spec.ts b/src/enface/__tests__/restoreSelection.spec.ts index 5a4eeb51a..71c4ac31f 100644 --- a/src/enface/__tests__/restoreSelection.spec.ts +++ b/src/enface/__tests__/restoreSelection.spec.ts @@ -63,6 +63,9 @@ it('keeps the projection ready and restores the non-first segment after deferred visible: true, locked: false, })), + imageSegmentSets: [ + { imageId: 'saved-oct', segmentIds: ['first', 'chosen'] }, + ], segmentations: [ { id: 'segmentation', @@ -102,8 +105,8 @@ it('keeps the projection ready and restores the non-first segment after deferred }, }, }); - const { segmentIdMap } = useSegmentStore().deserialize(manifest); const dataIDMap = { 'saved-oct': 'oct' }; + const { segmentIdMap } = useSegmentStore().deserialize(manifest, dataIDMap); useViewConfigStore().deserializeAll(manifest, dataIDMap, segmentIdMap); const reading = defer(); const release = defer(); diff --git a/src/enface/__tests__/thinRegionAppearance.spec.ts b/src/enface/__tests__/thinRegionAppearance.spec.ts index 889bce294..5b67101ac 100644 --- a/src/enface/__tests__/thinRegionAppearance.spec.ts +++ b/src/enface/__tests__/thinRegionAppearance.spec.ts @@ -20,7 +20,9 @@ function appearanceScene() { const store = useSegmentationStore(); const registry = useSegmentStore().segments; const group = store.ensureSegmentationForImage(parentId); - const segmentId = registry.addSegment({ color: [35, 215, 190, 255] }); + const segmentId = registry.addSegment(parentId, { + color: [35, 215, 190, 255], + }); const mask = store.createMask(group.id, segmentId); const binding = store.ensureLabelmapBinding(mask.id); const maskId = ref(mask.id); @@ -70,7 +72,9 @@ describe('thin-region appearance from the source segmentation', () => { it('changes appearance with the selected mask and drops missing selections', () => { const { appearance, registry, store, group, maskId } = appearanceScene(); - const second = registry.addSegment({ color: [0, 255, 0, 255] }); + const second = registry.addSegment(group.parentImageId, { + color: [0, 255, 0, 255], + }); const mask = store.createMask(group.id, second); store.ensureLabelmapBinding(mask.id); maskId.value = mask.id; diff --git a/src/io/import/__tests__/configJson.spec.ts b/src/io/import/__tests__/configJson.spec.ts index 48409d6a9..0752a7577 100644 --- a/src/io/import/__tests__/configJson.spec.ts +++ b/src/io/import/__tests__/configJson.spec.ts @@ -7,14 +7,17 @@ import { recognizeConfig, } from '@/src/io/import/configJson'; import { useSegmentStore } from '@/src/segmentation/segments'; +import { viewImage } from '@/src/segmentation/__tests__/segmentMaskFixtures'; const segments = () => useSegmentStore().segments; const segmentSummary = () => - segments().segmentList.value.map((segment) => ({ - name: segment.name, - color: segments().appearanceOf(segment.id).cssColor, - })); + segments() + .segmentList('image') + .map((segment) => ({ + name: segment.name, + color: segments().appearanceOf(segment.id).cssColor, + })); describe('config schema', () => { describe('shortcuts', () => { @@ -86,8 +89,9 @@ describe('config schema', () => { }); describe('segment config', () => { - beforeEach(() => { + beforeEach(async () => { setActivePinia(createPinia()); + await viewImage('image'); }); it('reports config colors it cannot parse', () => { @@ -100,7 +104,8 @@ describe('segment config', () => { Fine: { color: '#00ff00' }, }, }), - addError + addError, + ['image'] ); expect(addError).toHaveBeenCalledTimes(1); const [[message]] = addError.mock.calls; @@ -112,10 +117,11 @@ describe('segment config', () => { describe('legacy labels', () => { const applyLabels = (labels: unknown) => - applyPostStateConfig(config.parse({ labels })); + applyPostStateConfig(config.parse({ labels }), undefined, ['image']); - beforeEach(() => { + beforeEach(async () => { setActivePinia(createPinia()); + await viewImage('image'); }); // Only known top-level keys mark a file as config, so a config that names diff --git a/src/io/import/__tests__/crossImageAnnotationNotice.spec.ts b/src/io/import/__tests__/crossImageAnnotationNotice.spec.ts new file mode 100644 index 000000000..3ae5e3cd4 --- /dev/null +++ b/src/io/import/__tests__/crossImageAnnotationNotice.spec.ts @@ -0,0 +1,87 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { createStateFileRestorer } from '@/src/io/import/processors/restoreStateFile'; +import { useRulerStore } from '@/src/store/tools/rulers'; +import { useRectangleStore } from '@/src/store/tools/rectangles'; +import { usePolygonStore } from '@/src/store/tools/polygons'; +import { + activateAppPinia, + manifestForImages, + seatImage, +} from '@/src/segmentation/__tests__/segmentMaskFixtures'; + +const stores = { + rulers: useRulerStore, + rectangles: useRectangleStore, + polygons: usePolygonStore, +}; + +const shape = (imageID: string, segmentId: string, name: string) => ({ + imageID, + segmentId, + name, + placing: false, + slice: 0, + frameOfReference: { planeOrigin: [0, 0, 0], planeNormal: [0, 0, 1] }, + firstPoint: [0, 0, 0], + secondPoint: [1, 1, 0], + points: [ + [0, 0, 0], + [1, 0, 0], + [1, 1, 0], + ], +}); + +describe('cross-image annotation restore notices', () => { + beforeEach(async () => { + activateAppPinia(); + await seatImage('a'); + await seatImage('b'); + }); + + it.each(['rulers', 'rectangles', 'polygons'] as const)( + 'reports skipped %s while retaining valid annotations on both images', + async (kind) => { + const manifest = manifestForImages(['a', 'b'], { + segments: [ + { id: 'a-segment', name: 'Liver', color: [255, 0, 0, 255] }, + { id: 'b-segment', name: 'Kidney', color: [0, 0, 255, 255] }, + ], + imageSegmentSets: [ + { imageId: 'a', segmentIds: ['a-segment'] }, + { imageId: 'b', segmentIds: ['b-segment'] }, + ], + tools: { + [kind]: { + tools: [ + shape('a', 'a-segment', 'Healthy A'), + shape('b', 'b-segment', 'Healthy B'), + shape('b', 'a-segment', 'Foreign'), + ], + }, + }, + }); + const addWarning = vi.fn(); + + await createStateFileRestorer({ addWarning })(manifest, [], { + a: 'a', + b: 'b', + }); + + expect(addWarning).toHaveBeenCalledExactlyOnceWith( + 'Some scene content could not be restored', + { + details: + '- annotation: Foreign (its segment belongs to another image)', + } + ); + expect( + stores[kind]() + .serializeTools() + .tools.map(({ imageID, name }) => ({ imageID, name })) + ).toEqual([ + { imageID: 'a', name: 'Healthy A' }, + { imageID: 'b', name: 'Healthy B' }, + ]); + } + ); +}); diff --git a/src/io/import/__tests__/postStateSegmentConfig.spec.ts b/src/io/import/__tests__/postStateSegmentConfig.spec.ts index 2d1505f26..2e4923afe 100644 --- a/src/io/import/__tests__/postStateSegmentConfig.spec.ts +++ b/src/io/import/__tests__/postStateSegmentConfig.spec.ts @@ -3,7 +3,10 @@ import { createPinia, setActivePinia } from 'pinia'; import { beforeEach, describe, expect, it } from 'vitest'; import { nextTick } from 'vue'; import JSZip from 'jszip'; -import { manifestForImages } from '@/src/segmentation/__tests__/segmentMaskFixtures'; +import { + manifestForImages, + viewImage, +} from '@/src/segmentation/__tests__/segmentMaskFixtures'; import { importDataSources } from '@/src/io/import/importDataSources'; import { @@ -11,6 +14,14 @@ import { yieldsFor, } from '@/src/io/import/__tests__/restoreProcessorFixtures'; import { useSegmentStore } from '@/src/segmentation/segments'; +import type { DataSource } from '@/src/io/import/dataSource'; + +const restoredImageSource: DataSource = { + type: 'file', + file: new File(['{}'], 'restored-image.json'), + fileType: 'application/json', + stateFileLeaf: { stateID: 'image' }, +}; const sessionFile = () => new File(['{}'], 'session.volview.json', { type: 'application/json' }); @@ -28,9 +39,8 @@ const configFile = () => { type: 'application/json' } ); -const RESTORED_MANIFEST = { - version: '7.0.0', - dataSources: [], +const RESTORED_MANIFEST = manifestForImages(['image'], { + imageSegmentSets: [{ imageId: 'image', segmentIds: ['wire-restored'] }], segments: [ { id: 'wire-restored', @@ -40,23 +50,32 @@ const RESTORED_MANIFEST = { locked: false, }, ], -}; +}); -const setup = yieldsFor((source) => - source.type === 'file' && source.file.name === 'session.volview.json' +const setup = yieldsFor((source) => { + if (source === restoredImageSource) { + return { + type: 'data', + dataID: 'image', + dataType: 'image', + dataSource: source, + }; + } + return source.type === 'file' && source.file.name === 'session.volview.json' ? { type: 'stateFileSetup', - dataSources: [], + dataSources: [restoredImageSource], manifest: RESTORED_MANIFEST, stateFiles: [], missingFiles: [], } - : Skip -); + : Skip; +}); describe('post-state segment config', () => { - beforeEach(() => { + beforeEach(async () => { setActivePinia(createPinia()); + await viewImage('image'); }); it('applies configured segments after the restored registry', async () => { @@ -64,9 +83,9 @@ describe('post-state segment config', () => { const restore = recordingRestoreProcessors({ setup, completion: async () => { - useSegmentStore().deserialize(RESTORED_MANIFEST); + useSegmentStore().deserialize(RESTORED_MANIFEST, { image: 'image' }); configWasVisibleDuringRestore = - !!useSegmentStore().segments.findSegmentByName('Configured'); + !!useSegmentStore().segments.findSegmentByName('image', 'Configured'); }, }); @@ -89,7 +108,7 @@ describe('post-state segment config', () => { // Restore seats its registry first; the config layers on top of it. expect(configWasVisibleDuringRestore).toBe(false); - const manifest = manifestForImages([]); + const manifest = manifestForImages(['image']); useSegmentStore().serialize({ zip: new JSZip(), manifest }); expect(manifest.segments?.map((segment) => segment.name)).toEqual([ 'Restored', diff --git a/src/io/import/__tests__/repeatedSegmentIds.spec.ts b/src/io/import/__tests__/repeatedSegmentIds.spec.ts index 93a3a018b..ccf0eebaa 100644 --- a/src/io/import/__tests__/repeatedSegmentIds.spec.ts +++ b/src/io/import/__tests__/repeatedSegmentIds.spec.ts @@ -2,6 +2,8 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; import { createPinia, setActivePinia } from 'pinia'; import { createStateFileRestorer } from '@/src/io/import/processors/restoreStateFile'; +import { migrateManifest } from '@/src/io/state-file/migrations'; +import { ManifestSchema } from '@/src/io/state-file/schema'; import { manifestForImages, seatImage, @@ -22,6 +24,7 @@ describe('restoring a manifest whose segments repeat an id', () => { await seatImage('parent', { dimensions: [4, 4, 1] }); const manifest = manifestForImages(['parent'], { segments: [segment('s1', 'Liver'), segment('s1', 'Spleen')], + imageSegmentSets: [{ imageId: 'parent', segmentIds: ['s1'] }], }); const addWarning = vi.fn(); @@ -32,4 +35,49 @@ describe('restoring a manifest whose segments repeat an id', () => { { details: '- segment: Spleen (repeats the id of an earlier segment)' } ); }); + + it('reports a repeated legacy row once when its catalog is copied to several images', async () => { + await seatImage('a'); + await seatImage('b'); + const manifest = ManifestSchema.parse( + migrateManifest( + JSON.stringify( + manifestForImages(['a', 'b'], { + version: '7.0.0', + segments: [segment('s1', 'Liver'), segment('s1', 'Spleen')], + }) + ) + ) + ); + const addWarning = vi.fn(); + + await createStateFileRestorer({ addWarning })(manifest, [], { + a: 'a', + b: 'b', + }); + + expect(addWarning).toHaveBeenCalledExactlyOnceWith( + 'Some scene content could not be restored', + { details: '- segment: Spleen (repeats the id of an earlier segment)' } + ); + }); + + it.each([undefined, []])( + 'reports orphan definitions when image memberships are %j', + async (imageSegmentSets) => { + await seatImage('parent'); + const manifest = manifestForImages(['parent'], { + segments: [segment('orphan', 'Unassigned')], + imageSegmentSets, + }); + const addWarning = vi.fn(); + await createStateFileRestorer({ addWarning })(manifest, [], { + parent: 'parent', + }); + expect(addWarning).toHaveBeenCalledExactlyOnceWith( + 'Some scene content could not be restored', + { details: '- segment: Unassigned (is not assigned to an image)' } + ); + } + ); }); diff --git a/src/io/import/__tests__/restoreStateIdCollision.spec.ts b/src/io/import/__tests__/restoreStateIdCollision.spec.ts index 6f7f641cb..9c8b8ce47 100644 --- a/src/io/import/__tests__/restoreStateIdCollision.spec.ts +++ b/src/io/import/__tests__/restoreStateIdCollision.spec.ts @@ -132,7 +132,8 @@ const restoreOnto = async ( manifest: setup.manifest, stateFiles, dataIDMap, - segmentIdMap: useSegmentStore().deserialize(setup.manifest).segmentIdMap, + segmentIdMap: useSegmentStore().deserialize(setup.manifest, dataIDMap) + .segmentIdMap, labelmapSources: planLabelmapSources(setup.manifest).sources, io: artifactIO, }); diff --git a/src/io/import/configJson.ts b/src/io/import/configJson.ts index 1e97de55a..4a2ee053c 100644 --- a/src/io/import/configJson.ts +++ b/src/io/import/configJson.ts @@ -230,12 +230,13 @@ const reportUnparseableColors = ( // clears what an earlier config contributed to it. const applySegments = ( manifest: Config, - addError: (message: string) => void + addError: (message: string) => void, + imageIds: string[] ) => { const configured = manifest.segments; if (configured === undefined) return; if (configured) reportUnparseableColors(configured, addError); - useSegmentStore().segments.replaceConfigSegments(configured); + useSegmentStore().replaceConfigSegments(configured, imageIds); }; const applyLayout = (manifest: Config) => { @@ -333,7 +334,8 @@ export const applyPreStateConfig = async (manifest: Config) => { export const applyPostStateConfig = ( manifest: Config, addError: (message: string) => void = (message) => - useMessageStore().addError(message) + useMessageStore().addError(message), + imageIds: string[] ) => { - applySegments(manifest, addError); + applySegments(manifest, addError, imageIds); }; diff --git a/src/io/import/importDataSources.ts b/src/io/import/importDataSources.ts index 378d170eb..b90309159 100644 --- a/src/io/import/importDataSources.ts +++ b/src/io/import/importDataSources.ts @@ -11,6 +11,7 @@ import { ImportDataSourcesResult, asIntermediateResult, StateFileSetupResult, + isVolumeResult, } from '@/src/io/import/common'; import { DataSource, @@ -30,7 +31,9 @@ import { completeStateFileRestore, } from '@/src/io/import/processors/restoreStateFile'; import updateFileMimeType from '@/src/io/import/processors/updateFileMimeType'; -import handleConfig from '@/src/io/import/processors/handleConfig'; +import handleConfig, { + InvalidConfigError, +} from '@/src/io/import/processors/handleConfig'; import { applyPreStateConfig, applyPostStateConfig, @@ -47,6 +50,7 @@ import { Chunk } from '@/src/core/streaming/chunk'; import { useDatasetStore } from '@/src/store/datasets'; import { useDICOMStore } from '@/src/store/datasets-dicom'; import { useMessageStore } from '@/src/store/messages'; +import { useCurrentImage } from '@/src/composables/useCurrentImage'; const unhandledResource: ImportHandler = (dataSource) => { return asErrorResult(new Error('Failed to handle resource'), dataSource); @@ -69,11 +73,12 @@ function isSelectable(result: ImportResult): result is LoadableVolumeResult { } const applyConfigsPostState = ( - results: Array + results: Array, + imageIds: string[] ): (ConfigResult | ErrorResult)[] => results.map((result) => { try { - applyPostStateConfig(result.config); + applyPostStateConfig(result.config, undefined, imageIds); return result; } catch (err) { return asErrorResult(ensureError(err), result.dataSource); @@ -132,6 +137,33 @@ export type RestoreProcessors = { completeStateFileRestore: typeof completeStateFileRestore; }; +const hasDataIntent = (result: ImportResult) => { + if ( + result.type === 'config' || + (result.type === 'error' && result.error instanceof InvalidConfigError) + ) + return findStateFileLeaves(result.dataSource).length > 0; + if (result.type !== 'intermediate') return true; + return ( + result.dataSources.length === 0 || + result.dataSources.some( + (source) => + source.type === 'chunk' || findStateFileLeaves(source).length > 0 + ) + ); +}; + +// Config that arrives with data configures the loaded images; config alone +// configures the image whose view started the import. +const configTargetImageIds = ( + observed: readonly ImportResult[], + loadables: readonly LoadableResult[], + standaloneImageIds: string[] +) => + observed.some(hasDataIntent) + ? loadables.filter(isVolumeResult).map(({ dataID }) => dataID) + : standaloneImageIds; + export const appRestoreProcessors = (): RestoreProcessors => ({ restoreStateFile, completeStateFileRestore, @@ -140,7 +172,8 @@ export const appRestoreProcessors = (): RestoreProcessors => ({ async function importDataSourcesWithPolicy( dataSources: DataSource[], policy: ImportPolicy, - restore: RestoreProcessors = appRestoreProcessors() + restore: RestoreProcessors, + standaloneImageIds: string[] ): Promise { const cleanupHandlers: Array<() => void> = []; const onCleanup = (fn: () => void) => { @@ -154,7 +187,7 @@ async function importDataSourcesWithPolicy( fetchFileCache: new Map(), onCleanup, importDataSources: (sources: DataSource[]) => - importDataSourcesWithPolicy(sources, policy, restore), + importDataSourcesWithPolicy(sources, policy, restore, standaloneImageIds), }; // State files first: a manifest's top-level `segments` is also a config key. @@ -193,6 +226,7 @@ async function importDataSourcesWithPolicy( const configResults: ConfigResult[] = []; const stateFileSetups: StateFileSetupResult[] = []; const results: ImportDataSourcesResult[] = []; + const observedResults: ImportResult[] = []; let queue = dataSources.map((src) => ({ promise: evaluateChain(src, handlers, importContext), @@ -203,6 +237,7 @@ async function importDataSourcesWithPolicy( const { index } = await asyncSelect(queue.map((item) => item.promise)); const { promise, source } = queue[index]; const result = await promise.catch((err) => asErrorResult(err, source)); + observedResults.push(result); queue = queue.filter((_, i) => i !== index); switch (result.type) { @@ -313,7 +348,12 @@ async function importDataSourcesWithPolicy( } } - results.push(...applyConfigsPostState(configResults)); + results.push( + ...applyConfigsPostState( + configResults, + configTargetImageIds(observedResults, loadableResults, standaloneImageIds) + ) + ); // A failed state-file leaf is already counted in the restore's consolidated // missing-content notice, so this layer owns its reporting: it returns as an @@ -331,17 +371,38 @@ async function importDataSourcesWithPolicy( }); } +function importFromCurrentImage( + dataSources: DataSource[], + policy: ImportPolicy, + restore: RestoreProcessors +) { + const initiatingImageId = useCurrentImage('global').currentImageID.value; + // Standalone config belongs to the initiating image, even if focus changes + // while reading it. An explicit empty target updates only future defaults. + const standaloneImageIds = initiatingImageId ? [initiatingImageId] : []; + return importDataSourcesWithPolicy( + dataSources, + policy, + restore, + standaloneImageIds + ); +} + export function importDataSources( dataSources: DataSource[], restore: RestoreProcessors = appRestoreProcessors() ): Promise { - return importDataSourcesWithPolicy(dataSources, 'application', restore); + return importFromCurrentImage(dataSources, 'application', restore); } export function importVolumeDataSources( dataSources: DataSource[] ): Promise { - return importDataSourcesWithPolicy(dataSources, 'volume-data'); + return importFromCurrentImage( + dataSources, + 'volume-data', + appRestoreProcessors() + ); } export function toDataSelection(loadable: LoadableVolumeResult) { diff --git a/src/io/import/processors/handleConfig.ts b/src/io/import/processors/handleConfig.ts index 35ba494ea..8aab61ece 100644 --- a/src/io/import/processors/handleConfig.ts +++ b/src/io/import/processors/handleConfig.ts @@ -5,6 +5,10 @@ import { recognizeConfigFile } from '@/src/io/import/configJson'; import { Skip } from '@/src/utils/evaluateChain'; import { surfaceWarning } from '@/src/store/messages'; +// Validation failed after known section keys identified this as configuration. +// A JSON syntax/read failure has not established that role. +export class InvalidConfigError extends Error {} + // Forward-compat: a config that carries unknown top-level keys (e.g. a newer // config opened on an older client) still applies its known sections; the // unknown keys are stripped during recognition and surfaced here (console + @@ -52,7 +56,8 @@ const handleConfig: ImportHandler = async (dataSource) => { const error = ensureError(err); const detail = err instanceof ZodError ? z.prettifyError(err) : error.message; - throw new Error(`Failed to parse config file: ${detail}`, { + const ParseError = err instanceof ZodError ? InvalidConfigError : Error; + throw new ParseError(`Failed to parse config file: ${detail}`, { cause: error, }); } diff --git a/src/io/import/processors/restoreStateFile.ts b/src/io/import/processors/restoreStateFile.ts index 9f3917253..299a01d4d 100644 --- a/src/io/import/processors/restoreStateFile.ts +++ b/src/io/import/processors/restoreStateFile.ts @@ -203,8 +203,11 @@ export const createStateFileRestorer = ({ ); // Registries first: masks and shapes name the segment identities minted here. - const { segmentIdMap, repeated: repeatedSegments } = - useSegmentStore().deserialize(manifest); + const { + segmentIdMap, + repeated: repeatedSegments, + unassigned, + } = useSegmentStore().deserialize(manifest, stateIDToStoreID); // Restore camera and contrast before asynchronous mask decoding. useViewConfigStore().deserializeAll( @@ -252,7 +255,11 @@ export const createStateFileRestorer = ({ useLayersStore().deserialize(manifest, stateIDToStoreID); - useToolStore().deserialize(manifest, segmentIdMap, stateIDToStoreID); + const skippedTools = useToolStore().deserialize( + manifest, + segmentIdMap, + stateIDToStoreID + ); const missingBases = unresolvedDatasets.map((ds) => summarizeDataSource( @@ -290,9 +297,17 @@ export const createStateFileRestorer = ({ ...skippedLabelmaps.map( ({ name, reason }) => `- labelmap: ${name} (${reason})` ), - ...repeatedSegments.map( - ({ name }) => - `- segment: ${name} (repeats the id of an earlier segment)` + ...skippedTools.map( + ({ name, reason }) => `- annotation: ${name} (${reason})` + ), + ...new Set( + repeatedSegments.map( + ({ name }) => + `- segment: ${name} (repeats the id of an earlier segment)` + ) + ), + ...unassigned.map( + ({ name }) => `- segment: ${name} (is not assigned to an image)` ), ]; if (missing.length > 0) { diff --git a/src/io/state-file/__tests__/annotationSaveSnapshot.spec.ts b/src/io/state-file/__tests__/annotationSaveSnapshot.spec.ts index 4811e7105..de8c4d0f2 100644 --- a/src/io/state-file/__tests__/annotationSaveSnapshot.spec.ts +++ b/src/io/state-file/__tests__/annotationSaveSnapshot.spec.ts @@ -46,7 +46,7 @@ it.each(['encoding', 'queued update'])( segmentId: originalSegment, }); const reassign = () => { - const segmentId = segments.segments.mintSegment({ + const segmentId = segments.segments.mintSegment('ct', { name: 'Tumor', color: [255, 0, 0, 255], }); @@ -90,7 +90,9 @@ it.each(['encoding', 'queued update'])( setActivePinia(createPinia()); await seatImage('restored'); - const { segmentIdMap } = useSegmentStore().deserialize(saved); + const { segmentIdMap } = useSegmentStore().deserialize(saved, { + ct: 'restored', + }); const restored = useRulerStore(); restored.deserialize(saved, { ct: 'restored' }, segmentIdMap); const [ruler] = restored.serializeTools().tools; diff --git a/src/io/state-file/__tests__/annotationToolSource.spec.ts b/src/io/state-file/__tests__/annotationToolSource.spec.ts index 15a950482..bfd671465 100644 --- a/src/io/state-file/__tests__/annotationToolSource.spec.ts +++ b/src/io/state-file/__tests__/annotationToolSource.spec.ts @@ -94,7 +94,8 @@ describe('annotation tool source', () => { it('migrates a 6.4.0 tool onto a segment and keeps its source-free geometry', () => { const old = JSON.stringify({ version: '6.4.0', - dataSources: [], + dataSources: [{ id: 1, type: 'uri', uri: '/img-1.nrrd' }], + datasets: [{ id: 'img-1', dataSourceId: 1 }], tools: { rulers: { tools: [ruler()] } }, }); const migrated = migrateManifest(old); diff --git a/src/io/state-file/__tests__/boundedMaskRoundTrip.spec.ts b/src/io/state-file/__tests__/boundedMaskRoundTrip.spec.ts index 97a5cb022..169936b21 100644 --- a/src/io/state-file/__tests__/boundedMaskRoundTrip.spec.ts +++ b/src/io/state-file/__tests__/boundedMaskRoundTrip.spec.ts @@ -142,7 +142,10 @@ async function roundTrip(io: LabelmapIO, tamper?: (manifest: any) => void) { manifest: parsed, stateFiles: stateFiles, dataIDMap: { 'img-1': 'new-1', 'img-2': 'new-2' }, - segmentIdMap: useSegmentStore().deserialize(parsed).segmentIdMap, + segmentIdMap: useSegmentStore().deserialize(parsed, { + 'img-1': 'new-1', + 'img-2': 'new-2', + }).segmentIdMap, io: io, }); await nextTick(); @@ -164,10 +167,7 @@ describe('bounded masks through the state file', () => { await roundTrip(inMemoryArtifactIO()); expect(snapshot('new-1')).toEqual(before.first); - // The session repeats the name, so the later segment restores numbered. - expect(snapshot('new-2')).toEqual([ - { ...before.second[0], name: 'Tumor (2)' }, - ]); + expect(snapshot('new-2')).toEqual(before.second); // Spelled out once, so the equality above cannot pass on two full-extent // masks that happen to match each other. expect(snapshot('new-1')[0]).toMatchObject({ @@ -200,7 +200,8 @@ describe('bounded masks through the state file', () => { manifest: parsed, stateFiles: stateFiles, dataIDMap: dataIDMap, - segmentIdMap: useSegmentStore().deserialize(parsed).segmentIdMap, + segmentIdMap: useSegmentStore().deserialize(parsed, dataIDMap) + .segmentIdMap, io: io, }); await restore(); @@ -259,7 +260,7 @@ describe('bounded masks through the state file', () => { { name: 'Node', ...node }, { name: 'Planned', ...planned }, { name: 'Unbound', extent: undefined }, - { name: 'Tumor (3)', ...tumor }, + { name: 'Tumor (2)', ...tumor }, { name: 'Node (2)', ...node }, { name: 'Planned (2)', ...planned }, { name: 'Unbound (2)', extent: undefined }, @@ -268,8 +269,8 @@ describe('bounded masks through the state file', () => { { parent: 'img-2', masks: [ + { name: 'Tumor', ...other }, { name: 'Tumor (2)', ...other }, - { name: 'Tumor (4)', ...other }, ], }, ]); @@ -299,7 +300,10 @@ describe('bounded masks through the state file', () => { manifest: parsed, stateFiles: stateFiles, dataIDMap: { 'img-1': 'img-1', 'img-2': 'img-2' }, - segmentIdMap: useSegmentStore().deserialize(parsed).segmentIdMap, + segmentIdMap: useSegmentStore().deserialize(parsed, { + 'img-1': 'img-1', + 'img-2': 'img-2', + }).segmentIdMap, io: io, }); await nextTick(); @@ -349,7 +353,7 @@ describe('bounded masks through the state file', () => { ).find((mask) => nameOf(mask) === 'Tumor')!; const secondTumor = listMasks( store().getSegmentationForImage('new-2')! - ).find((mask) => nameOf(mask) === 'Tumor (2)')!; + ).find((mask) => nameOf(mask) === 'Tumor')!; expect(firstTumor.representations.labelmap).toBeDefined(); expect(secondTumor.representations.labelmap).toBeDefined(); expect(markedVoxels(firstTumor.id)).toEqual([ diff --git a/src/io/state-file/__tests__/droppedMaskReport.spec.ts b/src/io/state-file/__tests__/droppedMaskReport.spec.ts index e38c7942f..9e0b30efe 100644 --- a/src/io/state-file/__tests__/droppedMaskReport.spec.ts +++ b/src/io/state-file/__tests__/droppedMaskReport.spec.ts @@ -25,7 +25,7 @@ const buildScene = async () => { const restoreTampered = async ( tamper: (parsed: any) => void, segmentIdMapFor: (parsed: any) => Record = (parsed) => - useSegmentStore().deserialize(parsed).segmentIdMap, + useSegmentStore().deserialize(parsed, { 'img-1': 'new-1' }).segmentIdMap, dataIDMap: Record = { 'img-1': 'new-1' } ) => { await buildScene(); @@ -67,6 +67,7 @@ const restoreArtifactMasks = async (order: string[]) => { name: id, color: [255, 0, 0, 255], })), + imageSegmentSets: [{ imageId: 'parent', segmentIds: ['s1', 's3'] }], segmentations: [ { id: 'wire', @@ -102,7 +103,8 @@ const restoreArtifactMasks = async (order: string[]) => { { archivePath: 'labels.vti', file: new File([], 'labels.vti') }, ], dataIDMap: { parent: 'parent' }, - segmentIdMap: useSegmentStore().deserialize(manifest).segmentIdMap, + segmentIdMap: useSegmentStore().deserialize(manifest, { parent: 'parent' }) + .segmentIdMap, io: { read: async () => ({ image: labels }), write: async () => '' }, }); return { @@ -151,6 +153,17 @@ describe('masks dropped during restore', () => { ]); }); + it('distinguishes orphan definitions from segments missing from the file', async () => { + const { skipped, masks } = await restoreTampered((parsed) => { + delete parsed.imageSegmentSets; + }); + expect(masks).toEqual([]); + expect(skipped.map(({ reason }) => reason)).toEqual([ + 'its segment did not restore', + 'its segment did not restore', + ]); + }); + // A parent image the restore never mapped is as lost as one whose data // failed to load, and its masks go the same way: 'we dropped two masks' has // to read differently from 'that image had none'. @@ -234,7 +247,9 @@ describe('masks dropped during restore', () => { ? stateFiles.filter(({ file }) => file !== brokenFile) : stateFiles, dataIDMap: { 'img-1': 'new-1' }, - segmentIdMap: useSegmentStore().deserialize(parsed).segmentIdMap, + segmentIdMap: useSegmentStore().deserialize(parsed, { + 'img-1': 'new-1', + }).segmentIdMap, io: damagedIO, }); expect(skipped).toEqual([{ name: broken.name, reason }]); diff --git a/src/io/state-file/__tests__/imageSegmentSetsRoundTrip.spec.ts b/src/io/state-file/__tests__/imageSegmentSetsRoundTrip.spec.ts new file mode 100644 index 000000000..8aa791697 --- /dev/null +++ b/src/io/state-file/__tests__/imageSegmentSetsRoundTrip.spec.ts @@ -0,0 +1,263 @@ +import { beforeEach, describe, expect, it } from 'vitest'; +import { createPinia, setActivePinia } from 'pinia'; + +import { completeStateFileRestore } from '@/src/io/import/processors/restoreStateFile'; +import { migrateManifest } from '@/src/io/state-file/migrations'; +import { ManifestSchema } from '@/src/io/state-file/schema'; +import { + inMemoryArtifactIO, + manifestForImages, + seatImage, + serializeToStateFiles, + serializeAnnotations, + showImage, +} from '@/src/segmentation/__tests__/segmentMaskFixtures'; +import { useSegmentStore } from '@/src/segmentation/segments'; +import { useSegmentationStore } from '@/src/segmentation/store'; +import { useRulerStore } from '@/src/store/tools/rulers'; + +const legacyManifest = () => + ManifestSchema.parse( + migrateManifest( + JSON.stringify({ + ...manifestForImages(['a', 'b']), + version: '7.0.0', + segments: [ + { + id: 'shared', + name: 'Tumor', + color: [255, 0, 0, 255], + visible: true, + locked: false, + }, + ], + selectedSegment: 'shared', + segmentations: ['a', 'b'].map((parentImage) => ({ + id: `seg-${parentImage}`, + name: parentImage, + parentImage, + masks: [ + { + id: `mask-${parentImage}`, + segmentId: 'shared', + representations: {}, + }, + ], + order: [`mask-${parentImage}`], + })), + tools: { + rulers: { + tools: ['a', 'b'].map((imageID) => ({ + imageID, + segmentId: 'shared', + frameOfReference: { + planeOrigin: [0, 0, 0], + planeNormal: [0, 0, 1], + }, + slice: 0, + firstPoint: [0, 0, 0], + secondPoint: [1, 0, 0], + })), + }, + }, + }) + ) + ); + +describe('image segment set persistence', () => { + beforeEach(() => setActivePinia(createPinia())); + + it('restores the image set order and selection when catalog order differs', async () => { + await seatImage('runtime'); + const manifest = ManifestSchema.parse( + manifestForImages(['saved'], { + segments: ['A', 'B'].map((name) => ({ + id: name.toLowerCase(), + name, + color: [255, 0, 0, 255], + })), + imageSegmentSets: [ + { + imageId: 'saved', + segmentIds: ['b', 'a'], + selectedSegmentId: 'a', + }, + ], + }) + ); + const store = useSegmentStore(); + const { segmentIdMap } = store.deserialize(manifest, { saved: 'runtime' }); + expect( + store.segments.segmentList('runtime').map(({ name }) => name) + ).toEqual(['B', 'A']); + expect( + serializeAnnotations().imageSegmentSets?.find( + ({ imageId }) => imageId === 'runtime' + )?.selectedSegmentId + ).toBe(segmentIdMap.a); + }); + + it('reports repeated catalog identities and refuses shared image membership', async () => { + await seatImage('first'); + await seatImage('second'); + const manifest = ManifestSchema.parse( + manifestForImages(['a', 'b'], { + segments: [ + { id: 's1', name: 'A', color: [255, 0, 0, 255] }, + { id: 's1', name: 'Repeated A', color: [255, 0, 0, 255] }, + { id: 's2', name: 'B', color: [0, 255, 0, 255] }, + ], + imageSegmentSets: [ + { imageId: 'a', segmentIds: ['s2', 's1', 's2'] }, + { imageId: 'b', segmentIds: ['s2'], selectedSegmentId: 's2' }, + ], + }) + ); + const store = useSegmentStore(); + const { repeated, segmentIdMap } = store.deserialize(manifest, { + a: 'first', + b: 'second', + }); + expect(repeated.map(({ name }) => name)).toEqual(['Repeated A', 'B']); + expect(store.segments.segmentList('first').map(({ name }) => name)).toEqual( + ['B', 'A'] + ); + const saved = serializeAnnotations(); + expect( + saved.imageSegmentSets?.find(({ imageId }) => imageId === 'first') + ?.segmentIds + ).toContain(segmentIdMap.s2); + expect(store.segments.segmentList('second')).toEqual([]); + expect( + saved.imageSegmentSets?.find(({ imageId }) => imageId === 'second') + ).toEqual({ imageId: 'second', segmentIds: [] }); + }); + + it('keeps legacy masks and shapes independent and preserves an emptied image set', async () => { + await seatImage('image-a'); + await seatImage('image-b'); + await completeStateFileRestore(legacyManifest(), [], { + a: 'image-a', + b: 'image-b', + }); + + const registry = useSegmentStore().segments; + const a = registry.segmentList('image-a')[0]; + const b = registry.segmentList('image-b')[0]; + expect(a.name).toBe('Tumor'); + expect(b.name).toBe('Tumor'); + expect(a.id).not.toBe(b.id); + registry.deleteSegment(b.id); + + const manifest = manifestForImages(['image-a', 'image-b'], { + tools: { rulers: useRulerStore().serializeTools() }, + }); + const { parsed: saved } = await serializeToStateFiles( + manifest, + inMemoryArtifactIO() + ); + saved.imageSegmentSets.forEach((set: Record) => + expect(set).not.toHaveProperty('legacySegmentIds') + ); + expect( + saved.imageSegmentSets.find( + ({ imageId }: { imageId: string }) => imageId === 'image-b' + ) + ).toEqual({ imageId: 'image-b', segmentIds: [] }); + expect( + saved.segmentations.find( + ({ parentImage }: { parentImage: string }) => parentImage === 'image-a' + ).masks + ).toHaveLength(1); + expect( + saved.tools.rulers.tools.map( + ({ imageID }: { imageID: string }) => imageID + ) + ).toEqual(['image-a']); + + setActivePinia(createPinia()); + await seatImage('restored-a'); + await seatImage('restored-b'); + await completeStateFileRestore(saved, [], { + 'image-a': 'restored-a', + 'image-b': 'restored-b', + }); + + expect( + useSegmentStore() + .segments.segmentList('restored-a') + .map(({ name }) => name) + ).toEqual(['Tumor']); + expect(useSegmentStore().segments.segmentList('restored-b')).toEqual([]); + expect( + serializeAnnotations().imageSegmentSets?.find( + ({ imageId }) => imageId === 'restored-b' + ) + ).toEqual({ imageId: 'restored-b', segmentIds: [] }); + expect(useSegmentationStore().imageMasks('restored-a')).toHaveLength(1); + expect( + useRulerStore() + .serializeTools() + .tools.map(({ imageID }) => imageID) + ).toEqual(['restored-a']); + }); + + it('lets an untouched legacy image borrow labels added after restore', async () => { + const manifest = ManifestSchema.parse( + migrateManifest( + JSON.stringify({ + ...manifestForImages(['a', 'b']), + version: '7.0.0', + segments: [], + }) + ) + ); + await seatImage('image-a'); + await seatImage('image-b'); + await completeStateFileRestore(manifest, [], { + a: 'image-a', + b: 'image-b', + }); + const registry = useSegmentStore().segments; + registry.addSegment('image-a', { name: 'Tumor' }); + await showImage('image-b'); + expect(registry.segmentList('image-b').map(({ name }) => name)).toEqual([ + 'Tumor', + ]); + }); + + it('skips a mask that names another image segment without losing valid masks', async () => { + const manifest = legacyManifest(); + manifest.segmentations![1].masks[0].segmentId = + manifest.imageSegmentSets![0].segmentIds[0]; + await seatImage('image-a'); + await seatImage('image-b'); + const dataIDMap = { a: 'image-a', b: 'image-b' }; + const { segmentIdMap } = useSegmentStore().deserialize(manifest, dataIDMap); + const { skipped } = await useSegmentationStore().deserialize({ + manifest, + dataIDMap, + segmentIdMap, + stateFiles: [], + io: inMemoryArtifactIO(), + }); + expect(skipped.map(({ reason }) => reason)).toEqual([ + 'its segment belongs to another image', + ]); + expect(useSegmentationStore().imageMasks('image-a')).toHaveLength(1); + expect(useSegmentationStore().imageMasks('image-b')).toHaveLength(0); + }); + + it('does not restore definitions for a dataset whose image never loaded', async () => { + await seatImage('image-a'); + const registry = useSegmentStore(); + registry.deserialize(legacyManifest(), { + a: 'image-a', + b: 'missing-image', + }); + expect(registry.segments.segmentList('image-a')).toHaveLength(1); + expect( + serializeAnnotations().imageSegmentSets?.map(({ imageId }) => imageId) + ).toEqual(['image-a']); + }); +}); diff --git a/src/io/state-file/__tests__/manyToOneImageSegmentRestore.spec.ts b/src/io/state-file/__tests__/manyToOneImageSegmentRestore.spec.ts new file mode 100644 index 000000000..760f17122 --- /dev/null +++ b/src/io/state-file/__tests__/manyToOneImageSegmentRestore.spec.ts @@ -0,0 +1,269 @@ +import { beforeEach, describe, expect, it } from 'vitest'; +import { createPinia, setActivePinia } from 'pinia'; + +import { buildStateIDToStoreID } from '@/src/io/import/importDataSources'; +import { migrateManifest } from '@/src/io/state-file/migrations'; +import { ManifestSchema, type Manifest } from '@/src/io/state-file/schema'; +import { + makeLabelmap, + manifestForImages, + markedVoxels, + seatImage, +} from '@/src/segmentation/__tests__/segmentMaskFixtures'; +import { useSegmentStore } from '@/src/segmentation/segments'; +import { useSegmentationStore } from '@/src/segmentation/store'; +import { useRulerStore } from '@/src/store/tools/rulers'; + +const definitions = [ + { + id: 'red', + name: 'Tumor', + color: [255, 0, 0, 255], + fillOpacity: 0.25, + outlineOpacity: 0.5, + strokeWidth: 2, + visible: true, + locked: true, + }, + { + id: 'green', + name: 'Tumor', + color: [0, 255, 0, 255], + fillOpacity: 0.75, + outlineOpacity: 1, + strokeWidth: 4, + visible: false, + locked: false, + }, +]; + +const incoming = () => + ManifestSchema.parse( + manifestForImages(['a', 'b'], { + segments: definitions, + imageSegmentSets: [ + { imageId: 'a', segmentIds: ['red'], selectedSegmentId: 'red' }, + { imageId: 'b', segmentIds: ['green'], selectedSegmentId: 'green' }, + ], + segmentations: ['a', 'b'].map((parentImage, index) => ({ + id: `seg-${parentImage}`, + name: parentImage, + parentImage, + order: [`mask-${parentImage}`], + masks: [ + { + id: `mask-${parentImage}`, + segmentId: definitions[index].id, + representations: { + labelmap: { + path: `${parentImage}.vti`, + extent: [0, 1, 0, 1, 0, 0], + }, + }, + }, + ], + })), + tools: { + rulers: { + tools: ['a', 'b'].map((imageID, index) => ({ + imageID, + segmentId: definitions[index].id, + frameOfReference: { + planeOrigin: [0, 0, 0], + planeNormal: [0, 0, 1], + }, + slice: 0, + firstPoint: [0, index, 0], + secondPoint: [1, index, 0], + })), + }, + }, + }) + ); + +const restoreMerged = async (manifest: Manifest) => { + await seatImage('merged', { dimensions: [2, 2, 1] }); + const dataIDMap = buildStateIDToStoreID([ + { + type: 'data', + dataType: 'image', + dataID: 'merged', + dataSource: { + type: 'collection', + sources: ['a', 'b'].map((stateID) => ({ + type: 'uri', + uri: `/${stateID}.dcm`, + name: `${stateID}.dcm`, + stateFileLeaf: { stateID }, + })), + }, + }, + ]); + const { segmentIdMap, repeated } = useSegmentStore().deserialize( + manifest, + dataIDMap + ); + const labelmaps = new Map([ + [ + 'a.vti', + makeLabelmap({ + dimensions: [2, 2, 1], + values: new Uint8Array([1, 0, 0, 0]), + }), + ], + [ + 'b.vti', + makeLabelmap({ + dimensions: [2, 2, 1], + values: new Uint8Array([0, 1, 0, 0]), + }), + ], + ]); + const { skipped } = await useSegmentationStore().deserialize({ + manifest, + dataIDMap, + segmentIdMap, + stateFiles: [...labelmaps.keys()].map((name) => ({ + archivePath: name, + file: new File([], name), + })), + io: { + write: async () => '', + read: async (file) => ({ image: labelmaps.get(file.name)! }), + }, + }); + useRulerStore().deserialize(manifest, dataIDMap, segmentIdMap); + return { dataIDMap, segmentIdMap, skipped, repeated }; +}; + +const expectBothContents = () => { + const masks = useSegmentationStore().imageMasks('merged'); + expect(masks).toHaveLength(2); + expect(masks.map(({ id }) => markedVoxels(id))).toEqual([ + [[0, 0, 0, 1]], + [[1, 0, 0, 1]], + ]); + expect(masks[0].segmentId).not.toBe(masks[1].segmentId); + expect( + masks.map(({ segmentId }) => + useSegmentStore().segments.appearanceOf(segmentId) + ) + ).toMatchObject( + definitions.map( + ({ + color, + fillOpacity, + outlineOpacity, + strokeWidth, + visible, + locked, + }) => ({ + color, + fillOpacity, + outlineOpacity, + strokeWidth, + visible, + locked, + }) + ) + ); + const shapes = useRulerStore().serializeTools().tools; + expect( + shapes.map(({ segmentId, firstPoint, secondPoint, imageID }) => ({ + segmentId, + firstPoint, + secondPoint, + imageID, + })) + ).toEqual( + masks.map(({ segmentId }, index) => ({ + segmentId, + firstPoint: [0, index, 0], + secondPoint: [1, index, 0], + imageID: 'merged', + })) + ); +}; + +describe('saved image sets resolving to one runtime image', () => { + beforeEach(() => setActivePinia(createPinia())); + + it('preserves distinct same-name definitions, appearance, masks and shapes in an incoming 8.0.0 manifest', async () => { + const manifest = incoming(); + const { dataIDMap, segmentIdMap, skipped, repeated } = + await restoreMerged(manifest); + expect(dataIDMap).toEqual({ a: 'merged', b: 'merged' }); + expect(segmentIdMap.red).not.toBe(segmentIdMap.green); + expect( + useSegmentStore() + .segments.segmentList('merged') + .map(({ name }) => name) + ).toEqual(['Tumor', 'Tumor (2)']); + expect(useSegmentStore().segments.selectedSegmentId('merged')).toBe( + segmentIdMap.red + ); + expect(skipped).toEqual([]); + expect(repeated).toEqual([]); + expectBothContents(); + }); + + it('binds legacy masks and shapes from different slices to their shared definition', async () => { + const old = { ...incoming(), version: '7.0.0', selectedSegment: 'red' }; + old.segments = old.segments!.slice(0, 1); + old.segmentations = old.segmentations!.slice(0, 1); + old.tools!.rulers!.tools[1].segmentId = 'red'; + const manifest = ManifestSchema.parse(migrateManifest(JSON.stringify(old))); + const { skipped, repeated } = await restoreMerged(manifest); + const registry = useSegmentStore().segments; + expect(registry.segmentList('merged').map(({ name }) => name)).toEqual([ + 'Tumor', + ]); + const [mask] = useSegmentationStore().imageMasks('merged'); + expect( + useRulerStore() + .serializeTools() + .tools.map(({ segmentId }) => segmentId) + ).toEqual([mask.segmentId, mask.segmentId]); + expect(skipped).toEqual([]); + expect(repeated).toEqual([]); + }); + + it('preserves distinct legacy global definitions when their images resolve together', async () => { + const old = { ...incoming(), version: '7.0.0', selectedSegment: 'red' }; + const manifest = ManifestSchema.parse(migrateManifest(JSON.stringify(old))); + const { skipped, repeated } = await restoreMerged(manifest); + expect(skipped).toEqual([]); + expect(repeated).toEqual([]); + expect( + useSegmentStore() + .segments.segmentList('merged') + .map(({ name }) => name) + ).toEqual(['Tumor', 'Tumor (2)']); + expectBothContents(); + }); + + it('reports a repeated membership and keeps its first identity and mask', async () => { + const manifest = incoming(); + manifest.imageSegmentSets![1].segmentIds = ['red']; + manifest.imageSegmentSets![1].selectedSegmentId = 'red'; + manifest.segmentations![1].masks[0].segmentId = 'red'; + manifest.tools!.rulers!.tools[1].segmentId = 'red'; + const { segmentIdMap, repeated, skipped } = await restoreMerged(manifest); + expect(repeated.map(({ name }) => name)).toEqual(['Tumor']); + expect(segmentIdMap.green).toBeUndefined(); + expect(useSegmentStore().segments.segmentList('merged')).toHaveLength(1); + expect( + useSegmentStore().segments.getSegment(segmentIdMap.red)?.color + ).toEqual(definitions[0].color); + expect(skipped.map(({ reason }) => reason)).toEqual([ + 'the image already has a mask for its segment', + ]); + const masks = useSegmentationStore().imageMasks('merged'); + expect(masks.map(({ id }) => markedVoxels(id))).toEqual([[[0, 0, 0, 1]]]); + expect( + useRulerStore() + .serializeTools() + .tools.map(({ segmentId }) => segmentId) + ).toEqual([segmentIdMap.red, segmentIdMap.red]); + }); +}); diff --git a/src/io/state-file/__tests__/maskIoConcurrency.spec.ts b/src/io/state-file/__tests__/maskIoConcurrency.spec.ts index 3db582ec0..f0909f09d 100644 --- a/src/io/state-file/__tests__/maskIoConcurrency.spec.ts +++ b/src/io/state-file/__tests__/maskIoConcurrency.spec.ts @@ -138,7 +138,9 @@ describe('mask io concurrency', () => { setActivePinia(createPinia()); await seatSpecImage('new-1', 'CT A'); - const { segmentIdMap } = useSegmentStore().deserialize(parsed); + const { segmentIdMap } = useSegmentStore().deserialize(parsed, { + 'img-1': 'new-1', + }); const result = await store().deserialize({ manifest: parsed, stateFiles, @@ -187,7 +189,9 @@ describe('mask io concurrency', () => { setActivePinia(createPinia()); await seatSpecImage('new-1', 'CT A'); - const { segmentIdMap } = useSegmentStore().deserialize(parsed); + const { segmentIdMap } = useSegmentStore().deserialize(parsed, { + 'img-1': 'new-1', + }); await store().deserialize({ manifest: parsed, stateFiles, @@ -229,7 +233,8 @@ describe('mask io concurrency', () => { manifest: parsed, stateFiles, dataIDMap: { 'img-1': 'new-1' }, - segmentIdMap: useSegmentStore().deserialize(parsed).segmentIdMap, + segmentIdMap: useSegmentStore().deserialize(parsed, { 'img-1': 'new-1' }) + .segmentIdMap, io: save.io, }); await nextTick(); diff --git a/src/io/state-file/__tests__/migrate640To700.spec.ts b/src/io/state-file/__tests__/migrate640To700.spec.ts index 0ae4edc86..56d0ea4c8 100644 --- a/src/io/state-file/__tests__/migrate640To700.spec.ts +++ b/src/io/state-file/__tests__/migrate640To700.spec.ts @@ -3,7 +3,10 @@ import { setActivePinia, createPinia } from 'pinia'; import { nextTick } from 'vue'; import { ManifestSchema } from '@/src/io/state-file/schema'; -import { migrateManifest } from '@/src/io/state-file/migrations'; +import { + migrate640To700, + migrateManifest, +} from '@/src/io/state-file/migrations'; import { MANIFEST_VERSION } from '@/src/io/state-file/serialize'; import { leafStateId } from '@/src/io/import/dataSource'; import { completeStateFileRestore } from '@/src/io/import/processors/restoreStateFile'; @@ -55,7 +58,7 @@ const legacyManifest = (overrides: Record) => }); const migrate = (overrides: Record) => - migrateManifest(legacyManifest(overrides)) as any; + migrate640To700(JSON.parse(legacyManifest(overrides))) as any; type LegacyMask = { value: number; @@ -185,7 +188,7 @@ describe('migrate640To700: structural stage', () => { ], }); - expect(migrated.version).toBe(MANIFEST_VERSION); + expect(migrated.version).toBe('7.0.0'); expect(migrated.segmentGroups).toBeUndefined(); expect(migrated.segmentationArtifacts).toHaveLength(1); @@ -974,7 +977,7 @@ describe('migrate640To700: structural stage', () => { }, }); - expect(migrated.version).toBe(MANIFEST_VERSION); + expect(migrated.version).toBe('7.0.0'); expect(migrated.segments).toEqual([ { id: expect.any(String), @@ -996,7 +999,7 @@ describe('migrate640To700: structural stage', () => { // --------------------------------------------------------------------------- // Loaded stage + round trip: a migrated 6.4.0 file restores through the real // import path, resolves its placeholder extents against the loaded parent, and -// re-saves as 7.0.0 that reloads identically. +// re-saves with image-owned segment sets that reload identically. // --------------------------------------------------------------------------- const legacyScene = () => @@ -1094,9 +1097,9 @@ describe('migrated 6.4.0 state file: loaded stage and round trip', () => { const store = useSegmentationStore(); const segments = useSegmentStore().segments; const segmentation = store.getSegmentationForImage('store-ct')!; - expect(segments.appearanceOf(segments.selectedSegmentId.value).name).toBe( - 'Edema' - ); + expect( + segments.appearanceOf(segments.selectedSegmentId('store-ct')).name + ).toBe('Edema'); expect( segmentation.order.map( (id) => segments.appearanceOf(segmentation.masks[id].segmentId).name @@ -1128,7 +1131,7 @@ describe('migrated 6.4.0 state file: loaded stage and round trip', () => { 0.4 ); }); - const drawn = segments.findSegmentByName('Drawn')!; + const drawn = segments.findSegmentByName('store-ct', 'Drawn')!; expect(segments.appearanceOf(drawn.id)).toMatchObject({ fillOpacity: 1, outlineOpacity: 1, @@ -1159,9 +1162,9 @@ describe('migrated 6.4.0 state file: loaded stage and round trip', () => { await restoreLegacyScene(); const segments = useSegmentStore().segments; - expect(segments.segmentList.value.map((segment) => segment.name)).toContain( - 'Planned' - ); + expect( + segments.segmentList('store-ct').map((segment) => segment.name) + ).toContain('Planned'); // Offered, not painted: nothing was drawn with it. const segmentation = @@ -1173,7 +1176,7 @@ describe('migrated 6.4.0 state file: loaded stage and round trip', () => { ).not.toContain('Planned'); }); - it('re-saves as 7.0.0 and reloads identically', async () => { + it('re-saves with image ownership and reloads identically', async () => { await restoreLegacyScene(); const before = segmentationSnapshot('store-ct'); // Named after the loaded image, as a live segmentation is. @@ -1194,7 +1197,9 @@ describe('migrated 6.4.0 state file: loaded stage and round trip', () => { manifest: saved, stateFiles, dataIDMap: { 'store-ct': 'new-ct' }, - segmentIdMap: useSegmentStore().deserialize(saved).segmentIdMap, + segmentIdMap: useSegmentStore().deserialize(saved, { + 'store-ct': 'new-ct', + }).segmentIdMap, io, }); await nextTick(); diff --git a/src/io/state-file/__tests__/migrate700To800.spec.ts b/src/io/state-file/__tests__/migrate700To800.spec.ts new file mode 100644 index 000000000..d9c9960b5 --- /dev/null +++ b/src/io/state-file/__tests__/migrate700To800.spec.ts @@ -0,0 +1,227 @@ +import { describe, expect, it } from 'vitest'; + +import { migrateManifest } from '@/src/io/state-file/migrations'; +import { ManifestSchema } from '@/src/io/state-file/schema'; +import { omit } from '@/src/utils'; + +const tumor = { + id: 'tumor', + name: 'Tumor', + color: [255, 10, 20, 128], + visible: false, + locked: true, + fillOpacity: 0.4, + outlineOpacity: 0.8, + strokeWidth: 3, +}; +const planned = { + id: 'planned', + name: 'Planned', + color: [0, 255, 0, 255], +}; + +const migrate = (overrides: Record = {}) => + ManifestSchema.parse( + migrateManifest( + JSON.stringify({ + version: '7.0.0', + dataSources: [ + { id: 1, type: 'uri', uri: 'https://example.test/a.nrrd' }, + { id: 2, type: 'uri', uri: 'https://example.test/b.nrrd' }, + ], + datasets: [ + { id: 'a', dataSourceId: 1 }, + { id: 'b', dataSourceId: 2 }, + ], + segments: [tumor, planned], + selectedSegment: tumor.id, + ...overrides, + }) + ) + ); + +const mask = (id: string) => ({ + id, + segmentId: tumor.id, + representations: { + labelmap: { path: `${id}.vti`, extent: [0, 1, 0, 1, 0, 1] }, + }, +}); + +describe('7.0.0 image segment ownership migration', () => { + it('splits shared identities and preserves each image catalog and selection', () => { + const migrated = migrate({ + segmentations: ['a', 'b'].map((parentImage) => ({ + id: `segmentation-${parentImage}`, + name: parentImage, + parentImage, + masks: [mask(`mask-${parentImage}`)], + order: [`mask-${parentImage}`], + })), + }); + + expect(migrated.version).toBe('8.0.0'); + expect(migrated).not.toHaveProperty('selectedSegment'); + const [a, b] = migrated.imageSegmentSets!; + expect(a.imageId).toBe('a'); + expect(b.imageId).toBe('b'); + expect(a.segmentIds).toHaveLength(2); + expect(b.segmentIds).toHaveLength(2); + expect(new Set([...a.segmentIds, ...b.segmentIds]).size).toBe(4); + expect(a.selectedSegmentId).toBe(a.segmentIds[0]); + expect(b.selectedSegmentId).toBe(b.segmentIds[0]); + for (const set of [a, b]) { + expect( + set.segmentIds.map( + (id) => migrated.segments!.find((segment) => segment.id === id)?.name + ) + ).toEqual(['Tumor', 'Planned']); + expect( + migrated.segments!.find((segment) => segment.id === set.segmentIds[0]) + ).toMatchObject({ ...tumor, id: set.segmentIds[0] }); + } + expect( + migrated.segmentations!.map((segmentation) => ({ + segmentId: segmentation.masks[0].segmentId, + binding: segmentation.masks[0].representations.labelmap, + })) + ).toEqual([ + { + segmentId: a.segmentIds[0], + binding: mask('mask-a').representations.labelmap, + }, + { + segmentId: b.segmentIds[0], + binding: mask('mask-b').representations.labelmap, + }, + ]); + }); + + it('remaps each annotation kind through the shape image', () => { + const geometry = { + frameOfReference: { planeOrigin: [0, 0, 0], planeNormal: [0, 0, 1] }, + slice: 0, + firstPoint: [0, 0, 0], + secondPoint: [1, 1, 0], + points: [ + [0, 0, 0], + [1, 0, 0], + [1, 1, 0], + ], + }; + const tools = Object.fromEntries( + ['rulers', 'rectangles', 'polygons'].map((key) => [ + key, + { + tools: ['a', 'b'].map((imageID) => ({ + ...geometry, + imageID, + segmentId: tumor.id, + })), + }, + ]) + ); + const migrated = migrate({ tools }); + for (const key of ['rulers', 'rectangles', 'polygons'] as const) { + expect( + migrated.tools![key]!.tools.map(({ segmentId }) => segmentId) + ).toEqual( + migrated.imageSegmentSets!.map( + ({ selectedSegmentId }) => selectedSegmentId + ) + ); + } + }); + + it('remaps each en face segment through the image it displays', () => { + const enFace = (segmentId: string) => ({ + enFace: { + axis: 1, + method: 'mean', + depthStart: 0, + depthEnd: null, + highlightThin: true, + thresholdMicrons: 15, + segmentId, + }, + }); + const migrated = migrate({ + viewByID: { + view: { + id: 'view', + type: 'EnFace', + name: 'En face', + config: { a: enFace(planned.id), b: enFace(planned.id) }, + }, + }, + }); + + const [a, b] = migrated.imageSegmentSets!; + const { config } = migrated.viewByID!.view; + expect(config!.a.enFace!.segmentId).toBe(a.segmentIds[1]); + expect(config!.b.enFace!.segmentId).toBe(b.segmentIds[1]); + }); + + it('leaves images uninitialized when the legacy catalog is empty', () => { + const migrated = migrate({ segments: [], selectedSegment: 'missing' }); + expect(migrated.imageSegmentSets).toEqual([]); + }); + + it('preserves repeated ids for restore reporting without changing references', () => { + const migrated = migrate({ + segments: [tumor, { ...tumor, name: 'Repeated' }], + }); + const [a, b] = migrated.imageSegmentSets!; + expect(a.segmentIds).toHaveLength(1); + expect(b.segmentIds).toHaveLength(1); + expect(migrated.segments!.map(({ id }) => id)).toEqual([ + a.segmentIds[0], + a.segmentIds[0], + b.segmentIds[0], + b.segmentIds[0], + ]); + }); + + it('keeps a missing reference from aliasing a generated segment identity', () => { + const missingId = 'image-1-segment-1'; + const migrated = migrate({ + segmentations: [ + { + id: 'seg-a', + name: 'a', + parentImage: 'a', + order: ['broken'], + masks: [{ ...mask('broken'), segmentId: missingId }], + }, + ], + }); + expect(migrated.segmentations![0].masks[0].segmentId).toBe(missingId); + expect(migrated.segments!.some(({ id }) => id === missingId)).toBe(false); + }); + + it('retains an image-free catalog as seed defaults', () => { + const migrated = migrate({ datasets: [], dataSources: [] }); + expect(migrated.imageSegmentSets).toEqual([]); + expect(migrated.segments).toEqual([]); + expect(migrated.seedSegmentDefaults).toEqual( + [ + { ...tumor, id: undefined }, + { ...planned, id: undefined, visible: true, locked: false }, + ].map((definition) => omit(definition, ['id'])) + ); + }); + + it('uses legacy URI dataset identities when datasets were not saved', () => { + const migrated = migrate({ datasets: undefined }); + expect(migrated.imageSegmentSets!.map(({ imageId }) => imageId)).toEqual([ + '1', + '2', + ]); + }); + + it('leaves an 8.0.0 archive unchanged, including a deliberately empty set', () => { + const manifest = migrate({ segments: [] }); + manifest.imageSegmentSets = [{ imageId: 'a', segmentIds: [] }]; + expect(migrateManifest(JSON.stringify(manifest))).toEqual(manifest); + }); +}); diff --git a/src/io/state-file/__tests__/segmentationArtifactSource.spec.ts b/src/io/state-file/__tests__/segmentationArtifactSource.spec.ts index ad9664683..c95721428 100644 --- a/src/io/state-file/__tests__/segmentationArtifactSource.spec.ts +++ b/src/io/state-file/__tests__/segmentationArtifactSource.spec.ts @@ -169,9 +169,7 @@ describe('Segmentation wire shape', () => { }); describe('manifest version', () => { - // The segment model replaces `segmentGroups` with `segmentations` plus - // `segmentationArtifacts`, including display state in that breaking change. - it('pins MANIFEST_VERSION at 7.0.0', () => { - expect(MANIFEST_VERSION).toBe('7.0.0'); + it('writes the image segment ownership version', () => { + expect(MANIFEST_VERSION).toBe('8.0.0'); }); }); diff --git a/src/io/state-file/__tests__/segmentationDisplayRoundTrip.spec.ts b/src/io/state-file/__tests__/segmentationDisplayRoundTrip.spec.ts index c3d3b4130..276ff5b78 100644 --- a/src/io/state-file/__tests__/segmentationDisplayRoundTrip.spec.ts +++ b/src/io/state-file/__tests__/segmentationDisplayRoundTrip.spec.ts @@ -3,7 +3,6 @@ import { setActivePinia, createPinia } from 'pinia'; import { seatSpecImage, inMemoryArtifactIO, - mintSegment, manifestForImages, serializeToStateFiles, store, @@ -11,6 +10,7 @@ import { import { nextTick } from 'vue'; import { ManifestSchema } from '@/src/io/state-file/schema'; +import { migrateManifest } from '@/src/io/state-file/migrations'; import { useSegmentStore } from '@/src/segmentation/segments'; import { DEFAULT_SEGMENTATION_DISPLAY } from '@/src/segmentation/model'; @@ -25,14 +25,17 @@ const baseManifest = () => manifestForImages(['img-1']); /** A scene whose display state is nowhere near the defaults. */ function buildScene() { const segmentation = store().ensureSegmentationForImage('img-1'); - const tumorSegmentId = mintSegment({ + const tumorSegmentId = useSegmentStore().segments.mintSegment('img-1', { name: 'Tumor', fillOpacity: 0.25, outlineOpacity: 0.75, }); const tumor = store().createMask(segmentation.id, tumorSegmentId); store().ensureLabelmapBinding(tumor.id); - const plannedSegmentId = mintSegment({ name: 'Planned', fillOpacity: 0 }); + const plannedSegmentId = useSegmentStore().segments.mintSegment('img-1', { + name: 'Planned', + fillOpacity: 0, + }); store().createMask(segmentation.id, plannedSegmentId); const model = store().segmentations[segmentation.id]; @@ -52,7 +55,8 @@ const opacitiesOf = (segmentIds: string[]) => { // A 7.0.0 manifest that states no display state anywhere. const manifest700 = () => ({ version: '7.0.0', - dataSources: [], + dataSources: [{ id: 1, type: 'uri', uri: '/img-1.nrrd' }], + datasets: [{ id: 'img-1', dataSourceId: 1 }], segments: [{ id: 't-1', name: 'Tumor', color: [255, 0, 0, 255] }], segmentations: [ { @@ -109,7 +113,8 @@ describe('segmentation display state on the wire', () => { manifest: parsed, stateFiles: stateFiles, dataIDMap: { 'img-1': 'new-1' }, - segmentIdMap: useSegmentStore().deserialize(parsed).segmentIdMap, + segmentIdMap: useSegmentStore().deserialize(parsed, { 'img-1': 'new-1' }) + .segmentIdMap, io: io, }); await nextTick(); @@ -126,7 +131,9 @@ describe('segmentation display state on the wire', () => { }); it('fills defaults for a manifest that carries no display state', () => { - const parsed = ManifestSchema.parse(manifest700()); + const parsed = ManifestSchema.parse( + migrateManifest(JSON.stringify(manifest700())) + ); const wire = parsed.segmentations![0]; expect(wire).toMatchObject(DEFAULT_SEGMENTATION_DISPLAY); @@ -136,13 +143,16 @@ describe('segmentation display state on the wire', () => { }); it('restores a 7.0.0 manifest with default display state', async () => { - const parsed = ManifestSchema.parse(manifest700()); + const parsed = ManifestSchema.parse( + migrateManifest(JSON.stringify(manifest700())) + ); await store().deserialize({ manifest: parsed, stateFiles: [], dataIDMap: { 'img-1': 'img-1' }, - segmentIdMap: useSegmentStore().deserialize(parsed).segmentIdMap, + segmentIdMap: useSegmentStore().deserialize(parsed, { 'img-1': 'img-1' }) + .segmentIdMap, io: inMemoryArtifactIO(), }); await nextTick(); diff --git a/src/io/state-file/__tests__/segmentationRoundTrip.spec.ts b/src/io/state-file/__tests__/segmentationRoundTrip.spec.ts index 745580fbb..c5262623d 100644 --- a/src/io/state-file/__tests__/segmentationRoundTrip.spec.ts +++ b/src/io/state-file/__tests__/segmentationRoundTrip.spec.ts @@ -3,7 +3,6 @@ import { setActivePinia, createPinia } from 'pinia'; import { seatSpecImage, inMemoryArtifactIO, - mintSegment, segmentOfMask, manifestForImages, segmentationSnapshot, @@ -29,19 +28,25 @@ async function buildScene() { const first = store.ensureSegmentationForImage('img-1'); // No binding: a segment created by "add" has no voxels until a first edit. - const planned = store.createMask(first.id, mintSegment({ name: 'Planned' })); + const planned = store.createMask( + first.id, + useSegmentStore().segments.mintSegment('img-1', { name: 'Planned' }) + ); useSegmentStore().segments.updateSegment(segmentOfMask(planned.id), { locked: true, visible: false, }); - const tumor = store.createMask(first.id, mintSegment({ name: 'Tumor' })); + const tumor = store.createMask( + first.id, + useSegmentStore().segments.mintSegment('img-1', { name: 'Tumor' }) + ); store.ensureLabelmapBinding(tumor.id).source = SOURCE; // Same name on another image: still a distinct segment. const second = store.ensureSegmentationForImage('img-2'); const otherTumor = store.createMask( second.id, - mintSegment({ name: 'Tumor' }) + useSegmentStore().segments.mintSegment('img-2', { name: 'Tumor' }) ); store.ensureLabelmapBinding(otherTumor.id); @@ -74,7 +79,10 @@ describe('segmentation state-file round trip', () => { setActivePinia(createPinia()); await seatSpecImage('new-1', 'CT A'); await seatSpecImage('new-2', 'CT B'); - const { segmentIdMap } = useSegmentStore().deserialize(parsed); + const { segmentIdMap } = useSegmentStore().deserialize(parsed, { + 'img-1': 'new-1', + 'img-2': 'new-2', + }); await useSegmentationStore().deserialize({ manifest: parsed, stateFiles: stateFiles, @@ -85,9 +93,6 @@ describe('segmentation state-file round trip', () => { await nextTick(); expect(segmentationSnapshot('new-1')).toEqual(before.first); - expect(segmentationSnapshot('new-2')).toEqual({ - ...before.second, - segments: [{ ...before.second.segments[0], name: 'Tumor (2)' }], - }); + expect(segmentationSnapshot('new-2')).toEqual(before.second); }); }); diff --git a/src/io/state-file/__tests__/serializeResilience.spec.ts b/src/io/state-file/__tests__/serializeResilience.spec.ts index f9d1b6724..ff1eb299b 100644 --- a/src/io/state-file/__tests__/serializeResilience.spec.ts +++ b/src/io/state-file/__tests__/serializeResilience.spec.ts @@ -303,7 +303,14 @@ describe('state-file serialization resilience', () => { locked: true, }, ], - selectedSegment: 'segment-1', + imageSegmentSets: [ + { + imageId: 'dataset-1', + segmentIds: ['segment-1'], + selectedSegmentId: 'segment-1', + }, + ], + seedSegmentDefaults: [], } as unknown as Manifest; const zip = new JSZip(); @@ -329,7 +336,25 @@ describe('state-file serialization resilience', () => { locked: true, }, ]); - expect(normalized.manifest.selectedSegment).toBe('segment-1'); + expect(normalized.manifest.imageSegmentSets).toEqual([ + { + imageId: 'dataset-1', + segmentIds: ['segment-1'], + selectedSegmentId: 'segment-1', + }, + ]); + expect(normalized.manifest.seedSegmentDefaults).toEqual([]); + }); + + it('keeps an initialized empty image set through normalization', () => { + const manifest = { + ...manifestWithSelection('dataset-1'), + segments: [], + imageSegmentSets: [{ imageId: 'dataset-1', segmentIds: [] }], + }; + expect( + normalizeManifest(manifest, new JSZip()).manifest.imageSegmentSets + ).toEqual([{ imageId: 'dataset-1', segmentIds: [] }]); }); it('saves no import instructions', () => { diff --git a/src/io/state-file/migrations.ts b/src/io/state-file/migrations.ts index 24115681f..b7019bc28 100644 --- a/src/io/state-file/migrations.ts +++ b/src/io/state-file/migrations.ts @@ -297,11 +297,21 @@ const dropLegacyViewDisplay = (manifest: any, groupIds: string[]) => { }); }; +const ANNOTATION_TOOL_KEYS = ['rulers', 'rectangles', 'polygons']; + +// Suffixes a candidate until no id in `usedIds` has it, then claims it. +const uniqueIdAmong = (usedIds: Set) => (candidate: string) => { + let id = candidate; + for (let n = 2; usedIds.has(id); n += 1) id = `${candidate}-${n}`; + usedIds.add(id); + return id; +}; + // 6.4.0 -> 7.0.0 moves identity off segment groups and tool labels onto // segments. JSON only: no voxels are read here, so each group becomes an // artifact the loaded restore stage splits into bounded masks, and every // binding's extent is a placeholder that stage replaces. -const migrate640To700 = (inputManifest: any) => { +export const migrate640To700 = (inputManifest: any) => { const manifest = JSON.parse(JSON.stringify(inputManifest)); // Insertion order is the migrated order: groups in manifest order, then per @@ -310,13 +320,7 @@ const migrate640To700 = (inputManifest: any) => { const segments: any[] = []; // Joined legacy ids can collide, and restore keys a map on them. - const usedIds = new Set(); - const uniqueId = (candidate: string) => { - let id = candidate; - for (let n = 2; usedIds.has(id); n += 1) id = `${candidate}-${n}`; - usedIds.add(id); - return id; - }; + const uniqueId = uniqueIdAmong(new Set()); const addSegment = (id: string, segment: any) => { segments.push({ id, ...segment }); @@ -405,8 +409,6 @@ const migrate640To700 = (inputManifest: any) => { }; }); - const toolKeys = ['rulers', 'rectangles', 'polygons']; - const nextToolColor = cycle(TOOL_COLORS); const nextPaletteColor = () => cssColorToRGBA(nextToolColor()); @@ -417,7 +419,7 @@ const migrate640To700 = (inputManifest: any) => { tryCssColorToRGBA(css ?? '') ?? nextPaletteColor(); const labelNames = new Set( - toolKeys.flatMap((key) => + ANNOTATION_TOOL_KEYS.flatMap((key) => Object.entries(manifest.tools?.[key]?.labels ?? {}).map( ([labelId, label]: [string, any]) => label.labelName || labelId ) @@ -493,7 +495,7 @@ const migrate640To700 = (inputManifest: any) => { delete entry.labels; }; - toolKeys.forEach(migrateToolLabels); + ANNOTATION_TOOL_KEYS.forEach(migrateToolLabels); const segmentations = [...recordsByParent.entries()].map( ([parentImage, records]) => { @@ -526,6 +528,109 @@ const migrate640To700 = (inputManifest: any) => { return manifest; }; +// View configs are keyed by the image they display. +const remapEnFaceSegments = ( + manifest: any, + mappedSegmentId: (imageId: string, segmentId: string) => string +) => { + Object.values(manifest.viewByID ?? {}).forEach((view: any) => { + Object.entries(view?.config ?? {}).forEach(([imageId, config]: any) => { + if (typeof config?.enFace?.segmentId === 'string') + config.enFace.segmentId = mappedSegmentId( + imageId, + config.enFace.segmentId + ); + }); + }); +}; + +// Every saved image keeps the full catalog the global picker offered, including +// unused rows. Mask buffers stay in place; only their segment references split. +const migrate700To800 = (inputManifest: any) => { + const manifest = JSON.parse(JSON.stringify(inputManifest)); + const catalog: any[] = Array.isArray(manifest.segments) + ? manifest.segments + : []; + const datasets: any[] = + manifest.datasets ?? + (manifest.dataSources ?? []) + .filter((source: any) => source.type === 'uri') + .map((source: any) => ({ id: String(source.id) })); + const imageIds = [...new Set(datasets.map(({ id }) => id))]; + const referencedIds = [ + ...(manifest.segmentations ?? []).flatMap((segmentation: any) => + (segmentation.masks ?? []).map((mask: any) => mask.segmentId) + ), + ...ANNOTATION_TOOL_KEYS.flatMap((key) => + (manifest.tools?.[key]?.tools ?? []).map((tool: any) => tool.segmentId) + ), + ]; + // Missing references must stay missing rather than alias a newly minted id. + const uniqueId = uniqueIdAmong( + new Set([...catalog.map(({ id }) => id), ...referencedIds]) + ); + const idsByImage = new Map>(); + const segments: any[] = []; + const imageSegmentSets = imageIds.flatMap((imageId, imageIndex) => { + if (catalog.length === 0) return []; + const segmentIds = new Map(); + idsByImage.set(imageId, segmentIds); + catalog.forEach((segment, segmentIndex) => { + let id = segmentIds.get(segment.id); + if (id === undefined) { + id = uniqueId(`image-${imageIndex + 1}-segment-${segmentIndex + 1}`); + segmentIds.set(segment.id, id); + } + // A repeated wire id remains repeated so restore reports the bad row + // instead of converting it into a second, apparently valid definition. + segments.push({ + ...segment, + id, + ...(Array.isArray(segment.color) ? { color: [...segment.color] } : {}), + }); + }); + const selectedSegmentId = segmentIds.get(manifest.selectedSegment); + return { + imageId, + segmentIds: [...segmentIds.values()], + legacySegmentIds: Object.fromEntries( + [...segmentIds].map(([legacyId, id]) => [id, legacyId]) + ), + ...(selectedSegmentId ? { selectedSegmentId } : {}), + }; + }); + + const mappedSegmentId = (imageId: string, segmentId: string) => + idsByImage.get(imageId)?.get(segmentId) ?? segmentId; + (manifest.segmentations ?? []).forEach((segmentation: any) => { + (segmentation.masks ?? []).forEach((mask: any) => { + mask.segmentId = mappedSegmentId( + segmentation.parentImage, + mask.segmentId + ); + }); + }); + ANNOTATION_TOOL_KEYS.forEach((key) => { + (manifest.tools?.[key]?.tools ?? []).forEach((tool: any) => { + if (tool.segmentId !== undefined) + tool.segmentId = mappedSegmentId(tool.imageID, tool.segmentId); + }); + }); + remapEnFaceSegments(manifest, mappedSegmentId); + + if (imageIds.length === 0 && catalog.length > 0) { + manifest.seedSegmentDefaults = catalog.map((segment) => + omit(segment, ['id']) + ); + } + if (manifest.segments === undefined || Array.isArray(manifest.segments)) + manifest.segments = segments; + manifest.imageSegmentSets = imageSegmentSets; + delete manifest.selectedSegment; + manifest.version = '8.0.0'; + return manifest; +}; + export const migrateManifest = (manifestString: string) => { const inputManifest = JSON.parse(manifestString); return pipe( @@ -535,6 +640,7 @@ export const migrateManifest = (manifestString: string) => { migrateOrPass(['6.1.0', '6.1.1'], migrate610To620), migrateOrPass(['6.3.0'], migrate630To640), // No 6.2.0 -> 6.3.0 step exists, so a 6.2 manifest arrives here directly. - migrateOrPass(['6.2.0', '6.4.0'], migrate640To700) + migrateOrPass(['6.2.0', '6.4.0'], migrate640To700), + migrateOrPass(['7.0.0'], migrate700To800) ); }; diff --git a/src/io/state-file/schema.ts b/src/io/state-file/schema.ts index 224c56bed..3943234fa 100644 --- a/src/io/state-file/schema.ts +++ b/src/io/state-file/schema.ts @@ -382,6 +382,21 @@ export const Segment = z.object({ strokeWidth: z.number().optional(), }) satisfies z.ZodType; +export const ImageSegmentSet = z.object({ + imageId: z.string(), + segmentIds: z.string().array(), + selectedSegmentId: z.string().optional(), + // Migration-only: datasets can resolve to the same image, whose copies of + // each legacy shared definition must resolve to one segment. + legacySegmentIds: z.record(z.string(), z.string()).optional(), +}); + +export type ImageSegmentSet = z.infer; + +const SegmentDefaults = Segment.omit({ id: true }).extend({ + color: RGBAColor.optional(), +}); + export const Segmentation = z.object({ id: z.string(), name: z.string(), @@ -535,7 +550,8 @@ export const ManifestSchema = z.object({ segmentations: ImportedSegmentation.array().optional(), segmentationArtifacts: SegmentationArtifact.array().optional(), segments: Segment.array().optional(), - selectedSegment: z.string().optional(), + imageSegmentSets: ImageSegmentSet.array().optional(), + seedSegmentDefaults: SegmentDefaults.array().optional(), tools: Tools.optional(), activeView: z.string().optional().nullable(), isActiveViewMaximized: z.boolean().optional(), diff --git a/src/io/state-file/serialize.ts b/src/io/state-file/serialize.ts index 9a835f2cb..27a8c9b2b 100644 --- a/src/io/state-file/serialize.ts +++ b/src/io/state-file/serialize.ts @@ -42,7 +42,7 @@ declareManifestRefs('primarySelection', (manifest) => ); export const MANIFEST = 'manifest.json'; -export const MANIFEST_VERSION = '7.0.0'; +export const MANIFEST_VERSION = '8.0.0'; type ManifestCandidate = Record; @@ -250,7 +250,8 @@ export function normalizeManifest(manifest: Manifest, zip: JSZip) { 'layerDisplay', 'tools', 'segments', - 'selectedSegment', + 'imageSegmentSets', + 'seedSegmentDefaults', 'activeView', 'isActiveViewMaximized', 'viewByID', diff --git a/src/processing/__tests__/applyResults.annotations.spec.ts b/src/processing/__tests__/applyResults.annotations.spec.ts index a2e969ace..c541a59b3 100644 --- a/src/processing/__tests__/applyResults.annotations.spec.ts +++ b/src/processing/__tests__/applyResults.annotations.spec.ts @@ -27,14 +27,11 @@ import { useProcessingJobsStore } from '@/src/processing/store'; import { useMessageStore } from '@/src/store/messages'; import { seatDataSource } from '@/src/store/__tests__/datasetFixtures'; import { makeFakeProvider, registerFake } from './fakeProvider'; -import { - savedMasks, - serializeAnnotations, - serializeScene, -} from './serializedScene'; +import { savedMasks, serializeScene } from './serializedScene'; import { TOOL_COLORS } from '@/src/config'; import { mintSegment, + serializeAnnotations, lockSegment, } from '@/src/segmentation/__tests__/segmentMaskFixtures'; import { useRulerStore } from '@/src/store/tools/rulers'; @@ -64,6 +61,10 @@ const resultServer = () => { let results = resultServer(); const IMAGE_ID = 'img-1'; +const savedSelection = (imageId = IMAGE_ID) => + serializeAnnotations().imageSegmentSets?.find( + (set) => set.imageId === imageId + )?.selectedSegmentId; // A 20mm cube at the origin with unit spacing: world LPS mm and image indices // coincide, so a plane origin's z IS its slice. @@ -234,6 +235,27 @@ beforeEach(() => { }); describe('applyIntent — add-annotations', () => { + it('keeps the image copy when applying annotation results', async () => { + seatImage('previous'); + useViewStore().setDataForAllViews('previous'); + await nextTick(); + useSegmentStore().segments.addSegment('previous', { name: 'Liver' }); + useViewStore().setDataForAllViews(IMAGE_ID); + await nextTick(); + + expect((await apply(intent(), context(IMAGE_ID))).status).toBe('applied'); + const saved = serializeAnnotations(); + const set = saved.imageSegmentSets!.find( + ({ imageId }) => imageId === IMAGE_ID + )!; + expect( + set.segmentIds.map( + (id) => saved.segments!.find((segment) => segment.id === id)!.name + ) + ).toEqual(['Liver', 'roi', 'lesion']); + expect(toolCounts()).toEqual({ rulers: 1, rectangles: 1, polygons: 1 }); + }); + it('adds every tool kind to the job image, deriving the slice from the frame', async () => { const outcome = await apply(intent(), context(IMAGE_ID)); expect(outcome.status).toBe('applied'); @@ -371,7 +393,7 @@ describe('applyIntent — add-annotations', () => { it('binds an existing segment of the same name instead of minting one', async () => { const registry = useSegmentStore().segments; - const existingId = registry.addSegment({ + const existingId = registry.addSegment(IMAGE_ID, { name: 'Measured', color: cssColorToRGBA('#ff0000'), }); @@ -393,7 +415,7 @@ describe('applyIntent — add-annotations', () => { it('keeps a segment’s colour when a label states one the parser rejects', async () => { const registry = useSegmentStore().segments; - const existingId = registry.addSegment({ + const existingId = registry.addSegment(IMAGE_ID, { name: 'Measured', color: cssColorToRGBA('#ff0000'), }); @@ -446,7 +468,7 @@ describe('applyIntent — add-annotations', () => { it('binds a locked segment without touching its mask', async () => { const segmentationStore = useSegmentationStore(); const segmentation = segmentationStore.ensureSegmentationForImage(IMAGE_ID); - const lockedSegment = mintSegment({ + const lockedSegment = mintSegment(IMAGE_ID, { name: 'roi', color: [17, 34, 51, 255], }); @@ -484,8 +506,8 @@ describe('applyIntent — add-annotations', () => { it('leaves the picker where the user left it', async () => { const registry = useSegmentStore().segments; - const selectedBefore = registry.addSegment({ name: 'Chosen' }); - expect(serializeAnnotations().selectedSegment).toBe(selectedBefore); + const selectedBefore = registry.addSegment(IMAGE_ID, { name: 'Chosen' }); + expect(savedSelection()).toBe(selectedBefore); const file = annotationsFile(); // A name no segment carries, so binding must MINT one, the case that could @@ -495,7 +517,7 @@ describe('applyIntent — add-annotations', () => { serveFile(file); expect((await apply(intent(), context(IMAGE_ID))).status).toBe('applied'); - expect(serializeAnnotations().selectedSegment).toBe(selectedBefore); + expect(savedSelection()).toBe(selectedBefore); // The segment still landed; only the picker was left alone. const ruler = firstRuler(); expect( @@ -504,20 +526,29 @@ describe('applyIntent — add-annotations', () => { ).toBe('fresh'); }); - it('preserves the user’s selected segment while result segments land', async () => { + it('preserves each image’s selected segment while results land on their input image', async () => { seatImage('origin-image'); seatImage('next-image'); const segmentationStore = useSegmentationStore(); const originSegmentation = segmentationStore.ensureSegmentationForImage('origin-image'); - const selectedSegment = mintSegment({ name: 'User selection' }); + const selectedSegment = mintSegment('origin-image', { + name: 'User selection', + }); segmentationStore.createMask(originSegmentation.id, selectedSegment); useSegmentStore().segments.selectSegment(selectedSegment); - useViewStore().setDataForAllViews(IMAGE_ID); + const registry = useSegmentStore().segments; + const onTarget = registry.addSegment(IMAGE_ID, { name: 'Input selection' }); + const onNext = registry.addSegment('next-image', { + name: 'Next selection', + }); + useViewStore().setDataForAllViews('next-image'); await nextTick(); expect((await apply(intent(), context(IMAGE_ID))).status).toBe('applied'); - expect(serializeAnnotations().selectedSegment).toBe(selectedSegment); + expect(savedSelection('origin-image')).toBe(selectedSegment); + expect(savedSelection()).toBe(onTarget); + expect(savedSelection('next-image')).toBe(onNext); const next = segmentationStore.resolveEditTarget('next-image'); expect( @@ -525,7 +556,7 @@ describe('applyIntent — add-annotations', () => { await serializeScene(['origin-image', IMAGE_ID, 'next-image']), 'next-image' ) - ).toMatchObject([{ id: next, segmentId: selectedSegment }]); + ).toMatchObject([{ id: next, segmentId: onNext }]); }); it('leaves an unlabeled tool unlabeled', async () => { @@ -593,7 +624,9 @@ describe('applyIntent — add-annotations', () => { const otherImage = 'healthy-image'; seatImage(otherImage); const registry = useSegmentStore().segments; - const existingSegment = registry.addSegment({ name: 'Existing' }); + const existingSegment = registry.addSegment(otherImage, { + name: 'Existing', + }); const existingTool = useRulerStore().addTool({ imageID: otherImage, segmentId: existingSegment, diff --git a/src/processing/__tests__/applyResults.segments.spec.ts b/src/processing/__tests__/applyResults.segments.spec.ts index f34641b45..d44381891 100644 --- a/src/processing/__tests__/applyResults.segments.spec.ts +++ b/src/processing/__tests__/applyResults.segments.spec.ts @@ -16,6 +16,7 @@ import { usePolygonStore } from '@/src/store/tools/polygons'; import { defer } from '@/src/utils'; import { seatImage, + showImage, seedVoxel, store, } from '@/src/segmentation/__tests__/segmentMaskFixtures'; @@ -28,7 +29,7 @@ const blue = [0, 0, 255, 255] as [number, number, number, number]; const red = [255, 0, 0, 255] as [number, number, number, number]; const existingMask = (imageId: string, name: string) => { - const segmentId = registry().mintSegment({ name, color: red }); + const segmentId = registry().mintSegment(imageId, { name, color: red }); const maskId = store().resolveEditTarget(imageId, segmentId); seedVoxel(maskId, [1, 1, 1]); return { segmentId, maskId }; @@ -78,6 +79,29 @@ beforeEach(async () => { }); describe('processing segment identity', () => { + it('keeps the image copy when applying a segmentation result', async () => { + await showImage('parent-A'); + registry().addSegment('parent-A', { name: 'Liver' }); + await showImage('parent-B'); + + expect( + await importResult([{ value: 1, name: 'Tumor', color: blue }]) + ).toEqual({ status: 'applied' }); + const saved = await save(); + const set = saved.manifest.imageSegmentSets!.find( + ({ imageId }) => imageId === 'parent-B' + )!; + expect( + set.segmentIds.map( + (id) => + saved.manifest.segments!.find((segment) => segment.id === id)!.name + ) + ).toEqual(['Liver', 'Tumor']); + expect(savedMasks(saved, 'parent-B')).toMatchObject([ + { segment: { name: 'Tumor' } }, + ]); + }); + it("does not change another image's segment named after the output file", async () => { const { segmentId, maskId } = existingMask('parent-A', 'output'); const toolId = usePolygonStore().addTool({ @@ -111,7 +135,7 @@ describe('processing segment identity', () => { ]); }); - it('reuses the declared segment across images and keeps its existing appearance', async () => { + it('keeps same-named segments independent across images', async () => { const { segmentId } = existingMask('parent-A', 'Tumor'); registry().updateSegment(segmentId, { visible: false, locked: true }); @@ -121,12 +145,18 @@ describe('processing segment identity', () => { const saved = await save(); expect(savedMasks(saved, 'parent-B')).toMatchObject([ + { + segment: { name: 'Tumor', color: blue, visible: true, locked: false }, + }, + ]); + expect(savedMasks(saved, 'parent-B')[0].segmentId).not.toBe(segmentId); + expect(savedMasks(saved, 'parent-A')).toMatchObject([ { segmentId, segment: { name: 'Tumor', color: red, visible: false, locked: true }, }, ]); - expect(saved.manifest.segments).toHaveLength(1); + expect(saved.manifest.segments).toHaveLength(2); }); it('uses a distinct segment when the declared segment already has a mask on the target', async () => { diff --git a/src/processing/__tests__/declaredEmptySegments.spec.ts b/src/processing/__tests__/declaredEmptySegments.spec.ts index b97be925b..83c1c54ad 100644 --- a/src/processing/__tests__/declaredEmptySegments.spec.ts +++ b/src/processing/__tests__/declaredEmptySegments.spec.ts @@ -162,7 +162,9 @@ describe('a segment a result declares but leaves empty', () => { manifest: saved.manifest, stateFiles: saved.stateFiles, dataIDMap: { parent: 'new-parent' }, - segmentIdMap: useSegmentStore().deserialize(saved.manifest).segmentIdMap, + segmentIdMap: useSegmentStore().deserialize(saved.manifest, { + parent: 'new-parent', + }).segmentIdMap, io: saved.io, }); await nextTick(); diff --git a/src/processing/__tests__/job-adoption.spec.ts b/src/processing/__tests__/job-adoption.spec.ts index 7ea1864c7..9b703201a 100644 --- a/src/processing/__tests__/job-adoption.spec.ts +++ b/src/processing/__tests__/job-adoption.spec.ts @@ -11,8 +11,10 @@ import { seatDataSource, seatVolume, } from '@/src/store/__tests__/datasetFixtures'; -import { seatImage } from '@/src/segmentation/__tests__/segmentMaskFixtures'; -import { serializeAnnotations } from './serializedScene'; +import { + seatImage, + serializeAnnotations, +} from '@/src/segmentation/__tests__/segmentMaskFixtures'; const jobRef = { providerId: 'analysis', jobId: 'saved-job' }; const baseUris = ['/ct/2.dcm', '/ct/1.dcm']; diff --git a/src/processing/__tests__/segmentationPacking.spec.ts b/src/processing/__tests__/segmentationPacking.spec.ts index 9bee49bcc..4280bc723 100644 --- a/src/processing/__tests__/segmentationPacking.spec.ts +++ b/src/processing/__tests__/segmentationPacking.spec.ts @@ -139,10 +139,10 @@ const seedTwoImages = async () => { }; }; -describe('single-file packing against a selection from another image', () => { +describe('single-file packing with image-scoped selection', () => { beforeEach(() => setActivePinia(createPinia())); - it('packs a mask this image holds and says where to select', async () => { + it('keeps this image’s preferred mask when another image’s segment is selected', async () => { const { gamma, segmentation } = await seedTwoImages(); selectSegment(gamma); @@ -153,8 +153,7 @@ describe('single-file packing against a selection from another image', () => { namesAtVoxels(composeLabelmapPart(snapshot.parent, snapshot.parts[0])) ).toEqual(['Alpha', '', '', '', '', '', '', '']); expect(plan.warning).toBe( - 'This input accepts one labelmap. Omitted whole segments: Beta.' + - ' Select a segment on this image to prioritize it.' + 'This input accepts one labelmap. Omitted whole segments: Beta.' ); }); diff --git a/src/processing/__tests__/serializedScene.ts b/src/processing/__tests__/serializedScene.ts index 489a62a21..ecb4e0fec 100644 --- a/src/processing/__tests__/serializedScene.ts +++ b/src/processing/__tests__/serializedScene.ts @@ -1,26 +1,11 @@ -import JSZip from 'jszip'; import type { Manifest } from '@/src/io/state-file/schema'; -import { useRulerStore } from '@/src/store/tools/rulers'; -import { useRectangleStore } from '@/src/store/tools/rectangles'; -import { usePolygonStore } from '@/src/store/tools/polygons'; -import { useSegmentStore } from '@/src/segmentation/segments'; import { inMemoryArtifactIO, manifestForImages, serializeToStateFiles, + serializeAnnotations, } from '@/src/segmentation/__tests__/segmentMaskFixtures'; -export const serializeAnnotations = ( - manifest = manifestForImages([], { tools: {} }) -) => { - const state = { zip: new JSZip(), manifest }; - useSegmentStore().serialize(state); - useRulerStore().serialize(state); - useRectangleStore().serialize(state); - usePolygonStore().serialize(state); - return JSON.parse(JSON.stringify(manifest)) as Manifest; -}; - export const serializeScene = async (imageIds: string[]) => { const codec = inMemoryArtifactIO(); const snapshots = new Map< diff --git a/src/processing/applyResults.ts b/src/processing/applyResults.ts index fae8bde71..5c400edd7 100644 --- a/src/processing/applyResults.ts +++ b/src/processing/applyResults.ts @@ -256,6 +256,7 @@ const reportUnparseableColors = (rejected: Set) => { // Wire namespaces are per kind but bind into one registry, so the first kind to // bind a name sets its style. Unreferenced labels stay out of the picker. const bindReferencedSegments = ( + imageId: string, tools: readonly PreparedCore[], namespace: Record, rejected: Set @@ -268,6 +269,7 @@ const bindReferencedSegments = ( [...names].map((name) => [ name, segments.segmentNamed( + imageId, name, segmentInit(name, namespace[name] ?? {}, rejected) ), @@ -322,11 +324,13 @@ async function applyAnnotations( return { status: 'applied' }; } + useSegmentStore().prepareForEdit(parentSelection); const rejectedColors = new Set(); const segmentIds = Object.fromEntries( ANNOTATION_TOOL_KINDS.map((kind) => [ kind, bindReferencedSegments( + parentSelection, prepared[kind], decoded.labels[kind], rejectedColors @@ -388,8 +392,10 @@ export const appApplyDependencies = (): ApplyDependencies => ({ Object.values(useSegmentationStore().segmentations) .flatMap((segmentation) => listMasks(segmentation)) .map((mask) => mask.representations.labelmap?.source), - convertImageToLabelmap: (...args) => - useSegmentationStore().convertImageToLabelmap(...args), + convertImageToLabelmap: (...args) => { + useSegmentStore().prepareForEdit(args[1]); + return useSegmentationStore().convertImageToLabelmap(...args); + }, }, }); diff --git a/src/processing/components/JobsModule.vue b/src/processing/components/JobsModule.vue index 9843a798d..023acea5d 100644 --- a/src/processing/components/JobsModule.vue +++ b/src/processing/components/JobsModule.vue @@ -546,10 +546,11 @@ const overlapSignal = () => { return [ imageId, maskRevision.value, - segmentRegistry.selectedSegmentId.value, + segmentRegistry.selectedSegmentId(imageId), // The first or last finished tool can move a union input off the labelmap. finishedAnnotationCount.value > 0, - segmentRegistry.segmentList.value + segmentRegistry + .segmentList(imageId) .map(({ id, name }) => `${id}:${name}`) .join(), imageId diff --git a/src/processing/components/__tests__/JobsModule.spec.ts b/src/processing/components/__tests__/JobsModule.spec.ts index 8cd8a96ca..2d60224c6 100644 --- a/src/processing/components/__tests__/JobsModule.spec.ts +++ b/src/processing/components/__tests__/JobsModule.spec.ts @@ -490,7 +490,7 @@ describe('JobsModule segmentation staging', () => { segmentation.name = name; const segment = store.createMask( segmentation.id, - mintSegment({ name: 'Tumor' }) + mintSegment('image-1', { name: 'Tumor' }) ); // A segmentation with no voxel is no input, so it holds one. seedVoxel(segment.id, [1, 1, 1]); @@ -502,7 +502,7 @@ describe('JobsModule segmentation staging', () => { const { segmentationId, maskId } = seedSegmentation('Overlap'); const second = useSegmentationStore().createMask( segmentationId, - mintSegment({ + mintSegment('image-1', { name: 'Node', }) ); @@ -911,11 +911,11 @@ describe('JobsModule segmentation staging', () => { const segmentation = segmentStore.ensureSegmentationForImage('image-1'); const first = segmentStore.createMask( segmentation.id, - mintSegment({ name: 'A' }) + mintSegment('image-1', { name: 'A' }) ); const second = segmentStore.createMask( segmentation.id, - mintSegment({ name: 'B' }) + mintSegment('image-1', { name: 'B' }) ); seedVoxel(first.id, [0, 0, 0]); seedVoxel(second.id, [1, 1, 1]); diff --git a/src/processing/composables/useInputStaging.ts b/src/processing/composables/useInputStaging.ts index e9b8b6bcf..623c275bd 100644 --- a/src/processing/composables/useInputStaging.ts +++ b/src/processing/composables/useInputStaging.ts @@ -95,7 +95,7 @@ export function useInputStaging(write = writeSegmentation) { const annotationToolsView = computed(() => { const registry = useSegmentStore().segments; const labels = Object.fromEntries( - registry.segmentList.value.map((segment) => { + registry.segmentList(currentImageID.value).map((segment) => { const { name, cssColor, strokeWidth } = resolveSegmentAppearance(segment); return [segment.id, { labelName: name, color: cssColor, strokeWidth }]; diff --git a/src/processing/segmentationInput.ts b/src/processing/segmentationInput.ts index c243a9188..935ac238b 100644 --- a/src/processing/segmentationInput.ts +++ b/src/processing/segmentationInput.ts @@ -16,10 +16,9 @@ export function planSegmentationInput( return { ...base, parts, warning: undefined }; } const registry = useSegmentStore().segments; - // The selection is shared across images, so it can name a segment this image - // holds no mask for. Packing then starts from the first mask in registry - // order instead, and the advice to select a segment has to say where. - const selected = registry.selectedSegmentId.value; + // An unpainted definition has no mask to prioritize, so packing falls back + // to the first mask in this image's segment order. + const selected = registry.selectedSegmentId(parentImageId); const preferred = selected && segmentationStore.maskFor(parentImageId, selected) ? selected diff --git a/src/segmentation/README.md b/src/segmentation/README.md index ef30f012e..6c58174b1 100644 --- a/src/segmentation/README.md +++ b/src/segmentation/README.md @@ -1,17 +1,17 @@ # Segmentation -A segment is shared identity and appearance. A segmentation contains one -image's masks and display settings. Each segment has at most one mask on an -image; the mask's bounded labelmap is allocated only when needed. +Each image owns segment definitions, order and selection, shared by Paint, +rectangles, polygons and rulers. A segmentation holds its masks and display +settings. Each segment has at most one mask, allocated only when needed. ## Module ownership - `segment.ts`, `model.ts`, `geometry.ts`, and `color.ts` define the model and calculations. They do not depend on stores or components. -- `segments.ts` and `segmentRegistry.ts` own shared identity, selection, and - appearance. Stores holding masks or annotation tools declare their segment - references on the registry, which hands them the removal. Removing the last - image deletes every segment no config entry holds. +- `segments.ts` and `segmentRegistry.ts` own image sets, identity, selection and + appearance. Stores register mask and shape references for segment deletion. + On image eviction each store removes its own image content synchronously, and + the registry forgets its definitions. Configured defaults survive. - `deleteSegment.ts` guards locked segments before deleting their masks and shapes. - `store.ts` owns mask identity, attachment, lookup, and lifecycle. Import and @@ -33,6 +33,25 @@ stores or upper feature modules. ## Editing and persistence +On first activation, an image without a segment set copies the last active image's +definitions, order and selection, or configured defaults if no previous set exists. +Copies have fresh identities, start visible and unlocked, and contain no masks or +shapes. An image edited before it is viewed copies the same way. Existing sets, +including empty ones, are never reseeded. + +A copy stays borrowed until something edits it: a definition change, a mask or a +shape. A restored set replaces a borrowed copy, and a companion segmentation +replaces it with the configured defaults before binding its labels by name, so +loading never waits and editing is never blocked. Once the copy is edited, +incoming labels bind by name into it like any other import. + +Manifest 8.0.0 saves image membership, order, selection and defaults. Migration +splits older shared catalogs by image without duplicating voxels. Restore preserves +distinct saved identities even when datasets resolve to the same image, while +copies of the same legacy definition resolve to one segment on that image. +Open saved sessions with a build that supports their manifest version; earlier +builds cannot preserve the image ownership recorded in 8.0.0. + Process previews temporarily occupy live mask storage. Starting a competing edit cancels the preview before writing. Save and export also cancel an unconfirmed preview and read committed content, as does job input staging when @@ -44,9 +63,9 @@ An algorithm may modify its detached input buffer and return it. Cancellation uses a separate original snapshot. Resolved storage stays with the process manager and is checked again after asynchronous computation. -Mask creation refuses missing segment identities and duplicate image/segment -pairs. Attachment accepts prepared storage only for an unbound mask. Readers -must distinguish an existing mask record from allocated voxel storage. +Mask creation refuses missing or foreign-owned segment identities and duplicate +image/segment pairs. Attachment accepts prepared storage only for an unbound mask. +Readers must distinguish an existing mask record from allocated voxel storage. An erase never shrinks a mask's allocation, so an edit that leaves a mask holding no voxel deletes that mask when the edit ends: a paint or erase stroke, @@ -54,24 +73,20 @@ a polygon fill, or an applied process. The segment stays. ## Ordering and overlap -Registry order controls the sidebar, shortcuts, selection and picking, and on -export each mask's label value and the file it lands in. It does not control -what is drawn on top: overlapping segments blend in the slice view, and moving -one above another leaves the overlap looking the same. Per-image mask order -preserves insertion and restored file order. +Each image's segment order controls its sidebar, shortcuts, selection, picking, +export label values and file grouping. Overlapping segments blend in the slice view +regardless of order. Mask order preserves insertion and restored file order. Aimed writes, such as paint and polygon fills, clear unlocked neighbors and preserve locked neighbors. With the Paint panel's Allow Overlap switch on, which is never saved, aimed writes leave every neighbor alone and may overlap them. -Processes preserve voxels already held by other segments. Import matches -existing segment identities by exact name, sharing appearance and locks across -images, unless that segment already has a mask on the importing image, in which -case the import takes a new segment with a numbered name. Restore matches the -segments a file lists by exact name too, but joins an existing segment only -while it holds no mask or shape on any image, and gives it the session's -appearance; otherwise the restored segment takes a numbered name. Label values -no listed mask reads bind as import does, except that a migrated group carrying -display of its own always takes new segments. +Processes preserve voxels already held by other segments. + +Import and restore match exact segment names within the destination image. Import +reuses a segment without a mask, keeping its appearance and locks. Restore reuses +one without a mask or shape and applies the saved appearance. When a match cannot +be reused, a new segment gets a numbered name. Unlisted voxel labels follow import rules; +migrated groups with their own display settings always create new segments. ## Labelmap interchange diff --git a/src/segmentation/__tests__/annotationToolSegments.spec.ts b/src/segmentation/__tests__/annotationToolSegments.spec.ts index dc90c1b8d..e144ff9ea 100644 --- a/src/segmentation/__tests__/annotationToolSegments.spec.ts +++ b/src/segmentation/__tests__/annotationToolSegments.spec.ts @@ -30,12 +30,15 @@ const segmentationOf = (imageId: string) => const coloredSegment = () => ({ store: usePolygonStore(), - segmentId: segments().addSegment({ name: 'Tumor', color: [214, 0, 0, 255] }), + segmentId: segments().addSegment(IMAGE_ID, { + name: 'Tumor', + color: [214, 0, 0, 255], + }), }); const serializedToolForSegment = () => { const store = usePolygonStore(); - const segmentId = segments().addSegment({ name: 'Tumor' }); + const segmentId = segments().addSegment(IMAGE_ID, { name: 'Tumor' }); store.addTool({ imageID: IMAGE_ID, placing: false, segmentId }); return { store, @@ -51,30 +54,34 @@ describe('shape references to segments', () => { }); it('lists every segment in the shared registry, content or not', () => { - const segmentId = segments().addSegment({ name: 'Tumor' }); + const segmentId = segments().addSegment(IMAGE_ID, { name: 'Tumor' }); - expect(segments().segmentList.value.map((type) => type.id)).toEqual([ - segmentId, - ]); + expect( + segments() + .segmentList(IMAGE_ID) + .map((type) => type.id) + ).toEqual([segmentId]); expect(segments().appearanceOf(segmentId).name).toBe('Tumor'); expect(segmentationOf(IMAGE_ID)).toBeUndefined(); }); - it('keeps the selected segment when the viewed image changes', async () => { + it('selects a copied segment when the viewed image changes', async () => { const store = usePolygonStore(); - const segmentId = segments().addSegment({ name: 'Tumor' }); + const segmentId = segments().addSegment(IMAGE_ID, { name: 'Tumor' }); seatAndView('img-2'); await nextTick(); - expect(segments().selectedSegmentId.value).toBe(segmentId); + const copied = segments().selectedSegmentId('img-2'); + expect(copied).not.toBe(segmentId); + expect(segments().getSegment(copied)?.name).toBe('Tumor'); const id = store.addTool({ imageID: 'img-2', placing: false }); - expect(store.toolByID[id].segmentId).toBe(segmentId); + expect(store.toolByID[id].segmentId).toBe(copied); }); it('captures the selected segment when a tool is added', () => { const store = usePolygonStore(); - const segmentId = segments().addSegment({ name: 'Tumor' }); + const segmentId = segments().addSegment(IMAGE_ID, { name: 'Tumor' }); const id = store.addTool({ imageID: IMAGE_ID, placing: false }); @@ -97,7 +104,7 @@ describe('shape references to segments', () => { it('shows a rename without rewriting the tool', () => { const store = usePolygonStore(); - const segmentId = segments().addSegment({ name: 'Tumor' }); + const segmentId = segments().addSegment(IMAGE_ID, { name: 'Tumor' }); const id = store.addTool({ imageID: IMAGE_ID, placing: false, segmentId }); const before = store.toolByID[id]; @@ -122,7 +129,7 @@ describe('shape references to segments', () => { it('lets several shapes reference one segment', () => { const polygons = usePolygonStore(); const rectangles = useRectangleStore(); - const segmentId = segments().addSegment({ name: 'Tumor' }); + const segmentId = segments().addSegment(IMAGE_ID, { name: 'Tumor' }); const first = polygons.addTool({ imageID: IMAGE_ID, placing: false }); const second = polygons.addTool({ imageID: IMAGE_ID, placing: false }); @@ -136,7 +143,7 @@ describe('shape references to segments', () => { it('keeps the rectangle fill color a per-shape prop', () => { const rectangles = useRectangleStore(); - const segmentId = segments().addSegment({ name: 'Tumor' }); + const segmentId = segments().addSegment(IMAGE_ID, { name: 'Tumor' }); const id = rectangles.addTool({ imageID: IMAGE_ID, placing: false }); @@ -147,7 +154,7 @@ describe('shape references to segments', () => { it('allocates no voxels and no geometry for a segment', () => { const store = usePolygonStore(); - segments().addSegment({ name: 'Tumor' }); + segments().addSegment(IMAGE_ID, { name: 'Tumor' }); expect(segmentationOf(IMAGE_ID)).toBeUndefined(); expect(boundMasks()).toEqual([]); @@ -173,7 +180,7 @@ describe('shape references to segments', () => { setActivePinia(createPinia()); seatAndView(IMAGE_ID); const restored = usePolygonStore(); - const { idMap: adopted } = segments().adopt([ + const { idMap: adopted } = segments().adopt(IMAGE_ID, [ { id: segmentId, name: 'Tumor', @@ -233,7 +240,7 @@ describe('placing an annotation names its segment', () => { it('mints a segment for an annotation placed against nothing', () => { const store = useRectangleStore(); - expect(segments().selectedSegmentId.value).toBeUndefined(); + expect(segments().selectedSegmentId(IMAGE_ID)).toBeUndefined(); const id = place(store); @@ -262,7 +269,7 @@ describe('placing an annotation names its segment', () => { store.resolveToolSegment(id); store.placeTool(id); - expect(segments().segmentList.value).toHaveLength(1); + expect(segments().segmentList(IMAGE_ID)).toHaveLength(1); }); it('leaves the placed segment selected so the next one reuses it', () => { @@ -274,7 +281,7 @@ describe('placing an annotation names its segment', () => { expect(store.toolByID[second].segmentId).toBe( store.toolByID[first].segmentId ); - expect(segments().segmentList.value).toHaveLength(1); + expect(segments().segmentList(IMAGE_ID)).toHaveLength(1); }); it('hands the minted segment to the other delineation tools', () => { @@ -296,17 +303,17 @@ describe('placing an annotation names its segment', () => { it('places against the selected segment rather than minting beside it', () => { const store = useRectangleStore(); - const segmentId = segments().addSegment({ name: 'Tumor' }); + const segmentId = segments().addSegment(IMAGE_ID, { name: 'Tumor' }); const id = place(store); expect(store.toolByID[id].segmentId).toBe(segmentId); - expect(segments().segmentList.value).toHaveLength(1); + expect(segments().segmentList(IMAGE_ID)).toHaveLength(1); }); it('keeps the stub a widget is placing into when its segment is deleted', () => { const store = useRulerStore(); - const segmentId = segments().addSegment({ name: 'Long axis' }); + const segmentId = segments().addSegment(IMAGE_ID, { name: 'Long axis' }); const stub = store.addTool({ imageID: IMAGE_ID, placing: true, segmentId }); const placed = store.addTool({ imageID: IMAGE_ID, segmentId }); @@ -329,10 +336,10 @@ describe('placing an annotation names its segment', () => { // Dropping a config key deletes the segment it minted unless content // references it. const configured = () => { - segments().replaceConfigSegments({ 'Long axis': {} }); - return segments().findSegmentByName('Long axis')!.id; + segments().replaceConfigSegments(IMAGE_ID, { 'Long axis': {} }); + return segments().findSegmentByName(IMAGE_ID, 'Long axis')!.id; }; - const unconfigure = () => segments().replaceConfigSegments({}); + const unconfigure = () => segments().replaceConfigSegments(IMAGE_ID, {}); const stubbed = configured(); store.addTool({ imageID: IMAGE_ID, placing: true, segmentId: stubbed }); diff --git a/src/segmentation/__tests__/artifactParentRestore.spec.ts b/src/segmentation/__tests__/artifactParentRestore.spec.ts index 5b509588d..c876406a3 100644 --- a/src/segmentation/__tests__/artifactParentRestore.spec.ts +++ b/src/segmentation/__tests__/artifactParentRestore.spec.ts @@ -607,7 +607,9 @@ describe('an archive artifact', () => { manifest, stateFiles: [{ archivePath: 'a.vti', file: new File([''], 'a.vti') }], dataIDMap: { parent: 'parent' }, - segmentIdMap: useSegmentStore().deserialize(manifest).segmentIdMap, + segmentIdMap: useSegmentStore().deserialize(manifest, { + parent: 'parent', + }).segmentIdMap, io: { write: async () => '', read: async () => ({ image, headerMetadata }), @@ -632,6 +634,7 @@ describe('an archive artifact', () => { new Uint8Array([0, 1, 2, 0]), { segments: [{ id: 's1', name: 'Liver', color: [255, 0, 0, 255] }], + imageSegmentSets: [{ imageId: 'parent', segmentIds: ['s1'] }], segmentations: [ { id: 'wire', @@ -682,6 +685,7 @@ describe('an artifact-backed mask whose id spells a prototype member', () => { const manifest = ManifestSchema.parse( manifestForImages(['parent'], { segments: [{ id: 'tumor', name: 'Tumor', color: [255, 0, 0, 255] }], + imageSegmentSets: [{ imageId: 'parent', segmentIds: ['tumor'] }], segmentations: [ { id: 'segmentation', @@ -719,7 +723,9 @@ describe('an artifact-backed mask whose id spells a prototype member', () => { { archivePath: 'mask.vti', file: new File([''], 'mask.vti') }, ], dataIDMap: { parent: 'parent' }, - segmentIdMap: useSegmentStore().deserialize(manifest).segmentIdMap, + segmentIdMap: useSegmentStore().deserialize(manifest, { + parent: 'parent', + }).segmentIdMap, io: { write: async () => '', read: async () => ({ image }) }, }); return { result, mask: store().imageMasks('parent')[0] }; diff --git a/src/segmentation/__tests__/bulkImageRemoval.spec.ts b/src/segmentation/__tests__/bulkImageRemoval.spec.ts new file mode 100644 index 000000000..64f09ab9d --- /dev/null +++ b/src/segmentation/__tests__/bulkImageRemoval.spec.ts @@ -0,0 +1,205 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { createPinia, setActivePinia } from 'pinia'; +import type { Vector3 } from '@kitware/vtk.js/types'; + +import { + activateAppPinia, + manifestForImages, + makeLabelmap, + markedVoxels, + seatImage, + seedVoxel, + serializeAnnotations, + showImage, + viewImage, +} from '@/src/segmentation/__tests__/segmentMaskFixtures'; +import { useSegmentationEditsStore } from '@/src/segmentation/editing/coordinator'; +import { usePaintProcessStore } from '@/src/segmentation/editing/paintProcess'; +import { maskScalars } from '@/src/segmentation/model'; +import { useSegmentStore } from '@/src/segmentation/segments'; +import { useSegmentationStore } from '@/src/segmentation/store'; +import { useImageCacheStore } from '@/src/store/image-cache'; +import { useImageStatsStore } from '@/src/store/image-stats'; +import { usePolygonStore } from '@/src/store/tools/polygons'; +import { useRectangleStore } from '@/src/store/tools/rectangles'; +import { useRulerStore } from '@/src/store/tools/rulers'; +import { useToolSelectionStore } from '@/src/store/tools/toolSelection'; +import { AnnotationToolType } from '@/src/store/tools/types'; + +const detached = (value: T): T => JSON.parse(JSON.stringify(value)); + +const ruler = (imageID: string, segmentId: string, placing = false) => ({ + imageID, + segmentId, + placing, + slice: 0, + frameOfReference: { + planeOrigin: [0, 0, 0] as Vector3, + planeNormal: [0, 0, 1] as Vector3, + }, + firstPoint: [0, 0, 0] as Vector3, + secondPoint: [1, 1, 0] as Vector3, +}); + +const copiedCatalog = async (count: number) => { + const segmentStore = useSegmentStore(); + const rulers = useRulerStore(); + await seatImage('b'); + await seatImage('a'); + await showImage('b'); + segmentStore.replaceConfigSegments( + Object.fromEntries( + Array.from({ length: count }, (_, index) => [ + `Label ${index}`, + { color: '#336699' }, + ]) + ), + ['b'] + ); + segmentStore.segments.segmentList('b').forEach(({ id }) => { + rulers.addRuler(ruler('b', id)); + }); + await showImage('a'); + return { segmentStore, registry: segmentStore.segments, rulers }; +}; + +describe('bulk image segment removal', () => { + beforeEach(() => setActivePinia(createPinia())); + + it('removes a copied empty catalog and preserves the remaining definitions and complete ruler geometry', async () => { + const { registry, rulers } = await copiedCatalog(32); + const masks = useSegmentationStore(); + const beforeSegments = detached(registry.segmentList('b')); + const beforeRulers = detached(rulers.serializeTools()); + const removedIds = registry.segmentList('a').map(({ id }) => id); + expect( + new Set([...removedIds, ...beforeSegments.map(({ id }) => id)]).size + ).toBe(64); + expect(masks.segmentations).toEqual({}); + const cancelPreview = vi.fn(); + useSegmentationEditsStore().hold(cancelPreview); + + useImageCacheStore().removeImage('a'); + + const saved = serializeAnnotations(manifestForImages(['b'], { tools: {} })); + expect(saved.segments).toEqual(beforeSegments); + expect(saved.tools?.rulers).toEqual(beforeRulers); + expect(saved.imageSegmentSets?.map(({ imageId }) => imageId)).toEqual([ + 'b', + ]); + removedIds.forEach((removed) => + expect(saved.segments?.map(({ id }) => id)).not.toContain(removed) + ); + expect(masks.segmentations).toEqual({}); + expect(cancelPreview).not.toHaveBeenCalled(); + }); + + it('evicts masks and finished, placing and unnamed tools synchronously while preserving the other image', async () => { + const { registry, rulers } = await copiedCatalog(2); + const masks = useSegmentationStore(); + const aSegments = registry.segmentList('a'); + const b = registry.segmentList('b')[0].id; + const shapeStores = [rulers, useRectangleStore(), usePolygonStore()]; + const tools = shapeStores.map((store) => ({ + store, + removed: [ + ...aSegments.map(({ id }) => store.addTool(ruler('a', id))), + store.addTool(ruler('a', aSegments[0].id, true)), + store.addTool(ruler('a', '')), + ], + retained: store.addTool(ruler('b', b)), + })); + const aMasks = aSegments.map(({ id }) => masks.resolveEditTarget('a', id)); + aMasks.forEach((id, index) => seedVoxel(id, [index, 0, 0])); + const bMask = masks.resolveEditTarget('b', b); + seedVoxel(bMask, [1, 1, 0]); + const originalB = detached( + rulers.serializeTools().tools.filter(({ imageID }) => imageID === 'b') + ); + const segmentationA = masks.getSegmentationForImage('a')!; + masks.updateSegmentationDisplay(segmentationA.id, { + fillOpacity: 0.8, + outlineThickness: 4, + }); + const selection = useToolSelectionStore(); + const selectedA = tools[0].removed[0]; + const selectedB = tools[0].retained; + selection.addSelection(selectedA, AnnotationToolType.Ruler); + selection.addSelection(selectedB, AnnotationToolType.Ruler); + const cancelPreview = vi.fn(); + useSegmentationEditsStore().hold(cancelPreview); + + useImageCacheStore().removeImage('a'); + + const saved = serializeAnnotations(manifestForImages(['b'], { tools: {} })); + expect(saved.tools?.rulers?.tools).toEqual(originalB); + expect(saved.segments).toEqual(registry.segmentList('b')); + tools.forEach(({ store, removed, retained }) => { + expect(removed.every((id) => !store.toolByID[id])).toBe(true); + expect(store.toolByID[retained]?.segmentId).toBe(b); + }); + expect(selection.isSelected(selectedA)).toBe(false); + expect(selection.isSelected(selectedB)).toBe(true); + expect(aMasks.every((id) => !masks.maskExists(id))).toBe(true); + expect(masks.getSegmentationForImage('a')).toBeUndefined(); + expect(rulers.serializeTools().tools).toEqual(originalB); + expect(markedVoxels(bMask)).toEqual([[1, 1, 0, 1]]); + expect(cancelPreview).toHaveBeenCalledTimes(1); + }); + + it('rolls back a process preview before eviction removes its mask storage', async () => { + activateAppPinia(); + useImageStatsStore().stats.a = { + scalarMin: 0, + scalarMax: 0, + autoRangeValues: {}, + }; + await viewImage('a'); + const masks = useSegmentationStore(); + const maskId = masks.resolveEditTarget('a'); + seedVoxel(maskId, [0, 0, 0]); + const image = masks.findMaskBinding(maskId)!.image; + const original = maskScalars(image).slice(); + const process = usePaintProcessStore(); + await process.startProcess(async ({ scalars, maskExtent }) => ({ + scalars: new Uint8Array(scalars.length), + extent: maskExtent, + })); + expect(process.processStep).toBe('previewing'); + expect(maskScalars(image)).not.toEqual(original); + + useImageCacheStore().removeImage('a'); + + expect(process.processStep).toBe('start'); + expect(maskScalars(image)).toEqual(original); + expect(masks.maskExists(maskId)).toBe(false); + }); + + it('releases every removed mask name without reserving it against another image', async () => { + await seatImage('a', { name: 'CT' }); + await seatImage('b', { name: 'CT' }); + const store = useSegmentationStore(); + const registry = useSegmentStore().segments; + const values = new Uint8Array(64); + values[0] = 1; + values[1] = 2; + store.splitLabelmapIntoMasks( + 'a', + makeLabelmap({ values }), + [ + { value: 1, name: 'Tumor', color: [255, 0, 0, 255], visible: true }, + { value: 2, name: 'Node', color: [0, 255, 0, 255], visible: true }, + ], + { name: 'Segment Group 2 for CT' } + ); + + useImageCacheStore().removeImage('a'); + + const names = ['First', 'Second'].map((name) => { + const segmentId = registry.addSegment('b', { name }); + const maskId = store.resolveEditTarget('b', segmentId); + return store.ensureLabelmapBinding(maskId).name; + }); + expect(names).toEqual(['Segment Group 1 for CT', 'Segment Group 2 for CT']); + }); +}); diff --git a/src/segmentation/__tests__/configSegments.spec.ts b/src/segmentation/__tests__/configSegments.spec.ts index 3e446de6c..291a67a1a 100644 --- a/src/segmentation/__tests__/configSegments.spec.ts +++ b/src/segmentation/__tests__/configSegments.spec.ts @@ -8,6 +8,7 @@ import { } from '@/src/segmentation/__tests__/segmentMaskFixtures'; import { nextTick } from 'vue'; +import { useCurrentImage } from '@/src/composables/useCurrentImage'; import { useSegmentStore } from '@/src/segmentation/segments'; import { usePolygonStore } from '@/src/store/tools/polygons'; import { useRectangleStore } from '@/src/store/tools/rectangles'; @@ -17,13 +18,6 @@ import { applyPostStateConfig, config } from '@/src/io/import/configJson'; import { cssColorToRGBA } from '@/src/segmentation/color'; import { resolveSegmentAppearance } from '@/src/segmentation/segment'; -// --------------------------------------------------------------------------- -// Configured segments are declared once for a session, before any image loads. -// Configuring one creates nothing: no mask, no geometry, no image. A key keeps -// its segment id across config changes, and dropping a key only removes the -// segment when nothing references it. -// --------------------------------------------------------------------------- - const segments = () => useSegmentStore().segments; const viewImage = async (id: string) => { @@ -36,12 +30,22 @@ const seatAndView = async (id: string) => { await viewImage(id); }; -const applyConfig = (raw: unknown) => applyPostStateConfig(config.parse(raw)); +const applyConfig = (raw: unknown) => { + const { currentImageID } = useCurrentImage('global'); + applyPostStateConfig( + config.parse(raw), + undefined, + currentImageID.value ? [currentImageID.value] : [] + ); +}; -const typeNames = () => segments().segmentList.value.map((type) => type.name); +const typeNames = () => + segments() + .segmentList('img-1') + .map((type) => type.name); const segmentIdNamed = (name: string) => { - const type = segments().findSegmentByName(name); + const type = segments().findSegmentByName('img-1', name); if (!type) throw new Error(`No type named "${name}"`); return type.id; }; @@ -57,8 +61,9 @@ const TWO_TYPES = { }; describe('a configured segment creates nothing', () => { - beforeEach(() => { + beforeEach(async () => { setActivePinia(createPinia()); + await seatAndView('img-1'); }); it('touches no image when the config is applied', async () => { @@ -71,14 +76,22 @@ describe('a configured segment creates nothing', () => { expect(boundMasks()).toEqual([]); }); - it('offers the same segments on every image the user views', async () => { + it('copies configured definitions into independent image sets', async () => { applyConfig(TWO_TYPES); await seatAndView('img-1'); await seatAndView('img-2'); expect(typeNames()).toEqual(['Tumor', 'Node']); + expect( + segments() + .segmentList('img-2') + .map(({ name }) => name) + ).toEqual(['Tumor', 'Node']); expect(recordsOf('img-1')).toEqual([]); expect(recordsOf('img-2')).toEqual([]); + const copied = segments().findSegmentByName('img-2', 'Tumor'); + expect(copied).toBeDefined(); + expect(copied!.id).not.toBe(segmentIdNamed('Tumor')); }); it('takes the configured color and appearance', () => { @@ -101,7 +114,7 @@ describe('a configured segment creates nothing', () => { it('offers a selection without allocating for it', () => { applyConfig(TWO_TYPES); - expect(segments().selectedSegmentId.value).toBe(segmentIdNamed('Tumor')); + expect(segments().selectedSegmentId('img-1')).toBe(segmentIdNamed('Tumor')); expect(store().segmentations).toEqual({}); }); @@ -130,8 +143,9 @@ describe('a configured segment creates nothing', () => { }); describe('a second config replaces the first', () => { - beforeEach(() => { + beforeEach(async () => { setActivePinia(createPinia()); + await seatAndView('img-1'); }); it('keeps the id of a key both configs state', () => { @@ -285,7 +299,7 @@ describe('a second config replaces the first', () => { it('leaves a segment the user made alone', () => { applyConfig(TWO_TYPES); - const own = segments().addSegment({ name: 'Mine' }); + const own = segments().addSegment('img-1', { name: 'Mine' }); applyConfig({ segments: { Tumor: { color: 'blue' } } }); @@ -295,13 +309,14 @@ describe('a second config replaces the first', () => { }); describe('config applies after a restore', () => { - beforeEach(() => { + beforeEach(async () => { setActivePinia(createPinia()); + await seatAndView('img-1'); }); it('overlays a restored segment of the same name rather than adding one', () => { // Restore seats the segment first; the config then states its appearance. - const restored = segments().mintSegment({ + const restored = segments().mintSegment('img-1', { name: 'Tumor', color: [1, 2, 3, 255], }); @@ -315,7 +330,7 @@ describe('config applies after a restore', () => { }); it('restores session appearance when replacement removes a config override', () => { - const id = segments().mintSegment({ + const id = segments().mintSegment('img-1', { name: 'Tumor', color: [1, 2, 3, 255], fillOpacity: 0.6, @@ -336,7 +351,7 @@ describe('config applies after a restore', () => { }); it('keeps a restored segment the config does not name', () => { - const restored = segments().mintSegment({ name: 'Restored' }); + const restored = segments().mintSegment('img-1', { name: 'Restored' }); applyConfig(TWO_TYPES); @@ -345,7 +360,7 @@ describe('config applies after a restore', () => { }); it('keeps a restored segment it overlaid once a later config drops the key', () => { - const restored = segments().mintSegment({ name: 'Tumor' }); + const restored = segments().mintSegment('img-1', { name: 'Tumor' }); applyConfig({ segments: { Tumor: { color: 'blue' } } }); applyConfig({ segments: {} }); @@ -355,7 +370,7 @@ describe('config applies after a restore', () => { }); it('keeps a shape pointing at the segment the config took over', () => { - const restored = segments().mintSegment({ name: 'Tumor' }); + const restored = segments().mintSegment('img-1', { name: 'Tumor' }); const toolId = useRectangleStore().addTool({ imageID: 'img-1', segmentId: restored, @@ -369,3 +384,103 @@ describe('config applies after a restore', () => { ); }); }); + +describe('configured defaults without an active image', () => { + beforeEach(() => { + setActivePinia(createPinia()); + }); + + it('waits for an image before creating configured segment identities', async () => { + applyConfig(TWO_TYPES); + + expect(segments().imageSets.value).toEqual({}); + expect(store().segmentations).toEqual({}); + + await seatAndView('img-1'); + + expect(typeNames()).toEqual(['Tumor', 'Node']); + expect(segments().appearanceOf(segmentIdNamed('Tumor')).cssColor).toBe( + '#00ff00' + ); + expect(boundMasks()).toEqual([]); + }); + + it('does not apply an active image config to other initialized images', async () => { + applyConfig(TWO_TYPES); + await seatAndView('img-1'); + const first = segmentIdNamed('Tumor'); + await seatAndView('img-2'); + const second = segments().findSegmentByName('img-2', 'Tumor')!.id; + + applyConfig({ segments: { Tumor: { color: 'blue' } } }); + + expect(segments().appearanceOf(first).cssColor).toBe('#00ff00'); + expect(segments().appearanceOf(second).cssColor).toBe('#0000ff'); + expect(typeNames()).toEqual(['Tumor', 'Node']); + }); + + it('keeps configured identities when a copied image is renamed and reconfigured', async () => { + await seatAndView('img-1'); + applyConfig(TWO_TYPES); + const user = segments().mintSegment('img-1', { name: 'User' }); + await seatAndView('img-2'); + const copied = segments().findSegmentByName('img-2', 'Tumor')!.id; + segments().updateSegment(copied, { name: 'Lesion' }); + + applyConfig(TWO_TYPES); + + expect(segments().findSegmentByName('img-2', 'Tumor')!.id).toBe(copied); + expect( + segments() + .segmentList('img-2') + .map(({ name }) => name) + ).toEqual(['Tumor', 'Node', 'User']); + applyConfig({ segments: {} }); + expect( + segments() + .segmentList('img-2') + .map(({ name }) => name) + ).toEqual(['User']); + expect(segments().getSegment(user)?.name).toBe('User'); + expect(typeNames()).toEqual(['Tumor', 'Node', 'User']); + }); + + it('can clear defaults that initialized an image before any source image existed', async () => { + applyConfig(TWO_TYPES); + await seatAndView('img-1'); + + applyConfig({ segments: {} }); + + expect(typeNames()).toEqual([]); + }); + + it('resets omitted appearance when configuration initialized an image from defaults', async () => { + applyConfig({ segments: { Tumor: { color: 'red', fillOpacity: 0.3 } } }); + await seatAndView('img-1'); + const id = segmentIdNamed('Tumor'); + + applyConfig({ segments: { Tumor: {} } }); + + expect(segments().getSegment(id)?.fillOpacity).toBeUndefined(); + expect(segments().appearanceOf(id).fillOpacity).toBe(1); + expect(segmentIdNamed('Tumor')).toBe(id); + }); + + it('uses the same automatic configured colors for fresh rows and future defaults', async () => { + await seatAndView('img-1'); + segments().mintSegment('img-1', { name: 'User' }); + applyConfig({ segments: { First: {}, Second: {} } }); + const configuredColors = ['First', 'Second'].map( + (name) => segments().findSegmentByName('img-1', name)!.color + ); + + setActivePinia(createPinia()); + applyConfig({ segments: { First: {}, Second: {} } }); + await seatAndView('img-2'); + expect( + segments() + .segmentList('img-2') + .map(({ color }) => color) + ).toEqual(configuredColors); + }); +}); diff --git a/src/segmentation/__tests__/convertParentRemovedMidImport.spec.ts b/src/segmentation/__tests__/convertParentRemovedMidImport.spec.ts index 87ca56c7f..de55670dc 100644 --- a/src/segmentation/__tests__/convertParentRemovedMidImport.spec.ts +++ b/src/segmentation/__tests__/convertParentRemovedMidImport.spec.ts @@ -60,7 +60,9 @@ async function seatConvertible() { } const segmentNames = () => - useSegmentStore().segments.segmentList.value.map((segment) => segment.name); + useSegmentStore() + .segments.segmentList('parent') + .map((segment) => segment.name); const expectNothingMinted = () => { expect(Object.keys(store().segmentations)).toEqual([]); diff --git a/src/segmentation/__tests__/deleteSegment.spec.ts b/src/segmentation/__tests__/deleteSegment.spec.ts index 913795f0f..d3b1ee189 100644 --- a/src/segmentation/__tests__/deleteSegment.spec.ts +++ b/src/segmentation/__tests__/deleteSegment.spec.ts @@ -11,7 +11,7 @@ describe('deleteUnlockedSegment', () => { it('keeps a locked segment', () => { const registry = createSegmentRegistry(); - const id = registry.addSegment({ name: 'Tumor', locked: true }); + const id = registry.addSegment('img-1', { name: 'Tumor', locked: true }); deleteUnlockedSegment(registry, id); @@ -20,7 +20,7 @@ describe('deleteUnlockedSegment', () => { it('deletes an unlocked segment', () => { const registry = createSegmentRegistry(); - const id = registry.addSegment({ name: 'Tumor' }); + const id = registry.addSegment('img-1', { name: 'Tumor' }); deleteUnlockedSegment(registry, id); diff --git a/src/segmentation/__tests__/descriptorlessActiveSegment.spec.ts b/src/segmentation/__tests__/descriptorlessActiveSegment.spec.ts index 67caa601e..b3d768b42 100644 --- a/src/segmentation/__tests__/descriptorlessActiveSegment.spec.ts +++ b/src/segmentation/__tests__/descriptorlessActiveSegment.spec.ts @@ -84,7 +84,8 @@ const catalog = () => { // that segment has on this image. Both groups decode two segments each, in // ascending source value, so a segment's place says which value it came from. const selectedIndex = () => { - const segmentId = useSegmentStore().segments.selectedSegmentId.value; + const segmentId = + useSegmentStore().segments.selectedSegmentId('parent-store'); const index = catalog().findIndex( (segment) => segment.segmentId === segmentId ); @@ -123,11 +124,13 @@ describe('restoring a descriptorless active group', () => { let liver = ''; await restoreTwoGroups(1, () => { const registry = useSegmentStore().segments; - liver = registry.segmentNamed('Liver'); + liver = registry.segmentNamed('parent-store', 'Liver'); registry.selectSegment(liver); }); - expect(useSegmentStore().segments.selectedSegmentId.value).toBe(liver); + expect(useSegmentStore().segments.selectedSegmentId('parent-store')).toBe( + liver + ); }); it('selects the first segment when no source value matches', async () => { diff --git a/src/segmentation/__tests__/fillHoles.spec.ts b/src/segmentation/__tests__/fillHoles.spec.ts index 3b0cd2623..4c87eb67b 100644 --- a/src/segmentation/__tests__/fillHoles.spec.ts +++ b/src/segmentation/__tests__/fillHoles.spec.ts @@ -126,7 +126,7 @@ describe('Fill Holes store', () => { segmentationStore.ensureSegmentationForImage(parentImageID); const segment = segmentationStore.createMask( segmentation.id, - mintSegment({ + mintSegment(parentImageID, { name: 'Segment 1', }) ); @@ -311,7 +311,7 @@ describe('Fill Holes store', () => { segmentationStore.getSegmentationForImage(parentImageID)!; const other = segmentationStore.createMask( segmentation.id, - mintSegment({ + mintSegment(parentImageID, { name: 'Other', }) ); diff --git a/src/segmentation/__tests__/findMaskVoxels.spec.ts b/src/segmentation/__tests__/findMaskVoxels.spec.ts index 020aa90be..68fa54c1b 100644 --- a/src/segmentation/__tests__/findMaskVoxels.spec.ts +++ b/src/segmentation/__tests__/findMaskVoxels.spec.ts @@ -22,7 +22,7 @@ function seatMask(imageId: string) { const segmentation = store().ensureSegmentationForImage(imageId); const segment = store().createMask( segmentation.id, - mintSegment({ name: 'Tumor' }) + mintSegment(imageId, { name: 'Tumor' }) ); const voxels = store().maskVoxels(segment.id); voxels.materialize(); diff --git a/src/segmentation/__tests__/imageSetInitialization.spec.ts b/src/segmentation/__tests__/imageSetInitialization.spec.ts new file mode 100644 index 000000000..74ea3e657 --- /dev/null +++ b/src/segmentation/__tests__/imageSetInitialization.spec.ts @@ -0,0 +1,278 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { createPinia, setActivePinia } from 'pinia'; + +import { showImportedData } from '@/src/actions/loadUserFiles'; +import { asLoadableResult } from '@/src/io/import/common'; +import { importDataSources } from '@/src/io/import/importDataSources'; +import { completeStateFileRestore } from '@/src/io/import/processors/restoreStateFile'; +import { + manifestForImages, + seatImage, + serializeAnnotations, + segmentNames, + showImage, + viewImage, +} from '@/src/segmentation/__tests__/segmentMaskFixtures'; +import { useSegmentStore } from '@/src/segmentation/segments'; +import { useSegmentationStore } from '@/src/segmentation/store'; +import { useViewStore } from '@/src/store/views'; +import { defer } from '@/src/utils'; +import { Skip } from '@/src/utils/evaluateChain'; + +describe('image segment initialization boundaries', () => { + beforeEach(() => setActivePinia(createPinia())); + afterEach(() => vi.restoreAllMocks()); + + it('copies order and selection when an empty view is activated before assigning data', async () => { + await viewImage('a'); + const store = useSegmentStore(); + const selected = store.segments.addSegment('a', { name: 'Selected' }); + const first = store.segments.addSegment('a', { name: 'First' }); + store.segments.moveSegment(first, selected); + store.segments.selectSegment(selected); + await seatImage('b'); + const views = useViewStore(); + const emptyView = views.viewIDs.find((id) => id !== views.activeView)!; + views.setDataForView(emptyView, null); + + views.setActiveView(emptyView); + views.setDataForView(emptyView, 'b'); + + expect(segmentNames('b')).toEqual(['First', 'Selected']); + const copied = store.segments.findSegmentByName('b', 'Selected')!; + expect(store.segments.selectedSegmentId('b')).toBe(copied.id); + expect(copied.id).not.toBe(selected); + }); + + it.each([ + [], + [{ name: 'Session', color: [1, 2, 3, 255], visible: true, locked: false }], + ])( + 'uses restored defaults for an image without a saved set', + async (...defaults) => { + const store = useSegmentStore(); + store.replaceConfigSegments({ Site: { color: 'red' } }, []); + await seatImage('destination'); + const manifest = manifestForImages(['incoming'], { + primarySelection: 'incoming', + seedSegmentDefaults: defaults, + }); + + await completeStateFileRestore(manifest, [], { incoming: 'destination' }); + + expect(segmentNames('destination')).toEqual( + defaults.map(({ name }) => name) + ); + expect(serializeAnnotations().seedSegmentDefaults).toEqual(defaults); + } + ); + + it('keeps the previous image copy when a session carries neither sets nor defaults', async () => { + await viewImage('existing'); + const store = useSegmentStore(); + store.segments.addSegment('existing', { name: 'Liver' }); + await seatImage('destination'); + + await completeStateFileRestore( + manifestForImages(['incoming'], { primarySelection: 'incoming' }), + [], + { incoming: 'destination' } + ); + + expect(segmentNames('destination')).toEqual(['Liver']); + }); + + it('seeds an image edited before it is viewed like a viewed one', async () => { + await viewImage('existing'); + useSegmentStore().segments.addSegment('existing', { name: 'Liver' }); + await seatImage('destination'); + + useSegmentStore().segments.addSegment('destination', { name: 'Tumor' }); + + expect(segmentNames('destination')).toEqual(['Liver', 'Tumor']); + expect(segmentNames('existing')).toEqual(['Liver']); + }); + + it.each([true, false])( + 'distinguishes a saved empty set from its missing-entry negative control (saved: %s)', + async (savedEmptySet) => { + await viewImage('existing'); + useSegmentStore().segments.mintSegment('existing', { name: 'Existing' }); + await seatImage('destination'); + const manifest = manifestForImages(['incoming'], { + primarySelection: 'incoming', + segments: [], + seedSegmentDefaults: [ + { name: 'Fallback', visible: true, locked: false }, + ], + imageSegmentSets: savedEmptySet + ? [{ imageId: 'incoming', segmentIds: [] }] + : [], + }); + await completeStateFileRestore(manifest, [], { incoming: 'destination' }); + + expect(segmentNames('destination')).toEqual( + savedEmptySet ? [] : ['Fallback'] + ); + expect(segmentNames('existing')).toEqual(['Existing']); + if (savedEmptySet) + expect(serializeAnnotations().imageSegmentSets).toContainEqual({ + imageId: 'destination', + segmentIds: [], + }); + } + ); + + it('applies an accompanying config to loaded images while leaving the active image alone', async () => { + await viewImage('existing'); + useSegmentStore().segments.mintSegment('existing', { name: 'Existing' }); + await seatImage('new-image'); + const imageFile = new File([], 'new-image.nrrd'); + const configFile = new File( + [ + JSON.stringify({ + segments: { Configured: { color: '#00ff00' } }, + }), + ], + 'config.json', + { type: 'application/json' } + ); + await importDataSources( + [ + { type: 'file', file: imageFile, fileType: 'application/octet-stream' }, + { type: 'file', file: configFile, fileType: 'application/json' }, + ], + { + restoreStateFile: async (source) => + source.type === 'file' && source.file.name === imageFile.name + ? asLoadableResult('new-image', source, 'image') + : Skip, + completeStateFileRestore: async () => {}, + } + ); + + await showImage('new-image'); + expect(segmentNames('new-image')).toEqual(['Configured']); + expect(segmentNames('existing')).toEqual(['Existing']); + expect( + serializeAnnotations().seedSegmentDefaults?.map(({ name }) => name) + ).toEqual(['Configured']); + }); + + const companionImport = async (edit: () => void = () => {}) => { + await seatImage('base'); + await seatImage('labelmap', { values: new Uint8Array(64).fill(1) }); + const store = useSegmentationStore(); + const convert = store.startLabelmapConversion; + const entered = defer(); + const released = defer(); + let conversion: Promise = Promise.resolve(); + vi.spyOn(store, 'startLabelmapConversion').mockImplementation( + (imageId, parentId, options) => { + conversion = convert(imageId, parentId, { + ...options, + resample: async (_parent, image) => { + entered.resolve(); + await released.promise; + return image; + }, + }); + return conversion as ReturnType; + } + ); + showImportedData([ + asLoadableResult( + 'base', + { type: 'uri', uri: '/base.nrrd', name: 'base.nrrd' }, + 'image' + ), + asLoadableResult( + 'labelmap', + { type: 'uri', uri: '/base.seg.nrrd', name: 'base.seg.nrrd' }, + 'image' + ), + ]); + await entered.promise; + const pendingNames = segmentNames('base'); + edit(); + released.resolve(); + await conversion; + return pendingNames; + }; + + it('replaces an untouched copy with a companion segmentation', async () => { + await viewImage('existing'); + useSegmentStore().segments.mintSegment('existing', { name: 'Unrelated' }); + + const pendingNames = await companionImport(); + + expect(pendingNames).toEqual(['Unrelated']); + expect(segmentNames('base')).toHaveLength(1); + expect(segmentNames('base')).not.toContain('Unrelated'); + expect(useSegmentationStore().imageMasks('base')).toHaveLength(1); + expect(segmentNames('existing')).toEqual(['Unrelated']); + }); + + it('merges a companion segmentation into the configured defaults', async () => { + useSegmentStore().replaceConfigSegments({ Lung: {}, Lesion: {} }, []); + await viewImage('existing'); + useSegmentStore().segments.mintSegment('existing', { name: 'Unrelated' }); + + const pendingNames = await companionImport(); + + expect(pendingNames).toEqual(['Lung', 'Lesion', 'Unrelated']); + expect(segmentNames('base')).toEqual(['Lung', 'Lesion', 'labelmap']); + expect(segmentNames('existing')).toEqual(['Lung', 'Lesion', 'Unrelated']); + }); + + it.each(['new segment', 'paint'])( + 'keeps a copy edited by a %s before the companion segmentation lands', + async (action) => { + await viewImage('existing'); + useSegmentStore().segments.mintSegment('existing', { name: 'Liver' }); + + await companionImport(() => { + if (action === 'new segment') + useSegmentStore().segments.addSegment('base', { name: 'New' }); + else useSegmentationStore().resolveEditTarget('base'); + }); + + expect(segmentNames('base')).toEqual( + expect.arrayContaining( + action === 'new segment' ? ['Liver', 'New'] : ['Liver'] + ) + ); + expect(segmentNames('base')).toHaveLength( + action === 'new segment' ? 3 : 2 + ); + } + ); + + it('replaces an untouched copy with a restored set', async () => { + await viewImage('existing'); + useSegmentStore().segments.mintSegment('existing', { name: 'Liver' }); + await viewImage('destination'); + expect(segmentNames('destination')).toEqual(['Liver']); + + await completeStateFileRestore( + manifestForImages(['incoming'], { + primarySelection: 'incoming', + segments: [ + { + id: 's1', + name: 'Tumor', + color: [1, 2, 3, 255], + visible: true, + locked: false, + }, + ], + imageSegmentSets: [{ imageId: 'incoming', segmentIds: ['s1'] }], + }), + [], + { incoming: 'destination' } + ); + + expect(segmentNames('destination')).toEqual(['Tumor']); + expect(segmentNames('existing')).toEqual(['Liver']); + }); +}); diff --git a/src/segmentation/__tests__/lastImageRemoval.spec.ts b/src/segmentation/__tests__/lastImageRemoval.spec.ts index 524ae15a7..186f14878 100644 --- a/src/segmentation/__tests__/lastImageRemoval.spec.ts +++ b/src/segmentation/__tests__/lastImageRemoval.spec.ts @@ -5,76 +5,93 @@ import vtkImageData from '@kitware/vtk.js/Common/DataModel/ImageData'; import { ACTION_TO_FUNC } from '@/src/composables/actions'; import { applyPostStateConfig, config } from '@/src/io/import/configJson'; -import { maskOn } from '@/src/segmentation/__tests__/segmentMaskFixtures'; +import { + maskOn, + segmentNames, + serializeAnnotations, +} from '@/src/segmentation/__tests__/segmentMaskFixtures'; import { useSegmentStore } from '@/src/segmentation/segments'; import { useDatasetStore } from '@/src/store/datasets'; import { useImageStore } from '@/src/store/datasets-images'; import { useRulerStore } from '@/src/store/tools/rulers'; +import { useViewStore } from '@/src/store/views'; const segments = () => useSegmentStore().segments; -const names = () => segments().segmentList.value.map(({ name }) => name); -const seatBareImage = (id: string) => +const seatBareImage = (id: string) => { useImageStore().addVTKImageData('CT', vtkImageData.newInstance(), { id }); + useViewStore().setDataForAllViews(id); +}; const configure = (configured: Record) => - applyPostStateConfig(config.parse({ segments: configured })); + applyPostStateConfig(config.parse({ segments: configured }), undefined, []); -describe('segments once the last image is removed', () => { +describe('removing image segment sets', () => { beforeEach(() => { setActivePinia(createPinia()); useSegmentStore(); }); - it('keeps only the configured segments after Clear Scene', () => { - seatBareImage('img-1'); - seatBareImage('img-2'); - const matched = segments().addSegment({ name: 'Node' }); + it('clears image identities and keeps configured defaults after Clear Scene', () => { configure({ Tumor: { color: '#ff0000' }, Node: {} }); - const tumor = segments().findSegmentByName('Tumor')!.id; + seatBareImage('img-1'); + const tumor = segments().findSegmentByName('img-1', 'Tumor')!.id; maskOn('img-1', tumor); - maskOn('img-2', segments().addSegment({ name: 'Painted' })); - useRulerStore().addTool({ - imageID: 'img-1', - segmentId: segments().addSegment({ name: 'Measured' }), - }); - segments().addSegment({ name: 'Empty' }); + useRulerStore().addTool({ imageID: 'img-1', segmentId: tumor }); + seatBareImage('img-2'); + maskOn('img-2', segments().addSegment('img-2', { name: 'Painted' })); + segments().addSegment('img-2', { name: 'Empty' }); ACTION_TO_FUNC.clearScene(); - expect(names()).toEqual(['Node', 'Tumor']); - expect(segments().getSegment(matched)?.name).toBe('Node'); - expect(segments().selectedSegmentId.value).toBe(matched); + const saved = serializeAnnotations(); + expect(saved.imageSegmentSets).toEqual([]); + expect(saved.segments).toEqual([]); + expect(saved.tools?.rulers?.tools).toEqual([]); + expect(saved.seedSegmentDefaults?.map(({ name }) => name)).toEqual([ + 'Tumor', + 'Node', + ]); }); - it('prunes nothing while an image remains', () => { + it('removes one image set while preserving the remaining image', () => { seatBareImage('img-1'); + const first = segments().addSegment('img-1', { name: 'Tumor' }); seatBareImage('img-2'); - maskOn('img-2', segments().addSegment({ name: 'Painted' })); - segments().addSegment({ name: 'Empty' }); + const second = segments().findSegmentByName('img-2', 'Tumor')!.id; + maskOn('img-2', second); + segments().addSegment('img-2', { name: 'Empty' }); useDatasetStore().remove('img-2'); - expect(names()).toEqual(['Painted', 'Empty']); + const saved = serializeAnnotations(); + expect(saved.imageSegmentSets?.map(({ imageId }) => imageId)).toEqual([ + 'img-1', + ]); + expect(saved.segments?.map(({ id }) => id)).not.toContain(second); + expect(segmentNames('img-1')).toEqual(['Tumor']); + expect(saved.segments?.map(({ id }) => id)).toContain(first); }); - it('prunes when images removed one by one leave none', () => { + it('leaves no image sets when images are removed one by one', () => { seatBareImage('img-1'); + segments().addSegment('img-1', { name: 'Empty' }); seatBareImage('img-2'); - segments().addSegment({ name: 'Empty' }); useDatasetStore().remove('img-1'); useDatasetStore().remove('img-2'); - expect(names()).toEqual([]); + const saved = serializeAnnotations(); + expect(saved.imageSegmentSets).toEqual([]); + expect(saved.segments).toEqual([]); }); - it('keeps what a load brings after the last image left', async () => { + it('keeps definitions restored after the last image left', async () => { seatBareImage('img-1'); - segments().addSegment({ name: 'Old' }); + segments().addSegment('img-1', { name: 'Old' }); ACTION_TO_FUNC.clearScene(); - - segments().adopt([ + seatBareImage('img-2'); + segments().adopt('img-2', [ { id: 'saved', name: 'Restored', @@ -83,10 +100,9 @@ describe('segments once the last image is removed', () => { locked: false, }, ]); - seatBareImage('img-2'); - segments().addSegment({ name: 'Minted' }); + segments().addSegment('img-2', { name: 'Minted' }); await nextTick(); - expect(names()).toEqual(['Restored', 'Minted']); + expect(segmentNames('img-2')).toEqual(['Restored', 'Minted']); }); }); diff --git a/src/segmentation/__tests__/lockedEditCreatesNothing.spec.ts b/src/segmentation/__tests__/lockedEditCreatesNothing.spec.ts index efea7031d..9e41e90dd 100644 --- a/src/segmentation/__tests__/lockedEditCreatesNothing.spec.ts +++ b/src/segmentation/__tests__/lockedEditCreatesNothing.spec.ts @@ -30,7 +30,10 @@ const SQUARE: Vector3[] = [ const segments = () => useSegmentStore().segments; const lockedSelection = () => { - const segmentId = segments().mintSegment({ name: 'Locked', locked: true }); + const segmentId = segments().mintSegment('img-1', { + name: 'Locked', + locked: true, + }); segments().selectSegment(segmentId); return segmentId; }; @@ -73,8 +76,11 @@ describe('a refused edit on a locked segment', () => { }); it('creates no mask record for a polygon naming a locked segment', () => { - const locked = segments().mintSegment({ name: 'Locked', locked: true }); - const active = segments().mintSegment({ name: 'Active' }); + const locked = segments().mintSegment('img-1', { + name: 'Locked', + locked: true, + }); + const active = segments().mintSegment('img-1', { name: 'Active' }); segments().selectSegment(active); const result = rasterizePolygon({ diff --git a/src/segmentation/__tests__/maskAndNameLookups.spec.ts b/src/segmentation/__tests__/maskAndNameLookups.spec.ts index 64fc654ea..31085b003 100644 --- a/src/segmentation/__tests__/maskAndNameLookups.spec.ts +++ b/src/segmentation/__tests__/maskAndNameLookups.spec.ts @@ -50,7 +50,9 @@ describe('the mask index a segment is looked up through', () => { }); it('answers for the segments beside one that was detached', () => { - const [first, second, third] = ['a', 'b', 'c'].map(() => mintSegment()); + const [first, second, third] = ['a', 'b', 'c'].map(() => + mintSegment('img-1') + ); const masks = [first, second, third].map((segmentId) => { const segmentation = store().ensureSegmentationForImage('img-1'); return store().createMask(segmentation.id, segmentId).id; @@ -64,7 +66,7 @@ describe('the mask index a segment is looked up through', () => { }); it('reaches a computed when a mask appears and when it goes', () => { - const segmentId = mintSegment(); + const segmentId = mintSegment('img-1'); const seen = computed(() => maskIdFor(segmentId)); expect(seen.value).toBeUndefined(); diff --git a/src/segmentation/__tests__/paintProcess.spec.ts b/src/segmentation/__tests__/paintProcess.spec.ts index f1b4edf81..609ed3998 100644 --- a/src/segmentation/__tests__/paintProcess.spec.ts +++ b/src/segmentation/__tests__/paintProcess.spec.ts @@ -191,8 +191,11 @@ describe('Paint process store', () => { const processStore = usePaintProcessStore(); const segmentationStore = useSegmentationStore(); const { id } = segmentationStore.ensureSegmentationForImage('image-1'); - const first = segmentationStore.createMask(id, mintSegment({ name: 'A' })); - segmentationStore.createMask(id, mintSegment({ name: 'B' })); + const first = segmentationStore.createMask( + id, + mintSegment('image-1', { name: 'A' }) + ); + segmentationStore.createMask(id, mintSegment('image-1', { name: 'B' })); const voxels = segmentationStore.maskVoxels(first.id); voxels.materialize(); voxels.ensureContains([0, 1, 0, 0, 0, 0]); @@ -211,7 +214,7 @@ describe('Paint process store', () => { it('names a selected segment with no mask on this image as empty', () => { // The selection is shared across images while masks are per image, so a // selected segment routinely has no record on the image being viewed. - const segmentId = mintSegment({ name: 'Elsewhere' }); + const segmentId = mintSegment('image-1', { name: 'Elsewhere' }); useSegmentStore().segments.selectSegment(segmentId); expect(usePaintProcessStore().startRefusal('image-1')).toBe( @@ -263,7 +266,7 @@ describe('Paint process store', () => { segmentationStore.ensureSegmentationForImage('image-1'); const segment = segmentationStore.createMask( segmentation.id, - mintSegment({ + mintSegment('image-1', { name: 'Empty', }) ); @@ -286,7 +289,7 @@ describe('Paint process store', () => { segmentationStore.ensureSegmentationForImage('image-1'); const segment = segmentationStore.createMask( segmentation.id, - mintSegment({ + mintSegment('image-1', { name: 'Empty', }) ); @@ -339,7 +342,7 @@ describe('Paint process store', () => { const other = segmentationStore.createMask( segmentationId, - mintSegment({ + mintSegment('image-1', { name: 'Other', }) ); @@ -379,7 +382,7 @@ describe('Paint process store', () => { lockSegment(maskId, true); segmentationStore.createMask( segmentationId, - mintSegment({ name: 'Empty' }) + mintSegment('image-1', { name: 'Empty' }) ); await processStore.startProcess( diff --git a/src/segmentation/__tests__/paintProcessStorage.spec.ts b/src/segmentation/__tests__/paintProcessStorage.spec.ts index f3a630942..612f69c38 100644 --- a/src/segmentation/__tests__/paintProcessStorage.spec.ts +++ b/src/segmentation/__tests__/paintProcessStorage.spec.ts @@ -33,7 +33,7 @@ function addBoundSegment(segmentationId: string, name: string) { const segmentationStore = useSegmentationStore(); const segment = segmentationStore.createMask( segmentationId, - mintSegment({ name }) + mintSegment('image-1', { name }) ); const voxels = segmentationStore.maskVoxels(segment.id); voxels.materialize(); diff --git a/src/segmentation/__tests__/paintTarget.spec.ts b/src/segmentation/__tests__/paintTarget.spec.ts index a63ba9d23..714f58305 100644 --- a/src/segmentation/__tests__/paintTarget.spec.ts +++ b/src/segmentation/__tests__/paintTarget.spec.ts @@ -19,7 +19,10 @@ import { } from '@/src/segmentation/__tests__/segmentMaskFixtures'; function boundSegment(segmentationId: string, name: string) { - const segment = store().createMask(segmentationId, mintSegment({ name })); + const segment = store().createMask( + segmentationId, + mintSegment(store().segmentations[segmentationId].parentImageId, { name }) + ); store().ensureLabelmapBinding(segment.id); return segment; } @@ -57,9 +60,8 @@ describe('paint edit target', () => { strokeAt('img-2', [1, 1, 0]); - // The segment is shared; this image gets its own record and its own mask. const painted = store().findEditTarget('img-2')!; - expect(store().getMask(painted).segmentId).toBe(source.segmentId); + expect(store().getMask(painted).segmentId).not.toBe(source.segmentId); const paintedBinding = bindingOf(painted)!; const sourceBinding = bindingOf(source.id)!; expect(paintedBinding.image).not.toBe(sourceBinding.image); @@ -79,7 +81,9 @@ describe('paint edit target', () => { expect(useSegmentStore().segments.appearanceOf(segmentId).name).toBe( 'Segment 1' ); - expect(useSegmentStore().segments.selectedSegmentId.value).toBe(segmentId); + expect(useSegmentStore().segments.selectedSegmentId('img-1')).toBe( + segmentId + ); expect(maskValueAt(maskId, [1, 1, 0])).toBe(SEGMENT_VALUE); }); diff --git a/src/segmentation/__tests__/restoreSegmentLifetime.spec.ts b/src/segmentation/__tests__/restoreSegmentLifetime.spec.ts index e37256d35..3360c4325 100644 --- a/src/segmentation/__tests__/restoreSegmentLifetime.spec.ts +++ b/src/segmentation/__tests__/restoreSegmentLifetime.spec.ts @@ -35,6 +35,7 @@ async function pendingRestore(kind: 'own' | 'legacy') { const ids = ['deleted', 'healthy']; const manifest = ManifestSchema.parse( manifestForImages(['parent'], { + imageSegmentSets: [{ imageId: 'parent', segmentIds: ids }], segments: ids.map((id) => ({ id, name: id, @@ -128,23 +129,23 @@ describe.each(['own', 'legacy'] as const)( 'honors deletion during IO, same-name recreation: %s', async (recreate) => { const { completed, release, registry } = await pendingRestore(kind); - const deleted = registry.segmentList.value.find( - (segment) => segment.name === 'deleted' - )!; - const healthy = registry.segmentList.value.find( - (segment) => segment.name === 'healthy' - )!; + const deleted = registry + .segmentList('parent') + .find((segment) => segment.name === 'deleted')!; + const healthy = registry + .segmentList('parent') + .find((segment) => segment.name === 'healthy')!; registry.deleteSegment(deleted.id); const replacement = recreate - ? registry.addSegment({ name: deleted.name }) + ? registry.addSegment('parent', { name: deleted.name }) : undefined; release.resolve(); await completed; expect(registry.getSegment(deleted.id)).toBeUndefined(); - expect(registry.segmentList.value.map((segment) => segment.id)).toEqual( - replacement ? [healthy.id, replacement] : [healthy.id] - ); + expect( + registry.segmentList('parent').map((segment) => segment.id) + ).toEqual(replacement ? [healthy.id, replacement] : [healthy.id]); const masks = store().imageMasks('parent'); expect(masks.map((mask) => mask.segmentId)).toEqual([healthy.id]); expect(Array.from(store().maskVoxels(masks[0].id).scalars())).toContain( diff --git a/src/segmentation/__tests__/segmentDisplayState.spec.ts b/src/segmentation/__tests__/segmentDisplayState.spec.ts index 9798b0485..c56c41725 100644 --- a/src/segmentation/__tests__/segmentDisplayState.spec.ts +++ b/src/segmentation/__tests__/segmentDisplayState.spec.ts @@ -5,7 +5,6 @@ import { SPEC_DIMENSIONS as DIMENSIONS, labelmapValues, makeLabelmap, - mintSegment, store, } from '@/src/segmentation/__tests__/segmentMaskFixtures'; @@ -80,7 +79,7 @@ describe('segmentation display state', () => { const segmentation = store().ensureSegmentationForImage('img-1'); const segment = store().createMask( segmentation.id, - mintSegment({ name: 'Tumor' }) + segments().mintSegment('img-1', { name: 'Tumor' }) ); expect(segments().appearanceOf(segment.segmentId).fillOpacity).toBe(1); @@ -99,8 +98,14 @@ describe('segmentation display state', () => { it('keeps display state per segment', () => { const segmentation = store().ensureSegmentationForImage('img-1'); - const first = store().createMask(segmentation.id, mintSegment()); - const second = store().createMask(segmentation.id, mintSegment()); + const first = store().createMask( + segmentation.id, + segments().mintSegment('img-1') + ); + const second = store().createMask( + segmentation.id, + segments().mintSegment('img-1') + ); segments().updateSegment(first.segmentId, { fillOpacity: 0, @@ -124,7 +129,7 @@ describe('segmentation display state', () => { const segmentation = store().ensureSegmentationForImage('img-1'); const segment = store().createMask( segmentation.id, - mintSegment({ name: 'Tumor' }) + segments().mintSegment('img-1', { name: 'Tumor' }) ); store().maskVoxels(segment.id).materialize(); @@ -142,8 +147,14 @@ describe('segmentation display state', () => { // editing one segment must not hand every other mask a new array. it('leaves an untouched mask its previous list', () => { const segmentation = store().ensureSegmentationForImage('img-1'); - const edited = store().createMask(segmentation.id, mintSegment()); - const untouched = store().createMask(segmentation.id, mintSegment()); + const edited = store().createMask( + segmentation.id, + segments().mintSegment('img-1') + ); + const untouched = store().createMask( + segmentation.id, + segments().mintSegment('img-1') + ); store().maskVoxels(edited.id).materialize(); store().maskVoxels(untouched.id).materialize(); @@ -161,7 +172,10 @@ describe('segmentation display state', () => { // touched no mask must not invalidate it either. it('keeps the whole projection when nothing it covers changed', () => { const segmentation = store().ensureSegmentationForImage('img-1'); - const segment = store().createMask(segmentation.id, mintSegment()); + const segment = store().createMask( + segmentation.id, + segments().mintSegment('img-1') + ); store().maskVoxels(segment.id).materialize(); const before = store().labelmapDescriptorByMask; diff --git a/src/segmentation/__tests__/segmentImportPath.spec.ts b/src/segmentation/__tests__/segmentImportPath.spec.ts index 6b56a338b..566f8dcd9 100644 --- a/src/segmentation/__tests__/segmentImportPath.spec.ts +++ b/src/segmentation/__tests__/segmentImportPath.spec.ts @@ -77,7 +77,7 @@ describe('importing a labelmap onto a parent image', () => { await store().convertImageToLabelmap('child-img', 'parent-img'); const segments = segmentsOf('parent-img'); - expect(useSegmentStore().segments.selectedSegmentId.value).toBe( + expect(useSegmentStore().segments.selectedSegmentId('parent-img')).toBe( segments[0].segmentId ); expect(segments.map(appearanceOf)).toEqual([ @@ -187,6 +187,10 @@ describe('importing a labelmap onto a parent image', () => { async (_kind, values) => { await seatImage('parent-img', { name: 'CT' }); await seatImage('healthy-img', { name: 'MR' }); + const healthySegment = useSegmentStore().segments.addSegment( + 'healthy-img', + { name: 'Surviving label' } + ); await seatImage('child-img', { name: 'Tumor.seg.nrrd', values }); const decoding = defer(); const started = defer(); @@ -217,7 +221,12 @@ describe('importing a labelmap onto a parent image', () => { await expect(conversion).rejects.toThrow(/no longer loaded/i); expect(store().getSegmentationForImage('parent-img')).toBeUndefined(); expect(useImageCacheStore().imageById['healthy-img']).toBeDefined(); - expect(useSegmentStore().segments.segmentList.value).toEqual([]); + expect( + useSegmentStore() + .segments.segmentList('healthy-img') + .map(({ id }) => id) + ).toEqual([healthySegment]); + expect(useSegmentStore().segments.segmentList('parent-img')).toEqual([]); expect(boundMasks()).toEqual([]); } ); @@ -387,7 +396,7 @@ describe('the decode color cursor', () => { await seatImage('other-img', { name: 'MR' }); const other = store().ensureSegmentationForImage('other-img'); ['A', 'B', 'C'].forEach((name) => - store().createMask(other.id, mintSegment({ name })) + store().createMask(other.id, mintSegment('other-img', { name })) ); expect(await convertedColors()).toEqual([categorical(0), categorical(1)]); }); diff --git a/src/segmentation/__tests__/segmentMaskFixtures.ts b/src/segmentation/__tests__/segmentMaskFixtures.ts index 230de687f..abf94e455 100644 --- a/src/segmentation/__tests__/segmentMaskFixtures.ts +++ b/src/segmentation/__tests__/segmentMaskFixtures.ts @@ -12,6 +12,9 @@ import { ManifestSchema, type Manifest } from '@/src/io/state-file/schema'; import { MANIFEST_VERSION } from '@/src/io/state-file/serialize'; import { useImageCacheStore } from '@/src/store/image-cache'; import { usePaintToolStore } from '@/src/store/tools/paint'; +import { useRulerStore } from '@/src/store/tools/rulers'; +import { useRectangleStore } from '@/src/store/tools/rectangles'; +import { usePolygonStore } from '@/src/store/tools/polygons'; import { useViewStore } from '@/src/store/views'; import { useSegmentationStore, @@ -200,7 +203,7 @@ export const inMemoryArtifactIO = () => { export const segmentationSnapshot = (imageId: string) => { const segments = useSegmentStore().segments; const segmentation = store().getSegmentationForImage(imageId)!; - const selectedSegmentId = segments.selectedSegmentId.value; + const selectedSegmentId = segments.selectedSegmentId(imageId); return { name: segmentation.name, segments: segmentation.order.map((maskId) => { @@ -267,6 +270,17 @@ export const manifestForImages = ( ...extra, }) as unknown as Manifest; +export const serializeAnnotations = ( + manifest = manifestForImages(useImageCacheStore().imageIds, { tools: {} }) +) => { + const state = { zip: new JSZip(), manifest }; + useSegmentStore().serialize(state); + useRulerStore().serialize(state); + useRectangleStore().serialize(state); + usePolygonStore().serialize(state); + return JSON.parse(JSON.stringify(manifest)) as Manifest; +}; + const archivePathsIn = (parsed: any): string[] => [ ...(parsed.segmentationArtifacts ?? []).flatMap((artifact: any) => artifact.path ? [artifact.path] : [] @@ -307,15 +321,22 @@ export const makeSpecImage = () => makeImage(); export const parentImage = (imageId: string) => useImageCacheStore().getVtkImageData(imageId)!; -export const mintSegment = (init: SegmentInit | string = {}) => +/** An image's segment names, in its own order. */ +export const segmentNames = (imageId: string) => + useSegmentStore() + .segments.segmentList(imageId) + .map(({ name }) => name); + +export const mintSegment = (imageId: string, init: SegmentInit | string = {}) => useSegmentStore().segments.mintSegment( + imageId, typeof init === 'string' ? { name: init } : init ); /** A record with no storage: adding one never allocates voxels. */ export function addMask(imageId: string, name?: string) { const segmentation = store().ensureSegmentationForImage(imageId); - return store().createMask(segmentation.id, mintSegment(name)).id; + return store().createMask(segmentation.id, mintSegment(imageId, name)).id; } export const bindEmptyMasks = (imageId: string, names: string[]) => @@ -363,7 +384,7 @@ export function addActiveSegment( const segmentation = segmentationStore.ensureSegmentationForImage(imageId); const segment = segmentationStore.createMask( segmentation.id, - mintSegment({ + mintSegment(imageId, { name: 'Segment 1', }) ); diff --git a/src/segmentation/__tests__/segmentRegistry.spec.ts b/src/segmentation/__tests__/segmentRegistry.spec.ts index d393348e5..49e5f3d50 100644 --- a/src/segmentation/__tests__/segmentRegistry.spec.ts +++ b/src/segmentation/__tests__/segmentRegistry.spec.ts @@ -1,5 +1,6 @@ import { beforeEach, describe, expect, it } from 'vitest'; import { setActivePinia, createPinia } from 'pinia'; +import { computed } from 'vue'; import { maskOn, seatSpecImage, @@ -26,9 +27,8 @@ const savedSegment = (id: string, name = 'Tumor'): Segment => ({ locked: false, }); -const namesOf = (registry: { - segmentList: { value: Array<{ name: string }> }; -}) => registry.segmentList.value.map((segment) => segment.name); +const namesOf = (registry: SegmentRegistry, imageId = 'img-1') => + registry.segmentList(imageId).map((segment) => segment.name); describe('segment registry', () => { beforeEach(() => { @@ -38,27 +38,30 @@ describe('segment registry', () => { it('mints a named segment and selects it', () => { const registry = createSegmentRegistry(); - const id = registry.addSegment({ name: 'Tumor' }); + const id = registry.addSegment('img-1', { name: 'Tumor' }); expect(registry.getSegment(id)?.name).toBe('Tumor'); - expect(registry.selectedSegmentId.value).toBe(id); + expect(registry.selectedSegmentId('img-1')).toBe(id); }); it('keeps a segment id across a rename', () => { const registry = createSegmentRegistry(); - const id = registry.addSegment({ name: 'Tumor' }); + const id = registry.addSegment('img-1', { name: 'Tumor' }); registry.updateSegment(id, { name: 'Lesion' }); expect(registry.getSegment(id)?.name).toBe('Lesion'); - expect(registry.segmentList.value.map((segment) => segment.id)).toEqual([ + expect(registry.segmentList('img-1').map((segment) => segment.id)).toEqual([ id, ]); }); it('keeps the required fields a patch leaves undefined', () => { const registry = createSegmentRegistry(); - const id = registry.addSegment({ name: 'Tumor', color: [1, 2, 3, 255] }); + const id = registry.addSegment('img-1', { + name: 'Tumor', + color: [1, 2, 3, 255], + }); registry.updateSegment(id, { name: undefined, color: undefined }); @@ -66,40 +69,51 @@ describe('segment registry', () => { name: 'Tumor', color: [1, 2, 3, 255], }); - expect(registry.findSegmentByName('Tumor')?.id).toBe(id); + expect(registry.findSegmentByName('img-1', 'Tumor')?.id).toBe(id); }); it('assigns consecutive default segment names', () => { const registry = createSegmentRegistry(); - expect(registry.getSegment(registry.addSegment())?.name).toBe('Segment 1'); - expect(registry.getSegment(registry.addSegment())?.name).toBe('Segment 2'); + expect(registry.getSegment(registry.addSegment('img-1'))?.name).toBe( + 'Segment 1' + ); + expect(registry.getSegment(registry.addSegment('img-1'))?.name).toBe( + 'Segment 2' + ); }); it('mints the lowest free name again once a deletion or rename frees it', () => { const registry = createSegmentRegistry(); const nameOf = (id: string) => registry.getSegment(id)?.name; - const [first, second] = [registry.addSegment(), registry.addSegment()]; - registry.addSegment(); + const [first, second] = [ + registry.addSegment('img-1'), + registry.addSegment('img-1'), + ]; + registry.addSegment('img-1'); registry.deleteSegment(first); - expect(nameOf(registry.addSegment())).toBe('Segment 1'); + expect(nameOf(registry.addSegment('img-1'))).toBe('Segment 1'); registry.updateSegment(second, { name: 'Liver' }); - expect(nameOf(registry.addSegment())).toBe('Segment 2'); - expect(nameOf(registry.addSegment())).toBe('Segment 4'); + expect(nameOf(registry.addSegment('img-1'))).toBe('Segment 2'); + expect(nameOf(registry.addSegment('img-1'))).toBe('Segment 4'); - registry.addSegment({ name: registry.uniqueName('Liver') }); - const third = registry.addSegment({ name: registry.uniqueName('Liver') }); + registry.addSegment('img-1', { + name: registry.uniqueName('img-1', 'Liver'), + }); + const third = registry.addSegment('img-1', { + name: registry.uniqueName('img-1', 'Liver'), + }); expect(nameOf(third)).toBe('Liver (3)'); registry.deleteSegment(third); - expect(registry.uniqueName('Liver ')).toBe('Liver (3)'); + expect(registry.uniqueName('img-1', 'Liver ')).toBe('Liver (3)'); }); it('cycles the tool colors for minted segments', () => { const registry = createSegmentRegistry(); - const ids = TOOL_COLORS.map(() => registry.addSegment()); + const ids = TOOL_COLORS.map(() => registry.addSegment('img-1')); expect(ids.map((id) => registry.getSegment(id)?.color)).toEqual( TOOL_COLORS.map(cssColorToRGBA) @@ -108,20 +122,51 @@ describe('segment registry', () => { it('lists segments in creation order, which is the picking order', () => { const registry = createSegmentRegistry(); - const first = registry.addSegment({ name: 'Tumor' }); - const second = registry.addSegment({ name: 'Node' }); + const first = registry.addSegment('img-1', { name: 'Tumor' }); + const second = registry.addSegment('img-1', { name: 'Node' }); expect(namesOf(registry)).toEqual(['Tumor', 'Node']); expect(registry.orderIndexOf(first)).toBe(0); expect(registry.orderIndexOf(second)).toBe(1); }); + it('reuses an image list across readers and unrelated edits while keeping its contents reactive', () => { + const registry = createSegmentRegistry(); + const first = registry.addSegment('first', { name: 'A' }); + const other = registry.addSegment('second', { name: 'Other' }); + const list = computed(() => registry.segmentList('first')); + const initial = list.value; + + expect(registry.segmentList('first')).toBe(initial); + registry.selectSegment(first); + registry.updateSegment(other, { name: 'Unrelated' }); + registry.mintSegment('second'); + expect(registry.segmentList('first')).toBe(initial); + expect(list.value).toBe(initial); + + registry.updateSegment(first, { name: 'Renamed' }); + expect(list.value).not.toBe(initial); + expect(list.value.map(({ name }) => name)).toEqual(['Renamed']); + const next = registry.mintSegment('first', { name: 'B' }); + expect(list.value.map(({ id }) => id)).toEqual([first, next]); + registry.moveSegment(next, first); + expect(list.value.map(({ id }) => id)).toEqual([next, first]); + registry.deleteSegment(first); + expect(list.value.map(({ id }) => id)).toEqual([next]); + registry.removeImage('first'); + expect(list.value).toEqual([]); + const replacement = registry.addSegment('first', { name: 'Replacement' }); + expect(list.value.map(({ id }) => id)).toEqual([replacement]); + }); + it('keeps the list and the order index in step across a deletion', () => { const registry = createSegmentRegistry(); - const ids = ['A', 'B', 'C'].map((name) => registry.addSegment({ name })); + const ids = ['A', 'B', 'C'].map((name) => + registry.addSegment('img-1', { name }) + ); registry.deleteSegment(ids[1]); - const fourth = registry.addSegment({ name: 'D' }); + const fourth = registry.addSegment('img-1', { name: 'D' }); registry.updateSegment(ids[2], { name: 'C2' }); expect(namesOf(registry)).toEqual(['A', 'C2', 'D']); @@ -131,34 +176,34 @@ describe('segment registry', () => { it('forgets a deleted or renamed name', () => { const registry = createSegmentRegistry(); - const liver = registry.addSegment({ name: 'Liver' }); - const spleen = registry.addSegment({ name: 'Spleen' }); + const liver = registry.addSegment('img-1', { name: 'Liver' }); + const spleen = registry.addSegment('img-1', { name: 'Spleen' }); registry.deleteSegment(liver); registry.updateSegment(spleen, { name: 'Kidney' }); - expect(registry.findSegmentByName('Liver')).toBeUndefined(); - expect(registry.findSegmentByName('Spleen')).toBeUndefined(); - expect(registry.findSegmentByName('Kidney')?.id).toBe(spleen); - expect(registry.uniqueName(' Spleen ')).toBe('Spleen'); + expect(registry.findSegmentByName('img-1', 'Liver')).toBeUndefined(); + expect(registry.findSegmentByName('img-1', 'Spleen')).toBeUndefined(); + expect(registry.findSegmentByName('img-1', 'Kidney')?.id).toBe(spleen); + expect(registry.uniqueName('img-1', ' Spleen ')).toBe('Spleen'); }); it('finds the first in registry order when a name repeats', () => { const registry = createSegmentRegistry(); - const first = registry.addSegment({ name: 'Liver' }); - const second = registry.addSegment({ name: 'Other' }); + const first = registry.addSegment('img-1', { name: 'Liver' }); + const second = registry.addSegment('img-1', { name: 'Other' }); registry.updateSegment(second, { name: 'Liver' }); - expect(registry.findSegmentByName('Liver')?.id).toBe(first); + expect(registry.findSegmentByName('img-1', 'Liver')?.id).toBe(first); registry.moveSegment(second, first); - expect(registry.findSegmentByName('Liver')?.id).toBe(second); + expect(registry.findSegmentByName('img-1', 'Liver')?.id).toBe(second); }); it('resolves absent appearance to the app defaults', () => { const registry = createSegmentRegistry(); - const id = registry.addSegment({ name: 'Tumor' }); + const id = registry.addSegment('img-1', { name: 'Tumor' }); const appearance = registry.appearanceOf(id); @@ -169,18 +214,23 @@ describe('segment registry', () => { it('binds an exact name to the segment already carrying it', () => { const registry = createSegmentRegistry(); - const id = registry.addSegment({ name: 'Tumor', color: [1, 2, 3, 255] }); + const id = registry.addSegment('img-1', { + name: 'Tumor', + color: [1, 2, 3, 255], + }); - expect(registry.segmentNamed('Tumor', { color: [9, 9, 9, 255] })).toBe(id); + expect( + registry.segmentNamed('img-1', 'Tumor', { color: [9, 9, 9, 255] }) + ).toBe(id); // The registry's own color wins on a match. expect(registry.getSegment(id)?.color).toEqual([1, 2, 3, 255]); }); it('mints a segment when no name matches', () => { const registry = createSegmentRegistry(); - registry.addSegment({ name: 'Tumor' }); + registry.addSegment('img-1', { name: 'Tumor' }); - const id = registry.segmentNamed('Node'); + const id = registry.segmentNamed('img-1', 'Node'); expect(registry.getSegment(id)?.name).toBe('Node'); expect(namesOf(registry)).toEqual(['Tumor', 'Node']); @@ -188,12 +238,12 @@ describe('segment registry', () => { it('clears the selection when the last segment is deleted', () => { const registry = createSegmentRegistry(); - const id = registry.addSegment({ name: 'Tumor' }); + const id = registry.addSegment('img-1', { name: 'Tumor' }); registry.deleteSegment(id); - expect(registry.segmentList.value).toEqual([]); - expect(registry.selectedSegmentId.value).toBeFalsy(); + expect(registry.segmentList('img-1')).toEqual([]); + expect(registry.selectedSegmentId('img-1')).toBeFalsy(); }); it('hands a referenced segment to the removal callback before dropping it', () => { @@ -203,7 +253,7 @@ describe('segment registry', () => { has: () => false, remove: (segmentId) => removed.push(registry.getSegment(segmentId)?.name), }); - const id = registry.addSegment({ name: 'Tumor' }); + const id = registry.addSegment('img-1', { name: 'Tumor' }); registry.deleteSegment(id); @@ -214,54 +264,53 @@ describe('segment registry', () => { it('mints and selects a segment for the first edit that needs one', () => { const registry = createSegmentRegistry(); - const id = registry.ensureSelectedSegment(); + const id = registry.ensureSelectedSegment('img-1'); - expect(registry.selectedSegmentId.value).toBe(id); + expect(registry.selectedSegmentId('img-1')).toBe(id); // Idempotent: a second edit reuses the selection. - expect(registry.ensureSelectedSegment()).toBe(id); + expect(registry.ensureSelectedSegment('img-1')).toBe(id); }); it('selects the first existing segment without adding a placeholder', () => { const registry = createSegmentRegistry(); - const first = registry.mintSegment({ name: 'Liver' }); - const second = registry.mintSegment({ name: 'Spleen' }); - expect(registry.ensureSelectedSegment()).toBe(first); - expect(registry.segmentList.value.map((segment) => segment.name)).toEqual([ - 'Liver', - 'Spleen', - ]); + const first = registry.mintSegment('img-1', { name: 'Liver' }); + const second = registry.mintSegment('img-1', { name: 'Spleen' }); + expect(registry.ensureSelectedSegment('img-1')).toBe(first); + expect( + registry.segmentList('img-1').map((segment) => segment.name) + ).toEqual(['Liver', 'Spleen']); registry.selectSegment(second); - expect(registry.ensureSelectedSegment()).toBe(second); + expect(registry.ensureSelectedSegment('img-1')).toBe(second); }); it('selects the first segment in list order while none is chosen', () => { const registry = createSegmentRegistry(); - const first = registry.mintSegment({ name: 'Liver' }); - const second = registry.mintSegment({ name: 'Spleen' }); + const first = registry.mintSegment('img-1', { name: 'Liver' }); + const second = registry.mintSegment('img-1', { name: 'Spleen' }); - expect(registry.selectedSegmentId.value).toBe(first); + expect(registry.selectedSegmentId('img-1')).toBe(first); registry.moveSegment(second, first); - expect(registry.selectedSegmentId.value).toBe(second); + expect(registry.selectedSegmentId('img-1')).toBe(second); }); it('keeps a choice when asked to select nothing or a missing segment', () => { const registry = createSegmentRegistry(); - registry.mintSegment({ name: 'Liver' }); - const chosen = registry.mintSegment({ name: 'Spleen' }); + registry.mintSegment('img-1', { name: 'Liver' }); + const chosen = registry.mintSegment('img-1', { name: 'Spleen' }); registry.selectSegment(chosen); registry.selectSegment(undefined); registry.selectSegment(null); registry.selectSegment('nope'); - expect(registry.selectedSegmentId.value).toBe(chosen); + expect(registry.selectedSegmentId('img-1')).toBe(chosen); }); it('asks for the chosen row to be revealed again on reselection', () => { const registry = createSegmentRegistry(); - const chosen = registry.mintSegment({ name: 'Liver' }); + const chosen = registry.mintSegment('img-1', { name: 'Liver' }); registry.selectSegment(chosen); const revision = registry.selectionRevision.value; @@ -272,32 +321,35 @@ describe('segment registry', () => { it('falls back to the first segment once the chosen one is deleted', () => { const registry = createSegmentRegistry(); - const first = registry.mintSegment({ name: 'Liver' }); - const chosen = registry.mintSegment({ name: 'Spleen' }); - registry.mintSegment({ name: 'Kidney' }); + const first = registry.mintSegment('img-1', { name: 'Liver' }); + const chosen = registry.mintSegment('img-1', { name: 'Spleen' }); + registry.mintSegment('img-1', { name: 'Kidney' }); registry.selectSegment(chosen); registry.deleteSegment(chosen); - expect(registry.selectedSegmentId.value).toBe(first); + expect(registry.selectedSegmentId('img-1')).toBe(first); }); it('selects a configured segment without minting one beside it', () => { const registry = createSegmentRegistry(); - registry.replaceConfigSegments({ Tumor: {}, Edema: {} }); - const tumor = registry.findSegmentByName('Tumor')?.id; + registry.replaceConfigSegments('img-1', { Tumor: {}, Edema: {} }); + const tumor = registry.findSegmentByName('img-1', 'Tumor')?.id; - expect(registry.selectedSegmentId.value).toBe(tumor); - expect(registry.ensureSelectedSegment()).toBe(tumor); + expect(registry.selectedSegmentId('img-1')).toBe(tumor); + expect(registry.ensureSelectedSegment('img-1')).toBe(tumor); expect(namesOf(registry)).toEqual(['Tumor', 'Edema']); }); it('keeps the current color when a configured color does not parse', () => { const registry = createSegmentRegistry(); - const id = registry.mintSegment({ name: 'Tumor', color: [0, 255, 0, 255] }); + const id = registry.mintSegment('img-1', { + name: 'Tumor', + color: [0, 255, 0, 255], + }); - registry.replaceConfigSegments({ + registry.replaceConfigSegments('img-1', { Tumor: { color: 'rgb(255, 0, 0)', strokeWidth: 3 }, }); @@ -309,7 +361,7 @@ describe('segment registry', () => { it('adopts restored segments under fresh ids, overwriting nothing', () => { const registry = createSegmentRegistry(); - const existing = registry.addSegment({ + const existing = registry.addSegment('img-1', { name: 'Tumor', color: [1, 1, 1, 255], }); @@ -318,7 +370,7 @@ describe('segment registry', () => { remove: () => {}, }); - const { idMap } = registry.adopt([ + const { idMap } = registry.adopt('img-1', [ { id: existing, name: 'Tumor', @@ -336,13 +388,13 @@ describe('segment registry', () => { it('restores onto an empty segment of the same name', () => { const registry = createSegmentRegistry(); - const existing = registry.addSegment({ + const existing = registry.addSegment('img-1', { name: 'Tumor', color: [1, 1, 1, 255], strokeWidth: 3, }); - const { idMap } = registry.adopt([ + const { idMap } = registry.adopt('img-1', [ { ...savedSegment('saved'), visible: false, @@ -367,9 +419,12 @@ describe('segment registry', () => { it('seats one restored segment per existing name and suffixes the rest', () => { const registry = createSegmentRegistry(); - const existing = registry.addSegment({ name: 'Tumor' }); + const existing = registry.addSegment('img-1', { name: 'Tumor' }); - const { idMap } = registry.adopt([savedSegment('a'), savedSegment('b')]); + const { idMap } = registry.adopt('img-1', [ + savedSegment('a'), + savedSegment('b'), + ]); expect(idMap.a).toBe(existing); expect(idMap.b).not.toBe(existing); @@ -379,18 +434,49 @@ describe('segment registry', () => { it('suffixes a repeated name the same restore minted first', () => { const registry = createSegmentRegistry(); - const { idMap } = registry.adopt([savedSegment('a'), savedSegment('b')]); + const { idMap } = registry.adopt('img-1', [ + savedSegment('a'), + savedSegment('b'), + ]); expect(idMap.a).not.toBe(idMap.b); expect(namesOf(registry)).toEqual(['Tumor', 'Tumor (2)']); }); + it('keeps distinct identities across adoption calls in one restore', () => { + const registry = createSegmentRegistry(); + registry.replaceConfigSegments('img-1', { Tumor: { color: '#0000ff' } }); + const configured = registry.findSegmentByName('img-1', 'Tumor')!.id; + const claimed = new Set(); + const first = registry.adopt( + 'img-1', + [{ ...savedSegment('first'), color: [255, 0, 0, 255] }], + claimed + ); + + const second = registry.adopt( + 'img-1', + [{ ...savedSegment('second'), color: [0, 255, 0, 255] }], + claimed + ); + + expect(first.idMap.first).toBe(configured); + expect(second.idMap.second).not.toBe(configured); + expect(namesOf(registry)).toEqual(['Tumor', 'Tumor (2)']); + expect(registry.getSegment(configured)?.color).toEqual([0, 0, 255, 255]); + expect(registry.getSegment(second.idMap.second)?.color).toEqual([ + 0, 255, 0, 255, + ]); + registry.replaceConfigSegments('img-1', { Tumor: {} }); + expect(registry.getSegment(configured)?.color).toEqual([255, 0, 0, 255]); + }); + it('restores onto an empty configured segment beneath the config', () => { const registry = createSegmentRegistry(); - registry.replaceConfigSegments({ Tumor: { color: '#ff0000' } }); - const tumor = registry.findSegmentByName('Tumor')?.id; + registry.replaceConfigSegments('img-1', { Tumor: { color: '#ff0000' } }); + const tumor = registry.findSegmentByName('img-1', 'Tumor')?.id; - const { idMap } = registry.adopt([ + const { idMap } = registry.adopt('img-1', [ { ...savedSegment('saved'), visible: false, @@ -409,18 +495,18 @@ describe('segment registry', () => { }); // The session's appearance is what the config now layers over. - registry.replaceConfigSegments({ Tumor: {} }); + registry.replaceConfigSegments('img-1', { Tumor: {} }); expect(registry.getSegment(tumor)?.color).toEqual([2, 2, 2, 255]); // The segment is the session's too, so clearing the config keeps it. - registry.replaceConfigSegments(null); + registry.replaceConfigSegments('img-1', null); expect(registry.getSegment(tumor)?.name).toBe('Tumor'); }); it('adopts the first of a repeated id and seats no segment for the rest', () => { const registry = createSegmentRegistry(); - const { idMap, repeated } = registry.adopt([ + const { idMap, repeated } = registry.adopt('img-1', [ { id: 'dup', name: 'A', @@ -451,7 +537,7 @@ describe('segment registry', () => { it('adopts a segment whose id spells a prototype member', () => { const registry = createSegmentRegistry(); - const { idMap } = registry.adopt([ + const { idMap } = registry.adopt('img-1', [ { id: 'constructor', name: 'A', @@ -470,7 +556,7 @@ describe('segment registry', () => { describe('a restored selection', () => { const restoreSaved = (registry: SegmentRegistry) => - registry.adopt([ + registry.adopt('img-1', [ { id: 'saved', name: 'Saved', @@ -482,48 +568,194 @@ describe('segment registry', () => { it('replaces the selection a config offered', () => { const registry = createSegmentRegistry(); - registry.replaceConfigSegments({ Tumor: {}, Edema: {} }); + registry.replaceConfigSegments('img-1', { Tumor: {}, Edema: {} }); const saved = restoreSaved(registry); - registry.restoreSelection(saved); + registry.restoreSelection('img-1', saved); - expect(registry.selectedSegmentId.value).toBe(saved); + expect(registry.selectedSegmentId('img-1')).toBe(saved); }); it('replaces the first-segment fallback', () => { const registry = createSegmentRegistry(); - registry.mintSegment({ name: 'Tumor' }); + registry.mintSegment('img-1', { name: 'Tumor' }); const saved = restoreSaved(registry); - registry.restoreSelection(saved); + registry.restoreSelection('img-1', saved); - expect(registry.selectedSegmentId.value).toBe(saved); + expect(registry.selectedSegmentId('img-1')).toBe(saved); }); it('replaces a segment an edit seated automatically', () => { const registry = createSegmentRegistry(); - registry.ensureSelectedSegment(); + registry.ensureSelectedSegment('img-1'); const saved = restoreSaved(registry); - registry.restoreSelection(saved); + registry.restoreSelection('img-1', saved); - expect(registry.selectedSegmentId.value).toBe(saved); + expect(registry.selectedSegmentId('img-1')).toBe(saved); }); it('leaves a selection the user made', () => { const registry = createSegmentRegistry(); - registry.replaceConfigSegments({ Tumor: {}, Edema: {} }); - const edema = registry.findSegmentByName('Edema')?.id; + registry.replaceConfigSegments('img-1', { Tumor: {}, Edema: {} }); + const edema = registry.findSegmentByName('img-1', 'Edema')?.id; registry.selectSegment(edema); const saved = restoreSaved(registry); - registry.restoreSelection(saved); + registry.restoreSelection('img-1', saved); - expect(registry.selectedSegmentId.value).toBe(edema); + expect(registry.selectedSegmentId('img-1')).toBe(edema); }); }); }); +describe('image segment sets', () => { + beforeEach(() => { + setActivePinia(createPinia()); + }); + + it('copies definitions and selection without sharing later edits', () => { + const registry = createSegmentRegistry(); + const first = registry.addSegment('first', { + name: 'Tumor', + color: [1, 2, 3, 255], + visible: false, + locked: true, + strokeWidth: 4, + }); + const node = registry.addSegment('first', { name: 'Node' }); + registry.moveSegment(node, first); + registry.selectSegment(first); + + registry.initializeSet('second', registry.snapshot('first')); + const [copiedNode, copied] = registry.segmentList('second'); + + expect(copied).toMatchObject({ + name: 'Tumor', + color: [1, 2, 3, 255], + visible: true, + locked: false, + strokeWidth: 4, + }); + expect(copied.id).not.toBe(first); + expect(namesOf(registry, 'second')).toEqual(['Node', 'Tumor']); + expect(registry.selectedSegmentId('second')).toBe(copied.id); + registry.updateSegment(copied.id, { name: 'Lesion', locked: true }); + registry.moveSegment(copied.id, copiedNode.id); + registry.deleteSegment(copied.id); + + expect(namesOf(registry, 'first')).toEqual(['Node', 'Tumor']); + expect(registry.getSegment(first)?.locked).toBe(true); + expect(registry.selectedSegmentId('first')).toBe(first); + expect(namesOf(registry, 'second')).toEqual(['Node']); + }); + + it('keeps the captured definitions while the source changes', () => { + const registry = createSegmentRegistry(); + const first = registry.addSegment('first', { name: 'Tumor' }); + const snapshot = registry.snapshot('first'); + registry.updateSegment(first, { name: 'Lesion', color: [2, 3, 4, 255] }); + + registry.initializeSet('second', snapshot); + + expect(namesOf(registry, 'second')).toEqual(['Tumor']); + expect(registry.segmentList('second')[0].color).not.toEqual([2, 3, 4, 255]); + expect(registry.segmentList('second')[0].color).not.toBe( + snapshot.segments[0].color + ); + }); + + it('preserves an initialized empty set instead of copying again', () => { + const registry = createSegmentRegistry(); + registry.addSegment('first', { name: 'Tumor' }); + registry.initializeSet('second', registry.snapshot('first')); + registry.deleteSegment(registry.segmentList('second')[0].id); + + registry.initializeSet('second', registry.snapshot('first')); + + expect(namesOf(registry, 'second')).toEqual([]); + expect(registry.imageSets.value.second).toEqual({ segmentIds: [] }); + }); + + it('matches names, assigns defaults and applies config within one image', () => { + const registry = createSegmentRegistry(); + const first = registry.addSegment('first', { name: 'Tumor' }); + const second = registry.segmentNamed('second', 'Tumor'); + registry.replaceConfigSegments('second', { Tumor: { color: 'red' } }); + + expect(second).not.toBe(first); + expect(registry.findSegmentByName('first', 'Tumor')?.id).toBe(first); + expect(registry.uniqueName('second', 'Tumor')).toBe('Tumor (2)'); + expect(registry.getSegment(first)?.color).not.toEqual( + cssColorToRGBA('red') + ); + expect(registry.getSegment(second)?.color).toEqual(cssColorToRGBA('red')); + expect(registry.getSegment(registry.addSegment('first'))?.name).toBe( + 'Segment 1' + ); + expect(registry.getSegment(registry.addSegment('second'))?.name).toBe( + 'Segment 1' + ); + }); + + it('refuses moving or restoring selection across image ownership', () => { + const registry = createSegmentRegistry(); + const first = registry.addSegment('first', { name: 'Tumor' }); + const second = registry.mintSegment('second', { name: 'Node' }); + + registry.moveSegment(first, second); + registry.restoreSelection('second', first); + + expect(namesOf(registry, 'first')).toEqual(['Tumor']); + expect(namesOf(registry, 'second')).toEqual(['Node']); + expect(registry.selectedSegmentId('second')).toBe(second); + expect(registry.belongsToImage('second', first)).toBe(false); + expect(registry.ownerImageId(first)).toBe('first'); + }); + + it('forgets only the removed image metadata without invoking per-segment content removal', () => { + const registry = createSegmentRegistry(); + const first = registry.addSegment('first', { name: 'Tumor' }); + const another = registry.addSegment('first', { name: 'Node' }); + const second = registry.addSegment('second', { name: 'Tumor' }); + registry.replaceConfigSegments('first', { Configured: {} }); + let individualRemovals = 0; + registry.declareReferences('content', { + has: () => true, + remove: () => { + individualRemovals += 1; + }, + }); + + registry.removeImage('first'); + + expect(individualRemovals).toBe(0); + expect(registry.imageSets.value.first).toBeUndefined(); + expect(registry.getSegment(first)).toBeUndefined(); + expect(registry.getSegment(another)).toBeUndefined(); + expect(registry.ownerImageId(first)).toBeUndefined(); + expect(registry.getSegment(second)?.name).toBe('Tumor'); + expect(registry.selectedSegmentId('first')).toBeUndefined(); + expect(registry.selectedSegmentId('second')).toBe(second); + registry.replaceConfigSegments('first', { Configured: {} }); + expect(registry.segmentList('first').map(({ name }) => name)).toEqual([ + 'Configured', + ]); + const replacement = registry.addSegment('first'); + expect(registry.getSegment(replacement)?.name).toBe('Segment 1'); + }); + + it('reads absent image context without initializing anything', () => { + const registry = createSegmentRegistry(); + + expect(registry.segmentList(undefined)).toEqual([]); + expect(registry.selectedSegmentId(undefined)).toBeUndefined(); + expect(registry.belongsToImage(undefined, 'missing')).toBe(false); + expect(registry.imageSets.value).toEqual({}); + }); +}); + describe('the shared registry', () => { beforeEach(async () => { setActivePinia(createPinia()); @@ -531,7 +763,9 @@ describe('the shared registry', () => { }); it('shares one registry across paint, rectangles, polygons and rulers', () => { - const id = useSegmentStore().segments.addSegment({ name: 'Tumor' }); + const id = useSegmentStore().segments.addSegment('img-1', { + name: 'Tumor', + }); const tools = [usePolygonStore(), useRectangleStore(), useRulerStore()].map( (store) => store.toolByID[store.addTool({ imageID: 'img-1' })] @@ -542,18 +776,24 @@ describe('the shared registry', () => { }); it('starts the shared registry empty, so viewing creates nothing', () => { - expect(useSegmentStore().segments.segmentList.value).toEqual([]); + expect(useSegmentStore().segments.segmentList('img-1')).toEqual([]); expect(useSegmentationStore().segmentations).toEqual({}); }); it('suffixes a restored segment whose name holds a mask or a shape', () => { const shared = useSegmentStore().segments; - const masked = shared.addSegment({ name: 'Tumor', color: [1, 1, 1, 255] }); + const masked = shared.addSegment('img-1', { + name: 'Tumor', + color: [1, 1, 1, 255], + }); maskOn('img-1', masked); - const shaped = shared.addSegment({ name: 'Node', color: [1, 1, 1, 255] }); + const shaped = shared.addSegment('img-1', { + name: 'Node', + color: [1, 1, 1, 255], + }); usePolygonStore().addTool({ imageID: 'img-1', segmentId: shaped }); - const { idMap } = shared.adopt([ + const { idMap } = shared.adopt('img-1', [ savedSegment('tumor', 'Tumor'), savedSegment('node', 'Node'), ]); diff --git a/src/segmentation/__tests__/segmentSetAppearanceRestore.spec.ts b/src/segmentation/__tests__/segmentSetAppearanceRestore.spec.ts new file mode 100644 index 000000000..6533f0ef2 --- /dev/null +++ b/src/segmentation/__tests__/segmentSetAppearanceRestore.spec.ts @@ -0,0 +1,167 @@ +import { beforeEach, describe, expect, it } from 'vitest'; +import { createPinia, setActivePinia } from 'pinia'; + +import { TOOL_COLORS } from '@/src/config'; +import { completeStateFileRestore } from '@/src/io/import/processors/restoreStateFile'; +import { ManifestSchema } from '@/src/io/state-file/schema'; +import { cssColorToRGBA } from '@/src/segmentation/color'; +import { + createSegmentRegistry, + defaultsFromConfig, + snapshotFromDefaults, +} from '@/src/segmentation/segmentRegistry'; +import { useSegmentStore } from '@/src/segmentation/segments'; +import { + inMemoryArtifactIO, + manifestForImages, + serializeToStateFiles, + viewImage, +} from '@/src/segmentation/__tests__/segmentMaskFixtures'; + +const savedTumor = { + id: 'saved-tumor', + name: 'Tumor', + color: [255, 0, 0, 255] as [number, number, number, number], + visible: true, + locked: false, +}; + +describe('image segment appearance on restore', () => { + beforeEach(() => setActivePinia(createPinia())); + + it.each([ + { stated: {}, expected: savedTumor.color }, + { stated: { color: 'blue' }, expected: cssColorToRGBA('blue') }, + ])( + 'overlays only stated default appearance on a restored segment', + ({ stated, expected }) => { + const registry = createSegmentRegistry(); + registry.initializeSet( + 'image', + snapshotFromDefaults(defaultsFromConfig({ Tumor: stated })) + ); + + registry.adopt('image', [savedTumor]); + + expect(registry.serialize()).toEqual([ + expect.objectContaining({ name: 'Tumor', color: expected }), + ]); + } + ); + + it('preserves unstated default colors through the saved manifest', async () => { + useSegmentStore().replaceConfigSegments({ Tumor: {} }, []); + const { parsed: defaults } = await serializeToStateFiles( + manifestForImages([]), + inMemoryArtifactIO() + ); + expect(defaults.seedSegmentDefaults).toEqual([ + { name: 'Tumor', visible: true, locked: false }, + ]); + + setActivePinia(createPinia()); + useSegmentStore().deserialize(ManifestSchema.parse(defaults), {}); + await viewImage('destination'); + useSegmentStore().segments.adopt('destination', [savedTumor]); + const { parsed: restored } = await serializeToStateFiles( + manifestForImages(['destination']), + inMemoryArtifactIO() + ); + + expect(restored.segments).toEqual([ + expect.objectContaining({ name: 'Tumor', color: savedTumor.color }), + ]); + }); + + it('keeps a fully specified saved default color even when it equals the palette', async () => { + const color = cssColorToRGBA(TOOL_COLORS[0]); + useSegmentStore().deserialize( + ManifestSchema.parse( + manifestForImages([], { + seedSegmentDefaults: [ + { name: 'Tumor', color, visible: true, locked: false }, + ], + }) + ), + {} + ); + await viewImage('destination'); + useSegmentStore().segments.adopt('destination', [savedTumor]); + const { parsed } = await serializeToStateFiles( + manifestForImages(['destination']), + inMemoryArtifactIO() + ); + + expect(parsed.segments).toEqual([ + expect.objectContaining({ name: 'Tumor', color }), + ]); + }); + + it('copies the source next color after earlier rows have been deleted', async () => { + await viewImage('source'); + const registry = useSegmentStore().segments; + const removed = registry.mintSegment('source', { name: 'First' }); + registry.mintSegment('source', { name: 'Remaining' }); + registry.deleteSegment(removed); + await viewImage('destination'); + + registry.addSegment('source'); + registry.addSegment('destination'); + const { parsed } = await serializeToStateFiles( + manifestForImages(['source', 'destination']), + inMemoryArtifactIO() + ); + const colors = parsed.imageSegmentSets.map( + (set: { segmentIds: string[] }) => + set.segmentIds.map( + (id) => + parsed.segments.find((segment: { id: string }) => segment.id === id) + .color + ) + ); + expect(colors[1]).toEqual(colors[0]); + expect(colors[1][1]).toEqual(cssColorToRGBA(TOOL_COLORS[2])); + }); + + it('restores selection over a copied first-row fallback', async () => { + await viewImage('source'); + const registry = useSegmentStore().segments; + registry.mintSegment('source', { name: 'Tumor' }); + registry.mintSegment('source', { name: 'Liver' }); + await viewImage('destination'); + + await completeStateFileRestore( + ManifestSchema.parse( + manifestForImages(['saved'], { + primarySelection: 'saved', + segments: [ + savedTumor, + { ...savedTumor, id: 'saved-liver', name: 'Liver' }, + ], + imageSegmentSets: [ + { + imageId: 'saved', + segmentIds: ['saved-tumor', 'saved-liver'], + selectedSegmentId: 'saved-liver', + }, + ], + }) + ), + [], + { saved: 'destination' } + ); + const { parsed } = await serializeToStateFiles( + manifestForImages(['source', 'destination']), + inMemoryArtifactIO() + ); + const destination = parsed.imageSegmentSets.find( + (set: { imageId: string }) => set.imageId === 'destination' + ); + expect( + parsed.segments.find( + (segment: { id: string }) => + segment.id === destination.selectedSegmentId + ) + ).toMatchObject({ name: 'Liver' }); + }); +}); diff --git a/src/segmentation/__tests__/segmentationRestoreResilience.spec.ts b/src/segmentation/__tests__/segmentationRestoreResilience.spec.ts index 9245918f0..ea2d33c35 100644 --- a/src/segmentation/__tests__/segmentationRestoreResilience.spec.ts +++ b/src/segmentation/__tests__/segmentationRestoreResilience.spec.ts @@ -134,7 +134,8 @@ const restoreGroups = ( manifest, stateFiles, dataIDMap, - segmentIdMap: useSegmentStore().deserialize(manifest).segmentIdMap, + segmentIdMap: useSegmentStore().deserialize(manifest, dataIDMap) + .segmentIdMap, labelmapSources: planLabelmapSources(manifest).sources, io: artifactIO, }); diff --git a/src/segmentation/__tests__/segmentations.spec.ts b/src/segmentation/__tests__/segmentations.spec.ts index 2508a9234..59b948feb 100644 --- a/src/segmentation/__tests__/segmentations.spec.ts +++ b/src/segmentation/__tests__/segmentations.spec.ts @@ -23,7 +23,10 @@ const segments = () => useSegmentStore().segments; const labelmapOf = (maskId: string) => store().findMaskVoxels(maskId).image(); function makeBoundSegment(segmentationId: string, name?: string) { - const segment = store().createMask(segmentationId, mintSegment(name)); + const segment = store().createMask( + segmentationId, + mintSegment(store().segmentations[segmentationId].parentImageId, name) + ); const binding = store().ensureLabelmapBinding(segment.id); return { id: segment.id, binding }; } @@ -91,7 +94,7 @@ describe('segmentation store', () => { await seatSpecImage('img-1'); const first = store().ensureSegmentationForImage('img-1'); - store().createMask(first.id, mintSegment()); + store().createMask(first.id, mintSegment('img-1')); const second = store().ensureSegmentationForImage('img-1'); expect(second.id).toBe(first.id); @@ -118,7 +121,7 @@ describe('segmentation store', () => { const { id: segmentationId } = store().ensureSegmentationForImage('img-1'); - const segment = store().createMask(segmentationId, mintSegment()); + const segment = store().createMask(segmentationId, mintSegment('img-1')); expect(segment.representations.labelmap).toBeUndefined(); expect(bindingOf(segment.id)).toBeUndefined(); @@ -131,8 +134,8 @@ describe('segmentation store', () => { const { id: segmentationId } = store().ensureSegmentationForImage('img-1'); - const first = store().createMask(segmentationId, mintSegment()); - const second = store().createMask(segmentationId, mintSegment()); + const first = store().createMask(segmentationId, mintSegment('img-1')); + const second = store().createMask(segmentationId, mintSegment('img-1')); expect(store().segmentations[segmentationId].order).toEqual([ first.id, @@ -146,7 +149,7 @@ describe('segmentation store', () => { const { id: segmentationId } = store().ensureSegmentationForImage('img-1'); - const segment = store().createMask(segmentationId, mintSegment()); + const segment = store().createMask(segmentationId, mintSegment('img-1')); // The record is storage; what the user sets lives on its segment. expect(segments().appearanceOf(segment.segmentId).visible).toBe(true); @@ -157,7 +160,7 @@ describe('segmentation store', () => { await seatSpecImage('img-1'); const { id: segmentationId } = store().ensureSegmentationForImage('img-1'); - const segmentId = mintSegment('Tumor'); + const segmentId = mintSegment('img-1', 'Tumor'); const segment = store().createMask(segmentationId, segmentId); @@ -170,7 +173,7 @@ describe('segmentation store', () => { await seatSpecImage('img-1'); const { id: segmentationId } = store().ensureSegmentationForImage('img-1'); - const segmentId = mintSegment('Tumor'); + const segmentId = mintSegment('img-1', 'Tumor'); const segment = store().createMask(segmentationId, segmentId); segments().updateSegment(segmentId, { name: 'Lesion' }); @@ -179,21 +182,21 @@ describe('segmentation store', () => { expect(segments().appearanceOf(segment.segmentId).name).toBe('Lesion'); }); - it('lets two images hold a record for one segment', async () => { + it('refuses using one image segment on another image', async () => { await seatSpecImage('img-1'); await seatSpecImage('img-2', 'PET'); - const segmentId = mintSegment('Tumor'); + const segmentId = mintSegment('img-1', 'Tumor'); const first = maskOn('img-1', segmentId); - const second = maskOn('img-2', segmentId); - - expect(second.id).not.toBe(first.id); - expect(second.segmentId).toBe(first.segmentId); + expect(() => maskOn('img-2', segmentId)).toThrow( + 'Segment belongs to another image' + ); + expect(store().getMask(first.id).segmentId).toBe(segmentId); }); it('keeps one record per segment on an image', async () => { await seatSpecImage('img-1'); - const segmentId = mintSegment('Tumor'); + const segmentId = mintSegment('img-1', 'Tumor'); const first = maskOn('img-1', segmentId); const again = maskOn('img-1', segmentId); @@ -208,7 +211,7 @@ describe('segmentation store', () => { await seatSpecImage('img-1'); const { id: segmentationId } = store().ensureSegmentationForImage('img-1'); - const segmentId = mintSegment('Tumor'); + const segmentId = mintSegment('img-1', 'Tumor'); const first = store().createMask(segmentationId, segmentId); expect(() => store().createMask(segmentationId, segmentId)).toThrow( @@ -275,9 +278,11 @@ describe('segmentation store', () => { expect(store().getMask(second.id).id).toBe(second.id); // The move reorders the registry; the image's masks keep their own // insertion order, and every one of them still draws. - expect(segments().segmentList.value.map((segment) => segment.id)).toEqual( - [segmentOfMask(second.id), segmentOfMask(first.id)] - ); + expect( + segments() + .segmentList('img-1') + .map((segment) => segment.id) + ).toEqual([segmentOfMask(second.id), segmentOfMask(first.id)]); expect(store().boundMaskIds('img-1')).toEqual([first.id, second.id]); }); @@ -285,8 +290,14 @@ describe('segmentation store', () => { await seatSpecImage('img-1'); const { id: segmentationId } = store().ensureSegmentationForImage('img-1'); - const first = store().createMask(segmentationId, mintSegment('Tumor')); - const second = store().createMask(segmentationId, mintSegment('Node')); + const first = store().createMask( + segmentationId, + mintSegment('img-1', 'Tumor') + ); + const second = store().createMask( + segmentationId, + mintSegment('img-1', 'Node') + ); segments().updateSegment(first.segmentId, { visible: false }); @@ -357,7 +368,7 @@ describe('segmentation store', () => { await seatSpecImage('img-1'); const { id: segmentationId } = store().ensureSegmentationForImage('img-1'); - const segment = store().createMask(segmentationId, mintSegment()); + const segment = store().createMask(segmentationId, mintSegment('img-1')); deleteSegmentOf(segment.id); @@ -489,7 +500,7 @@ describe('segmentation store', () => { it('creates nothing when a segment is selected', async () => { const { one, two } = await seatTwoImages(); - segments().addSegment({ name: 'Tumor' }); + segments().addSegment('img-1', { name: 'Tumor' }); expect(store().segmentations[one].order).toEqual([]); expect(store().segmentations[two].order).toEqual([]); @@ -500,7 +511,7 @@ describe('segmentation store', () => { await seatSpecImage('img-1'); await seatSpecImage('img-2', 'PET'); const one = store().ensureSegmentationForImage('img-1').id; - segments().addSegment({ name: 'Tumor' }); + segments().addSegment('img-1', { name: 'Tumor' }); expect(store().getSegmentationForImage('img-2')).toBeFalsy(); expect(Object.keys(store().segmentations)).toEqual([one]); @@ -508,7 +519,7 @@ describe('segmentation store', () => { it('resolves the selected segment to this image record', async () => { const { one } = await seatTwoImages(); - const segmentId = segments().addSegment({ name: 'Tumor' }); + const segmentId = segments().addSegment('img-1', { name: 'Tumor' }); const record = maskOn('img-1', segmentId); const target = store().resolveEditTarget('img-1'); @@ -517,15 +528,15 @@ describe('segmentation store', () => { expect(store().segmentations[one].order).toEqual([record.id]); }); - it('gives the second image its own record for the selected segment', async () => { + it('gives the second image its own segment and record', async () => { const { one, two } = await seatTwoImages(); - const segmentId = segments().addSegment({ name: 'Tumor' }); + const segmentId = segments().addSegment('img-1', { name: 'Tumor' }); const source = store().resolveEditTarget('img-1'); const target = store().resolveEditTarget('img-2'); expect(target).not.toBe(source); - expect(maskOf(target).segmentId).toBe(segmentId); + expect(maskOf(target).segmentId).not.toBe(segmentId); expect(maskOf(source).segmentId).toBe(segmentId); expect(store().segmentations[two].order).toEqual([target]); expect(store().segmentations[one].order).toEqual([source]); @@ -533,7 +544,7 @@ describe('segmentation store', () => { it('reuses each image record for the rest of the session', async () => { const { one, two } = await seatTwoImages(); - segments().addSegment({ name: 'Tumor' }); + segments().addSegment('img-1', { name: 'Tumor' }); const source = store().resolveEditTarget('img-1'); const first = store().resolveEditTarget('img-2'); @@ -546,9 +557,9 @@ describe('segmentation store', () => { expect(store().segmentations[one].order).toEqual([source]); }); - it('renames every image record at once, because they share one segment', async () => { + it('renames only the owning image segment', async () => { await seatTwoImages(); - const segmentId = segments().addSegment({ name: 'Tumor' }); + const segmentId = segments().addSegment('img-1', { name: 'Tumor' }); const source = store().resolveEditTarget('img-1'); const target = store().resolveEditTarget('img-2'); @@ -558,13 +569,13 @@ describe('segmentation store', () => { 'Lesion' ); expect(segments().appearanceOf(maskOf(target).segmentId).name).toBe( - 'Lesion' + 'Segment 1' ); }); it('never binds a second record of one segment to an image', async () => { const { two } = await seatTwoImages(); - const segmentId = segments().addSegment({ name: 'Tumor' }); + const segmentId = segments().addSegment('img-2', { name: 'Tumor' }); const existing = maskOn('img-2', segmentId); const target = store().resolveEditTarget('img-2'); @@ -575,7 +586,7 @@ describe('segmentation store', () => { it('creates a record again when its segment was deleted', async () => { const { two } = await seatTwoImages(); - const tumor = segments().addSegment({ name: 'Tumor' }); + const tumor = segments().addSegment('img-2', { name: 'Tumor' }); const first = store().resolveEditTarget('img-2'); deleteSegmentOf(first); @@ -589,8 +600,8 @@ describe('segmentation store', () => { it('follows the selection to another segment', async () => { const { one } = await seatTwoImages(); - const tumor = segments().addSegment({ name: 'Tumor' }); - const node = segments().addSegment({ name: 'Node' }); + const tumor = segments().addSegment('img-1', { name: 'Tumor' }); + const node = segments().addSegment('img-1', { name: 'Node' }); segments().selectSegment(tumor); const forTumor = store().resolveEditTarget('img-1'); @@ -614,14 +625,14 @@ describe('segmentation store', () => { const appearance = segments().appearanceOf(segment.segmentId); expect(appearance.visible).toBe(true); expect(appearance.locked).toBe(false); - expect(segments().selectedSegmentId.value).toBe(segment.segmentId); + expect(segments().selectedSegmentId('img-1')).toBe(segment.segmentId); expect(segments().appearanceOf(segment.segmentId).name).toBe('Segment 1'); }); it('falls back to the first segment and reuses the freed default name', async () => { await seatSpecImage('img-1'); const first = store().resolveEditTarget('img-1'); - const selected = segments().addSegment(); + const selected = segments().addSegment('img-1'); segments().deleteSegment(selected); const replacement = store().resolveEditTarget('img-1'); @@ -631,7 +642,7 @@ describe('segmentation store', () => { 'Segment 1' ); // The next minted segment takes the next free default name. - expect(segments().appearanceOf(segments().addSegment()).name).toBe( + expect(segments().appearanceOf(segments().addSegment('img-1')).name).toBe( 'Segment 2' ); }); @@ -659,7 +670,7 @@ describe('segmentation store', () => { it('finds no target for a segment this image has no record for', async () => { await seatSpecImage('img-1'); - segments().addSegment({ name: 'Tumor' }); + segments().addSegment('img-1', { name: 'Tumor' }); expect(store().findEditTarget('img-1')).toBeUndefined(); expect(store().getSegmentationForImage('img-1')).toBeFalsy(); @@ -667,23 +678,23 @@ describe('segmentation store', () => { it('targets the first segment while none has been chosen', async () => { const { one } = await seatTwoImages(); - const first = store().createMask(one, mintSegment('Tumor')); - store().createMask(one, mintSegment('Node')); + const first = store().createMask(one, mintSegment('img-1', 'Tumor')); + store().createMask(one, mintSegment('img-1', 'Node')); expect(store().findEditTarget('img-1')).toBe(first.id); }); it('asks the lock of the segment it targets', async () => { const { one } = await seatTwoImages(); - const first = store().createMask(one, mintSegment('Tumor')); - const second = store().createMask(one, mintSegment('Node')); + const first = store().createMask(one, mintSegment('img-1', 'Tumor')); + const second = store().createMask(one, mintSegment('img-1', 'Node')); segments().updateSegment(first.segmentId, { locked: true }); - expect(store().editTargetLocked()).toBe(true); + expect(store().editTargetLocked('img-1')).toBe(true); segments().selectSegment(second.segmentId); - expect(store().editTargetLocked()).toBe(false); + expect(store().editTargetLocked('img-1')).toBe(false); expect(store().findEditTarget('img-1')).toBe(second.id); }); }); diff --git a/src/segmentation/__tests__/sharedSegmentAcrossImages.spec.ts b/src/segmentation/__tests__/sharedSegmentAcrossImages.spec.ts index 57ead4d71..1a4cbd397 100644 --- a/src/segmentation/__tests__/sharedSegmentAcrossImages.spec.ts +++ b/src/segmentation/__tests__/sharedSegmentAcrossImages.spec.ts @@ -15,12 +15,6 @@ import { store, } from '@/src/segmentation/__tests__/segmentMaskFixtures'; -// --------------------------------------------------------------------------- -// One segment, every image. The registry is independent of the viewed image, so -// switching images keeps the selection and an edit takes this image's mask -// for the selected segment. Nothing is cloned and no identity is matched by name. -// --------------------------------------------------------------------------- - const segments = () => useSegmentStore().segments; const viewImage = async (id: string) => { @@ -31,9 +25,6 @@ const viewImage = async (id: string) => { const recordIdsOf = (imageId: string) => store().getSegmentationForImage(imageId)?.order ?? []; -const nameOf = (maskId: string) => - segments().appearanceOf(store().getMask(maskId).segmentId).name; - const SQUARE: Vector3[] = [ [0, 0, 0], [2, 0, 0], @@ -50,95 +41,103 @@ const rasterizeOn = (imageId: string, segmentId: string) => viewAxis: 'Axial', }); -describe('one segment across images', () => { +const copiedTumor = async () => { + const source = segments().addSegment('img-1', { name: 'Tumor' }); + await viewImage('img-2'); + const copied = segments().findSegmentByName('img-2', 'Tumor')!.id; + return { source, copied }; +}; + +describe('independent segments across images', () => { beforeEach(async () => { setActivePinia(createPinia()); await seatSpecImage('img-1'); await seatSpecImage('img-2'); + await viewImage('img-1'); }); - it('keeps the selection when the viewed image changes', async () => { - const segmentId = segments().addSegment({ name: 'Tumor' }); + it('offers copied definitions and selection without allocating masks', async () => { + const { source, copied } = await copiedTumor(); - await viewImage('img-2'); - - expect(segments().selectedSegmentId.value).toBe(segmentId); + expect(copied).not.toBe(source); + expect(segments().getSegment(copied)?.color).toEqual( + segments().getSegment(source)?.color + ); + expect(segments().selectedSegmentId('img-2')).toBe(copied); expect(recordIdsOf('img-2')).toEqual([]); - }); - it('hides and locks the segment on every image at once', () => { - const segmentId = segments().addSegment({ name: 'Tumor' }); - const first = store().resolveEditTarget('img-1'); - const second = store().resolveEditTarget('img-2'); - - segments().updateSegment(segmentId, { visible: false, locked: true }); - - // Both describe the thing, so a record cannot disagree with its segment. - [first, second].forEach((maskId) => { - const appearance = segments().appearanceOf( - store().getMask(maskId).segmentId - ); - expect(appearance.visible).toBe(false); - expect(appearance.locked).toBe(true); - expect(store().isLocked(maskId)).toBe(true); - }); - }); + await viewImage('img-1'); - it('deletes the segment on every image at once', () => { - const segmentId = segments().addSegment({ name: 'Tumor' }); - store().resolveEditTarget('img-1'); - store().resolveEditTarget('img-2'); + expect(segments().selectedSegmentId('img-1')).toBe(source); + }); - segments().deleteSegment(segmentId); + it('changes visibility, lock and name only on the edited image', async () => { + const { source, copied } = await copiedTumor(); + const first = store().resolveEditTarget('img-1', source); + const second = store().resolveEditTarget('img-2', copied); - expect(recordIdsOf('img-1')).toEqual([]); - expect(recordIdsOf('img-2')).toEqual([]); - }); -}); + segments().updateSegment(copied, { + name: 'Lesion', + visible: false, + locked: true, + }); -describe('placing and rasterizing on another image', () => { - beforeEach(async () => { - setActivePinia(createPinia()); - await seatSpecImage('img-1'); - await seatSpecImage('img-2'); - await viewImage('img-1'); + expect(segments().appearanceOf(source)).toMatchObject({ + name: 'Tumor', + visible: true, + locked: false, + }); + expect(store().isLocked(first)).toBe(false); + expect(store().isLocked(second)).toBe(true); }); - it('places an annotation on another image against the selected segment', async () => { - const segmentId = segments().addSegment({ name: 'Tumor' }); + it('deletes only the selected image segment and its masks and shapes', async () => { + const { source, copied } = await copiedTumor(); + const first = store().resolveEditTarget('img-1', source); + store().resolveEditTarget('img-2', copied); const polygons = usePolygonStore(); + const kept = polygons.addTool({ imageID: 'img-1', segmentId: source }); + const removed = polygons.addTool({ imageID: 'img-2', segmentId: copied }); - await viewImage('img-2'); - const toolId = polygons.addTool({ imageID: 'img-2', placing: false }); + segments().deleteSegment(copied); - expect(polygons.toolByID[toolId].segmentId).toBe(segmentId); - expect(segments().selectedSegmentId.value).toBe(segmentId); + expect(recordIdsOf('img-1')).toEqual([first]); + expect(recordIdsOf('img-2')).toEqual([]); + expect(polygons.toolByID[kept]).toBeDefined(); + expect(polygons.toolByID[removed]).toBeUndefined(); + await viewImage('img-2'); + expect(segments().segmentList('img-2')).toEqual([]); }); - it("rasterizes into this image's record for the polygon's segment", async () => { - const segmentId = segments().addSegment({ name: 'Tumor' }); - segments().updateSegment(segmentId, { name: 'Lesion' }); - const origin = store().resolveEditTarget('img-1'); + it('places an annotation against the destination image selection', async () => { + const { source, copied } = await copiedTumor(); + const polygons = usePolygonStore(); - await viewImage('img-2'); - const result = rasterizeOn('img-2', segmentId); + const toolId = polygons.addTool({ imageID: 'img-2', placing: false }); - expect(result.segmentId).toBe(segmentId); - expect(nameOf(result.maskId!)).toBe('Lesion'); - expect(recordIdsOf('img-2')).toEqual([result.maskId]); - expect(recordIdsOf('img-1')).toEqual([origin]); + expect(polygons.toolByID[toolId].segmentId).toBe(copied); + expect(polygons.toolByID[toolId].segmentId).not.toBe(source); }); - it('never rasterizes into the other image mask', async () => { - const segmentId = segments().addSegment({ name: 'Tumor' }); - const origin = store().resolveEditTarget('img-1'); + it('rasterizes the destination copy without changing source voxels', async () => { + const { source, copied } = await copiedTumor(); + const origin = store().resolveEditTarget('img-1', source); store().ensureLabelmapBinding(origin); - await viewImage('img-2'); - const { maskId } = rasterizeOn('img-2', segmentId); + const { maskId } = rasterizeOn('img-2', copied); expect(maskId).not.toBe(origin); expect(maskValueAt(maskId!, [1, 1, 0])).toBe(SEGMENT_VALUE); expect(markedVoxels(origin)).toEqual([]); + expect(recordIdsOf('img-1')).toEqual([origin]); + }); + + it('refuses a segment owned by a different image before creating a mask', async () => { + const { source } = await copiedTumor(); + + expect(() => rasterizeOn('img-2', source)).toThrow( + 'Segment belongs to another image' + ); + expect(recordIdsOf('img-2')).toEqual([]); }); }); diff --git a/src/segmentation/components/SegmentAssignmentList.vue b/src/segmentation/components/SegmentAssignmentList.vue index caf4c7d09..ec69e611d 100644 --- a/src/segmentation/components/SegmentAssignmentList.vue +++ b/src/segmentation/components/SegmentAssignmentList.vue @@ -3,13 +3,13 @@ import { computed } from 'vue'; import ColorDot from '@/src/components/ColorDot.vue'; import { useSegmentStore } from '@/src/segmentation/segments'; -defineProps<{ segmentId?: string }>(); +const props = defineProps<{ imageId: string; segmentId?: string }>(); defineEmits<{ select: [segmentId: string] }>(); const registry = useSegmentStore().segments; const items = computed(() => - registry.segmentList.value.map(({ id }) => { + registry.segmentList(props.imageId).map(({ id }) => { const { displayName, cssColor } = registry.appearanceOf(id); return { id, name: displayName, cssColor }; }) diff --git a/src/segmentation/components/SegmentEditor.vue b/src/segmentation/components/SegmentEditor.vue index acf84a523..c8950391b 100644 --- a/src/segmentation/components/SegmentEditor.vue +++ b/src/segmentation/components/SegmentEditor.vue @@ -113,13 +113,13 @@ const onDelete = () => { diff --git a/src/segmentation/components/SegmentList.vue b/src/segmentation/components/SegmentList.vue index cdc82c805..469c13e4a 100644 --- a/src/segmentation/components/SegmentList.vue +++ b/src/segmentation/components/SegmentList.vue @@ -1,5 +1,5 @@