diff --git a/docs/configuration_file.md b/docs/configuration_file.md index 2f574e207..063035c94 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,44 @@ 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. On a new segment, omitted fields use the app defaults. +On an existing segment matched by name, omitted fields preserve its underlying +appearance. -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. +Opening a saved session replaces these starting defaults when the session includes +its own defaults. An explicitly empty list clears them; a session without saved +defaults leaves them unchanged. Loading a configuration afterward sets the starting +defaults again. Each image's saved segment set is restored independently of these +defaults. -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 only unreferenced segments that +the configuration created. Segments matched by name and segments that hold masks +or shapes remain. ### 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..68e90c5a0 100644 --- a/docs/toolbar.md +++ b/docs/toolbar.md @@ -14,19 +14,23 @@ 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. Until edited, that copy can be replaced by a +session, companion segmentation or configuration loaded with data. 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 +99,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..bf20423ab --- /dev/null +++ b/src/actions/__tests__/loadImageSegmentSets.spec.ts @@ -0,0 +1,484 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { enableAutoUnmount, flushPromises, mount } from '@vue/test-utils'; +import { nextTick } from 'vue'; +import { createVuetify } from 'vuetify'; +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 MessageCenter from '@/src/components/MessageCenter.vue'; +import SegmentAssignmentList from '@/src/segmentation/components/SegmentAssignmentList.vue'; +import { planLabelmapExport } from '@/src/segmentation/io/composition'; +import { writeLabelmapParts } from '@/src/segmentation/io/export'; +import { + activateAppPinia, + 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(); + // Read VTI/STL geometry in process and supply synthetic segmentation metadata. + 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 = async (imageId: string) => { + strokeAt(imageId, [1, 1, 0]); + const names: Array = []; + await writeLabelmapParts( + { parentId: imageId, ...planLabelmapExport(imageId) }, + 'painted', + 'vti', + { + deliver: () => {}, + write: async (_format, image, segments) => { + const label = image.getPointData().getScalars().getData()[5]; + if (label) + names.push(segments.find(({ value }) => value === label)?.name); + return ''; + }, + } + ); + expect(names).toHaveLength(1); + return names[0]; +}; +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'); + useViewStore().setDataForAllViews(completed); + await nextTick(); + expect(picker(completed)()).toEqual(['Liver']); + expect(await 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(await 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(await 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(await 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 notifications = mount(MessageCenter, { + global: { plugins: [createVuetify()] }, + }); + 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']); + const errors = notifications.findAll('.error-message'); + expect(errors).toHaveLength(1); + await errors[0].trigger('click'); + expect(notifications.get('.details').text()).toContain( + '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); + 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(await 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..73b1c1192 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,19 @@ const moveBy = async (id: string, offset: number, event: KeyboardEvent) => {
    - -
    - mdi-plus - {{ createText }} -
    -     + + +
    @@ -205,13 +208,19 @@ const moveBy = async (id: string, offset: number, event: KeyboardEvent) => { } .v-list-item.item-row, -.v-list-item.create-row { +.create-row-content { border: 1px solid transparent; border-radius: 4px; /* The default inset would dwarf the gaps between the row's parts. */ padding-inline: 4px; } +.create-row-content { + display: flex; + align-items: center; + min-height: 40px; +} + .item-row.v-list-item--active { background-color: rgb(var(--v-theme-selection-bg-color)); border-color: rgb(var(--v-theme-selection-border-color)); @@ -231,6 +240,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..bd04c523c --- /dev/null +++ b/src/components/__tests__/EditableItemList.spec.ts @@ -0,0 +1,37 @@ +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.find('[role="listitem"]').exists()).toBe(false); + 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..66385e0fc 100644 --- a/src/components/tools/paint/PaintWidget2D.vue +++ b/src/components/tools/paint/PaintWidget2D.vue @@ -70,11 +70,11 @@ export default defineComponent({ // --- widget representation config --- // - const { segments } = useSegmentStore(); + const segmentStore = useSegmentStore(); + const pointerInView = ref(false); watchEffect(() => { - const [r, g, b] = segments.appearanceOf( - segments.selectedSegmentId.value - ).color; + if (!pointerInView.value) return; + const [r, g, b] = segmentStore.previewSegmentColor(imageId.value); widgetState.getBrush().setColor3([r, g, b]); }); @@ -151,7 +151,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 +172,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..a73d73767 100644 --- a/src/enface/__tests__/restoreSelection.spec.ts +++ b/src/enface/__tests__/restoreSelection.spec.ts @@ -8,7 +8,11 @@ import { CurrentImageInjectionKey } from '@/src/composables/useCurrentImage'; import { ManifestSchema, type StateFile } from '@/src/io/state-file/schema'; import { useViewConfigStore } from '@/src/store/view-configs'; import { useImageStatsStore } from '@/src/store/image-stats'; -import { useEnFaceViewStore } from '@/src/enface/store'; +import { + defaultEnFaceViewConfig, + useEnFaceViewStore, +} from '@/src/enface/store'; +import { completeStateFileRestore } from '@/src/io/import/processors/restoreStateFile'; import { useEnFaceInput } from '@/src/enface/useEnFaceInput'; import { useEnFaceOccupiedDepth } from '@/src/enface/useEnFaceOccupiedDepth'; import EnFaceControls from '@/src/enface/EnFaceControls.vue'; @@ -21,6 +25,7 @@ import { } from '@/src/segmentation/measurements/occupiedDepth'; import { activateAppPinia, + manifestForImages, makeImage, seatImage, } from '@/src/segmentation/__tests__/segmentMaskFixtures'; @@ -42,6 +47,52 @@ const measureInProcess = () => ({ release: () => {}, }); +it.each(['foreign', 'missing'])( + 'drops a %s segment selection when restoring an en-face view', + async (segmentId) => { + await seatImage('a'); + await seatImage('b'); + const manifest = ManifestSchema.parse( + manifestForImages(['saved-a', 'saved-b'], { + segments: [ + { id: 'foreign', name: 'Other image', color: [255, 0, 0, 255] }, + ], + imageSegmentSets: [ + { imageId: 'saved-a', segmentIds: ['foreign'] }, + { imageId: 'saved-b', segmentIds: [] }, + ], + viewByID: { + view: { + id: 'view', + type: 'EnFace', + name: 'En face', + config: { + 'saved-b': { + enFace: { ...defaultEnFaceViewConfig(), segmentId }, + }, + }, + }, + }, + }) + ); + await completeStateFileRestore(manifest, [], { + 'saved-a': 'a', + 'saved-b': 'b', + }); + + const saved: StateFile = { + zip: new JSZip(), + manifest: manifestForImages(['a', 'b'], { + viewByID: { view: { id: 'view', type: 'EnFace', name: 'En face' } }, + }), + }; + useEnFaceViewStore().serialize(saved); + expect( + saved.manifest.viewByID!.view.config!.b.enFace?.segmentId + ).toBeNull(); + } +); + it('keeps the projection ready and restores the non-first segment after deferred mask decoding', async () => { useImageStatsStore().stats.oct = { scalarMin: 0, @@ -53,7 +104,7 @@ it('keeps the projection ready and restores the non-first segment after deferred spacing: [0.01, 0.01, 0.01], }); const manifest = ManifestSchema.parse({ - version: '7.0.0', + version: '8.0.0', datasets: [], dataSources: [], segments: ['first', 'chosen'].map((id, index) => ({ @@ -63,6 +114,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 +156,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__/store.spec.ts b/src/enface/__tests__/store.spec.ts index d0515c772..c402c0b0b 100644 --- a/src/enface/__tests__/store.spec.ts +++ b/src/enface/__tests__/store.spec.ts @@ -7,6 +7,8 @@ import { parseNamedLayouts } from '@/src/utils/layoutParsing'; import { computeEffectiveView } from '@/src/core/views/effectiveView'; import { useViewConfigStore } from '@/src/store/view-configs'; import { useWindowingStore } from '@/src/store/view-configs/windowing'; +import { useSegmentStore } from '@/src/segmentation/segments'; +import { manifestForImages } from '@/src/segmentation/__tests__/segmentMaskFixtures'; beforeEach(() => setActivePinia(createPinia())); @@ -126,6 +128,9 @@ describe('En face view settings', () => { it('remaps saved segment ids and drops unknown ones on restore', () => { const store = useEnFaceViewStore(); + const chosen = useSegmentStore().segments.addSegment('kept', { + name: 'Layer', + }); const saved = (segmentId: string) => ({ enFace: { axis: 1 as const, @@ -140,10 +145,18 @@ describe('En face view settings', () => { store.deserialize( 'view', { kept: saved('saved-layer'), dropped: saved('missing-layer') }, - { 'saved-layer': 'chosen' } + { 'saved-layer': chosen } ); - expect(store.configFor('view', 'kept').segmentId).toBe('chosen'); - expect(store.configFor('view', 'dropped').segmentId).toBeNull(); + const state: StateFile = { + zip: new JSZip(), + manifest: manifestForImages(['kept', 'dropped'], { + viewByID: { view: { id: 'view', type: 'EnFace', name: 'En face' } }, + }), + }; + store.serialize(state); + const config = state.manifest.viewByID!.view.config!; + expect(config.kept.enFace?.segmentId).toBe(chosen); + expect(config.dropped.enFace?.segmentId).toBeNull(); }); it('resolves custom enface layouts without treating the projection as a slice', () => { 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/enface/store.ts b/src/enface/store.ts index 1a5884955..7ad7ee980 100644 --- a/src/enface/store.ts +++ b/src/enface/store.ts @@ -9,6 +9,7 @@ import { import type { ViewConfig } from '@/src/io/state-file/schema'; import type { SlabAxis, SlabMethod } from '@/src/core/slab/types'; import type { Maybe } from '@/src/types'; +import { useSegmentStore } from '@/src/segmentation/segments'; export type EnFaceViewConfig = { axis: SlabAxis; @@ -101,11 +102,15 @@ export const useEnFaceViewStore = defineStore('enface-view', () => { config: Record, segmentIdMap: Record ) { + const { segments } = useSegmentStore(); Object.entries(config).forEach(([dataID, viewConfig]) => { const saved = viewConfig.enFace; if (!saved) return; + const segmentId = saved.segmentId && segmentIdMap[saved.segmentId]; Object.assign(configFor(viewID, dataID), saved, { - segmentId: (saved.segmentId && segmentIdMap[saved.segmentId]) || null, + segmentId: segments.belongsToImage(dataID, segmentId) + ? segmentId + : null, }); }); } diff --git a/src/io/import/__tests__/configJson.spec.ts b/src/io/import/__tests__/configJson.spec.ts index 48409d6a9..c6843a359 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 + ['image'], + { addError } ); 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 }), ['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__/missingArchiveMember.spec.ts b/src/io/import/__tests__/missingArchiveMember.spec.ts index b16dab9f1..783af2be0 100644 --- a/src/io/import/__tests__/missingArchiveMember.spec.ts +++ b/src/io/import/__tests__/missingArchiveMember.spec.ts @@ -64,7 +64,7 @@ describe('restore with a partially missing dataset archive', () => { ManifestSchema.parse(manifest), [], { 'ds-ct': 'store-ct' }, // the dataset resolved from its surviving files - [{ stateID: 'ds-ct', path: 'data/20/b.dcm' }] + { missingFiles: [{ stateID: 'ds-ct', path: 'data/20/b.dcm' }] } ); const warning = useMessageStore().messages.find( 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..4d59870d1 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(); @@ -29,7 +32,104 @@ describe('restoring a manifest whose segments repeat an id', () => { await restore(manifest, [], { parent: 'parent' }); expect(addWarning).toHaveBeenCalledExactlyOnceWith( 'Some scene content could not be restored', - { details: '- segment: Spleen (repeats the id of an earlier segment)' } + { + details: + '- segment: Spleen (catalog row 2; image: parent.nrrd; 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 migrated = migrateManifest( + JSON.stringify( + manifestForImages(['a', 'b'], { + version: '7.0.0', + segments: [segment('s1', 'Liver'), segment('s1', 'Spleen')], + }) + ) + ); + const addWarning = vi.fn(); + + await createStateFileRestorer({ addWarning })( + ManifestSchema.parse(migrated.manifest), + [], + { a: 'a', b: 'b' }, + { migration: migrated.migration } + ); + + expect(addWarning).toHaveBeenCalledExactlyOnceWith( + 'Some scene content could not be restored', + { + details: + '- segment: Spleen (catalog row 2; images: a.nrrd, b.nrrd; repeats the id of an earlier segment)', + } + ); + }); + + it.each(['7.0.0', '8.0.0'])( + 'reports distinct same-name rows separately in version %s', + async (version) => { + await seatImage('a'); + await seatImage('b'); + const migrated = migrateManifest( + JSON.stringify( + manifestForImages(['a', 'b'], { + version, + segments: [ + segment('first', 'First'), + segment('first', 'Repeated'), + segment('second', 'Second'), + segment('second', 'Repeated'), + ], + imageSegmentSets: [ + { imageId: 'a', segmentIds: ['first'] }, + { imageId: 'b', segmentIds: ['second'] }, + ], + }) + ) + ); + const addWarning = vi.fn(); + await createStateFileRestorer({ addWarning })( + ManifestSchema.parse(migrated.manifest), + [], + { a: 'a', b: 'b' }, + { migration: migrated.migration } + ); + + const images = (imageId: string) => + version === '7.0.0' + ? 'images: a.nrrd, b.nrrd' + : `image: ${imageId}.nrrd`; + expect(addWarning).toHaveBeenCalledExactlyOnceWith( + 'Some scene content could not be restored', + { + details: [ + `- segment: Repeated (catalog row 2; ${images('a')}; repeats the id of an earlier segment)`, + `- segment: Repeated (catalog row 4; ${images('b')}; repeats the id of an earlier segment)`, + ].join('\n'), + } + ); + } + ); + + 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__/restoreCoveredErrors.spec.ts b/src/io/import/__tests__/restoreCoveredErrors.spec.ts index a2cc43d30..39af2fad6 100644 --- a/src/io/import/__tests__/restoreCoveredErrors.spec.ts +++ b/src/io/import/__tests__/restoreCoveredErrors.spec.ts @@ -118,8 +118,10 @@ describe('importDataSources — restore-covered failures return as ok, not error await openSession(restore); - const [, , , , failedLeaves] = restore.completions[0]; - expect(failedLeaves).toEqual([{ stateID: 'ds-a', name: 'ds-a.bin' }]); + const options = restore.completions[0][3]; + expect(options?.failedLeaves).toEqual([ + { stateID: 'ds-a', name: 'ds-a.bin' }, + ]); }); it('keeps a leaf failure as an error when the restore never completed', async () => { diff --git a/src/io/import/__tests__/restoreStateIdCollision.spec.ts b/src/io/import/__tests__/restoreStateIdCollision.spec.ts index 6f7f641cb..9ee6d24ef 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, }); @@ -191,7 +192,8 @@ describe('restore stateID namespaces (collision)', () => { parent: BASE_STORE_ID, }); expect(views.setDataForAllViews).toHaveBeenCalledExactlyOnceWith( - BASE_STORE_ID + BASE_STORE_ID, + undefined ); }); diff --git a/src/io/import/common.ts b/src/io/import/common.ts index 801ba0c84..b29ce5174 100644 --- a/src/io/import/common.ts +++ b/src/io/import/common.ts @@ -6,6 +6,7 @@ import { Config } from '@/src/io/import/configJson'; import { ChainHandler } from '@/src/utils/evaluateChain'; import type { Manifest } from '@/src/io/state-file/schema'; import type { FileEntry } from '@/src/io/types'; +import type { ManifestMigration } from '@/src/io/state-file/migrations'; export type LoadableResult = { type: 'data'; @@ -42,6 +43,7 @@ export type StateFileSetupResult = { type: 'stateFileSetup'; dataSources: DataSource[]; manifest: Manifest; + migration?: ManifestMigration; stateFiles: FileEntry[]; // Archive members the state file recorded but does not contain, keyed by the // dataset they belong to — a dataset that still resolves from its surviving diff --git a/src/io/import/configJson.ts b/src/io/import/configJson.ts index 1e97de55a..14df00a93 100644 --- a/src/io/import/configJson.ts +++ b/src/io/import/configJson.ts @@ -230,12 +230,21 @@ const reportUnparseableColors = ( // clears what an earlier config contributed to it. const applySegments = ( manifest: Config, - addError: (message: string) => void + imageIds: string[], + { + addError, + replaceBorrowed, + }: { + addError: (message: string) => void; + replaceBorrowed: boolean; + } ) => { const configured = manifest.segments; if (configured === undefined) return; if (configured) reportUnparseableColors(configured, addError); - useSegmentStore().segments.replaceConfigSegments(configured); + useSegmentStore().replaceConfigSegments(configured, imageIds, { + replaceBorrowed, + }); }; const applyLayout = (manifest: Config) => { @@ -332,8 +341,14 @@ export const applyPreStateConfig = async (manifest: Config) => { export const applyPostStateConfig = ( manifest: Config, - addError: (message: string) => void = (message) => - useMessageStore().addError(message) + imageIds: string[], + { + addError = (message: string) => useMessageStore().addError(message), + replaceBorrowed = false, + }: { + addError?: (message: string) => void; + replaceBorrowed?: boolean; + } = {} ) => { - applySegments(manifest, addError); + applySegments(manifest, imageIds, { addError, replaceBorrowed }); }; diff --git a/src/io/import/importDataSources.ts b/src/io/import/importDataSources.ts index 378d170eb..bdb5f8fac 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, @@ -28,9 +29,12 @@ import handleRemoteManifest from '@/src/io/import/processors/remoteManifest'; import { restoreStateFile, completeStateFileRestore, + type RestoreOptions, } 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 +51,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 +74,13 @@ function isSelectable(result: ImportResult): result is LoadableVolumeResult { } const applyConfigsPostState = ( - results: Array + results: Array, + imageIds: string[], + replaceBorrowed: boolean ): (ConfigResult | ErrorResult)[] => results.map((result) => { try { - applyPostStateConfig(result.config); + applyPostStateConfig(result.config, imageIds, { replaceBorrowed }); return result; } catch (err) { return asErrorResult(ensureError(err), result.dataSource); @@ -120,7 +127,13 @@ async function importDicomChunkSources(sources: ChunkSource[]) { ); } -type ImportPolicy = 'application' | 'volume-data'; +type ImportPolicy = + | { + kind: 'application'; + restore: RestoreProcessors; + standaloneImageIds: string[]; + } + | { kind: 'volume-data' }; /** * The two halves of a state-file restore: the handler that reads the manifest @@ -132,15 +145,84 @@ export type RestoreProcessors = { completeStateFileRestore: typeof completeStateFileRestore; }; +const hasDataIntent = (result: ImportResult) => { + if ( + result.type === 'config' || + (result.type === 'error' && result.error instanceof InvalidConfigError) + ) + return false; + if (result.type !== 'intermediate') return true; + return ( + result.dataSources.length === 0 || + result.dataSources.some((source) => source.type === 'chunk') + ); +}; + export const appRestoreProcessors = (): RestoreProcessors => ({ restoreStateFile, completeStateFileRestore, }); +async function completeStateFileSetups( + stateFileSetups: StateFileSetupResult[], + restore: RestoreProcessors, + loadableResults: LoadableResult[], + failedLeaves: RestoreOptions['failedLeaves'] +) { + const stateIDToStoreID = buildStateIDToStoreID(loadableResults); + // Leaf stateIDs covered by a consolidated notice that actually ran: only + // their errors may be suppressed by the import pipeline. + const reportedStateIDs = new Set(); + for (const setup of stateFileSetups) { + try { + await restore.completeStateFileRestore( + setup.manifest, + setup.stateFiles, + stateIDToStoreID, + { + missingFiles: setup.missingFiles, + failedLeaves, + migration: setup.migration, + } + ); + setup.dataSources.forEach((src) => { + findStateFileLeaves(src).forEach((leaf) => + reportedStateIDs.add(leaf.stateID) + ); + }); + setup.missingFiles.forEach(({ stateID }) => + reportedStateIDs.add(stateID) + ); + } catch (err) { + // Auto-degrade to an ephemeral open: a mid-restore throw leaves the + // already-loaded bases as plain datasets and the session's attached set + // stays empty, so a later save prunes every launch-composition entry. + // Restore steps already applied (layout, view bindings) are not rolled + // back. One notice here; this setup's leaf errors stay unflagged so the + // generic load-error dialog still names them. + useMessageStore().addWarning( + 'Could not restore the saved session; opened its images instead', + { details: ensureError(err).message } + ); + } + } + return reportedStateIDs; +} + +// Config imported with data targets its loaded images; standalone config keeps +// the image whose view started the import. +const configTargetImageIds = ( + importedData: boolean, + loadables: readonly LoadableResult[], + standaloneImageIds: string[] +) => + importedData + ? loadables.filter(isVolumeResult).map(({ dataID }) => dataID) + : standaloneImageIds; + async function importDataSourcesWithPolicy( dataSources: DataSource[], - policy: ImportPolicy, - restore: RestoreProcessors = appRestoreProcessors() + policy: ImportPolicy ): Promise { const cleanupHandlers: Array<() => void> = []; const onCleanup = (fn: () => void) => { @@ -154,13 +236,13 @@ async function importDataSourcesWithPolicy( fetchFileCache: new Map(), onCleanup, importDataSources: (sources: DataSource[]) => - importDataSourcesWithPolicy(sources, policy, restore), + importDataSourcesWithPolicy(sources, policy), }; // State files first: a manifest's top-level `segments` is also a config key. const applicationHandlers = - policy === 'application' - ? [restore.restoreStateFile, handleConfig, handleRemoteManifest] + policy.kind === 'application' + ? [policy.restore.restoreStateFile, handleConfig, handleRemoteManifest] : []; const handlers = [ @@ -193,6 +275,7 @@ async function importDataSourcesWithPolicy( const configResults: ConfigResult[] = []; const stateFileSetups: StateFileSetupResult[] = []; const results: ImportDataSourcesResult[] = []; + let importedData = false; let queue = dataSources.map((src) => ({ promise: evaluateChain(src, handlers, importContext), @@ -203,6 +286,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)); + importedData ||= hasDataIntent(result); queue = queue.filter((_, i) => i !== index); switch (result.type) { @@ -278,42 +362,27 @@ async function importDataSourcesWithPolicy( })) ); - const stateIDToStoreID = buildStateIDToStoreID(loadableResults); - // Leaf stateIDs covered by a consolidated notice that actually ran — only - // their errors may be suppressed below. - const reportedStateIDs = new Set(); - for (const setup of stateFileSetups) { - try { - await restore.completeStateFileRestore( - setup.manifest, - setup.stateFiles, - stateIDToStoreID, - setup.missingFiles, - failedLeaves - ); - setup.dataSources.forEach((src) => { - findStateFileLeaves(src).forEach((leaf) => - reportedStateIDs.add(leaf.stateID) - ); - }); - setup.missingFiles.forEach(({ stateID }) => - reportedStateIDs.add(stateID) - ); - } catch (err) { - // Auto-degrade to an ephemeral open: a mid-restore throw leaves the - // already-loaded bases as plain datasets and the session's attached set - // stays empty, so a later save prunes every launch-composition entry. - // Restore steps already applied (layout, view bindings) are not rolled - // back. One notice here; this setup's leaf errors stay unflagged so the - // generic load-error dialog still names them. - useMessageStore().addWarning( - 'Could not restore the saved session; opened its images instead', - { details: ensureError(err).message } - ); - } - } + let reportedStateIDs = new Set(); + if (policy.kind === 'application') { + reportedStateIDs = await completeStateFileSetups( + stateFileSetups, + policy.restore, + loadableResults, + failedLeaves + ); - results.push(...applyConfigsPostState(configResults)); + results.push( + ...applyConfigsPostState( + configResults, + configTargetImageIds( + importedData, + loadableResults, + policy.standaloneImageIds + ), + importedData + ) + ); + } // 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 @@ -335,13 +404,20 @@ export function importDataSources( dataSources: DataSource[], restore: RestoreProcessors = appRestoreProcessors() ): Promise { - return importDataSourcesWithPolicy(dataSources, 'application', restore); + 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. + return importDataSourcesWithPolicy(dataSources, { + kind: 'application', + restore, + standaloneImageIds: initiatingImageId ? [initiatingImageId] : [], + }); } export function importVolumeDataSources( dataSources: DataSource[] ): Promise { - return importDataSourcesWithPolicy(dataSources, 'volume-data'); + return importDataSourcesWithPolicy(dataSources, { kind: 'volume-data' }); } 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..8a33301fe 100644 --- a/src/io/import/processors/restoreStateFile.ts +++ b/src/io/import/processors/restoreStateFile.ts @@ -10,7 +10,7 @@ import { StateFileSetupResult, } from '@/src/io/import/common'; import { MANIFEST, isStateFile } from '@/src/io/state-file/serialize'; -import { partition, getURLBasename } from '@/src/utils'; +import { partition, getURLBasename, plural } from '@/src/utils'; import { basename } from '@/src/utils/path'; import { planLabelmapSources } from '@/src/io/import/labelmapImports'; import { useSegmentationStore } from '@/src/segmentation/store'; @@ -22,7 +22,10 @@ import type { FileEntry } from '@/src/io/types'; import { Skip } from '@/src/utils/evaluateChain'; import { useViewStore } from '@/src/store/views'; import { useViewConfigStore } from '@/src/store/view-configs'; -import { migrateManifest } from '@/src/io/state-file/migrations'; +import { + migrateManifest, + type ManifestMigration, +} from '@/src/io/state-file/migrations'; import { useMessageStore } from '@/src/store/messages'; type LeafSource = @@ -183,6 +186,34 @@ type RestoreServices = { >; }; +export type RestoreOptions = { + missingFiles?: Array<{ stateID: string; path: string }>; + failedLeaves?: Array<{ stateID: string; name: string }>; + migration?: ManifestMigration; +}; + +const restoreOptions = ({ + missingFiles = [], + failedLeaves = [], + migration, +}: RestoreOptions = {}) => ({ missingFiles, failedLeaves, migration }); + +const repeatedSegmentNotices = ( + repeated: Array<{ name: string; imageId: string; sourceRow: number }>, + imageName: (imageId: string) => string +) => { + const rows = new Map }>(); + repeated.forEach(({ name, imageId, sourceRow }) => { + const row = rows.get(sourceRow) ?? { name, images: new Set() }; + row.images.add(imageId); + rows.set(sourceRow, row); + }); + return [...rows].map( + ([sourceRow, { name, images }]) => + `- segment: ${name} (catalog row ${sourceRow + 1}; ${plural(images.size, 'image')}: ${[...images].map(imageName).join(', ')}; repeats the id of an earlier segment)` + ); +}; + export const createStateFileRestorer = ({ addWarning = (title, options) => useMessageStore().addWarning(title, options), views, @@ -191,20 +222,34 @@ export const createStateFileRestorer = ({ manifest: Manifest, stateFiles: FileEntry[], stateIDToStoreID: Record, - missingFiles: Array<{ stateID: string; path: string }> = [], - failedLeaves: Array<{ stateID: string; name: string }> = [] + options?: RestoreOptions ) { + const { missingFiles, failedLeaves, migration } = restoreOptions(options); const viewStore = views ?? useViewStore(); const byId = dataSourcesById(manifest); const datasets = manifestDatasets(manifest); + const imageName = (imageId: string) => { + const dataset = datasets.find(({ id }) => id === imageId); + return dataset + ? summarizeDataSource( + dataset.dataSourceId, + byId, + manifest.datasetFilePath, + imageId + ) + : imageId; + }; const resolvedDatasets = datasets.filter((ds) => ds.id in stateIDToStoreID); const unresolvedDatasets = datasets.filter( (ds) => !(ds.id in stateIDToStoreID) ); // 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, migration); // Restore camera and contrast before asynchronous mask decoding. useViewConfigStore().deserializeAll( @@ -226,7 +271,7 @@ export const createStateFileRestorer = ({ if (defaultStoreID !== undefined) { if (!manifest.viewByID) { - viewStore.setDataForAllViews(defaultStoreID); + viewStore.setDataForAllViews(defaultStoreID, manifest.activeView); } else { // A saved view whose image could not be restored uses the default image. const unresolvedIDs = new Set(unresolvedDatasets.map((ds) => ds.id)); @@ -252,16 +297,13 @@ export const createStateFileRestorer = ({ useLayersStore().deserialize(manifest, stateIDToStoreID); - useToolStore().deserialize(manifest, segmentIdMap, stateIDToStoreID); - - const missingBases = unresolvedDatasets.map((ds) => - summarizeDataSource( - ds.dataSourceId, - byId, - manifest.datasetFilePath, - String(ds.id) - ) + const skippedTools = useToolStore().deserialize( + manifest, + segmentIdMap, + stateIDToStoreID ); + + const missingBases = unresolvedDatasets.map(({ id }) => imageName(id)); // Partially restored datasets need member notices; unresolved datasets are named above. const missingMembers = missingFiles .filter(({ stateID }) => stateID in stateIDToStoreID) @@ -290,9 +332,12 @@ 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})` + ), + ...repeatedSegmentNotices(repeatedSegments, imageName), + ...unassigned.map( + ({ name }) => `- segment: ${name} (is not assigned to an image)` ), ]; if (missing.length > 0) { @@ -335,8 +380,11 @@ export const restoreStateFile: ImportHandler = async (dataSource) => { // Parse/validate BEFORE applying any restore: a file that fails here // opened nothing, so the live session must stay exactly as it was. let manifest: Manifest; + let migration: ManifestMigration; try { - manifest = ManifestSchema.parse(migrateManifest(manifestString)); + const migrated = migrateManifest(manifestString); + manifest = ManifestSchema.parse(migrated.manifest); + migration = migrated.migration; } catch (e) { return asErrorResult( new Error(`Unsupported state file schema or version: ${e}`), @@ -354,6 +402,7 @@ export const restoreStateFile: ImportHandler = async (dataSource) => { type: 'stateFileSetup', dataSources, manifest, + migration, stateFiles, missingFiles, } as StateFileSetupResult; 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..22e82693c 100644 --- a/src/io/state-file/__tests__/annotationToolSource.spec.ts +++ b/src/io/state-file/__tests__/annotationToolSource.spec.ts @@ -94,10 +94,11 @@ 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); + const migrated = migrateManifest(old).manifest; expect(migrated.version).toBe(MANIFEST_VERSION); expect(() => ManifestSchema.parse(migrated)).not.toThrow(); // An unlabelled tool gains the segment its appearance mints, nothing else. diff --git a/src/io/state-file/__tests__/boundedMaskRoundTrip.spec.ts b/src/io/state-file/__tests__/boundedMaskRoundTrip.spec.ts index 97a5cb022..c472745ff 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([ @@ -590,7 +594,7 @@ const legacyPairManifest = (scene: Record) => datasets: [{ id: 'ds-ct', dataSourceId: 1 }], ...scene, }) - ) + ).manifest ); const restoreLegacyPair = async (manifest: Manifest) => { 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..068522121 --- /dev/null +++ b/src/io/state-file/__tests__/imageSegmentSetsRoundTrip.spec.ts @@ -0,0 +1,252 @@ +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, + savedSegmentNames, + 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], + })), + }, + }, + }) + ).manifest + ); + +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(savedSegmentNames('runtime')).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']); + const saved = serializeAnnotations(); + expect(savedSegmentNames('first', saved)).toEqual(['B', 'A']); + expect( + saved.imageSegmentSets?.find(({ imageId }) => imageId === 'first') + ?.segmentIds + ).toContain(segmentIdMap.s2); + 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(savedSegmentNames('restored-a')).toEqual(['Tumor']); + 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: [], + }) + ).manifest + ); + 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(savedSegmentNames('image-b')).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(savedSegmentNames('image-a')).toEqual(['Tumor']); + 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..07649ef5c --- /dev/null +++ b/src/io/state-file/__tests__/manyToOneImageSegmentRestore.spec.ts @@ -0,0 +1,294 @@ +import { beforeEach, describe, expect, it } from 'vitest'; +import { createPinia, setActivePinia } from 'pinia'; + +import { buildStateIDToStoreID } from '@/src/io/import/importDataSources'; +import type { ManifestMigration } from '@/src/io/state-file/migrations'; +import { restoreStateFile } from '@/src/io/import/processors/restoreStateFile'; +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'; +import { Skip } from '@/src/utils/evaluateChain'; + +const readSession = async (manifest: unknown) => { + const setup = await restoreStateFile({ + type: 'file', + file: new File([JSON.stringify(manifest)], 'session.volview.json', { + type: 'application/json', + }), + fileType: 'application/json', + }); + if (setup === Skip || setup.type !== 'stateFileSetup') + throw new Error('Expected a session restore setup'); + return setup; +}; + +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, + migration?: ManifestMigration +) => { + 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, + migration + ); + 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 migrated = await readSession(old); + const { skipped, repeated } = await restoreMerged( + migrated.manifest, + migrated.migration + ); + 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 migrated = await readSession(old); + const { skipped, repeated } = await restoreMerged( + migrated.manifest, + migrated.migration + ); + 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..1349ada86 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; @@ -170,7 +173,7 @@ describe('migrate640To700: structural stage', () => { (version) => { const migrated = migrateManifest( JSON.stringify({ version, dataSources: [] }) - ) as any; + ).manifest as any; expect(migrated.version).toBe(MANIFEST_VERSION); } ); @@ -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 = () => @@ -1050,7 +1053,7 @@ const legacyScene = () => }); const restoreLegacyScene = async (scene = legacyScene()) => { - const manifest = ManifestSchema.parse(migrateManifest(scene)); + const manifest = ManifestSchema.parse(migrateManifest(scene).manifest); await seatImage('store-ct', { name: 'CT', dimensions: SPEC_DIMENSIONS }); const values = new Uint8Array(SPEC_VOXEL_COUNT); values.fill(1, 4, 12); @@ -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..2881de252 --- /dev/null +++ b/src/io/state-file/__tests__/migrate700To800.spec.ts @@ -0,0 +1,257 @@ +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], + visible: true, + locked: false, +}; + +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, + }) + ).manifest + ); + +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('keeps a missing en face reference from selecting a generated segment', () => { + const missingId = 'image-1-segment-1'; + const migrated = migrate({ + viewByID: { + view: { + id: 'view', + type: 'EnFace', + name: 'En face', + config: { + a: { + enFace: { + axis: 1, + method: 'mean', + depthStart: 0, + depthEnd: null, + segmentId: missingId, + }, + }, + }, + }, + }, + }); + + expect(migrated.viewByID!.view.config!.a.enFace!.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, planned].map((definition) => + omit(definition, ['id', 'visible', 'locked']) + ) + ); + }); + + 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)).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..15cc58a65 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())).manifest + ); 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())).manifest + ); 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..9cc32c6e2 100644 --- a/src/io/state-file/migrations.ts +++ b/src/io/state-file/migrations.ts @@ -4,6 +4,12 @@ import { emptyExtent } from '@/src/segmentation/geometry'; import { cssColorToRGBA, tryCssColorToRGBA } from '@/src/segmentation/color'; import { makeDefaultSegmentName } from '@/src/segmentation/model'; import { TOOL_COLORS } from '@/src/config'; +import { manifestDatasets } from '@/src/io/state-file/schema'; + +export type ManifestMigration = { + legacySegmentIds: Record; + segmentSourceRows: number[]; +}; const migrateOrPass = (versions: Array, migrationFunc: (manifest: any) => any) => @@ -297,11 +303,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 +326,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 +415,6 @@ const migrate640To700 = (inputManifest: any) => { }; }); - const toolKeys = ['rulers', 'rectangles', 'polygons']; - const nextToolColor = cycle(TOOL_COLORS); const nextPaletteColor = () => cssColorToRGBA(nextToolColor()); @@ -417,7 +425,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 +501,7 @@ const migrate640To700 = (inputManifest: any) => { delete entry.labels; }; - toolKeys.forEach(migrateToolLabels); + ANNOTATION_TOOL_KEYS.forEach(migrateToolLabels); const segmentations = [...recordsByParent.entries()].map( ([parentImage, records]) => { @@ -526,15 +534,127 @@ 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 + ); + }); + }); +}; + +const enFaceSegmentIds = (manifest: any) => + Object.values(manifest.viewByID ?? {}).flatMap((view: any) => + Object.values(view?.config ?? {}).map( + (config: any) => 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, migration: ManifestMigration) => { + const manifest = JSON.parse(JSON.stringify(inputManifest)); + const catalog: any[] = Array.isArray(manifest.segments) + ? manifest.segments + : []; + const datasets = manifestDatasets(manifest); + 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) + ), + ...enFaceSegmentIds(manifest), + ]; + // 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); + migration.legacySegmentIds[id] = segment.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] } : {}), + }); + migration.segmentSourceRows.push(segmentIndex); + }); + const selectedSegmentId = segmentIds.get(manifest.selectedSegment); + return { + imageId, + segmentIds: [...segmentIds.values()], + ...(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', 'visible', 'locked']) + ); + } + 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( + const migration: ManifestMigration = { + legacySegmentIds: Object.create(null), + segmentSourceRows: [], + }; + const manifest = pipe( inputManifest, migrateOrPass(['5.0.1'], migrate501To600), migrateOrPass(['6.0.0'], migrate600To610), 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'], (legacy) => migrate700To800(legacy, migration)) ); + return { manifest, migration }; }; diff --git a/src/io/state-file/schema.ts b/src/io/state-file/schema.ts index 224c56bed..389068dca 100644 --- a/src/io/state-file/schema.ts +++ b/src/io/state-file/schema.ts @@ -382,6 +382,20 @@ export const Segment = z.object({ strokeWidth: z.number().optional(), }) satisfies z.ZodType; +const ImageSegmentSet = z.object({ + imageId: z.string(), + segmentIds: z.string().array(), + selectedSegmentId: z.string().optional(), +}); + +const SegmentDefaults = Segment.omit({ + id: true, + visible: true, + locked: true, +}).extend({ + color: RGBAColor.optional(), +}); + export const Segmentation = z.object({ id: z.string(), name: z.string(), @@ -535,7 +549,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(), @@ -551,9 +566,11 @@ export type Manifest = z.infer; // The manifest's base datasets; older manifests carry no `datasets`, so every // uri source stands in for one, keyed by its stringified source id. -export const manifestDatasets = (manifest: Manifest) => +export const manifestDatasets = ( + manifest: Partial> +) => manifest.datasets ?? - manifest.dataSources + (manifest.dataSources ?? []) .filter((ds) => ds.type === 'uri') .map((ds) => ({ id: String(ds.id), dataSourceId: ds.id })); 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..83fe6011f 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,28 @@ stores or upper feature modules. ## Editing and persistence +On first activation or edit, an image without a segment set copies the last active +image's definitions, order and selection, falling back to configured defaults. +Copies have fresh identities, start visible and unlocked, and contain no masks or +shapes. + +A copy stays borrowed until its definitions, masks or shapes are edited. A restored +set or a configuration loaded with data replaces a borrowed copy. A companion +segmentation replaces it with configured defaults before binding its labels by +name. Restored sets and companion labels merge into edited sets. + +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. +Legacy identity and source-row mappings belong to the import context and are +not part of the saved manifest. +An image without a saved set uses saved defaults when supplied, otherwise it +inherits normally. Artifact-only sessions without saved defaults use configured +defaults. Saved empty sets do not inherit. +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 +66,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 +76,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..65805e104 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', @@ -231,9 +238,56 @@ describe('placing an annotation names its segment', () => { return id; }; + const copyToDestination = () => { + segments().addSegment(IMAGE_ID, { name: 'Custom copied row' }); + seatAndView('destination'); + const store = useRulerStore(); + const id = store.addTool({ imageID: 'destination', placing: true }); + return { store, id }; + }; + + it('keeps a copied set after a finished annotation is removed', () => { + const { store, id } = copyToDestination(); + store.placeTool(id); + store.removeTool(id); + + useSegmentStore().seedDefaults('destination'); + + expect( + segments() + .segmentList('destination') + .map(({ name }) => name) + ).toEqual(['Custom copied row']); + }); + + it('keeps the begun annotation in its copied segment while incoming defaults arrive', () => { + const { store, id } = copyToDestination(); + store.resolveToolSegment(id); + + useSegmentStore().seedDefaults('destination'); + store.updateTool(id, { firstPoint: [0, 0, 0], secondPoint: [1, 1, 0] }); + store.placeTool(id); + + expect(store.getPoints(id)).toEqual([ + [0, 0, 0], + [1, 1, 0], + ]); + expect(store.appearanceOfTool(id).name).toBe('Custom copied row'); + }); + + it('replaces a copied set while its annotation is still an idle stub', () => { + const { store, id } = copyToDestination(); + + useSegmentStore().seedDefaults('destination'); + expect(segments().segmentList('destination')).toEqual([]); + store.placeTool(id); + + expect(store.appearanceOfTool(id).name).toBe('Segment 1'); + }); + 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 +316,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 +328,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 +350,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 +383,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..0c08f4f53 --- /dev/null +++ b/src/segmentation/__tests__/bulkImageRemoval.spec.ts @@ -0,0 +1,256 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { enableAutoUnmount, flushPromises, mount } from '@vue/test-utils'; +import { nextTick } from 'vue'; +import { createVuetify } from 'vuetify'; +import { createPinia, setActivePinia } from 'pinia'; +import type { Vector3 } from '@kitware/vtk.js/types'; + +import { + activateAppPinia, + inMemoryArtifactIO, + manifestForImages, + makeLabelmap, + seatImage, + seedVoxel, + serializeAnnotations, + serializeToStateFiles, + showImage, + viewImage, +} from '@/src/segmentation/__tests__/segmentMaskFixtures'; +import { useSegmentationEditsStore } from '@/src/segmentation/editing/coordinator'; +import ProcessWorkflow from '@/src/segmentation/components/ProcessWorkflow.vue'; +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 { removeSelectedTools, useToolStore } from '@/src/store/tools'; +import { AnnotationToolType, Tools } from '@/src/store/tools/types'; + +enableAutoUnmount(afterEach); + +const detached = (value: T): T => JSON.parse(JSON.stringify(value)); + +const annotation = (imageID: string, segmentId: string, placing = false) => ({ + imageID, + segmentId, + placing, + slice: 0, + frameOfReference: { + planeOrigin: [0, 0, 0] as Vector3, + planeNormal: [0, 0, 1] as Vector3, + }, +}); + +const ruler = (imageID: string, segmentId: string, placing = false) => ({ + ...annotation(imageID, segmentId, placing), + firstPoint: [0, 0, 0] as Vector3, + secondPoint: [1, 1, 0] as Vector3, +}); + +const polygon = (imageID: string, segmentId: string, placing = false) => ({ + ...annotation(imageID, segmentId, placing), + points: [ + [0, 0, 0], + [2, 0, 0], + [1, 2, 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 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); + 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(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 = [ + { store: rulers, makeTool: ruler }, + { store: useRectangleStore(), makeTool: ruler }, + { store: usePolygonStore(), makeTool: polygon }, + ]; + const tools = shapeStores.map(({ store, makeTool }) => ({ + store, + removed: [ + ...aSegments.map(({ id }) => store.addTool(makeTool('a', id))), + store.addTool(makeTool('a', aSegments[0].id, true)), + store.addTool(makeTool('a', '')), + ], + retained: store.addTool(makeTool('b', b)), + })); + const originalTools = tools.map(({ store }) => + detached( + store.serializeTools().tools.filter(({ imageID }) => imageID === '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 = originalTools[0]; + 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 }, index) => { + removed.forEach((id) => store.placeTool(id)); + expect(store.serializeTools().tools).toEqual(originalTools[index]); + }); + expect(cancelPreview).toHaveBeenCalledTimes(1); + removeSelectedTools(); + expect(rulers.serializeTools().tools).toEqual( + originalB.filter(({ id }) => id !== selectedB) + ); + const io = inMemoryArtifactIO(); + const { parsed } = await serializeToStateFiles( + manifestForImages(['b']), + io + ); + expect(parsed.segmentations).toMatchObject([ + { + parentImage: 'b', + masks: [ + { + segmentId: b, + representations: { labelmap: { extent: [1, 1, 1, 1, 0, 0] } }, + }, + ], + }, + ]); + expect( + io.snapshots.flatMap(({ values }) => values.filter(Boolean)) + ).toEqual([1]); + }); + + it('closes the process preview when its image is removed', 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]); + useToolStore().setCurrentTool(Tools.Paint); + const workflow = mount(ProcessWorkflow, { + props: { + algorithm: async ({ scalars, maskExtent }) => ({ + scalars: new Uint8Array(scalars.length), + extent: maskExtent, + }), + }, + global: { plugins: [createVuetify()] }, + }); + await workflow.get('button').trigger('click'); + await flushPromises(); + expect(workflow.text()).toContain('Processed'); + expect(workflow.text()).toContain('Cancel'); + + useImageCacheStore().removeImage('a'); + + await nextTick(); + expect(workflow.text()).not.toContain('Cancel'); + expect(workflow.get('button').text()).toBe('Preview'); + expect(workflow.get('button').attributes('disabled')).toBeDefined(); + const { parsed } = await serializeToStateFiles( + manifestForImages([]), + inMemoryArtifactIO() + ); + expect(parsed.segmentations).toEqual([]); + }); + + 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..a34c73de2 100644 --- a/src/segmentation/__tests__/configSegments.spec.ts +++ b/src/segmentation/__tests__/configSegments.spec.ts @@ -8,6 +8,8 @@ import { } from '@/src/segmentation/__tests__/segmentMaskFixtures'; import { nextTick } from 'vue'; +import { useCurrentImage } from '@/src/composables/useCurrentImage'; +import { TOOL_COLORS } from '@/src/config'; import { useSegmentStore } from '@/src/segmentation/segments'; import { usePolygonStore } from '@/src/store/tools/polygons'; import { useRectangleStore } from '@/src/store/tools/rectangles'; @@ -17,13 +19,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 +31,21 @@ 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), + 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,106 @@ 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().serialize()).toEqual([]); + expect(boundMasks()).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('starts future defaults at the first palette color independently of existing rows', 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 + ); + expect(configuredColors).toEqual( + TOOL_COLORS.slice(1, 3).map(cssColorToRGBA) + ); + + setActivePinia(createPinia()); + applyConfig({ segments: { First: {}, Second: {} } }); + await seatAndView('img-2'); + expect( + segments() + .segmentList('img-2') + .map(({ color }) => color) + ).toEqual(TOOL_COLORS.slice(0, 2).map(cssColorToRGBA)); + }); +}); 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..c9a95326a 100644 --- a/src/segmentation/__tests__/descriptorlessActiveSegment.spec.ts +++ b/src/segmentation/__tests__/descriptorlessActiveSegment.spec.ts @@ -9,7 +9,11 @@ import { MANIFEST_VERSION } from '@/src/io/state-file/serialize'; import { useSegmentationStore } from '@/src/segmentation/store'; import { useSegmentStore } from '@/src/segmentation/segments'; import { listMasks } from '@/src/segmentation/model'; -import { seatImage } from '@/src/segmentation/__tests__/segmentMaskFixtures'; +import { + seatImage, + serializeAnnotations, + viewImage, +} from '@/src/segmentation/__tests__/segmentMaskFixtures'; // --------------------------------------------------------------------------- // A pre-7 group with no descriptors has no segments to activate when @@ -58,7 +62,7 @@ const twoGroupManifest = (activeValue: number): Manifest => paint: { activeSegmentGroupID: 'sg-b', activeSegment: activeValue }, }, }) - ) + ).manifest ); async function restoreTwoGroups(activeValue: number, populate = () => {}) { @@ -84,7 +88,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 ); @@ -119,15 +124,40 @@ describe('restoring a descriptorless active group', () => { expect(selectedIndex()).toBe(3); }); + it('restores defaults and the active group over an unrelated image selection', async () => { + const store = useSegmentStore(); + store.replaceConfigSegments({ Site: {} }, []); + await viewImage('existing'); + store.segments.addSegment('existing', { name: 'Unrelated' }); + + await restoreTwoGroups(2); + + const saved = serializeAnnotations(); + const set = saved.imageSegmentSets?.find( + ({ imageId }) => imageId === 'parent-store' + ); + const names = (set?.segmentIds ?? []).map( + (id) => saved.segments?.find((segment) => segment.id === id)?.name + ); + expect(names).toHaveLength(5); + expect(names[0]).toBe('Site'); + expect(names).not.toContain('Unrelated'); + expect( + saved.segments?.find(({ id }) => id === set?.selectedSegmentId)?.name + ).toMatch(/^B/); + }); + it('leaves a selection the scene already has alone', async () => { 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..89375d914 --- /dev/null +++ b/src/segmentation/__tests__/imageSetInitialization.spec.ts @@ -0,0 +1,529 @@ +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 { migrateManifest } from '@/src/io/state-file/migrations'; +import { ManifestSchema } from '@/src/io/state-file/schema'; +import { + inMemoryArtifactIO, + makeLabelmap, + manifestForImages, + savedSegmentNames, + seatImage, + serializeAnnotations, + serializeToStateFiles, + 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 { useImageStatsStore } from '@/src/store/image-stats'; +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] }]])( + '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' }], + 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([false, true])( + "captures a companion conversion's defaults while preserving intervening edits: %s", + async (edited) => { + const store = useSegmentStore(); + store.replaceConfigSegments({ Lung: {} }, []); + await viewImage('existing'); + store.segments.addSegment('existing', { name: 'Unrelated' }); + + await companionImport(() => { + if (edited) store.segments.addSegment('base', { name: 'My edit' }); + store.replaceConfigSegments({ Other: {} }, []); + }); + + expect(segmentNames('base')).toEqual( + edited + ? ['Lung', 'Unrelated', 'My edit', 'labelmap'] + : ['Lung', 'labelmap'] + ); + expect(serializeAnnotations().seedSegmentDefaults).toEqual([ + { name: 'Other' }, + ]); + } + ); + + it.each([ + { version: '7.0.0' }, + { version: '8.0.0' }, + { version: '8.0.0', segments: [] }, + { version: '8.0.0', segments: [], imageSegmentSets: [] }, + ])( + 'initializes artifact-only images from configured defaults: %j', + async (wire) => { + const store = useSegmentStore(); + store.replaceConfigSegments({ Site: {} }, []); + await viewImage('existing'); + store.segments.addSegment('existing', { name: 'Unrelated' }); + await viewImage('destination-a'); + await seatImage('destination-b'); + const image = await seatImage('artifact', { + values: new Uint8Array(64).fill(1), + }); + const segmentation = useSegmentationStore(); + const deserialize = segmentation.deserialize; + vi.spyOn(segmentation, 'deserialize').mockImplementation((options) => + deserialize({ + ...options, + io: { read: async () => ({ image }), write: async () => '' }, + }) + ); + const ids = ['a', 'b']; + const migrated = migrateManifest( + JSON.stringify( + manifestForImages(ids, { + ...wire, + primarySelection: 'a', + segmentationArtifacts: ids.map((id) => ({ + id: `artifact-${id}`, + name: `Mask ${id}`, + parentImage: id, + path: `${id}.vti`, + })), + }) + ) + ); + await completeStateFileRestore( + ManifestSchema.parse(migrated.manifest), + ids.map((id) => ({ + archivePath: `${id}.vti`, + file: new File([], `${id}.vti`), + })), + { a: 'destination-a', b: 'destination-b' }, + { migration: migrated.migration } + ); + + ids.forEach((id) => { + expect(savedSegmentNames(`destination-${id}`)).toEqual([ + 'Site', + `Mask ${id}`, + ]); + expect(segmentation.imageMasks(`destination-${id}`)).toHaveLength(1); + }); + await showImage('destination-b'); + expect(savedSegmentNames('destination-b')).toEqual(['Site', 'Mask b']); + expect(savedSegmentNames('existing')).toEqual(['Site', 'Unrelated']); + } + ); + + it.each([false, true])( + 'inherits before matching a mixed manifest artifact, with destination viewed: %s', + async (viewed) => { + await viewImage('existing'); + const segments = useSegmentStore(); + segments.segments.addSegment('existing', { name: 'Tumor' }); + segments.segments.addSegment('existing', { name: 'Unused' }); + await seatImage('destination'); + await seatImage('other'); + if (viewed) await showImage('destination'); + const manifest = manifestForImages(['target', 'other'], { + imageSegmentSets: [{ imageId: 'other', segmentIds: [] }], + segmentationArtifacts: [ + { + id: 'artifact', + name: 'Tumor.vti', + path: 'Tumor.vti', + parentImage: 'target', + }, + ], + }); + const dataIDMap = { target: 'destination', other: 'other' }; + const { segmentIdMap } = segments.deserialize(manifest, dataIDMap); + const io = inMemoryArtifactIO(); + const encoded = await io.write( + 'vti', + makeLabelmap({ values: new Uint8Array(64).fill(1) }) + ); + + await useSegmentationStore().deserialize({ + manifest, + dataIDMap, + segmentIdMap, + stateFiles: [ + { archivePath: 'Tumor.vti', file: new File([encoded], 'Tumor.vti') }, + ], + io, + }); + + expect(savedSegmentNames('destination')).toEqual(['Tumor', 'Unused']); + expect(savedSegmentNames('other')).toEqual([]); + const { parsed: saved } = await serializeToStateFiles( + manifestForImages(['destination']), + io + ); + const { segmentIds } = saved.imageSegmentSets.find( + ({ imageId }: { imageId: string }) => imageId === 'destination' + ); + expect(saved.segmentations).toEqual([ + expect.objectContaining({ + parentImage: 'destination', + masks: [expect.objectContaining({ segmentId: segmentIds[0] })], + }), + ]); + } + ); + + it('keeps defaults separate when two sessions share an import map', async () => { + await seatImage('destination-a'); + await seatImage('destination-b'); + const dataIDMap = { a: 'destination-a', b: 'destination-b' }; + + await completeStateFileRestore( + manifestForImages(['a'], { seedSegmentDefaults: [{ name: 'Tumor' }] }), + [], + dataIDMap + ); + await completeStateFileRestore( + manifestForImages(['b'], { + seedSegmentDefaults: [{ name: 'Kidney' }], + segments: [ + { + id: 'kidney', + name: 'Kidney', + color: [1, 2, 3, 255], + visible: true, + locked: false, + }, + ], + imageSegmentSets: [{ imageId: 'b', segmentIds: ['kidney'] }], + }), + [], + dataIDMap + ); + + expect(savedSegmentNames('destination-a')).toEqual(['Tumor']); + expect(savedSegmentNames('destination-b')).toEqual(['Kidney']); + }); + + it.each([ + { savedLayout: false, savedViews: true, slotCount: 2 }, + { savedLayout: true, savedViews: true, slotCount: 2 }, + { savedLayout: true, savedViews: false, slotCount: 2 }, + { savedLayout: true, savedViews: false, slotCount: 1 }, + { savedLayout: true, savedViews: false, slotCount: 2, activeSlot: 0 }, + { savedLayout: true, savedViews: false, slotCount: 2, activeSlot: 1 }, + ])( + 'ignores old view contents while replacing a restored layout: %j', + async ({ savedLayout, savedViews, slotCount, activeSlot }) => { + ['existing', 'other', 'destination'].forEach((id) => { + useImageStatsStore().stats[id] = { + scalarMin: 0, + scalarMax: 0, + autoRangeValues: {}, + }; + }); + await viewImage('existing'); + const store = useSegmentStore(); + store.segments.addSegment('existing', { name: 'Previous' }); + await seatImage('other'); + store.segments.addSegment('other', { name: 'Wrong source' }); + await seatImage('destination'); + const views = useViewStore(); + const previousViewId = views.viewIDs[1]; + views.setDataForView(previousViewId, 'existing'); + views.setActiveView(previousViewId); + const incomingViewId = views.viewIDs[0]; + views.setDataForView(incomingViewId, 'other'); + const activeView = activeSlot === 1 ? previousViewId : incomingViewId; + const manifest = manifestForImages(['saved'], { + ...(savedViews || activeSlot !== undefined ? { activeView } : {}), + ...(savedLayout + ? { + layout: { + direction: 'row', + items: Array.from({ length: slotCount }, (_, slotIndex) => ({ + type: 'slot', + slotIndex, + })), + }, + } + : {}), + layoutSlots: [incomingViewId, previousViewId], + isActiveViewMaximized: true, + ...(savedViews + ? { + viewByID: { + [incomingViewId]: { + ...views.viewByID[incomingViewId], + dataID: 'saved', + }, + [previousViewId]: { + ...views.viewByID[previousViewId], + dataID: null, + }, + }, + } + : {}), + }); + + views.deserializeLayout(manifest); + await completeStateFileRestore(manifest, [], { saved: 'destination' }); + + expect(savedSegmentNames('destination')).toEqual(['Previous']); + expect(views.visibleViews.map(({ dataID }) => dataID)).toEqual([ + 'destination', + ]); + if (activeSlot !== undefined) + expect(views.visibleViews.map(({ id }) => id)).toEqual([activeView]); + } + ); + + 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..4d87e4b1c 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 }), []); -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__/paintPreview.spec.ts b/src/segmentation/__tests__/paintPreview.spec.ts new file mode 100644 index 000000000..503991c57 --- /dev/null +++ b/src/segmentation/__tests__/paintPreview.spec.ts @@ -0,0 +1,84 @@ +import { beforeEach, describe, expect, it } from 'vitest'; + +import { useSegmentStore } from '@/src/segmentation/segments'; +import { useSegmentationStore } from '@/src/segmentation/store'; +import { SEGMENT_VALUE } from '@/src/segmentation/masks/labelValue'; +import { useViewStore } from '@/src/store/views'; +import { + activateAppPinia, + seatImage, + maskValueAt, + serializeAnnotations, + strokeAt, + viewImage, +} from '@/src/segmentation/__tests__/segmentMaskFixtures'; + +const showInactiveImage = async (imageId: string) => { + await seatImage(imageId); + const views = useViewStore(); + const viewId = views.viewIDs.find((id) => id !== views.activeView)!; + views.setDataForView(viewId, imageId); + return () => views.setActiveView(viewId); +}; + +const paintedSegmentColor = (imageId: string) => { + const [painted] = useSegmentationStore().imageMasks(imageId); + expect(maskValueAt(painted.id, [1, 1, 0])).toBe(SEGMENT_VALUE); + return serializeAnnotations().segments?.find( + ({ id }) => id === painted.segmentId + )?.color; +}; + +describe('the next painted segment color', () => { + beforeEach(activateAppPinia); + + it("previews an inactive image's copied selection without initializing it", async () => { + await viewImage('source'); + const store = useSegmentStore(); + store.segments.addSegment('source', { name: 'First' }); + store.segments.addSegment('source', { + name: 'Selected', + color: [11, 22, 33, 255], + }); + const activate = await showInactiveImage('destination'); + + const preview = store.previewSegmentColor('destination'); + + expect( + serializeAnnotations().imageSegmentSets?.some( + ({ imageId }) => imageId === 'destination' + ) + ).toBe(false); + activate(); + strokeAt('destination', [1, 1, 0]); + expect(paintedSegmentColor('destination')).toEqual(preview); + expect(preview).toEqual([11, 22, 33, 255]); + }); + + it("previews an inherited empty set's next color after earlier rows were deleted", async () => { + await viewImage('source'); + const store = useSegmentStore(); + const deleted = store.segments.addSegment('source'); + store.segments.deleteSegment(deleted); + const activate = await showInactiveImage('destination'); + + const preview = store.previewSegmentColor('destination'); + activate(); + strokeAt('destination', [1, 1, 0]); + + expect(paintedSegmentColor('destination')).toEqual(preview); + }); + + it('previews configured defaults before the first image is activated', async () => { + const store = useSegmentStore(); + store.replaceConfigSegments({ Configured: { color: '#102030' } }, []); + const activate = await showInactiveImage('destination'); + + const preview = store.previewSegmentColor('destination'); + activate(); + strokeAt('destination', [1, 1, 0]); + + expect(paintedSegmentColor('destination')).toEqual(preview); + expect(preview).toEqual([16, 32, 48, 255]); + }); +}); diff --git a/src/segmentation/__tests__/paintProcess.spec.ts b/src/segmentation/__tests__/paintProcess.spec.ts index f1b4edf81..2923de62f 100644 --- a/src/segmentation/__tests__/paintProcess.spec.ts +++ b/src/segmentation/__tests__/paintProcess.spec.ts @@ -16,6 +16,10 @@ import { addMask, boundMasks, markedVoxels, + makeLabelmap, + serializeAnnotations, + serializeToStateFiles, + inMemoryArtifactIO, seedVoxel, } from '@/src/segmentation/__tests__/segmentMaskFixtures'; import { SEGMENT_VALUE } from '@/src/segmentation/masks/labelValue'; @@ -191,8 +195,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]); @@ -209,9 +216,8 @@ 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' }); + // A segment selected on this image need not have a mask yet. + const segmentId = mintSegment('image-1', { name: 'Unpainted' }); useSegmentStore().segments.selectSegment(segmentId); expect(usePaintProcessStore().startRefusal('image-1')).toBe( @@ -263,7 +269,7 @@ describe('Paint process store', () => { segmentationStore.ensureSegmentationForImage('image-1'); const segment = segmentationStore.createMask( segmentation.id, - mintSegment({ + mintSegment('image-1', { name: 'Empty', }) ); @@ -286,7 +292,7 @@ describe('Paint process store', () => { segmentationStore.ensureSegmentationForImage('image-1'); const segment = segmentationStore.createMask( segmentation.id, - mintSegment({ + mintSegment('image-1', { name: 'Empty', }) ); @@ -339,7 +345,7 @@ describe('Paint process store', () => { const other = segmentationStore.createMask( segmentationId, - mintSegment({ + mintSegment('image-1', { name: 'Other', }) ); @@ -379,7 +385,7 @@ describe('Paint process store', () => { lockSegment(maskId, true); segmentationStore.createMask( segmentationId, - mintSegment({ name: 'Empty' }) + mintSegment('image-1', { name: 'Empty' }) ); await processStore.startProcess( @@ -402,6 +408,34 @@ describe('Paint process store', () => { extent: target.maskExtent, }); + it('keeps imported definitions and selection after a process removes their last mask', async () => { + const segments = useSegmentStore(); + segments.replaceConfigSegments({ Lung: {} }, ['image-1']); + segments.segments.addSegment('image-1', { name: 'Unrelated' }); + await viewImage('image-2', TWO_VOXELS); + const segmentationStore = useSegmentationStore(); + segmentationStore.splitLabelmapIntoMasks( + 'image-2', + makeLabelmap({ ...TWO_VOXELS, values: new Uint8Array([1, 1]) }), + [{ value: 1, name: 'Unrelated', color: [255, 0, 0, 255], visible: true }] + ); + const saved = serializeAnnotations(); + const processStore = usePaintProcessStore(); + + await processStore.startProcess(emptyingProcess); + processStore.confirmProcess(); + segments.seedDefaults('image-2'); + + expect(serializeAnnotations()).toEqual(saved); + const { parsed } = await serializeToStateFiles( + serializeAnnotations(), + inMemoryArtifactIO() + ); + expect(parsed.segmentations).toEqual([ + expect.objectContaining({ parentImage: 'image-2', masks: [] }), + ]); + }); + it('deletes a mask the applied result leaves empty once it is applied', async () => { const processStore = usePaintProcessStore(); const segmentationStore = useSegmentationStore(); 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..f7a1b0c5d 100644 --- a/src/segmentation/__tests__/restoreSegmentLifetime.spec.ts +++ b/src/segmentation/__tests__/restoreSegmentLifetime.spec.ts @@ -1,6 +1,6 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { createPinia, disposePinia } from 'pinia'; -import { completeStateFileRestore } from '@/src/io/import/processors/restoreStateFile'; +import { createStateFileRestorer } from '@/src/io/import/processors/restoreStateFile'; import { ManifestSchema } from '@/src/io/state-file/schema'; import { useSegmentStore } from '@/src/segmentation/segments'; import { useRulerStore } from '@/src/store/tools/rulers'; @@ -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, @@ -109,7 +110,8 @@ async function pendingRestore(kind: 'own' | 'legacy') { }, }) ); - const completed = completeStateFileRestore( + const addWarning = vi.fn(); + const completed = createStateFileRestorer({ addWarning })( manifest, ids.map((id) => ({ archivePath: `${id}.vti`, @@ -118,7 +120,12 @@ async function pendingRestore(kind: 'own' | 'legacy') { { parent: 'parent' } ); await reading.promise; - return { completed, release, registry: useSegmentStore().segments }; + return { + completed, + release, + registry: useSegmentStore().segments, + addWarning, + }; } describe.each(['own', 'legacy'] as const)( @@ -127,24 +134,25 @@ describe.each(['own', 'legacy'] as const)( it.each([false, true])( '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 { completed, release, registry, addWarning } = + await pendingRestore(kind); + 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( @@ -156,6 +164,12 @@ describe.each(['own', 'legacy'] as const)( healthy.id, '', ]); + expect(addWarning).not.toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ + details: expect.stringContaining('belongs to another image'), + }) + ); } ); } 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..fea27c53f 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,29 @@ 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; +}; + +export const savedSegmentNames = ( + imageId: string, + manifest = serializeAnnotations() +) => { + const set = manifest.imageSegmentSets?.find( + (entry) => entry.imageId === imageId + ); + return (set?.segmentIds ?? []).map( + (id) => manifest.segments?.find((segment) => segment.id === id)?.name + ); +}; + const archivePathsIn = (parsed: any): string[] => [ ...(parsed.segmentationArtifacts ?? []).flatMap((artifact: any) => artifact.path ? [artifact.path] : [] @@ -307,15 +333,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 +396,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..bb8b8db19 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,90 +264,135 @@ 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; + const revision = registry.selectionRevision('img-1'); registry.selectSegment(chosen); - expect(registry.selectionRevision.value).toBe(revision + 1); + expect(registry.selectionRevision('img-1')).toBe(revision + 1); }); 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('continues the image palette beneath colors supplied by a config', () => { + const registry = createSegmentRegistry(); + registry.addSegment('img-1'); + registry.addSegment('img-1'); + + registry.replaceConfigSegments('img-1', { + Tumor: {}, + Liver: { color: 'blue' }, + }); + + expect(registry.segmentList('img-1').map(({ color }) => color)).toEqual( + [...TOOL_COLORS.slice(0, 3), 'blue'].map(cssColorToRGBA) + ); + + registry.replaceConfigSegments('img-1', { Tumor: {}, Liver: {} }); + expect(registry.findSegmentByName('img-1', 'Liver')?.color).toEqual( + cssColorToRGBA(TOOL_COLORS[3]) + ); + expect(registry.getSegment(registry.addSegment('img-1'))?.color).toEqual( + cssColorToRGBA(TOOL_COLORS[4]) + ); + }); + + it('gives a prepended config key the next color without changing retained keys', () => { + const registry = createSegmentRegistry(); + registry.replaceConfigSegments('img-1', { Tumor: {}, Liver: {} }); + const retained = registry.serialize(); + + registry.replaceConfigSegments('img-1', { + Node: {}, + Tumor: {}, + Liver: {}, + }); + + expect(registry.segmentList('img-1')).toEqual([ + ...retained, + expect.objectContaining({ + name: 'Node', + color: cssColorToRGBA(TOOL_COLORS[2]), + }), + ]); + }); + 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 +404,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 +413,7 @@ describe('segment registry', () => { remove: () => {}, }); - const { idMap } = registry.adopt([ + const { idMap } = registry.adopt('img-1', [ { id: existing, name: 'Tumor', @@ -336,13 +431,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 +462,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 +477,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 +538,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 +580,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 +599,7 @@ describe('segment registry', () => { describe('a restored selection', () => { const restoreSaved = (registry: SegmentRegistry) => - registry.adopt([ + registry.adopt('img-1', [ { id: 'saved', name: 'Saved', @@ -482,48 +611,247 @@ 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.each([ + {}, + { color: [1, 2, 3, 255] as [number, number, number, number] }, + { fillOpacity: undefined }, + ])( + 'keeps an unchanged copy replaceable after a no-op update (%o)', + (patch) => { + const registry = createSegmentRegistry(); + registry.addSegment('source', { name: 'Copied', color: [1, 2, 3, 255] }); + registry.initializeSet('destination', registry.snapshot('source'), { + copied: true, + }); + registry.updateSegment(registry.segmentList('destination')[0].id, patch); + + registry.releaseBorrowed('destination'); + registry.initializeSet('destination', { + segments: [{ name: 'Incoming' }], + configured: [], + }); + + expect(namesOf(registry, 'destination')).toEqual(['Incoming']); + } + ); + + it('copies snapshot colors independently into every destination', () => { + const registry = createSegmentRegistry(); + registry.replaceConfigSegments('source', { Tumor: { color: 'red' } }); + const snapshot = registry.snapshot('source'); + registry.initializeSet('first', snapshot); + registry.initializeSet('second', snapshot); + + const first = registry.findSegmentByName('first', 'Tumor')!; + first.color[0] = 64; + registry.replaceConfigSegments('first', { Tumor: {} }); + + expect(registry.findSegmentByName('second', 'Tumor')?.color).toEqual( + cssColorToRGBA('red') + ); + expect(registry.findSegmentByName('source', 'Tumor')?.color).toEqual( + cssColorToRGBA('red') + ); + registry.replaceConfigSegments('second', { Tumor: {} }); + expect(registry.findSegmentByName('second', 'Tumor')?.color).not.toEqual( + first.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([]); + }); + + 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.belongsToImage('first', first)).toBe(true); + }); + + 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.segmentList('first')).toEqual([]); + expect(registry.getSegment(first)).toBeUndefined(); + expect(registry.getSegment(another)).toBeUndefined(); + expect(registry.belongsToImage('first', first)).toBe(false); + 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(); + + [undefined, 'missing'].forEach((imageId) => { + expect(registry.segmentList(imageId)).toEqual([]); + expect(registry.selectedSegmentId(imageId)).toBeUndefined(); + expect(registry.belongsToImage(imageId, 'missing')).toBe(false); + }); + expect(registry.serialize()).toEqual([]); + registry.initializeSet('missing', { + segments: [{ name: 'Initialized' }], + configured: [], + }); + expect(namesOf(registry, 'missing')).toEqual(['Initialized']); + }); +}); + describe('the shared registry', () => { beforeEach(async () => { setActivePinia(createPinia()); @@ -531,7 +859,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 +872,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..51c6a04b8 --- /dev/null +++ b/src/segmentation/__tests__/segmentSetAppearanceRestore.spec.ts @@ -0,0 +1,163 @@ +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' }]); + + 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 }], + }) + ), + {} + ); + 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__/segmentationDescriptorlessParity.spec.ts b/src/segmentation/__tests__/segmentationDescriptorlessParity.spec.ts index 8d0c25857..d95353a91 100644 --- a/src/segmentation/__tests__/segmentationDescriptorlessParity.spec.ts +++ b/src/segmentation/__tests__/segmentationDescriptorlessParity.spec.ts @@ -33,7 +33,7 @@ const labelValues = () => new Uint8Array(4 * 4 * 4).fill(1, 20, 44).fill(2, 44); // NO segments. The migration turns it into an artifact no mask names, and the // LOADED restore stage is what enumerates its voxels. const migrated = (manifest: Record): Manifest => - ManifestSchema.parse(migrateManifest(JSON.stringify(manifest))); + ManifestSchema.parse(migrateManifest(JSON.stringify(manifest)).manifest); const descriptorlessComposedManifest = (visibility = true) => migrated({ diff --git a/src/segmentation/__tests__/segmentationRestoreResilience.spec.ts b/src/segmentation/__tests__/segmentationRestoreResilience.spec.ts index 9245918f0..6f04354bd 100644 --- a/src/segmentation/__tests__/segmentationRestoreResilience.spec.ts +++ b/src/segmentation/__tests__/segmentationRestoreResilience.spec.ts @@ -71,7 +71,7 @@ const manifestWith = (groups: Array>): Manifest => datasets: [{ id: 'ds-ct', dataSourceId: 1 }], segmentGroups: groups, }) - ) + ).manifest ); function makeImage(fillValue = 0) { @@ -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..c94f1ec89 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) => { @@ -29,10 +23,9 @@ const viewImage = async (id: string) => { }; const recordIdsOf = (imageId: string) => - store().getSegmentationForImage(imageId)?.order ?? []; - -const nameOf = (maskId: string) => - segments().appearanceOf(store().getMask(maskId).segmentId).name; + store() + .imageMasks(imageId) + .map(({ id }) => id); const SQUARE: Vector3[] = [ [0, 0, 0], @@ -50,95 +43,105 @@ 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(); + polygons.addTool({ imageID: 'img-1', segmentId: source, placing: false }); + polygons.addTool({ imageID: 'img-2', segmentId: copied, placing: false }); - 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.serializeTools().tools).toEqual([ + expect.objectContaining({ imageID: 'img-1', segmentId: source }), + ]); + 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 { 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.serializeTools().tools).toEqual([ + expect.objectContaining({ id: toolId, segmentId: copied }), + ]); }); - 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..ed0cf5d97 100644 --- a/src/segmentation/components/SegmentList.vue +++ b/src/segmentation/components/SegmentList.vue @@ -1,5 +1,5 @@