diff --git a/backend-contract/README.md b/backend-contract/README.md index e6fc24fec..d5467df83 100644 --- a/backend-contract/README.md +++ b/backend-contract/README.md @@ -109,9 +109,28 @@ Two versions on separate clocks: - **Artifact version**: `package.json` `version` and the OpenAPI `info.version`, kept in lockstep by `processing/__tests__/openapi.spec.ts`. Versions this package as a published thing. -- **Shape versions**: `INTENT_VOCABULARY_VERSION` (`processing/wire.ts`) and - the task-spec `specVersion`. These version the wire vocabulary for additive - compatibility negotiation. +- **Shape versions**: `INTENT_VOCABULARY_VERSION` (`processing/wire.ts`) names + the shape of the result intent vocabulary in the generated OpenAPI + description and in release notes. It never travels on the wire, so adding an + intent rests on both sides failing open on a name they do not know. The + task-spec `specVersion` does travel: every task spec carries it, and a client + rejects a spec whose version it does not know. Bump it only on a shape + change, never for a new optional field. + +### Result instruction rollout + +Contract artifact 0.3.0 uses intent vocabulary 3 and names segmentation import +`import-segmentation`. A current client still reads the earlier name, +`add-segment-group`, as the same instruction (`LEGACY_RESULT_INTENT_NAMES` in +`processing/wire.ts`), so a producer may move to the new name after the client. +The reverse order does not hold: an older client treats `import-segmentation` +as an ordinary result and will not apply its segmentation automatically. +Update Girder's pinned VolView package before the producer emits the new name. + +This vocabulary change does not change task-spec versions or saved-session +schemas. Girder projects stored job outputs into current instructions when +results are requested; stored output references and mask provenance keep their +identities. ## Regenerating diff --git a/backend-contract/fixtures/negative/wrong-length-color.json b/backend-contract/fixtures/negative/wrong-length-color.json index f5aa4fee1..cc483faa6 100644 --- a/backend-contract/fixtures/negative/wrong-length-color.json +++ b/backend-contract/fixtures/negative/wrong-length-color.json @@ -1,6 +1,6 @@ { "id": "6600000000000000000000e1", - "intent": "add-segment-group", + "intent": "import-segmentation", "url": "/api/v1/file/6600000000000000000000e1/proxiable/otsu.nii.gz", "name": "otsu.nii.gz", "segments": [ diff --git a/backend-contract/fixtures/wire/intent.add-segment-group.embedded.json b/backend-contract/fixtures/wire/intent.import-segmentation.embedded.json similarity index 88% rename from backend-contract/fixtures/wire/intent.add-segment-group.embedded.json rename to backend-contract/fixtures/wire/intent.import-segmentation.embedded.json index 5c6f74ba9..a0a1da4c5 100644 --- a/backend-contract/fixtures/wire/intent.add-segment-group.embedded.json +++ b/backend-contract/fixtures/wire/intent.import-segmentation.embedded.json @@ -1,6 +1,6 @@ { "id": "6600000000000000000000e2", - "intent": "add-segment-group", + "intent": "import-segmentation", "url": "/api/v1/file/6600000000000000000000e2/proxiable/threshold.seg.nrrd", "name": "threshold.seg.nrrd", "source": { diff --git a/backend-contract/fixtures/wire/intent.add-segment-group.with-segments.json b/backend-contract/fixtures/wire/intent.import-segmentation.with-segments.json similarity index 95% rename from backend-contract/fixtures/wire/intent.add-segment-group.with-segments.json rename to backend-contract/fixtures/wire/intent.import-segmentation.with-segments.json index fa0f30949..87cb530d3 100644 --- a/backend-contract/fixtures/wire/intent.add-segment-group.with-segments.json +++ b/backend-contract/fixtures/wire/intent.import-segmentation.with-segments.json @@ -1,6 +1,6 @@ { "id": "6600000000000000000000e1", - "intent": "add-segment-group", + "intent": "import-segmentation", "url": "/api/v1/file/6600000000000000000000e1/proxiable/otsu.nii.gz", "name": "otsu.nii.gz", "segments": [ diff --git a/backend-contract/generated/job-results.schema.json b/backend-contract/generated/job-results.schema.json index 79b8f1c1f..c1dc3f5bb 100644 --- a/backend-contract/generated/job-results.schema.json +++ b/backend-contract/generated/job-results.schema.json @@ -114,7 +114,7 @@ "properties": { "intent": { "type": "string", - "const": "add-segment-group" + "const": "import-segmentation" }, "id": { "type": "string", @@ -155,10 +155,12 @@ "value": { "type": "integer", "minimum": 1, - "maximum": 9007199254740991 + "maximum": 9007199254740991, + "description": "The label value, from 1 to 65535; 0 is background. The client stores labels in at most 16 bits: voxels holding a larger value import as background with a warning, and a segment declared with one arrives empty." }, "name": { - "type": "string" + "type": "string", + "description": "Binds the segment by exact name. A segment the client already holds under this name, with no mask on the target image yet, takes these voxels and keeps its own color and visibility. Otherwise the client creates a segment from this descriptor, with a numbered name when the existing one already has a mask on that image." }, "color": { "minItems": 4, diff --git a/backend-contract/generated/openapi.json b/backend-contract/generated/openapi.json index 71adfe668..15816ab98 100644 --- a/backend-contract/generated/openapi.json +++ b/backend-contract/generated/openapi.json @@ -3,8 +3,8 @@ "jsonSchemaDialect": "https://json-schema.org/draft/2020-12/schema", "info": { "title": "VolView neutral backend contract", - "version": "0.2.0", - "description": "DRAFT 0.x — shapes may change until a second backend passes the conformance kit (the pinned 1.0 criterion). The neutral REST surface the VolView client calls to run processing tasks against a backend. A conforming server-side BACKEND implements these endpoints and the referenced wire schemas — no VolView client change is needed to bring a new backend online. Everything here is neutral: no backend routes, ids, status enums, or URL shapes leak. The artifact version is the draft artifact version, distinct from the shape versions: the result-intent vocabulary is at version 2 (INTENT_VOCABULARY_VERSION); the task-spec shape at version 1 (specVersion)." + "version": "0.3.0", + "description": "DRAFT 0.x — shapes may change until a second backend passes the conformance kit (the pinned 1.0 criterion). The neutral REST surface the VolView client calls to run processing tasks against a backend. A conforming server-side BACKEND implements these endpoints and the referenced wire schemas — no VolView client change is needed to bring a new backend online. Everything here is neutral: no backend routes, ids, status enums, or URL shapes leak. The artifact version is the draft artifact version, distinct from the shape versions: the result-intent vocabulary is at version 3 (INTENT_VOCABULARY_VERSION); the task-spec shape at version 1 (specVersion)." }, "servers": [ { @@ -347,7 +347,7 @@ ], "responses": { "200": { - "description": "The resolved results as the { resultState, intents, missing } envelope (JobResults). Each entry of `intents` is a ResultIntent — a result row carrying a required `id` display key and required `name`/`url`, plus optional/null `mimeType`/`size` file metadata. `missing` counts declared outputs that never arrived plus recorded outputs that cannot be read. Total loss is a valid incomplete response with an empty intents array.", + "description": "The resolved results as the { resultState, intents, missing } envelope (JobResults). Each entry of `intents` is a ResultIntent — a result row carrying a required `id` display key, unique within the job, and required `name`/`url`, plus optional/null `mimeType`/`size` file metadata. `missing` counts declared outputs that never arrived plus recorded outputs that cannot be read. Total loss is a valid incomplete response with an empty intents array.", "content": { "application/json": { "schema": { @@ -896,7 +896,8 @@ "name", "referenceImage" ], - "additionalProperties": false + "additionalProperties": false, + "description": "The staged bytes are a `.seg.nrrd` labelmap on the reference image's voxel grid. Label values run from 1 to N within each file, in the client's segment order, with 0 as background. Voxels are unsigned 8-bit for up to 255 labels and unsigned 16-bit beyond. Segment names and colors ride in the header." }, { "type": "object", @@ -1203,7 +1204,7 @@ "properties": { "intent": { "type": "string", - "const": "add-segment-group" + "const": "import-segmentation" }, "id": { "type": "string", @@ -1244,10 +1245,12 @@ "value": { "type": "integer", "minimum": 1, - "maximum": 9007199254740991 + "maximum": 9007199254740991, + "description": "The label value, from 1 to 65535; 0 is background. The client stores labels in at most 16 bits: voxels holding a larger value import as background with a warning, and a segment declared with one arrives empty." }, "name": { - "type": "string" + "type": "string", + "description": "Binds the segment by exact name. A segment the client already holds under this name, with no mask on the target image yet, takes these voxels and keeps its own color and visibility. Otherwise the client creates a segment from this descriptor, with a numbered name when the existing one already has a mask on that image." }, "color": { "minItems": 4, diff --git a/backend-contract/generated/result-intent.schema.json b/backend-contract/generated/result-intent.schema.json index e0b27e58b..7892d5210 100644 --- a/backend-contract/generated/result-intent.schema.json +++ b/backend-contract/generated/result-intent.schema.json @@ -102,7 +102,7 @@ "properties": { "intent": { "type": "string", - "const": "add-segment-group" + "const": "import-segmentation" }, "id": { "type": "string", @@ -143,10 +143,12 @@ "value": { "type": "integer", "minimum": 1, - "maximum": 9007199254740991 + "maximum": 9007199254740991, + "description": "The label value, from 1 to 65535; 0 is background. The client stores labels in at most 16 bits: voxels holding a larger value import as background with a warning, and a segment declared with one arrives empty." }, "name": { - "type": "string" + "type": "string", + "description": "Binds the segment by exact name. A segment the client already holds under this name, with no mask on the target image yet, takes these voxels and keeps its own color and visibility. Otherwise the client creates a segment from this descriptor, with a numbered name when the existing one already has a mask on that image." }, "color": { "minItems": 4, diff --git a/backend-contract/generated/stage-input-descriptor.schema.json b/backend-contract/generated/stage-input-descriptor.schema.json index ffa29090a..1cb2af2e1 100644 --- a/backend-contract/generated/stage-input-descriptor.schema.json +++ b/backend-contract/generated/stage-input-descriptor.schema.json @@ -42,7 +42,8 @@ "name", "referenceImage" ], - "additionalProperties": false + "additionalProperties": false, + "description": "The staged bytes are a `.seg.nrrd` labelmap on the reference image's voxel grid. Label values run from 1 to N within each file, in the client's segment order, with 0 as background. Voxels are unsigned 8-bit for up to 255 labels and unsigned 16-bit beyond. Segment names and colors ride in the header." }, { "type": "object", diff --git a/backend-contract/package.json b/backend-contract/package.json index 1afe62c13..f1a07f920 100644 --- a/backend-contract/package.json +++ b/backend-contract/package.json @@ -1,5 +1,5 @@ { "name": "@volview/backend-contract", - "version": "0.2.0", + "version": "0.3.0", "private": true } diff --git a/backend-contract/processing/__tests__/wire.spec.ts b/backend-contract/processing/__tests__/wire.spec.ts index 603c1bed6..488eac5f0 100644 --- a/backend-contract/processing/__tests__/wire.spec.ts +++ b/backend-contract/processing/__tests__/wire.spec.ts @@ -15,6 +15,7 @@ import { jobHistoryDetailSchema, jobResultsSchema, jobResultsErrorSchema, + currentResultIntentName, } from '../wire'; import { loadFixture, loadFixtureDir } from './loadFixtures'; @@ -193,13 +194,28 @@ describe('neutral job status fixtures', () => { // Result intents // --------------------------------------------------------------------------- +describe('currentResultIntentName', () => { + it('reads the earlier segmentation import name as the current one', () => { + expect(currentResultIntentName('add-segment-group')).toBe( + 'import-segmentation' + ); + }); + + it.each(['toString', 'constructor', '__proto__', 'add-mesh'])( + 'passes %s through unchanged', + (intent) => { + expect(currentResultIntentName(intent)).toBe(intent); + } + ); +}); + describe('result intent fixtures', () => { - it('exports vocabulary version 2 and the exactly-four state intents', () => { - expect(INTENT_VOCABULARY_VERSION).toBe(2); + it('exports vocabulary version 3 and the exactly-four state intents', () => { + expect(INTENT_VOCABULARY_VERSION).toBe(3); expect([...RESULT_INTENTS]).toEqual([ 'add-base-image', 'add-layer', - 'add-segment-group', + 'import-segmentation', 'add-annotations', ]); expect(wire).not.toHaveProperty('intent.download'); @@ -208,8 +224,8 @@ describe('result intent fixtures', () => { it.each([ 'intent.add-base-image', 'intent.add-layer', - 'intent.add-segment-group.with-segments', - 'intent.add-segment-group.embedded', + 'intent.import-segmentation.with-segments', + 'intent.import-segmentation.embedded', 'intent.add-annotations', 'intent.unknown', ])('validates %s', (name) => { @@ -253,11 +269,11 @@ describe('result intent fixtures', () => { ).toBe(false); }); - it('parses add-segment-group WITH segments and a source provenance tag', () => { + it('parses import-segmentation WITH segments and a source provenance tag', () => { const parsed = resultIntentSchema.parse( - wire['intent.add-segment-group.with-segments'] + wire['intent.import-segmentation.with-segments'] ) as Record; - expect(parsed.intent).toBe('add-segment-group'); + expect(parsed.intent).toBe('import-segmentation'); expect(Array.isArray(parsed.segments)).toBe(true); expect(parsed.source).toEqual({ providerId: 'analysis-provider', @@ -266,18 +282,18 @@ describe('result intent fixtures', () => { }); }); - it('parses add-segment-group WITHOUT segments (embedded metadata) but with source', () => { + it('parses import-segmentation WITHOUT segments (embedded metadata) but with source', () => { const parsed = resultIntentSchema.parse( - wire['intent.add-segment-group.embedded'] + wire['intent.import-segmentation.embedded'] ) as Record; - expect(parsed.intent).toBe('add-segment-group'); + expect(parsed.intent).toBe('import-segmentation'); expect(parsed.segments).toBeUndefined(); expect(parsed.source).toMatchObject({ outputId: 'outputLabelmap' }); }); - it('rejects a segment-group source without provider identity', () => { + it('rejects a segmentation source without provider identity', () => { const value = structuredClone( - wire['intent.add-segment-group.with-segments'] + wire['intent.import-segmentation.with-segments'] ) as { source: { providerId?: string } }; delete value.source.providerId; expect(knownResultIntentSchema.safeParse(value).success).toBe(false); @@ -364,16 +380,14 @@ describe('result intent fixtures', () => { }); it('rejects a wrong-length segment color (the tuple-length parity pin)', () => { - // The negative fixture carries a 3-element color. The STRICT union must - // reject it — and the generated JSON Schema must agree (backend side: - // test_contract_fixtures.py), so both validators close fixed-length - // tuples identically. The full union still accepts the row, demoted to an - // ordinary result with no state action (the designed fail-open). + // Only the strict known-intent union rejects the 3-element color; the + // published result-intent schema accepts the row and demotes it to an + // ordinary result with no state action. const short = loadFixture('negative/wrong-length-color.json'); expect(knownResultIntentSchema.safeParse(short).success).toBe(false); expect(resultIntentSchema.safeParse(short).success).toBe(true); - const good = wire['intent.add-segment-group.with-segments'] as { + const good = wire['intent.import-segmentation.with-segments'] as { segments: { color: number[] }[]; }; const long = structuredClone(good); diff --git a/backend-contract/processing/annotations.ts b/backend-contract/processing/annotations.ts index 07588353c..431c221e5 100644 --- a/backend-contract/processing/annotations.ts +++ b/backend-contract/processing/annotations.ts @@ -1,6 +1,9 @@ // Vector annotations staged into a task or returned as a result. Coordinates -// are world LPS millimeters. Tool records exclude session identity and state; -// labels are namespaced by tool kind because the client stores are independent. +// are world LPS millimeters. Tool records exclude session identity and state. +// Labels are namespaced by tool kind on the wire, but the client binds every +// kind into one segment registry by name: a name repeated across kinds is one +// segment, the first kind to bind a new name (rulers, rectangles, polygons) +// sets its style, and a label no tool references creates nothing. // Unknown envelope fields survive round-trip without gaining behavior. import { z } from 'zod'; diff --git a/backend-contract/processing/openapi.ts b/backend-contract/processing/openapi.ts index 8a62458d6..0615ff1a7 100644 --- a/backend-contract/processing/openapi.ts +++ b/backend-contract/processing/openapi.ts @@ -429,11 +429,11 @@ const paths = (): Record => ({ description: 'The resolved results as the { resultState, intents, missing } envelope ' + '(JobResults). Each entry of `intents` is a ResultIntent — a result ' + - 'row carrying a required `id` display key and required `name`/`url`, ' + - 'plus optional/null `mimeType`/`size` file metadata. `missing` counts ' + - 'declared outputs that never arrived plus recorded outputs that cannot ' + - 'be read. Total loss is a valid incomplete response with an empty ' + - 'intents array.', + 'row carrying a required `id` display key, unique within the job, and ' + + 'required `name`/`url`, plus optional/null `mimeType`/`size` file ' + + 'metadata. `missing` counts declared outputs that never arrived plus ' + + 'recorded outputs that cannot be read. Total loss is a valid ' + + 'incomplete response with an empty intents array.', content: json(ref('JobResults')), }, '409': resultReadErrorResponse, @@ -474,7 +474,7 @@ export const buildOpenApiDocument = (): Record => ({ // VERSION / specVersion) below. It is deliberately literal, not derived from // the shape-version constants — the artifact and the shapes version on // separate clocks. - version: '0.2.0', + version: '0.3.0', description: 'DRAFT 0.x — shapes may change until a second backend passes the ' + 'conformance kit (the pinned 1.0 criterion). ' + diff --git a/backend-contract/processing/wire.ts b/backend-contract/processing/wire.ts index 27c2d41ba..df394d50b 100644 --- a/backend-contract/processing/wire.ts +++ b/backend-contract/processing/wire.ts @@ -18,10 +18,9 @@ import { } from './task-spec'; import { pathSegmentIdSchema } from './ids'; -// Bump when the intent vocabulary's shape changes so producers and the applier -// can negotiate compatibility. Adding an intent is a compatible bump: an older -// client demotes the unknown intent through the fail-open branch above. -export const INTENT_VOCABULARY_VERSION = 2; +// Names the intent vocabulary's shape in the OpenAPI text; never sent. An older +// client demotes an unknown intent through `resultIntentSchema` below. +export const INTENT_VOCABULARY_VERSION = 3; // --------------------------------------------------------------------------- // Input value: what the client sends at submit @@ -59,11 +58,15 @@ const stagedDescriptorCommon = { referenceImage: stagedReferenceImageSchema, }; -// A parent-bound labelmap: the segment-group bytes overlaying the image. -const stageLabelmapDescriptorSchema = z.strictObject({ - type: z.literal(TYPE_TAG_LABELMAP), - ...stagedDescriptorCommon, -}); +// A parent-bound labelmap: the segmentation bytes overlaying the image. +const stageLabelmapDescriptorSchema = z + .strictObject({ + type: z.literal(TYPE_TAG_LABELMAP), + ...stagedDescriptorCommon, + }) + .describe( + "The staged bytes are a `.seg.nrrd` labelmap on the reference image's voxel grid. Label values run from 1 to N within each file, in the client's segment order, with 0 as background. Voxels are unsigned 8-bit for up to 255 labels and unsigned 16-bit beyond. Segment names and colors ride in the header." + ); // A parent-bound annotations file: the vector annotations (rulers, rectangles, // polygons) drawn on the image, as the `annotations.ts` interchange format. @@ -158,11 +161,22 @@ export type NeutralJobStatus = z.infer; export const RESULT_INTENTS = [ 'add-base-image', 'add-layer', - 'add-segment-group', + 'import-segmentation', 'add-annotations', ] as const; export type ResultIntentName = (typeof RESULT_INTENTS)[number]; +// Names an earlier vocabulary gave an intent whose shape has not changed since. +// A client reads one as its current name, so a producer still on the old +// vocabulary keeps applying and the two sides need not deploy in lockstep. +// A Map, so a wire string naming an Object.prototype key resolves to nothing. +export const LEGACY_RESULT_INTENT_NAMES: ReadonlyMap = + new Map([['add-segment-group', 'import-segmentation']]); + +export const currentResultIntentName = (intent: unknown) => + (typeof intent === 'string' && LEGACY_RESULT_INTENT_NAMES.get(intent)) || + intent; + // Provenance tag on a result: the durable idempotency identity the client // preserves on generated scene state so restored results can be recognized. export const resultSourceSchema = z.object({ @@ -177,8 +191,18 @@ const colorChannel = z.number().int().min(0).max(255); // A segment descriptor: `value` is a label index >= 1 (0 is reserved // background), `color` is RGBA 0-255. export const segmentDescriptorSchema = z.object({ - value: z.number().int().min(1), - name: z.string(), + value: z + .number() + .int() + .min(1) + .describe( + 'The label value, from 1 to 65535; 0 is background. The client stores labels in at most 16 bits: voxels holding a larger value import as background with a warning, and a segment declared with one arrives empty.' + ), + name: z + .string() + .describe( + 'Binds the segment by exact name. A segment the client already holds under this name, with no mask on the target image yet, takes these voxels and keeps its own color and visibility. Otherwise the client creates a segment from this descriptor, with a numbered name when the existing one already has a mask on that image.' + ), color: z.tuple([colorChannel, colorChannel, colorChannel, colorChannel]), visible: z.boolean().optional(), }); @@ -186,9 +210,10 @@ export type SegmentDescriptor = z.infer; // The ONE canonical result-list-item shape, shared by every producer, the // client, the generated OpenAPI, the fixtures, and the backend copy. `id` is the -// display key (required, nonempty); `name`/`url` are required; `mimeType`/`size` -// are advisory file metadata that may be null. Every intent branch is built FROM -// this shape, so there is no payload the contract accepts but the client rejects. +// display key, unique within the job (required, nonempty); `name`/`url` are +// required; `mimeType`/`size` are advisory file metadata that may be null. Every +// intent branch is built FROM this shape, so there is no payload the contract +// accepts but the client rejects. export const resultListItemSchema = z.object({ id: z.string().min(1), name: z.string(), @@ -212,13 +237,13 @@ const addLayer = z .object({ intent: z.literal('add-layer'), ...resultListItemSchema.shape }) .passthrough(); -// `add-segment-group` carries OPTIONAL `segments` (the bare-labelmap + +// `import-segmentation` carries OPTIONAL `segments` (the bare-labelmap + // labels-sidecar case; a `seg.nrrd` with embedded metadata carries none — the // client uses `segments` when present, else the file's own metadata) and an // optional `source` provenance tag (the idempotency key). -const addSegmentGroup = z +const importSegmentation = z .object({ - intent: z.literal('add-segment-group'), + intent: z.literal('import-segmentation'), ...resultListItemSchema.shape, segments: z.array(segmentDescriptorSchema).optional(), source: resultSourceSchema.optional(), @@ -244,7 +269,7 @@ const addAnnotations = z export const knownResultIntentSchema = z.discriminatedUnion('intent', [ addBaseImage, addLayer, - addSegmentGroup, + importSegmentation, addAnnotations, ]); diff --git a/docs/configuration_file.md b/docs/configuration_file.md index 177890c4d..a5fe2da20 100644 --- a/docs/configuration_file.md +++ b/docs/configuration_file.md @@ -167,16 +167,31 @@ visibility and lock state. } ``` -Fields: `color`, `fillOpacity`, `outlineOpacity`, `strokeWidth`. +Fields: -Omitting the key leaves the registry alone. An empty record (`{}`) or `null` clears what -an earlier config contributed, keeping any segment your content still references with its -last configured appearance. A configured segment keeps its id across config changes, so -renaming or recoloring one never detaches the masks and shapes that reference it. +- `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. -### Pre-7.0 `labels` +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 pre-7.0 `labels` section is converted into `segments` at configuration ingestion, with +Entries are keyed by name. A configured segment keeps its id while its key stays the +same, so recoloring it never detaches its masks and shapes. Changing a key moves the +entry to the segment of the new name, adding one if none exists, and drops the old entry +as above. Applying a config again restores each key's name and configured appearance, +undoing a rename made in the app. + +Configured segments outlive the images. Removing the last image deletes every other +segment and keeps these. + +### 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 @@ -200,13 +215,13 @@ Converting a config by hand: } ``` -becomes +becomes, with the segments in the order VolView reads them: ```json { "segments": { - "lesion": { "color": "#ff0000" }, - "big": { "color": "#ff0000" } + "big": { "color": "#ff0000" }, + "lesion": { "color": "#ff0000" } } } ``` @@ -224,10 +239,8 @@ VolView will include in the volview.zip file. } ``` -The legacy `io.segmentGroupSaveFormat` key is migrated at ingestion. Matching -old and new values are accepted; conflicting values are rejected. This setting -controls mask files inside saved sessions, independently of the explicit -segmentation export dialog. Existing saved-session encodings remain readable. +This setting controls mask files inside saved sessions, independently of the +explicit segmentation export dialog. Working mask file formats: @@ -240,7 +253,7 @@ Example: `base.[extension].nrrd` will match `base.nii`. The extension must appear anywhere in the filename after splitting by dots, and the filename must start with the same prefix as the base image (everything before the first dot). Files matching `base.[extension]...` will be associated with a base image named `base.*`. -**Ordering:** When multiple layers/segmentations match a base image, they are sorted alphabetically by filename and added to the stack in that order. To control the stacking order explicitly, you could use numeric prefixes in your filenames. +**Ordering:** When multiple layers match a base image, they are sorted alphabetically by filename and added to the stack in that order. To control the stacking order explicitly, you could use numeric prefixes in your filenames. Matching segmentation files each add their labels as segments to the base image's one segmentation. For example, with a base image `patient001.nrrd`: @@ -249,23 +262,10 @@ For example, with a base image `patient001.nrrd`: Both features default to `''` which disables them. -### Configuration migration - -Use `io.segmentationExtension` in new configuration. The old -`io.segmentGroupExtension` key is accepted at ingestion and converted to the -new key. If both keys are present, their values must match; conflicting values -are rejected. An explicit empty string disables automatic matching. - -The value `seg` is the filename marker in `patient.seg.nii.gz`; `nii.gz` is -its encoding extension. This setting preserves the existing filename matching -rule and does not add support for additional segmentation formats. - -Directly loading an old key in VolView also reports a deprecation warning. - ### Segmentations -Use `segmentationExtension` to automatically convert matching non-DICOM images to segmentations. -For example, `myFile.seg.nrrd` becomes a segmentation for `myFile.nii`. +Use `segmentationExtension` to automatically import the labels of matching non-DICOM images as segments. +For example, `myFile.seg.nrrd` adds its labels as segments on `myFile.nii`. Defaults to `''` which disables matching. ```json @@ -289,9 +289,16 @@ Defaults to `''` which disables matching. } ``` +## Renamed `io` Keys + +The keys `io.segmentGroupExtension` and `io.segmentGroupSaveFormat` are still read, +converted to `io.segmentationExtension` and `io.segmentationSaveFormat`, and reported +with a deprecation warning. If both spellings of a key are present, their values must +match or the configuration is rejected. + ## Keyboard Shortcuts -Configure the keys to activate tools, change selected labels, and more. +Configure the keys to activate tools, change the selected segment, and more. All [shortcut actions](https://github.com/Kitware/VolView/blob/main/src/constants.ts#L53) are under the `ACTIONS` variable. To configure a key for an action, add its action name and the key(s) under the `shortcuts` section. For key combinations, use `+` like `Ctrl+f`. diff --git a/docs/quick_start_guide.md b/docs/quick_start_guide.md index 39a5fe7dd..c4baa90de 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 and Ruler: When the ruler tool selected, the left mouse button is used to place and adjust ruler end-markers. Right clicking on a end-marker displays a pop-up menu for deleting that ruler. Switch to the "Annotations" tab to see a list of annotations made to currently loaded data. Select the location icon next to a listed ruler to jump to its slice. Select the trashcan to delete that ruler. When the paint tool is selected, you can paint in any 2D window. Click on the paint tool a second time to bring up a menu of colors and adjust the brush size. Note: Segment groups and measurements are only visible in the UI when their associated base image's view is selected. ![Paint and Ruler](./assets/11-volview-paint-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) - 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) @@ -66,4 +66,4 @@ VolView reads the DICOM tags of your data to determine appropriate preset parame ## 4. Saving / loading state -Once you have made the measures and generated the visualizations that you want to store to recall later or share with others, use the icons at the top of the toolbar to Load and Save state files. For more information on the json format of these state files and how they can be used to integrate VolView with workflows and other services, see [State Files](state_files.html). +Use the Load and Save icons in the toolbar to reopen or save your images, annotations, and view settings. See [State Files](./state_files.md) for details on saving and sharing your work. diff --git a/docs/state_files.md b/docs/state_files.md index b34e00faf..050ccc6a5 100644 --- a/docs/state_files.md +++ b/docs/state_files.md @@ -1,136 +1,17 @@ # State Files -VolView state files save your scene configuration: annotations, camera positions, colormaps, layouts, and more. There are two formats: +State files save your scene so you can return to your work later or share it with others. They preserve annotations, segmentations, camera positions, colormaps, layouts, and other view settings. -## Zip State Files (`*.volview.zip`) +## Saving a Scene -Save by clicking the "Disk" icon in the toolbar. This embeds your image data that was loaded from local files alongside the application state. Useful for sharing annotations with collaborators. +Click the **Save** (disk) icon in the toolbar to download a `*.volview.zip` file. Images loaded from local files are included alongside the saved scene. -## Sparse Manifest Files (`*.volview.json`) +## Linked State Files -JSON files that reference remote data via URIs instead of embedding it. Useful for: +A `*.volview.json` file references data hosted on a server instead of including it in the file. This lets workflows and external applications open a prepared scene without copying large datasets. Anyone opening the scene needs access to the referenced data. -- Linking to data hosted on servers -- Sharing annotations without duplicating large datasets -- Integrating with external systems (AI pipelines, access control, etc.) +## Loading a Saved Scene -### Current manifest (version 7.0.0) - -A segmentation owns one image's segment masks; labelmaps encode those masks for -storage or interchange. The top-level `segments` list holds the identities the -masks paint (name, color, visibility), each mask names the segment it carries -voxels for, and `order` lists the masks of that segmentation. - -A mask saved into a zip names its own archive entry with `path`. A sparse -manifest instead points at a whole label volume: `segmentationArtifacts` names -that volume, its `dataSourceId` says where the bytes come from, and each mask -whose `artifactId` points at it is filled from the `sourceValue` it declares. -An artifact is a single-component label volume; one with several components is -skipped on restore. Extents are placeholders until the volume is read. - -```json -{ - "version": "7.0.0", - "dataSources": [ - { "id": 0, "type": "uri", "uri": "https://example.com/scan.zip" }, - { "id": 1, "type": "uri", "uri": "https://example.com/segmentation.nii.gz" } - ], - "segments": [ - { - "id": "segment-tumor", - "name": "Tumor", - "color": [255, 0, 0, 255], - "visible": true, - "locked": false - } - ], - "segmentations": [ - { - "id": "segmentation-0", - "name": "Tumor Segmentation", - "parentImage": "0", - "masks": [ - { - "id": "mask-tumor", - "segmentId": "segment-tumor", - "representations": { - "labelmap": { - "artifactId": "labelmap-1", - "sourceValue": 1, - "extent": [0, -1, 0, -1, 0, -1] - } - } - } - ], - "order": ["mask-tumor"] - } - ], - "segmentationArtifacts": [ - { - "id": "labelmap-1", - "parentImage": "0", - "name": "Tumor Segmentation", - "dataSourceId": 1 - } - ], - "selectedSegment": "segment-tumor" -} -``` - -### Legacy 6.2.0 manifest (the pre-7.0.0 form, still read on import) - -The historical `segmentGroups` field is migrated into the current segmentation -model on load, and the per-tool `labels` records become segments the tools -reference by id. Nothing writes this form any more. - -```json -{ - "version": "6.2.0", - "dataSources": [ - { "id": 0, "type": "uri", "uri": "https://example.com/scan.zip" }, - { "id": 1, "type": "uri", "uri": "https://example.com/segmentation.nii.gz" } - ], - "segmentGroups": [ - { - "id": "seg-1", - "dataSourceId": 1, - "metadata": { - "name": "Tumor Segmentation", - "parentImage": "0", - "segments": { - "order": [1], - "byValue": { - "1": { "value": 1, "name": "Tumor", "color": [255, 0, 0, 255] } - } - } - } - } - ], - "tools": { - "rectangles": { - "tools": [ - { - "imageID": "0", - "frameOfReference": { - "planeNormal": [0, 0, 1], - "planeOrigin": [0, 0, 50] - }, - "slice": 50, - "firstPoint": [-20, -20, 50], - "secondPoint": [20, 20, 50], - "label": "lesion" - } - ], - "labels": { - "lesion": { "color": "red" } - } - } - } -} -``` - -## Loading State Files - -- **Drag and drop** onto VolView -- **File browser** via the "Folder" icon below the save button -- **URL parameter**: `?urls=[https://example.com/session.volview.json]` +- Drag and drop a state file onto VolView. +- Click the **Load** (folder) icon in the toolbar and select a state file. +- Open a link that includes a state file in the `urls` parameter, such as `?urls=[https://example.com/session.volview.json]`. diff --git a/docs/toolbar.md b/docs/toolbar.md index 5067a16dd..5d9837135 100644 --- a/docs/toolbar.md +++ b/docs/toolbar.md @@ -53,6 +53,9 @@ its voxels: painting goes around it. Turn on "Allow Overlap" to paint over other segments without taking anything from them, so the segments overlap. The same rules apply when a polygon is rasterized. +Erasing or painting over everything a segment holds on an image removes its mask +there. The segment stays in the list, with nothing on that image to reveal or save. + ### Rectangle When the rectangle tool is selected, the left mouse button is used to place and adjust rectangle control points. diff --git a/package-lock.json b/package-lock.json index 95d2d2632..ca1fdb4ef 100644 --- a/package-lock.json +++ b/package-lock.json @@ -25,7 +25,6 @@ "@thi.ng/rasterize": "^1.0.171", "@types/color-name": "^1.1.5", "@types/cors": "^2.8.19", - "@types/deep-equal": "^1.0.4", "@types/express": "^5.0.5", "@types/file-saver": "^2.0.7", "@types/mocha": "^10.0.10", @@ -46,7 +45,6 @@ "core-js": "3.47.0", "cors": "^2.8.5", "cross-env": "^10.1.0", - "deep-equal": "^2.2.3", "dicom-parser": "^1.8.21", "dicomweb-client-typed": "^0.8.6", "eslint": "^9.39.1", @@ -5178,13 +5176,6 @@ "dev": true, "license": "MIT" }, - "node_modules/@types/deep-equal": { - "version": "1.0.4", - "resolved": "https://registry.npmjs.org/@types/deep-equal/-/deep-equal-1.0.4.tgz", - "integrity": "sha512-tqdiS4otQP4KmY0PR3u6KbZ5EWvhNdUoS/jc93UuK23C220lOZ/9TvjfxdPcKvqwwDVtmtSCrnr0p/2dirAxkA==", - "dev": true, - "license": "MIT" - }, "node_modules/@types/emscripten": { "version": "1.41.5", "resolved": "https://registry.npmjs.org/@types/emscripten/-/emscripten-1.41.5.tgz", @@ -7360,23 +7351,6 @@ "node": ">= 0.4" } }, - "node_modules/array-buffer-byte-length": { - "version": "1.0.2", - "resolved": "https://registry.npmjs.org/array-buffer-byte-length/-/array-buffer-byte-length-1.0.2.tgz", - "integrity": "sha512-LHE+8BuR7RYGDKvnrmcuSq3tDcKv9OFEXQt/HpbZhY7V6h0zlUXutnAD82GiFx9rdieCMjkvtcsPqBwgUl1Iiw==", - "dev": true, - "license": "MIT", - "dependencies": { - "call-bound": "^1.0.3", - "is-array-buffer": "^3.0.5" - }, - "engines": { - "node": ">= 0.4" - }, - "funding": { - "url": "https://github.com/sponsors/ljharb" - } - }, "node_modules/array-ify": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/array-ify/-/array-ify-1.0.0.tgz", @@ -9832,39 +9806,6 @@ "node": ">=6" } }, - "node_modules/deep-equal": { - "version": "2.2.3", - "resolved": "https://registry.npmjs.org/deep-equal/-/deep-equal-2.2.3.tgz", - "integrity": "sha512-ZIwpnevOurS8bpT4192sqAowWM76JDKSHYzMLty3BZGSswgq6pBaH3DhCSW5xVAZICZyKdOBPjwww5wfgT/6PA==", - "dev": true, - "license": "MIT", - "dependencies": { - "array-buffer-byte-length": "^1.0.0", - "call-bind": "^1.0.5", - "es-get-iterator": "^1.1.3", - "get-intrinsic": "^1.2.2", - "is-arguments": "^1.1.1", - "is-array-buffer": "^3.0.2", - "is-date-object": "^1.0.5", - "is-regex": "^1.1.4", - "is-shared-array-buffer": "^1.0.2", - "isarray": "^2.0.5", - "object-is": "^1.1.5", - "object-keys": "^1.1.1", - "object.assign": "^4.1.4", - "regexp.prototype.flags": "^1.5.1", - "side-channel": "^1.0.4", - "which-boxed-primitive": "^1.0.2", - "which-collection": "^1.0.1", - "which-typed-array": "^1.1.13" - }, - "engines": { - "node": ">= 0.4" - }, - "funding": { - "url": "https://github.com/sponsors/ljharb" - } - }, "node_modules/deep-is": { "version": "0.1.4", "resolved": "https://registry.npmjs.org/deep-is/-/deep-is-0.1.4.tgz", @@ -9934,24 +9875,6 @@ "node": ">=8" } }, - "node_modules/define-properties": { - "version": "1.2.1", - "resolved": "https://registry.npmjs.org/define-properties/-/define-properties-1.2.1.tgz", - "integrity": "sha512-8QmQKqEASLd5nx0U1B1okLElbUuuttJ/AnYmRXbbbGDWh6uS208EjD4Xqq/I9wK7u0v6O08XhTWnt5XtEbR6Dg==", - "dev": true, - "license": "MIT", - "dependencies": { - "define-data-property": "^1.0.1", - "has-property-descriptors": "^1.0.0", - "object-keys": "^1.1.1" - }, - "engines": { - "node": ">= 0.4" - }, - "funding": { - "url": "https://github.com/sponsors/ljharb" - } - }, "node_modules/degenerator": { "version": "5.0.1", "resolved": "https://registry.npmjs.org/degenerator/-/degenerator-5.0.1.tgz", @@ -10526,27 +10449,6 @@ "node": ">= 0.4" } }, - "node_modules/es-get-iterator": { - "version": "1.1.3", - "resolved": "https://registry.npmjs.org/es-get-iterator/-/es-get-iterator-1.1.3.tgz", - "integrity": "sha512-sPZmqHBe6JIiTfN5q2pEi//TwxmAFHwj/XEuYjTuse78i8KxaqMTTzxPoFKuzRpDpTJ+0NAbpfenkmH2rePtuw==", - "dev": true, - "license": "MIT", - "dependencies": { - "call-bind": "^1.0.2", - "get-intrinsic": "^1.1.3", - "has-symbols": "^1.0.3", - "is-arguments": "^1.1.1", - "is-map": "^2.0.2", - "is-set": "^2.0.2", - "is-string": "^1.0.7", - "isarray": "^2.0.5", - "stop-iteration-iterator": "^1.0.0" - }, - "funding": { - "url": "https://github.com/sponsors/ljharb" - } - }, "node_modules/es-module-lexer": { "version": "2.1.0", "resolved": "https://registry.npmjs.org/es-module-lexer/-/es-module-lexer-2.1.0.tgz", @@ -11849,16 +11751,6 @@ "url": "https://github.com/sponsors/ljharb" } }, - "node_modules/functions-have-names": { - "version": "1.2.3", - "resolved": "https://registry.npmjs.org/functions-have-names/-/functions-have-names-1.2.3.tgz", - "integrity": "sha512-xckBUXyTIqT97tq2x2AMb+g163b5JFysYk0x4qxNFwbfQkmNZoiRHb6sPzI9/QV33WeuvVYBUIiD4NzNIyqaRQ==", - "dev": true, - "license": "MIT", - "funding": { - "url": "https://github.com/sponsors/ljharb" - } - }, "node_modules/geckodriver": { "version": "6.1.1", "resolved": "https://registry.npmjs.org/geckodriver/-/geckodriver-6.1.1.tgz", @@ -12205,19 +12097,6 @@ "node": ">=20.0.0" } }, - "node_modules/has-bigints": { - "version": "1.1.0", - "resolved": "https://registry.npmjs.org/has-bigints/-/has-bigints-1.1.0.tgz", - "integrity": "sha512-R3pbpkcIqv2Pm3dUwgjclDRVmWpTJW2DcMzcIhEXEx1oh/CEMObMm3KLmRJOdvhM7o4uQBnwr8pzRK2sJWIqfg==", - "dev": true, - "license": "MIT", - "engines": { - "node": ">= 0.4" - }, - "funding": { - "url": "https://github.com/sponsors/ljharb" - } - }, "node_modules/has-flag": { "version": "4.0.0", "resolved": "https://registry.npmjs.org/has-flag/-/has-flag-4.0.0.tgz", @@ -12740,21 +12619,6 @@ "dev": true, "license": "Apache-2.0 OR MIT" }, - "node_modules/internal-slot": { - "version": "1.1.0", - "resolved": "https://registry.npmjs.org/internal-slot/-/internal-slot-1.1.0.tgz", - "integrity": "sha512-4gd7VpWNQNB4UKKCFFVcp1AVv+FMOgs9NKzjHKusc8jTMhd5eL1NqQqOpE0KzMds804/yHlglp3uxgluOqAPLw==", - "dev": true, - "license": "MIT", - "dependencies": { - "es-errors": "^1.3.0", - "hasown": "^2.0.2", - "side-channel": "^1.1.0" - }, - "engines": { - "node": ">= 0.4" - } - }, "node_modules/internmap": { "version": "2.0.3", "resolved": "https://registry.npmjs.org/internmap/-/internmap-2.0.3.tgz", @@ -12879,41 +12743,6 @@ "progress-events": "^1.0.1" } }, - "node_modules/is-arguments": { - "version": "1.2.0", - "resolved": "https://registry.npmjs.org/is-arguments/-/is-arguments-1.2.0.tgz", - "integrity": "sha512-7bVbi0huj/wrIAOzb8U1aszg9kdi3KN/CyU19CTI7tAoZYEZoL9yCDXpbXN+uPsuWnP02cyug1gleqq+TU+YCA==", - "dev": true, - "license": "MIT", - "dependencies": { - "call-bound": "^1.0.2", - "has-tostringtag": "^1.0.2" - }, - "engines": { - "node": ">= 0.4" - }, - "funding": { - "url": "https://github.com/sponsors/ljharb" - } - }, - "node_modules/is-array-buffer": { - "version": "3.0.5", - "resolved": "https://registry.npmjs.org/is-array-buffer/-/is-array-buffer-3.0.5.tgz", - "integrity": "sha512-DDfANUiiG2wC1qawP66qlTugJeL5HyzMpfr8lLK+jMQirGzNod0B12cFB/9q838Ru27sBwfw78/rdoU7RERz6A==", - "dev": true, - "license": "MIT", - "dependencies": { - "call-bind": "^1.0.8", - "call-bound": "^1.0.3", - "get-intrinsic": "^1.2.6" - }, - "engines": { - "node": ">= 0.4" - }, - "funding": { - "url": "https://github.com/sponsors/ljharb" - } - }, "node_modules/is-arrayish": { "version": "0.2.1", "resolved": "https://registry.npmjs.org/is-arrayish/-/is-arrayish-0.2.1.tgz", @@ -12921,22 +12750,6 @@ "dev": true, "license": "MIT" }, - "node_modules/is-bigint": { - "version": "1.1.0", - "resolved": "https://registry.npmjs.org/is-bigint/-/is-bigint-1.1.0.tgz", - "integrity": "sha512-n4ZT37wG78iz03xPRKJrHTdZbe3IicyucEtdRsV5yglwc3GyUfbAfpSeD0FJ41NbUNSt5wbhqfp1fS+BgnvDFQ==", - "dev": true, - "license": "MIT", - "dependencies": { - "has-bigints": "^1.0.2" - }, - "engines": { - "node": ">= 0.4" - }, - "funding": { - "url": "https://github.com/sponsors/ljharb" - } - }, "node_modules/is-binary-path": { "version": "2.1.0", "resolved": "https://registry.npmjs.org/is-binary-path/-/is-binary-path-2.1.0.tgz", @@ -12950,23 +12763,6 @@ "node": ">=8" } }, - "node_modules/is-boolean-object": { - "version": "1.2.2", - "resolved": "https://registry.npmjs.org/is-boolean-object/-/is-boolean-object-1.2.2.tgz", - "integrity": "sha512-wa56o2/ElJMYqjCjGkXri7it5FbebW5usLw/nPmCMs5DeZ7eziSYZhSmPRn0txqeW4LnAmQQU7FgqLpsEFKM4A==", - "dev": true, - "license": "MIT", - "dependencies": { - "call-bound": "^1.0.3", - "has-tostringtag": "^1.0.2" - }, - "engines": { - "node": ">= 0.4" - }, - "funding": { - "url": "https://github.com/sponsors/ljharb" - } - }, "node_modules/is-callable": { "version": "1.2.7", "resolved": "https://registry.npmjs.org/is-callable/-/is-callable-1.2.7.tgz", @@ -13016,23 +12812,6 @@ "url": "https://github.com/sponsors/ljharb" } }, - "node_modules/is-date-object": { - "version": "1.1.0", - "resolved": "https://registry.npmjs.org/is-date-object/-/is-date-object-1.1.0.tgz", - "integrity": "sha512-PwwhEakHVKTdRNVOw+/Gyh0+MzlCl4R6qKvkhuvLtPMggI1WAHt9sOwZxQLSGpUaDnrdyDsomoRgNnCfKNSXXg==", - "dev": true, - "license": "MIT", - "dependencies": { - "call-bound": "^1.0.2", - "has-tostringtag": "^1.0.2" - }, - "engines": { - "node": ">= 0.4" - }, - "funding": { - "url": "https://github.com/sponsors/ljharb" - } - }, "node_modules/is-docker": { "version": "2.2.1", "resolved": "https://registry.npmjs.org/is-docker/-/is-docker-2.2.1.tgz", @@ -13088,19 +12867,6 @@ "node": ">=0.10.0" } }, - "node_modules/is-map": { - "version": "2.0.3", - "resolved": "https://registry.npmjs.org/is-map/-/is-map-2.0.3.tgz", - "integrity": "sha512-1Qed0/Hr2m+YqxnM09CjA2d/i6YZNfF6R2oRAOj36eUdS6qIV/huPJNSEpKbupewFs+ZsJlxsjjPbc0/afW6Lw==", - "dev": true, - "license": "MIT", - "engines": { - "node": ">= 0.4" - }, - "funding": { - "url": "https://github.com/sponsors/ljharb" - } - }, "node_modules/is-natural-number": { "version": "4.0.1", "resolved": "https://registry.npmjs.org/is-natural-number/-/is-natural-number-4.0.1.tgz", @@ -13118,23 +12884,6 @@ "node": ">=0.12.0" } }, - "node_modules/is-number-object": { - "version": "1.1.1", - "resolved": "https://registry.npmjs.org/is-number-object/-/is-number-object-1.1.1.tgz", - "integrity": "sha512-lZhclumE1G6VYD8VHe35wFaIif+CTy5SJIi5+3y4psDgWu4wPDoBhF8NxUOinEc7pHgiTsT6MaBb92rKhhD+Xw==", - "dev": true, - "license": "MIT", - "dependencies": { - "call-bound": "^1.0.3", - "has-tostringtag": "^1.0.2" - }, - "engines": { - "node": ">= 0.4" - }, - "funding": { - "url": "https://github.com/sponsors/ljharb" - } - }, "node_modules/is-obj": { "version": "2.0.0", "resolved": "https://registry.npmjs.org/is-obj/-/is-obj-2.0.0.tgz", @@ -13175,54 +12924,6 @@ "dev": true, "license": "MIT" }, - "node_modules/is-regex": { - "version": "1.2.1", - "resolved": "https://registry.npmjs.org/is-regex/-/is-regex-1.2.1.tgz", - "integrity": "sha512-MjYsKHO5O7mCsmRGxWcLWheFqN9DJ/2TmngvjKXihe6efViPqc274+Fx/4fYj/r03+ESvBdTXK0V6tA3rgez1g==", - "dev": true, - "license": "MIT", - "dependencies": { - "call-bound": "^1.0.2", - "gopd": "^1.2.0", - "has-tostringtag": "^1.0.2", - "hasown": "^2.0.2" - }, - "engines": { - "node": ">= 0.4" - }, - "funding": { - "url": "https://github.com/sponsors/ljharb" - } - }, - "node_modules/is-set": { - "version": "2.0.3", - "resolved": "https://registry.npmjs.org/is-set/-/is-set-2.0.3.tgz", - "integrity": "sha512-iPAjerrse27/ygGLxw+EBR9agv9Y6uLeYVJMu+QNCoouJ1/1ri0mGrcWpfCqFZuzzx3WjtwxG098X+n4OuRkPg==", - "dev": true, - "license": "MIT", - "engines": { - "node": ">= 0.4" - }, - "funding": { - "url": "https://github.com/sponsors/ljharb" - } - }, - "node_modules/is-shared-array-buffer": { - "version": "1.0.4", - "resolved": "https://registry.npmjs.org/is-shared-array-buffer/-/is-shared-array-buffer-1.0.4.tgz", - "integrity": "sha512-ISWac8drv4ZGfwKl5slpHG9OwPNty4jOWPRIhBpxOoD+hqITiwuipOQ2bNthAzwA3B4fIjO4Nln74N0S9byq8A==", - "dev": true, - "license": "MIT", - "dependencies": { - "call-bound": "^1.0.3" - }, - "engines": { - "node": ">= 0.4" - }, - "funding": { - "url": "https://github.com/sponsors/ljharb" - } - }, "node_modules/is-stream": { "version": "4.0.1", "resolved": "https://registry.npmjs.org/is-stream/-/is-stream-4.0.1.tgz", @@ -13236,41 +12937,6 @@ "url": "https://github.com/sponsors/sindresorhus" } }, - "node_modules/is-string": { - "version": "1.1.1", - "resolved": "https://registry.npmjs.org/is-string/-/is-string-1.1.1.tgz", - "integrity": "sha512-BtEeSsoaQjlSPBemMQIrY1MY0uM6vnS1g5fmufYOtnxLGUZM2178PKbhsk7Ffv58IX+ZtcvoGwccYsh0PglkAA==", - "dev": true, - "license": "MIT", - "dependencies": { - "call-bound": "^1.0.3", - "has-tostringtag": "^1.0.2" - }, - "engines": { - "node": ">= 0.4" - }, - "funding": { - "url": "https://github.com/sponsors/ljharb" - } - }, - "node_modules/is-symbol": { - "version": "1.1.1", - "resolved": "https://registry.npmjs.org/is-symbol/-/is-symbol-1.1.1.tgz", - "integrity": "sha512-9gGx6GTtCQM73BgmHQXfDmLtfjjTUDSyoxTCbp5WtoixAhfgsDirWIcVQ/IHpvI5Vgd5i/J5F7B9cN/WlVbC/w==", - "dev": true, - "license": "MIT", - "dependencies": { - "call-bound": "^1.0.2", - "has-symbols": "^1.1.0", - "safe-regex-test": "^1.1.0" - }, - "engines": { - "node": ">= 0.4" - }, - "funding": { - "url": "https://github.com/sponsors/ljharb" - } - }, "node_modules/is-typed-array": { "version": "1.1.15", "resolved": "https://registry.npmjs.org/is-typed-array/-/is-typed-array-1.1.15.tgz", @@ -13313,36 +12979,6 @@ ], "license": "MIT" }, - "node_modules/is-weakmap": { - "version": "2.0.2", - "resolved": "https://registry.npmjs.org/is-weakmap/-/is-weakmap-2.0.2.tgz", - "integrity": "sha512-K5pXYOm9wqY1RgjpL3YTkF39tni1XajUIkawTLUo9EZEVUFga5gSQJF8nNS7ZwJQ02y+1YCNYcMh+HIf1ZqE+w==", - "dev": true, - "license": "MIT", - "engines": { - "node": ">= 0.4" - }, - "funding": { - "url": "https://github.com/sponsors/ljharb" - } - }, - "node_modules/is-weakset": { - "version": "2.0.4", - "resolved": "https://registry.npmjs.org/is-weakset/-/is-weakset-2.0.4.tgz", - "integrity": "sha512-mfcwb6IzQyOKTs84CQMrOwW4gQcaTOAWJ0zzJCl2WSPDrWk/OzDaImWFH3djXhb24g4eudZfLRozAvPGw4d9hQ==", - "dev": true, - "license": "MIT", - "dependencies": { - "call-bound": "^1.0.3", - "get-intrinsic": "^1.2.6" - }, - "engines": { - "node": ">= 0.4" - }, - "funding": { - "url": "https://github.com/sponsors/ljharb" - } - }, "node_modules/is-what": { "version": "5.5.0", "resolved": "https://registry.npmjs.org/is-what/-/is-what-5.5.0.tgz", @@ -15839,23 +15475,6 @@ "url": "https://github.com/sponsors/ljharb" } }, - "node_modules/object-is": { - "version": "1.1.6", - "resolved": "https://registry.npmjs.org/object-is/-/object-is-1.1.6.tgz", - "integrity": "sha512-F8cZ+KfGlSGi09lJT7/Nd6KJZ9ygtvYC0/UYYLI9nmQKLMnydpB9yvbv9K1uSkEu7FU9vYPmVwLg328tX+ot3Q==", - "dev": true, - "license": "MIT", - "dependencies": { - "call-bind": "^1.0.7", - "define-properties": "^1.2.1" - }, - "engines": { - "node": ">= 0.4" - }, - "funding": { - "url": "https://github.com/sponsors/ljharb" - } - }, "node_modules/object-keys": { "version": "1.1.1", "resolved": "https://registry.npmjs.org/object-keys/-/object-keys-1.1.1.tgz", @@ -15866,27 +15485,6 @@ "node": ">= 0.4" } }, - "node_modules/object.assign": { - "version": "4.1.7", - "resolved": "https://registry.npmjs.org/object.assign/-/object.assign-4.1.7.tgz", - "integrity": "sha512-nK28WOo+QIjBkDduTINE4JkF/UJJKyf2EJxvJKfblDpyg0Q+pkOHNTL0Qwy6NP6FhE/EnzV73BxxqcJaXY9anw==", - "dev": true, - "license": "MIT", - "dependencies": { - "call-bind": "^1.0.8", - "call-bound": "^1.0.3", - "define-properties": "^1.2.1", - "es-object-atoms": "^1.0.0", - "has-symbols": "^1.1.0", - "object-keys": "^1.1.1" - }, - "engines": { - "node": ">= 0.4" - }, - "funding": { - "url": "https://github.com/sponsors/ljharb" - } - }, "node_modules/obug": { "version": "2.1.1", "resolved": "https://registry.npmjs.org/obug/-/obug-2.1.1.tgz", @@ -17400,27 +16998,6 @@ "dev": true, "license": "MIT" }, - "node_modules/regexp.prototype.flags": { - "version": "1.5.4", - "resolved": "https://registry.npmjs.org/regexp.prototype.flags/-/regexp.prototype.flags-1.5.4.tgz", - "integrity": "sha512-dYqgNSZbDwkaJ2ceRd9ojCGjBq+mOm9LmtXnAnEGyHhN/5R7iDW2TRw3h+o/jCFxus3P2LfWIIiwowAjANm7IA==", - "dev": true, - "license": "MIT", - "dependencies": { - "call-bind": "^1.0.8", - "define-properties": "^1.2.1", - "es-errors": "^1.3.0", - "get-proto": "^1.0.1", - "gopd": "^1.2.0", - "set-function-name": "^2.0.2" - }, - "engines": { - "node": ">= 0.4" - }, - "funding": { - "url": "https://github.com/sponsors/ljharb" - } - }, "node_modules/relateurl": { "version": "0.2.7", "resolved": "https://registry.npmjs.org/relateurl/-/relateurl-0.2.7.tgz", @@ -17798,24 +17375,6 @@ "dev": true, "license": "MIT" }, - "node_modules/safe-regex-test": { - "version": "1.1.0", - "resolved": "https://registry.npmjs.org/safe-regex-test/-/safe-regex-test-1.1.0.tgz", - "integrity": "sha512-x/+Cz4YrimQxQccJf5mKEbIa1NzeCRNI5Ecl/ekmlYaampdNLPalVyIcCZNNH3MvmqBugV5TMYZXv0ljslUlaw==", - "dev": true, - "license": "MIT", - "dependencies": { - "call-bound": "^1.0.2", - "es-errors": "^1.3.0", - "is-regex": "^1.2.1" - }, - "engines": { - "node": ">= 0.4" - }, - "funding": { - "url": "https://github.com/sponsors/ljharb" - } - }, "node_modules/safe-regex2": { "version": "5.1.1", "resolved": "https://registry.npmjs.org/safe-regex2/-/safe-regex2-5.1.1.tgz", @@ -18031,22 +17590,6 @@ "node": ">= 0.4" } }, - "node_modules/set-function-name": { - "version": "2.0.2", - "resolved": "https://registry.npmjs.org/set-function-name/-/set-function-name-2.0.2.tgz", - "integrity": "sha512-7PGFlmtwsEADb0WYyvCMa1t+yke6daIG4Wirafur5kcf+MhUnPms1UeR0CKQdTZD81yESwMHbtn+TR+dMviakQ==", - "dev": true, - "license": "MIT", - "dependencies": { - "define-data-property": "^1.1.4", - "es-errors": "^1.3.0", - "functions-have-names": "^1.2.3", - "has-property-descriptors": "^1.0.2" - }, - "engines": { - "node": ">= 0.4" - } - }, "node_modules/setimmediate": { "version": "1.0.5", "resolved": "https://registry.npmjs.org/setimmediate/-/setimmediate-1.0.5.tgz", @@ -18537,20 +18080,6 @@ "dev": true, "license": "MIT" }, - "node_modules/stop-iteration-iterator": { - "version": "1.1.0", - "resolved": "https://registry.npmjs.org/stop-iteration-iterator/-/stop-iteration-iterator-1.1.0.tgz", - "integrity": "sha512-eLoXW/DHyl62zxY4SCaIgnRhuMr6ri4juEYARS8E6sCEqzKpOiE521Ucofdx+KnDZl5xmvGYaaKCk5FEOxJCoQ==", - "dev": true, - "license": "MIT", - "dependencies": { - "es-errors": "^1.3.0", - "internal-slot": "^1.1.0" - }, - "engines": { - "node": ">= 0.4" - } - }, "node_modules/stream-buffers": { "version": "3.0.3", "resolved": "https://registry.npmjs.org/stream-buffers/-/stream-buffers-3.0.3.tgz", @@ -21338,45 +20867,6 @@ "node": ">= 8" } }, - "node_modules/which-boxed-primitive": { - "version": "1.1.1", - "resolved": "https://registry.npmjs.org/which-boxed-primitive/-/which-boxed-primitive-1.1.1.tgz", - "integrity": "sha512-TbX3mj8n0odCBFVlY8AxkqcHASw3L60jIuF8jFP78az3C2YhmGvqbHBpAjTRH2/xqYunrJ9g1jSyjCjpoWzIAA==", - "dev": true, - "license": "MIT", - "dependencies": { - "is-bigint": "^1.1.0", - "is-boolean-object": "^1.2.1", - "is-number-object": "^1.1.1", - "is-string": "^1.1.1", - "is-symbol": "^1.1.1" - }, - "engines": { - "node": ">= 0.4" - }, - "funding": { - "url": "https://github.com/sponsors/ljharb" - } - }, - "node_modules/which-collection": { - "version": "1.0.2", - "resolved": "https://registry.npmjs.org/which-collection/-/which-collection-1.0.2.tgz", - "integrity": "sha512-K4jVyjnBdgvc86Y6BkaLZEN933SwYOuBFkdmBu9ZfkcAbdVbpITnDmjvZ/aQjRXQrv5EPkTnD1s39GiiqbngCw==", - "dev": true, - "license": "MIT", - "dependencies": { - "is-map": "^2.0.3", - "is-set": "^2.0.3", - "is-weakmap": "^2.0.2", - "is-weakset": "^2.0.3" - }, - "engines": { - "node": ">= 0.4" - }, - "funding": { - "url": "https://github.com/sponsors/ljharb" - } - }, "node_modules/which-typed-array": { "version": "1.1.20", "resolved": "https://registry.npmjs.org/which-typed-array/-/which-typed-array-1.1.20.tgz", diff --git a/package.json b/package.json index bbda03ac8..04acf74b4 100644 --- a/package.json +++ b/package.json @@ -51,7 +51,6 @@ "@thi.ng/rasterize": "^1.0.171", "@types/color-name": "^1.1.5", "@types/cors": "^2.8.19", - "@types/deep-equal": "^1.0.4", "@types/express": "^5.0.5", "@types/file-saver": "^2.0.7", "@types/mocha": "^10.0.10", @@ -72,7 +71,6 @@ "core-js": "3.47.0", "cors": "^2.8.5", "cross-env": "^10.1.0", - "deep-equal": "^2.2.3", "dicom-parser": "^1.8.21", "dicomweb-client-typed": "^0.8.6", "eslint": "^9.39.1", diff --git a/src/components/EditableItemList.vue b/src/components/EditableItemList.vue index c520e58fc..60fb9bdfd 100644 --- a/src/components/EditableItemList.vue +++ b/src/components/EditableItemList.vue @@ -1,50 +1,22 @@ - diff --git a/src/segmentation/components/SegmentList.vue b/src/segmentation/components/SegmentList.vue index 9c4f78aa8..cdc82c805 100644 --- a/src/segmentation/components/SegmentList.vue +++ b/src/segmentation/components/SegmentList.vue @@ -10,12 +10,15 @@ import SaveSegmentationDialog from '@/src/segmentation/components/SaveSegmentati import SegmentEditor from '@/src/segmentation/components/SegmentEditor.vue'; import SegmentListActions from '@/src/segmentation/components/SegmentListActions.vue'; import { useCurrentImage } from '@/src/composables/useCurrentImage'; -import { deleteSegmentAndReport } from '@/src/segmentation/deleteSegment'; +import { deleteUnlockedSegment } from '@/src/segmentation/deleteSegment'; import { useSegmentEditing } from '@/src/segmentation/composables/useSegmentEditing'; import { pulseSegmentMask } from '@/src/segmentation/rendering/revealPulse'; -import { revealSegmentContent } from '@/src/core/annotations/locator'; +import { + revealSegmentContent, + type SegmentContent, +} from '@/src/core/annotations/locator'; import { isCineImage } from '@/src/core/cine/isCineImage'; -import { NO_NAME, SEGMENT_SHORTCUT_ACTIONS } from '@/src/constants'; +import { SEGMENT_SHORTCUT_ACTIONS } from '@/src/constants'; import { actionToKey, readableBinding, @@ -27,13 +30,23 @@ import useLoadDataStore from '@/src/store/load-data'; import type { LPSAxis } from '@/src/types/lps'; import { DEFAULT_SEGMENTATION_DISPLAY, - listMasks, maskHasContent, maskScalars, + segmentationHasContent, type SegmentMask, type SegmentationDisplayPatch, } from '@/src/segmentation/model'; import { markedSlices } from '@/src/segmentation/geometry'; +import { sameFields } from '@/src/utils'; + +const props = withDefaults( + defineProps<{ + // Spelled out: `typeof` an import compiles to an untyped prop, whose + // function default Vue would call as a factory. + reveal?: (imageId: string, content: SegmentContent) => void; + }>(), + { reveal: revealSegmentContent } +); const registry = useSegmentStore().segments; const { shapesOf } = useSegmentShapes(); @@ -64,15 +77,6 @@ type Row = { shapeCount: number; }; -// Fields compare by identity, which the constraint keeps meaningful: a row -// that gained an object or array field would never equal its predecessor. -const sameRow = < - R extends Record, ->( - one: R, - other: R -) => (Object.keys(one) as (keyof R)[]).every((key) => one[key] === other[key]); - // Unchanged rows keep their identity: EditableItemList memoizes on it, and a // ruler drag recomputes every row's shape count per pointer move. // Rows omit mask bounds, so growing a painted mask does not rebuild the list. @@ -85,14 +89,14 @@ const rows = computed((previous?: Row[]) => { shortcut: SEGMENT_SHORTCUT_ACTIONS[index] ? readableBinding(actionToKey.value[SEGMENT_SHORTCUT_ACTIONS[index]]) : undefined, - name: appearance.name || NO_NAME, + name: appearance.displayName, color: appearance.cssColor, visible: appearance.visible, locked: appearance.locked, shapeCount: shapesOf(segment.id).length, }; const kept = before.get(segment.id); - return kept && sameRow(kept, row) ? kept : row; + return kept && sameFields(kept, row) ? kept : row; }); }); @@ -135,7 +139,7 @@ const savableReason = computed(() => { const segmentation = viewedSegmentation.value; // Records alone save nothing: a segment resolved here but never painted // leaves an empty mask, so what is offered follows the voxels. - if (!segmentation || !listMasks(segmentation).some(maskHasContent)) + if (!segmentation || !segmentationHasContent(segmentation)) return 'Nothing is painted on this image yet'; return ''; }); @@ -186,7 +190,7 @@ function revealSlice(row: Row) { shapes.forEach(({ frame, axis, slice }) => { if (frame == null && axis) (slicesByAxis[axis] ??= []).push(slice); }); - revealSegmentContent(imageId, { + props.reveal(imageId, { paintedSlicesByIJK: mask && paintedSlices(mask), slicesByAxis, frames: shapes.flatMap((shape) => @@ -353,11 +357,9 @@ const { @update:model-value="registry.selectSegment" :selection-revision="registry.selectionRevision.value" :items="rows" - reorderable @move="registry.moveSegment" - item-key="id" - item-title="name" create-text="New segment" + reorder-hint="Drag to reorder segments and shortcuts. Earlier segments win picking and come first on export; overlaps blend in the view. Alt+Up or Alt+Down also moves this segment." @create="registry.addSegment()" class="segment-items" > @@ -385,12 +387,16 @@ const { diff --git a/src/segmentation/components/SegmentListActions.vue b/src/segmentation/components/SegmentListActions.vue index 2056e234f..5c39b1252 100644 --- a/src/segmentation/components/SegmentListActions.vue +++ b/src/segmentation/components/SegmentListActions.vue @@ -20,7 +20,7 @@ defineEmits<{ // Nothing else on screen says what a lock does to other segments' painting. const lockTooltip = (locked: boolean) => locked - ? 'Unlock. Painting over this segment takes its voxels, unless Allow Overlap is on.' + ? 'Unlock. Painting over this segment replaces its voxels, unless Allow Overlap is on.' : 'Lock. Painting other segments goes around it, or overlaps it with Allow Overlap on.'; diff --git a/src/segmentation/components/__tests__/ProcessWorkflow.spec.ts b/src/segmentation/components/__tests__/ProcessWorkflow.spec.ts index c6c20941e..dd62db242 100644 --- a/src/segmentation/components/__tests__/ProcessWorkflow.spec.ts +++ b/src/segmentation/components/__tests__/ProcessWorkflow.spec.ts @@ -1,13 +1,17 @@ -import { beforeEach, describe, expect, it } from 'vitest'; -import { createPinia, setActivePinia } from 'pinia'; -import { createApp, defineComponent, nextTick } from 'vue'; -import { flushPromises, mount, VueWrapper } from '@vue/test-utils'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { defineComponent, nextTick } from 'vue'; +import { + enableAutoUnmount, + flushPromises, + mount, + VueWrapper, +} from '@vue/test-utils'; import ProcessWorkflow from '@/src/segmentation/components/ProcessWorkflow.vue'; -import { CorePiniaProviderPlugin } from '@/src/core/provider'; import { addActiveSegment, - seatImage, + activateAppPinia, + viewImage, } from '@/src/segmentation/__tests__/segmentMaskFixtures'; import type { ProcessTarget } from '@/src/segmentation/editing/paintProcess'; import { useViewStore } from '@/src/store/views'; @@ -15,11 +19,7 @@ import { useToolStore } from '@/src/store/tools'; import { Tools } from '@/src/store/tools/types'; import { markCine } from '@/src/core/cine/__tests__/cineFixtures'; -// --------------------------------------------------------------------------- -// The Original/Processed pair is a segmented choice, not a switch: the toggle -// is mandatory, so clicking the button already selected keeps the selection -// but still fires the click. Each button therefore states what it shows. -// --------------------------------------------------------------------------- +enableAutoUnmount(afterEach); const BtnStub = defineComponent({ name: 'VBtn', @@ -67,63 +67,52 @@ const selected = (wrapper: VueWrapper) => wrapper.get('.btn-toggle').attributes('data-selected'); beforeEach(async () => { - const pinia = createPinia().use(CorePiniaProviderPlugin()); - createApp({}).use(pinia); - setActivePinia(pinia); - await seatImage('image-1', { dimensions: [2, 1, 1] }); - useViewStore().setDataForAllViews('image-1'); - await nextTick(); + activateAppPinia(); + await viewImage('image-1', { dimensions: [2, 1, 1] }); useToolStore().setCurrentTool(Tools.Paint); }); describe('the process preview toggle', () => { const previewing = async () => { - const { labelMap } = addActiveSegment(new Uint8Array([1, 0])); + addActiveSegment(new Uint8Array([1, 0])); const wrapper = mount(ProcessWorkflow, { props: { algorithm: processed }, global: globalOptions, }); await button(wrapper, 'Preview').trigger('click'); await flushPromises(); - const values = () => - Array.from(labelMap.getPointData().getScalars().getData()); expect(wrapper.find('.btn-toggle').exists()).toBe(true); - return { wrapper, values }; + return wrapper; }; it('leaves the preview alone when the showing button is clicked again', async () => { - const { wrapper, values } = await previewing(); + const wrapper = await previewing(); expect(selected(wrapper)).toBe('1'); - expect(values()).toEqual([1, 1]); await button(wrapper, 'Processed').trigger('click'); expect(selected(wrapper)).toBe('1'); - expect(values()).toEqual([1, 1]); }); it('shows the original once, however often its button is clicked', async () => { - const { wrapper, values } = await previewing(); + const wrapper = await previewing(); await button(wrapper, 'Original').trigger('click'); expect(selected(wrapper)).toBe('0'); - expect(values()).toEqual([1, 0]); await button(wrapper, 'Original').trigger('click'); expect(selected(wrapper)).toBe('0'); - expect(values()).toEqual([1, 0]); }); it('still moves between the two', async () => { - const { wrapper, values } = await previewing(); + const wrapper = await previewing(); await button(wrapper, 'Original').trigger('click'); await button(wrapper, 'Processed').trigger('click'); expect(selected(wrapper)).toBe('1'); - expect(values()).toEqual([1, 1]); }); }); diff --git a/src/segmentation/components/__tests__/SaveSegmentationDialog.spec.ts b/src/segmentation/components/__tests__/SaveSegmentationDialog.spec.ts new file mode 100644 index 000000000..2730ced3e --- /dev/null +++ b/src/segmentation/components/__tests__/SaveSegmentationDialog.spec.ts @@ -0,0 +1,148 @@ +import { + afterEach, + beforeEach, + describe, + expect, + it, + vi, + type Mock, +} from 'vitest'; +import { setActivePinia, createPinia } from 'pinia'; +import { defineComponent, nextTick } from 'vue'; +import { enableAutoUnmount, flushPromises, mount } from '@vue/test-utils'; + +import SaveSegmentationDialog from '@/src/segmentation/components/SaveSegmentationDialog.vue'; +import { + mintSegment, + seatSpecImage, + seedVoxel, + store, +} from '@/src/segmentation/__tests__/segmentMaskFixtures'; +import { defer, type Deferred } from '@/src/utils'; +import type { saveLabelmapExport } from '@/src/segmentation/components/saveLabelmapExport'; + +enableAutoUnmount(afterEach); + +const CardStub = defineComponent({ + name: 'VCard', + template: '
', +}); + +const globalOptions = { + stubs: { + VCard: CardStub, + VCardTitle: { template: '
' }, + VCardText: { template: '
' }, + VCardActions: { template: '
' }, + VForm: { template: '
' }, + VTextField: { + props: ['modelValue'], + template: '', + }, + // Vuetify's select consumes Enter to open its menu. + VSelect: { + props: ['modelValue', 'items'], + template: '', + emits: ['update:modelValue'], + template: + '', }); const SliderStub = defineComponent({ @@ -68,7 +72,7 @@ describe('segment editor name validation', () => { expect(rule('Tumor')).toBe(true); }); - it('rejects changing to another segment’s name', async () => { + it("rejects changing to another segment's name", async () => { const wrapper = mountEditor(); await wrapper.setProps({ name: ' Node ' }); @@ -149,3 +153,29 @@ describe('segment editor stroke width', () => { expect(wrapper.emitted('update:strokeWidth')).toEqual([[4]]); }); }); + +describe('segment editor field events', () => { + it.each([ + ['Fill Opacity', 'update:fillOpacity'], + ['Outline Opacity', 'update:outlineOpacity'], + ])('emits %s changes', (name, event) => { + const wrapper = mountEditor(); + const slider = wrapper + .findAllComponents(SliderStub) + .find((candidate) => candidate.props('name') === name)!; + slider.vm.$emit('update:modelValue', 0.35); + expect(wrapper.emitted(event)).toEqual([[0.35]]); + }); + + it('emits a name change from the name field', async () => { + const wrapper = mountEditor(); + await wrapper.get('input').setValue('Lesion'); + expect(wrapper.emitted('update:name')).toEqual([['Lesion']]); + }); + + it('finishes from Enter in the name field', async () => { + const wrapper = mountEditor(); + await wrapper.get('input').trigger('keydown', { key: 'Enter' }); + expect(wrapper.emitted('done')).toEqual([[]]); + }); +}); diff --git a/src/segmentation/components/__tests__/SegmentList.spec.ts b/src/segmentation/components/__tests__/SegmentList.spec.ts index a038169fe..860bc3414 100644 --- a/src/segmentation/components/__tests__/SegmentList.spec.ts +++ b/src/segmentation/components/__tests__/SegmentList.spec.ts @@ -1,4 +1,4 @@ -import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { setActivePinia, createPinia } from 'pinia'; import { type Index3, @@ -6,70 +6,39 @@ import { lockSegment, seedVoxel, seatImage, + showImage, + mintSegment, seatSpecImage, store, } from '@/src/segmentation/__tests__/segmentMaskFixtures'; -import { defineComponent, nextTick, ref } from 'vue'; +import { defineComponent, nextTick } from 'vue'; import { enableAutoUnmount, mount, VueWrapper } from '@vue/test-utils'; import SegmentList from '@/src/segmentation/components/SegmentList.vue'; -import { useMessageStore } from '@/src/store/messages'; +import { messageTitles } from '@/src/components/__tests__/messageDisplay'; import useLoadDataStore from '@/src/store/load-data'; import { useSegmentStore } from '@/src/segmentation/segments'; -import { DEFAULT_SEGMENTATION_FILL_OPACITY } from '@/src/segmentation/model'; -import { extentSize, maskOffset } from '@/src/segmentation/geometry'; -import { useViewStore } from '@/src/store/views'; -import useViewSliceStore from '@/src/store/view-configs/slicing'; +import { DEFAULT_SEGMENTATION_DISPLAY } from '@/src/segmentation/model'; import { seatCineImage } from '@/src/core/cine/__tests__/cineFixtures'; -import { SEGMENT_VALUE } from '@/src/segmentation/masks/labelValue'; -import { - useCurrentTools, - usePlacingAnnotationTool, -} from '@/src/composables/annotationTool'; import { useRulerStore } from '@/src/store/tools/rulers'; -import { useRectangleStore } from '@/src/store/tools/rectangles'; -import { usePolygonStore } from '@/src/store/tools/polygons'; import { AXIAL_FRAME_OF_REFERENCE } from '@/src/utils/frameOfReference'; -import useCinePlaybackStore from '@/src/store/view-configs/cine-playback'; -import useViewCameraStore from '@/src/store/view-configs/camera'; enableAutoUnmount(afterEach); -// --------------------------------------------------------------------------- -// One flat list of segments: rows are the shared registry's segments, keyed -// on segment id, offered whether or not this image has a mask for them. The -// visibility and lock controls belong to the shared segment, and the -// display sliders to the viewed image's segmentation. -// --------------------------------------------------------------------------- - const segments = () => useSegmentStore().segments; -const viewImage = async (id: string) => { - useViewStore().setDataForAllViews(id); - await nextTick(); -}; - const makeMask = (imageId: string, name: string) => { const segmentId = segments().mintSegment({ name }); const record = maskOn(imageId, segmentId); - return { id: segmentId, segmentId, maskId: record.id, record }; + return { segmentId, maskId: record.id }; }; const makeSegment = (name: string) => segments().mintSegment({ name }); -// The item list stands in for the real one so the per-row slot renders without -// Vuetify: rows carry their segment id, and the row buttons keep the icon names -// the list uses today. +// Slots expose row controls without mounting the Vuetify list. const ItemListStub = defineComponent({ name: 'EditableItemList', - props: [ - 'items', - 'itemKey', - 'itemTitle', - 'modelValue', - 'createText', - 'hideCreate', - ], + props: ['items', 'modelValue', 'createText'], emits: ['update:model-value', 'create', 'select', 'edit'], template: `
@@ -82,7 +51,7 @@ const ItemListStub = defineComponent({
- `, -}); - -const SaveDialogStub = defineComponent({ - name: 'SaveSegmentationDialog', - props: ['id'], - emits: ['done'], - template: `
`, -}); - -// Either dialog host works: the slot renders unless the host is explicitly -// closed, so a `v-model`-gated host and an inner `v-if` both read correctly. -const DialogHostStub = (name: string) => - defineComponent({ - name, - props: ['modelValue', 'maxWidth'], - emits: ['update:modelValue'], - template: `
`, - }); - -const globalOptions = { - stubs: { - EditableItemList: ItemListStub, - SegmentEditor: { template: '
' }, - SaveSegmentationDialog: SaveDialogStub, - IsolatedDialog: DialogHostStub('IsolatedDialog'), - CloseableDialog: DialogHostStub('CloseableDialog'), - VDialog: DialogHostStub('VDialog'), - VBtn: BtnStub, - VIcon: { template: '' }, - VTooltip: { template: '' }, - VMenu: { - template: '
', - }, - VList: { template: '
' }, - VListItem: { template: '
' }, - VSpacer: { template: '' }, - VSlider: { props: ['label', 'modelValue'], template: '' }, - VExpansionPanels: { template: '
' }, - VExpansionPanel: { template: '
' }, - VExpansionPanelTitle: { template: '' }, - VExpansionPanelText: { template: '
' }, - VDivider: { template: '
' }, - }, -}; - -const mountList = () => - mount(SegmentList, { - props: { - registry: useSegmentStore().segments, - noun: 'segment', - masked: true, - }, - global: globalOptions, - }); - -const saveButton = (wrapper: VueWrapper) => - wrapper.find('[data-testid="save-segments-button"]'); - -const saveDialog = (wrapper: VueWrapper) => - wrapper.findComponent(SaveDialogStub); - -const paintMask = (imageId: string, name: string) => { - const segmentation = store().ensureSegmentationForImage(imageId); - const mask = store().createMask(segmentation.id, mintSegment({ name })); - seedVoxel(mask.id, [1, 1, 0]); - return segmentation; -}; - -describe('saving from the flat segment panel', () => { - beforeEach(async () => { - setActivePinia(createPinia()); - await seatImage('img-1'); - await seatImage('img-2', 'MR'); - await viewImage('img-1'); - }); - - it('offers the save affordance disabled, saying why, until something is painted', async () => { - const wrapper = mountList(); - await nextTick(); - - expect(saveButton(wrapper).exists()).toBe(true); - expect(saveButton(wrapper).attributes('disabled')).toBeDefined(); - expect(wrapper.text()).toContain('Nothing is painted on this image yet'); - }); - - it('offers one save affordance once the viewed image has segments', async () => { - paintMask('img-1', 'Tumor'); - const wrapper = mountList(); - await nextTick(); - - expect( - wrapper.findAll('[data-testid="save-segments-button"]') - ).toHaveLength(1); - }); - - // A segment resolved as an edit target mints a record, and allocating its - // storage does not put a voxel in it: neither is anything to write out. - it('keeps the save affordance disabled for masks that hold nothing', async () => { - const segmentation = store().ensureSegmentationForImage('img-1'); - store().createMask(segmentation.id, mintSegment({ name: 'Resolved' })); - const allocated = store().createMask( - segmentation.id, - mintSegment({ name: 'Allocated' }) - ); - store().maskVoxels(allocated.id).materialize(); - const wrapper = mountList(); - await nextTick(); - - expect(saveButton(wrapper).attributes('disabled')).toBeDefined(); - expect(wrapper.text()).toContain('Nothing is painted on this image yet'); - }); - - it('opens the save dialog on the viewed image segmentation', async () => { - const segmentation = paintMask('img-1', 'Tumor'); - const wrapper = mountList(); - await nextTick(); - - expect(saveDialog(wrapper).exists()).toBe(false); - expect(saveButton(wrapper).exists()).toBe(true); - - await saveButton(wrapper).trigger('click'); - await nextTick(); - - expect(saveDialog(wrapper).props('id')).toBe(segmentation.id); - }); - - // The create affordance names the row it adds, and it reads as an expression - // rather than a literal attribute, so the source scan below cannot see it. - it('names what the create affordance adds without a storage word', async () => { - const wrapper = mountList(); - await nextTick(); - - expect(wrapper.findComponent(ItemListStub).props('createText')).toBe( - 'New segment' - ); - }); - - it('follows the viewed image rather than the selected segment', async () => { - const first = paintMask('img-1', 'Tumor'); - const second = store().ensureSegmentationForImage('img-2'); - const onSecond = store().createMask( - second.id, - mintSegment({ name: 'Node' }) - ); - // The selected segment has its mask on the image that is NOT being viewed. - useSegmentStore().segments.selectSegment(onSecond.segmentId); - const wrapper = mountList(); - await nextTick(); - - expect(saveButton(wrapper).exists()).toBe(true); - await saveButton(wrapper).trigger('click'); - await nextTick(); - - expect(saveDialog(wrapper).props('id')).toBe(first.id); - }); -}); - -// --- user-visible panel text --- // const exists = (rel: string) => fs.existsSync(path.resolve(repoRoot, rel)); const read = (rel: string) => @@ -222,6 +16,7 @@ const VISIBLE_ATTRIBUTES = [ 'subtitle', 'aria-label', 'create-text', + 'reorder-hint', ]; /** @@ -286,21 +81,18 @@ const componentFiles = (dir: string): string[] => describe('panel language', () => { it('keeps the segmentation panel free of group and storage words', () => { - // These two must be present, so the scan is never vacuous; a renamed save - // dialog simply drops out of the list. - expect(exists('src/components/AnnotationsModule.vue')).toBe(true); - expect(exists('src/segmentation/components/SegmentList.vue')).toBe(true); + expect(SEGMENTATION_PANEL.filter((rel) => !exists(rel))).toEqual([]); const banned = /segment group|labelmap|label value|layer/i; - const hits = SEGMENTATION_PANEL.filter(exists).flatMap((rel) => + const hits = SEGMENTATION_PANEL.flatMap((rel) => bannedIn(rel, banned).map((line) => `${rel}: ${line}`) ); expect(hits).toEqual([]); }); - it('keeps the panel’s notification titles free of storage words', () => { - const files = SEGMENTATION_PANEL.filter(exists); + it("keeps the panel's notification titles free of storage words", () => { + const files = SEGMENTATION_PANEL; // The panel reports at least one failure to the user, so the scan reads // something rather than passing on an empty match set. const messages = files.flatMap((rel) => bannedMessagesIn(rel, /.*/)); diff --git a/src/segmentation/components/saveLabelmapExport.ts b/src/segmentation/components/saveLabelmapExport.ts new file mode 100644 index 000000000..7bc88fae3 --- /dev/null +++ b/src/segmentation/components/saveLabelmapExport.ts @@ -0,0 +1,29 @@ +import { saveAs } from 'file-saver'; +import { planLabelmapExport } from '@/src/segmentation/io/composition'; +import { useSegmentationEditsStore } from '@/src/segmentation/editing/coordinator'; +import { + bundleExportFiles, + writeLabelmapParts, + type ExportFile, +} from '@/src/segmentation/io/export'; + +export async function saveLabelmapExport( + parentId: string, + stem: string, + format: string +) { + useSegmentationEditsStore().beforeRead(); + const files: ExportFile[] = []; + await writeLabelmapParts( + { parentId, parts: planLabelmapExport(parentId).parts }, + stem, + format, + { + deliver: (file) => { + files.push(file); + }, + } + ); + const bundle = await bundleExportFiles(stem, files); + saveAs(bundle.blob, bundle.name); +} diff --git a/src/segmentation/composables/__tests__/useSegmentShapes.spec.ts b/src/segmentation/composables/__tests__/useSegmentShapes.spec.ts index 74e7ea3fb..5a681709b 100644 --- a/src/segmentation/composables/__tests__/useSegmentShapes.spec.ts +++ b/src/segmentation/composables/__tests__/useSegmentShapes.spec.ts @@ -1,29 +1,24 @@ import { beforeEach, describe, expect, it } from 'vitest'; import { createPinia, setActivePinia } from 'pinia'; -import vtkImageData from '@kitware/vtk.js/Common/DataModel/ImageData'; import { useSegmentShapes } from '@/src/segmentation/composables/useSegmentShapes'; import { useSegmentStore } from '@/src/segmentation/segments'; -import { useImageCacheStore } from '@/src/store/image-cache'; -import { useViewStore } from '@/src/store/views'; +import { + seatImage, + viewImage, +} from '@/src/segmentation/__tests__/segmentMaskFixtures'; import { useRulerStore } from '@/src/store/tools/rulers'; import { useRectangleStore } from '@/src/store/tools/rectangles'; import { AXIAL_FRAME_OF_REFERENCE } from '@/src/utils/frameOfReference'; -const seat = (id: string) => - useImageCacheStore().addVTKImageData(vtkImageData.newInstance(), 'CT', { - id, - }); - describe('segment shapes', () => { - beforeEach(() => { + beforeEach(async () => { setActivePinia(createPinia()); - seat('img-1'); - seat('img-2'); - useViewStore().setDataForAllViews('img-1'); + await seatImage('img-2'); + await viewImage('img-1'); }); - it('groups the viewed image’s finished shapes under their segments', () => { + it("groups the viewed image's finished shapes under their segments", () => { const { segments } = useSegmentStore(); const tumor = segments.mintSegment({ name: 'Tumor' }); const node = segments.mintSegment({ name: 'Node' }); diff --git a/src/segmentation/composables/useMaskRevision.ts b/src/segmentation/composables/useMaskRevision.ts index ba0a4db3b..8183cc0df 100644 --- a/src/segmentation/composables/useMaskRevision.ts +++ b/src/segmentation/composables/useMaskRevision.ts @@ -2,17 +2,18 @@ import { ref, watchEffect } from 'vue'; import { useSegmentationStore } from '@/src/segmentation/store'; import { listMasks } from '@/src/segmentation/model'; +import type { Maybe } from '@/src/types'; /** - * A counter every mask change bumps, voxel writes included. A mask's extent is - * reactive but a write inside the box it already has moves nothing, so this is - * the only trace of one a consumer can watch. It says something changed and - * nothing about what. A stroke bumps it once per changed mask per sample, - * so debounce anything expensive that reads it. + * A counter every change to one image's masks bumps, voxel writes included. A + * mask's extent is reactive but a write inside the box it already has moves + * nothing, so this is the only trace of one a consumer can watch. It says + * something changed and nothing about what. A stroke bumps it once per changed + * mask per sample, so debounce anything expensive that reads it. * * Scoped to the caller: the masks are watched only while it is alive. */ -export function useMaskRevision() { +export function useMaskRevision(imageId: () => Maybe) { const segmentationStore = useSegmentationStore(); const revision = ref(0); @@ -20,9 +21,12 @@ export function useMaskRevision() { // announces itself to vtk, so watching the mask itself catches the ones that // reach the buffer without going through the store. watchEffect((onCleanup) => { - const subscriptions = Object.values(segmentationStore.segmentations) - .flatMap((segmentation) => listMasks(segmentation)) - .flatMap((segment) => segment.representations.labelmap ?? []) + const id = imageId(); + const segmentation = id + ? segmentationStore.getSegmentationForImage(id) + : undefined; + const subscriptions = (segmentation ? listMasks(segmentation) : []) + .flatMap((mask) => mask.representations.labelmap ?? []) .map((binding) => binding.image.onModified(() => { revision.value += 1; diff --git a/src/segmentation/composables/useSegmentEditing.ts b/src/segmentation/composables/useSegmentEditing.ts index a31c227bc..389584e0e 100644 --- a/src/segmentation/composables/useSegmentEditing.ts +++ b/src/segmentation/composables/useSegmentEditing.ts @@ -4,7 +4,7 @@ import type { SegmentRegistry } from '@/src/segmentation/segmentRegistry'; import type { Maybe } from '@/src/types'; import { cssColorToRGBA } from '@/src/segmentation/color'; import { cleanUndefined } from '@/src/utils'; -import { deleteSegmentAndReport } from '@/src/segmentation/deleteSegment'; +import { deleteUnlockedSegment } from '@/src/segmentation/deleteSegment'; /** * The segment edit dialog: one editor, one set of fields, one place that @@ -86,7 +86,7 @@ export function useSegmentEditing(registry: SegmentRegistry) { // Deleting a segment takes its masks on every image and its shapes with it. function deleteEditingSegment() { const id = editingSegmentId.value; - if (id) deleteSegmentAndReport(registry, id); + if (id) deleteUnlockedSegment(registry, id); stopEditing(false); } diff --git a/src/segmentation/composables/useSegmentShapes.ts b/src/segmentation/composables/useSegmentShapes.ts index 96b123d3f..515f8c3da 100644 --- a/src/segmentation/composables/useSegmentShapes.ts +++ b/src/segmentation/composables/useSegmentShapes.ts @@ -91,8 +91,7 @@ const useSegmentShapesStore = defineStore('segmentShapes', () => { }); /** - * The shapes drawn on the viewed image, grouped by the segment each one names. - * A segment's row lists these under it, so the sidebar holds no second list of - * the same annotations. + * Finished shapes on the viewed image, shared by the Measurements list and the + * segment rows' reveal. */ export const useSegmentShapes = () => useSegmentShapesStore().segmentShapes; diff --git a/src/segmentation/deleteSegment.ts b/src/segmentation/deleteSegment.ts index e5181ba50..fa426a5e8 100644 --- a/src/segmentation/deleteSegment.ts +++ b/src/segmentation/deleteSegment.ts @@ -1,43 +1,6 @@ -import { maskHasContent } from '@/src/segmentation/model'; import type { SegmentRegistry } from '@/src/segmentation/segmentRegistry'; -import { useSegmentationStore } from '@/src/segmentation/store'; -import { useMessageStore } from '@/src/store/messages'; -import { AnnotationToolStoreMap } from '@/src/store/tools'; -import { plural } from '@/src/utils'; -/** - * What deleting a segment is about to take with it, counted before the cascade - * runs: its mask on every image, and every finished annotation naming it. A - * tool still being placed is not counted, because the cascade leaves it alone, - * and neither is a mask record with nothing in it: the cascade drops the - * record, but the user never put anything on that image to lose. - */ -function countCascade(segmentId: string) { - // A segment has at most one mask per image, so this counts both. - const images = Object.values(useSegmentationStore().segmentations).filter( - (segmentation) => - Object.values(segmentation.masks).some( - (mask) => mask.segmentId === segmentId && maskHasContent(mask) - ) - ).length; - const annotations = Object.values(AnnotationToolStoreMap).reduce( - (total, useStore) => - total + - useStore().finishedTools.filter((tool) => tool.segmentId === segmentId) - .length, - 0 - ); - return { images, annotations }; -} - -/** - * Deletes a segment and says what went with it. The cascade reaches masks on - * images this one is not viewing and annotations on other slices and axes, so - * its scope is invisible from here and there is no undo: the same reason - * `removeSelectedTools` reports its count. No dialog asks first, which is what - * the rest of the app does. - */ -export function deleteSegmentAndReport( +export function deleteUnlockedSegment( registry: SegmentRegistry, segmentId: string ) { @@ -46,14 +9,5 @@ export function deleteSegmentAndReport( registry.appearanceOf(segmentId).locked ) return; - const { images, annotations } = countCascade(segmentId); registry.deleteSegment(segmentId); - - const removed = [ - images > 0 && - `${images} ${plural(images, 'mask')} on ${images} ${plural(images, 'image')}`, - annotations > 0 && `${annotations} ${plural(annotations, 'annotation')}`, - ].filter((part): part is string => !!part); - if (removed.length > 0) - useMessageStore().addInfo(`Deleted ${removed.join(' and ')}`); } diff --git a/src/segmentation/editing/__tests__/rasterizePolygon.spec.ts b/src/segmentation/editing/__tests__/rasterizePolygon.spec.ts index 5bb648e7a..8d98d2dc5 100644 --- a/src/segmentation/editing/__tests__/rasterizePolygon.spec.ts +++ b/src/segmentation/editing/__tests__/rasterizePolygon.spec.ts @@ -2,13 +2,16 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; import { setActivePinia, createPinia } from 'pinia'; import type { Vector3 } from '@kitware/vtk.js/types'; -import { rasterizePolygon } from '@/src/segmentation/editing/rasterizePolygon'; +import { + rasterizePolygon, + rasterizeTargetDisabledReason, +} from '@/src/segmentation/editing/rasterizePolygon'; +import { useMessageStore } from '@/src/store/messages'; import { useSegmentStore } from '@/src/segmentation/segments'; import { listMasks } from '@/src/segmentation/model'; import { addMask, extentOf, - labelValueOf, maskValueAt, markedVoxels, seatImage, @@ -18,16 +21,9 @@ import { type Index3, lockSegment, } from '@/src/segmentation/__tests__/segmentMaskFixtures'; +import { SEGMENT_VALUE } from '@/src/segmentation/masks/labelValue'; -// --------------------------------------------------------------------------- -// The mask grows to hold the polygon before `fillPoly` runs (a mask that does -// not reach the polygon silently swallows every pixel), and the filled voxels -// are cleared in the other UNLOCKED segments of the image. A locked one keeps -// its voxels and the fill goes around them. -// -// Unit spacing and a zero origin make world points index points, and an -// identity direction maps the Axial view axis to K. -// --------------------------------------------------------------------------- +// Unit spacing and a zero origin make world points index points, Axial on K. const DIMENSIONS: Index3 = [6, 6, 2]; @@ -64,7 +60,7 @@ const rasterize = (segmentId: string | undefined, points = SQUARE, slice = 0) => viewAxis: 'Axial', }); -/** What a polygon carries: the type, not the record it lands in. */ +/** What a polygon carries: the segment, not the mask it lands in. */ const rasterizeInto = ( maskId: string | undefined, points = SQUARE, @@ -73,9 +69,11 @@ const rasterizeInto = ( const segments = () => useSegmentStore().segments; +const UNLOCK_REASON = 'Unlock this segment to rasterize into it'; + const segmentNamesOf = (imageId: string) => listMasks(store().getSegmentationForImage(imageId)!).map( - (segment) => segments().appearanceOf(segment.segmentId).name + (mask) => segments().appearanceOf(mask.segmentId).name ); describe('rasterizing a polygon into a bounded mask', () => { @@ -88,10 +86,8 @@ describe('rasterizing a polygon into a bounded mask', () => { const maskId = addMask('img-1', 'Tumor'); rasterizeInto(maskId); - - const labelValue = labelValueOf(maskId); - expect(maskValueAt(maskId, [2, 3, 0])).toBe(labelValue); - expect(maskValueAt(maskId, [3, 3, 0])).toBe(labelValue); + expect(maskValueAt(maskId, [2, 3, 0])).toBe(SEGMENT_VALUE); + expect(maskValueAt(maskId, [3, 3, 0])).toBe(SEGMENT_VALUE); const extent = extentOf(maskId)!; expect(extent[0]).toBeLessThanOrEqual(1); expect(extent[1]).toBeGreaterThanOrEqual(4); @@ -111,8 +107,8 @@ describe('rasterizing a polygon into a bounded mask', () => { expect(markedVoxels(maskId)).toHaveLength(16); }); - it('clears the filled voxels in another segment’s mask', () => { - const neighbor = addMask('img-1', 'Neighbour'); + it("clears the filled voxels in another segment's mask", () => { + const neighbor = addMask('img-1', 'Neighbor'); seedVoxel(neighbor, [2, 3, 0]); seedVoxel(neighbor, [0, 0, 0]); const maskId = addMask('img-1', 'Tumor'); @@ -120,10 +116,10 @@ describe('rasterizing a polygon into a bounded mask', () => { rasterizeInto(maskId); expect(maskValueAt(neighbor, [2, 3, 0])).toBe(0); - expect(maskValueAt(neighbor, [0, 0, 0])).toBe(labelValueOf(neighbor)); + expect(maskValueAt(neighbor, [0, 0, 0])).toBe(SEGMENT_VALUE); }); - it('fills around the voxels a locked neighbour holds', () => { + it('fills around the voxels a locked neighbor holds', () => { const locked = addMask('img-1', 'Locked'); const unlocked = addMask('img-1', 'Unlocked'); seedVoxel(locked, [2, 3, 0]); @@ -134,14 +130,41 @@ describe('rasterizing a polygon into a bounded mask', () => { rasterizeInto(maskId); - expect(maskValueAt(locked, [2, 3, 0])).toBe(labelValueOf(locked)); - expect(maskValueAt(unlocked, [2, 3, 0])).toBe(labelValueOf(unlocked)); + expect(maskValueAt(locked, [2, 3, 0])).toBe(SEGMENT_VALUE); + expect(maskValueAt(unlocked, [2, 3, 0])).toBe(SEGMENT_VALUE); expect(maskValueAt(maskId, [2, 3, 0])).toBeFalsy(); expect(maskValueAt(unlocked, [3, 3, 0])).toBe(0); - expect(maskValueAt(maskId, [3, 3, 0])).toBe(labelValueOf(maskId)); + expect(maskValueAt(maskId, [3, 3, 0])).toBe(SEGMENT_VALUE); + }); + + it('deletes a neighbor the fill takes every voxel from', () => { + const neighbor = addMask('img-1', 'Neighbor'); + seedVoxel(neighbor, [2, 3, 0]); + const maskId = addMask('img-1', 'Tumor'); + + rasterizeInto(maskId); + + expect(store().maskExists(neighbor)).toBe(false); + expect(maskValueAt(maskId, [2, 3, 0])).toBe(SEGMENT_VALUE); }); - it('shares the filled voxels with every neighbour while overlap is allowed', () => { + it('deletes the mask of a fill that lands nowhere and names none', () => { + const locked = addMask('img-1', 'Locked'); + const voxels = store().maskVoxels(locked); + voxels.materialize(); + voxels.ensureContains([0, 5, 0, 5, 0, 1]); + voxels.scalars().fill(SEGMENT_VALUE); + lockSegment(locked, true); + const tumor = segments().addSegment({ name: 'Tumor' }); + + const result = rasterize(tumor); + + expect(result).toEqual({ segmentId: tumor, maskId: undefined }); + expect(store().maskFor('img-1', tumor)).toBeUndefined(); + expect(markedVoxels(locked)).toHaveLength(72); + }); + + it('shares the filled voxels with every neighbor while overlap is allowed', () => { const locked = addMask('img-1', 'Locked'); const unlocked = addMask('img-1', 'Unlocked'); seedVoxel(locked, [2, 3, 0]); @@ -152,10 +175,10 @@ describe('rasterizing a polygon into a bounded mask', () => { rasterizeInto(maskId); - expect(maskValueAt(locked, [2, 3, 0])).toBe(labelValueOf(locked)); - expect(maskValueAt(unlocked, [3, 3, 0])).toBe(labelValueOf(unlocked)); - expect(maskValueAt(maskId, [2, 3, 0])).toBe(labelValueOf(maskId)); - expect(maskValueAt(maskId, [3, 3, 0])).toBe(labelValueOf(maskId)); + expect(maskValueAt(locked, [2, 3, 0])).toBe(SEGMENT_VALUE); + expect(maskValueAt(unlocked, [3, 3, 0])).toBe(SEGMENT_VALUE); + expect(maskValueAt(maskId, [2, 3, 0])).toBe(SEGMENT_VALUE); + expect(maskValueAt(maskId, [3, 3, 0])).toBe(SEGMENT_VALUE); }); it('publishes each changed neighbor once before returning from a fill', () => { @@ -165,7 +188,8 @@ describe('rasterizing a polygon into a bounded mask', () => { const voxels = store().maskVoxels(id); voxels.materialize(); voxels.ensureContains([0, 5, 0, 5, 0, 1]); - if (name === 'First' || name === 'Second') voxels.scalars().fill(1); + if (name === 'First' || name === 'Second') + voxels.scalars().fill(SEGMENT_VALUE); // Held inside the polygon, so the fill goes around it. if (name === 'Locked') { seedVoxel(id, [1, 2, 0]); @@ -194,8 +218,8 @@ describe('rasterizing a polygon into a bounded mask', () => { expect(neighbors.map(({ modified }) => modified.mock.calls.length)).toEqual( [2, 2, 0, 0] ); - expect(maskValueAt(target, [2, 3, 1])).toBe(1); - expect(maskValueAt(neighbors[0].id, [0, 0, 0])).toBe(1); + expect(maskValueAt(target, [2, 3, 1])).toBe(SEGMENT_VALUE); + expect(maskValueAt(neighbors[0].id, [0, 0, 0])).toBe(SEGMENT_VALUE); }); it('creates nothing for a polygon that covers no voxel', () => { @@ -215,8 +239,8 @@ describe('rasterizing a polygon into a bounded mask', () => { }); it('refuses a locked segment and leaves every mask as it was', () => { - const neighbour = addMask('img-1', 'Neighbour'); - seedVoxel(neighbour, [2, 3, 0]); + const neighbor = addMask('img-1', 'Neighbor'); + seedVoxel(neighbor, [2, 3, 0]); const maskId = addMask('img-1', 'Tumor'); lockSegment(maskId, true); @@ -226,8 +250,55 @@ describe('rasterizing a polygon into a bounded mask', () => { expect(result.maskId).toBeUndefined(); expect(result.segmentId).toBe(segmentOfMask(maskId)); expect(maskValueAt(maskId, [2, 3, 0])).toBeFalsy(); - // The clearer never ran, so the neighbour keeps what a fill would take. - expect(maskValueAt(neighbour, [2, 3, 0])).toBe(labelValueOf(neighbour)); + // The clearer never ran, so the neighbor keeps what a fill would take. + expect(maskValueAt(neighbor, [2, 3, 0])).toBe(SEGMENT_VALUE); + expect(store().getMask(maskId).representations.labelmap).toBeUndefined(); + expect(useMessageStore().messages.map(({ title }) => title)).toContain( + 'Cannot rasterize into a locked segment' + ); + expect(rasterizeTargetDisabledReason(segmentOfMask(maskId))).toBe( + UNLOCK_REASON + ); + }); + + it('names the locked first segment for a polygon carrying none, without allocating it', () => { + const first = addMask('img-1', 'First'); + lockSegment(first, true); + + expect(rasterizeTargetDisabledReason('')).toBe(UNLOCK_REASON); + expect(store().getMask(first).representations.labelmap).toBeUndefined(); + }); + + it('falls back to the selected segment when the polygon names a deleted one', () => { + const stale = segmentOfMask(addMask('img-1', 'Deleted')); + const fallback = addMask('img-1', 'Selected'); + segments().selectSegment(segmentOfMask(fallback)); + segments().deleteSegment(stale); + lockSegment(fallback, true); + + expect(rasterizeTargetDisabledReason(stale)).toBe(UNLOCK_REASON); + expect(rasterizeTargetDisabledReason('')).toBe(UNLOCK_REASON); + + lockSegment(fallback, false); + expect(rasterizeTargetDisabledReason(stale)).toBe(''); + expect(rasterize(stale)).toEqual({ + segmentId: segmentOfMask(fallback), + maskId: fallback, + }); + expect(maskValueAt(fallback, [2, 3, 0])).toBe(SEGMENT_VALUE); + }); + + it('mints a segment when the polygon names the only one, since deleted', () => { + const deleted = segments().addSegment({ name: 'Tumor' }); + segments().deleteSegment(deleted); + + const result = rasterize(deleted); + + expect(result.segmentId).not.toBe(deleted); + expect(store().getSegmentationForImage('img-1')!.order).toEqual([ + result.maskId, + ]); + expect(maskValueAt(result.maskId!, [2, 3, 0])).toBe(SEGMENT_VALUE); }); it('keeps an earlier polygon when a later one grows the mask', () => { @@ -244,10 +315,8 @@ describe('rasterizing a polygon into a bounded mask', () => { ], 1 ); - - const labelValue = labelValueOf(maskId); - expect(maskValueAt(maskId, [2, 3, 0])).toBe(labelValue); - expect(maskValueAt(maskId, [2, 3, 1])).toBe(labelValue); + expect(maskValueAt(maskId, [2, 3, 0])).toBe(SEGMENT_VALUE); + expect(maskValueAt(maskId, [2, 3, 1])).toBe(SEGMENT_VALUE); }); it('stays inside the parent image for a polygon that overhangs it', () => { @@ -265,7 +334,7 @@ describe('rasterizing a polygon into a bounded mask', () => { const extent = extentOf(maskId)!; expect(extent[0]).toBe(0); expect(extent[2]).toBe(0); - expect(maskValueAt(maskId, [1, 1, 0])).toBe(labelValueOf(maskId)); + expect(maskValueAt(maskId, [1, 1, 0])).toBe(SEGMENT_VALUE); }); it('rasterizes into a segment it resolves when the polygon carries none', () => { @@ -273,7 +342,7 @@ describe('rasterizing a polygon into a bounded mask', () => { const segmentation = store().getSegmentationForImage('img-1')!; expect(segmentation.order).toEqual([maskId]); - expect(maskValueAt(maskId, [2, 3, 0])).toBe(labelValueOf(maskId)); + expect(maskValueAt(maskId, [2, 3, 0])).toBe(SEGMENT_VALUE); }); it('rasterizes into the segment the polygon names, not the selected one', () => { @@ -287,7 +356,7 @@ describe('rasterizing a polygon into a bounded mask', () => { expect(segmentOfMask(maskId)).toBe(tumor); expect(segmentNamesOf('img-1')).toEqual(['Tumor']); - expect(maskValueAt(maskId, [2, 3, 0])).toBe(labelValueOf(maskId)); + expect(maskValueAt(maskId, [2, 3, 0])).toBe(SEGMENT_VALUE); const nextEdit = store().resolveEditTarget('img-1'); expect(segmentOfMask(nextEdit)).toBe(node); @@ -314,26 +383,9 @@ describe('rasterizing a polygon into a bounded mask', () => { expect(segments().selectedSegmentId.value).toBe(tumor); expect(segmentNamesOf('img-1')).toEqual(['Tumor']); }); - - it('rasterizes into the segment it was given, not the selected one', () => { - const active = addMask('img-1', 'Active'); - segments().selectSegment(segmentOfMask(active)); - const named = addMask('img-1', 'Named'); - - const result = rasterizeInto(named); - - expect(result.maskId).toBe(named); - expect(maskValueAt(named, [2, 3, 0])).toBe(labelValueOf(named)); - expect(maskValueAt(active, [2, 3, 0])).toBeFalsy(); - }); }); -// --------------------------------------------------------------------------- -// The voxels a polygon fills are a property of the polygon and the parent -// image, not of how much of the image its mask currently holds. Triangles with -// integer vertices are where an edge can cross a scanline exactly on a pixel -// centre, which is the tie an allocation-dependent fill resolves differently. -// --------------------------------------------------------------------------- +// Integer-vertex triangles put edges on pixel centers, where allocation must not decide. const GRID: Index3 = [40, 40, 1]; const GRID_EXTENT = [0, 39, 0, 39, 0, 0] as const; diff --git a/src/segmentation/editing/__tests__/rasterizeTarget.spec.ts b/src/segmentation/editing/__tests__/rasterizeTarget.spec.ts deleted file mode 100644 index c3decc139..000000000 --- a/src/segmentation/editing/__tests__/rasterizeTarget.spec.ts +++ /dev/null @@ -1,206 +0,0 @@ -import { beforeEach, describe, expect, it } from 'vitest'; -import { setActivePinia, createPinia } from 'pinia'; -import { - seatSpecImage as seatImage, - maskOn, - lockSegment, - store, -} from '@/src/segmentation/__tests__/segmentMaskFixtures'; - -import { - rasterizeTargetDisabledReason, - resolveRasterizeTarget, -} from '@/src/segmentation/editing/rasterizePolygon'; -import { useMessageStore } from '@/src/store/messages'; -import { useSegmentStore } from '@/src/segmentation/segments'; -import { SEGMENT_VALUE } from '@/src/segmentation/masks/labelValue'; - -const segments = () => useSegmentStore().segments; - -const makeMask = (imageId: string, name: string) => { - const segmentId = segments().mintSegment({ name }); - return { segmentId, record: maskOn(imageId, segmentId) }; -}; - -const targetOf = (imageId: string, segmentId: string | undefined) => - resolveRasterizeTarget(imageId, segmentId)!; - -describe('polygon rasterize target', () => { - beforeEach(() => { - setActivePinia(createPinia()); - }); - - it('allocates storage for a record that has none', async () => { - await seatImage('img-1'); - const segment = makeMask('img-1', 'Tumor'); - - const target = targetOf('img-1', segment.segmentId); - - expect(store().boundMaskIds('img-1')).toEqual([target.maskId]); - expect(target.voxels.image()).toBe( - store().findMaskBinding(target.maskId)!.image - ); - expect( - store().getMask(segment.record.id).representations.labelmap!.image - ).toBe(target.voxels.image()); - }); - - it('resolves the given segment rather than the first one', async () => { - await seatImage('img-1'); - const first = makeMask('img-1', 'Other'); - store().maskVoxels(first.record.id).materialize(); - const second = makeMask('img-1', 'Tumor'); - - const target = targetOf('img-1', second.segmentId); - - expect(target.voxels.image()).not.toBe( - store().findMaskBinding(first.record.id)!.image - ); - }); - - it('reuses the same binding on a second rasterize', async () => { - await seatImage('img-1'); - const segment = makeMask('img-1', 'Tumor'); - - const first = targetOf('img-1', segment.segmentId); - const second = targetOf('img-1', segment.segmentId); - - expect(second.voxels.image()).toBe(first.voxels.image()); - expect(store().boundMaskIds('img-1')).toHaveLength(1); - }); - - it('leaves the selected segment alone', async () => { - await seatImage('img-1'); - const active = makeMask('img-1', 'Active'); - const other = makeMask('img-1', 'Other'); - segments().selectSegment(active.segmentId); - - targetOf('img-1', other.segmentId); - - expect(segments().selectedSegmentId.value).toBe(active.segmentId); - }); - - it('takes this image record for a segment painted on another image', async () => { - await seatImage('img-1'); - await seatImage('img-2'); - const elsewhere = makeMask('img-2', 'Tumor'); - - const target = targetOf('img-1', elsewhere.segmentId); - - expect(target.maskId).not.toBe(elsewhere.record.id); - expect(store().getMask(target.maskId).segmentId).toBe(elsewhere.segmentId); - }); - - it('refuses a locked record before allocating storage for it', async () => { - await seatImage('img-1'); - const segment = makeMask('img-1', 'Tumor'); - lockSegment(segment.record.id, true); - - expect(resolveRasterizeTarget('img-1', segment.segmentId)).toBeUndefined(); - - expect( - store().getMask(segment.record.id).representations.labelmap - ).toBeUndefined(); - expect(store().boundMaskIds('img-1')).toEqual([]); - expect( - useMessageStore().messages.map((message) => message.title) - ).toContain('Cannot rasterize into a locked segment'); - expect(rasterizeTargetDisabledReason(segment.segmentId)).toBe( - 'Unlock this segment to rasterize into it' - ); - }); - - it('describes the same selected fallback execution will use', async () => { - await seatImage('img-1'); - const stale = makeMask('img-1', 'Deleted'); - const fallback = makeMask('img-1', 'Selected'); - segments().selectSegment(fallback.segmentId); - segments().deleteSegment(stale.segmentId); - lockSegment(fallback.record.id, true); - - expect(rasterizeTargetDisabledReason(stale.segmentId)).toBe( - 'Unlock this segment to rasterize into it' - ); - expect(rasterizeTargetDisabledReason('')).toBe( - 'Unlock this segment to rasterize into it' - ); - - lockSegment(fallback.record.id, false); - expect(rasterizeTargetDisabledReason(stale.segmentId)).toBe(''); - expect(targetOf('img-1', stale.segmentId).segmentId).toBe( - fallback.segmentId - ); - }); - - it('describes the first-segment fallback without allocating it', async () => { - await seatImage('img-1'); - const first = makeMask('img-1', 'First'); - lockSegment(first.record.id, true); - - expect(rasterizeTargetDisabledReason('')).toBe( - 'Unlock this segment to rasterize into it' - ); - expect( - store().getMask(first.record.id).representations.labelmap - ).toBeUndefined(); - }); - - it('rasterizes into a minted segment when nothing is selected', async () => { - await seatImage('img-1'); - - const target = targetOf('img-1', undefined); - - const segmentation = store().getSegmentationForImage('img-1'); - expect(Object.keys(segmentation!.masks)).toHaveLength(1); - expect(segments().selectedSegmentId.value).toBe(target.segmentId); - expect(target.voxels.image()).toBe( - store().findMaskBinding(target.maskId)!.image - ); - expect(target.maskId).toBe(Object.keys(segmentation!.masks)[0]); - }); - - it('reuses the default segment on a second rasterize', async () => { - await seatImage('img-1'); - - const first = targetOf('img-1', undefined); - const second = targetOf('img-1', undefined); - - expect(second.voxels.image()).toBe(first.voxels.image()); - expect( - Object.keys(store().getSegmentationForImage('img-1')!.masks) - ).toHaveLength(1); - }); - - it('hands back the accessor the polygon writes through', async () => { - await seatImage('img-1'); - const segment = makeMask('img-1', 'Tumor'); - - const target = targetOf('img-1', segment.segmentId); - target.voxels.ensureContains([0, 3, 0, 0, 0, 0]); - // fillPoly writes voxel offsets into the live buffer, so a copy would be - // rasterized and thrown away. - target.voxels.scalars()[3] = SEGMENT_VALUE; - - expect( - store() - .findMaskBinding(target.maskId)! - .image.getPointData() - .getScalars() - .getData()[3] - ).toBe(SEGMENT_VALUE); - }); - - it('rasterizes into a minted segment when the tool names a deleted one', async () => { - await seatImage('img-1'); - const segment = makeMask('img-1', 'Tumor'); - segments().deleteSegment(segment.segmentId); - - // The tool keeps the deleted segment's id; that must not block rasterizing. - const target = targetOf('img-1', segment.segmentId); - - expect(target.segmentId).not.toBe(segment.segmentId); - expect(store().getSegmentationForImage('img-1')!.masks).toHaveProperty( - target.maskId - ); - }); -}); diff --git a/src/segmentation/editing/__tests__/rasterizeWithProcess.spec.ts b/src/segmentation/editing/__tests__/rasterizeWithProcess.spec.ts index 7591ea1a8..0ef658cfd 100644 --- a/src/segmentation/editing/__tests__/rasterizeWithProcess.spec.ts +++ b/src/segmentation/editing/__tests__/rasterizeWithProcess.spec.ts @@ -1,16 +1,13 @@ import { beforeEach, describe, expect, it } from 'vitest'; -import { createPinia, setActivePinia } from 'pinia'; -import { createApp, nextTick } from 'vue'; import type { Vector3 } from '@kitware/vtk.js/types'; -import { CorePiniaProviderPlugin } from '@/src/core/provider'; import { rasterizePolygon } from '@/src/segmentation/editing/rasterizePolygon'; import { addMask, extentOf, - labelValueOf, maskValueAt, - seatImage, + activateAppPinia, + viewImage, seedVoxel, store, type Index3, @@ -18,8 +15,8 @@ import { segmentOfMask, } from '@/src/segmentation/__tests__/segmentMaskFixtures'; import { usePaintProcessStore } from '@/src/segmentation/editing/paintProcess'; -import { useViewStore } from '@/src/store/views'; import type { Extent3D } from '@/src/segmentation/geometry'; +import { SEGMENT_VALUE } from '@/src/segmentation/masks/labelValue'; const DIMENSIONS: Index3 = [6, 6, 1]; const SQUARE: Vector3[] = [ @@ -47,12 +44,8 @@ function rasterize(maskId: string) { } async function setUpRasterizeView() { - const pinia = createPinia().use(CorePiniaProviderPlugin()); - createApp({}).use(pinia); - setActivePinia(pinia); - await seatImage('img-1', { dimensions: DIMENSIONS }); - useViewStore().setDataForAllViews('img-1'); - await nextTick(); + activateAppPinia(); + await viewImage('img-1', { dimensions: DIMENSIONS }); } function setUpOverlappingSegments(extent: Extent3D) { @@ -60,37 +53,35 @@ function setUpOverlappingSegments(extent: Extent3D) { growMask(target, extent); const neighbor = addMask('img-1', 'Neighbor'); seedVoxel(neighbor, [2, 3, 0]); - return { target, neighbor, labelValue: labelValueOf(target)! }; + // Outside the polygon, so the neighbor keeps a voxel and its mask. + seedVoxel(neighbor, [5, 5, 0]); + return { target, neighbor }; } describe('polygon rasterize action', () => { beforeEach(setUpRasterizeView); it('restores the original before rasterization grows the mask', async () => { - const { target, neighbor, labelValue } = setUpOverlappingSegments([ - 0, 1, 0, 1, 0, 0, - ]); + const { target, neighbor } = setUpOverlappingSegments([0, 1, 0, 1, 0, 0]); const processStore = usePaintProcessStore(); await processStore.startProcess(async ({ scalars, maskExtent }) => ({ - scalars: new Uint8Array(scalars.length).fill(labelValue), + scalars: new Uint8Array(scalars.length).fill(SEGMENT_VALUE), extent: maskExtent, })); - expect(maskValueAt(target, [0, 0, 0])).toBe(labelValue); + expect(maskValueAt(target, [0, 0, 0])).toBe(SEGMENT_VALUE); rasterize(target); expect(processStore.processState.step).toBe('start'); expect(extentOf(target)).toEqual([0, 4, 0, 4, 0, 0]); expect(maskValueAt(target, [0, 0, 0])).toBe(0); - expect(maskValueAt(target, [2, 3, 0])).toBe(labelValue); + expect(maskValueAt(target, [2, 3, 0])).toBe(SEGMENT_VALUE); expect(maskValueAt(neighbor, [2, 3, 0])).toBe(0); }); it('leaves same-sized rasterization intact after the preview is reset', async () => { - const { target, neighbor, labelValue } = setUpOverlappingSegments([ - 0, 5, 0, 5, 0, 0, - ]); + const { target, neighbor } = setUpOverlappingSegments([0, 5, 0, 5, 0, 0]); seedVoxel(target, [0, 0, 0]); const processStore = usePaintProcessStore(); @@ -106,8 +97,8 @@ describe('polygon rasterize action', () => { expect(processStore.processState.step).toBe('start'); expect(extentOf(target)).toEqual([0, 5, 0, 5, 0, 0]); - expect(maskValueAt(target, [0, 0, 0])).toBe(labelValue); - expect(maskValueAt(target, [2, 3, 0])).toBe(labelValue); + expect(maskValueAt(target, [0, 0, 0])).toBe(SEGMENT_VALUE); + expect(maskValueAt(target, [2, 3, 0])).toBe(SEGMENT_VALUE); expect(maskValueAt(neighbor, [2, 3, 0])).toBe(0); }); }); diff --git a/src/segmentation/editing/algorithms/fillHoles.ts b/src/segmentation/editing/algorithms/fillHoles.ts index a7373a6a0..cec9b0e11 100644 --- a/src/segmentation/editing/algorithms/fillHoles.ts +++ b/src/segmentation/editing/algorithms/fillHoles.ts @@ -1,5 +1,6 @@ import { TypedArray } from '@kitware/vtk.js/types'; import { SEGMENT_VALUE } from '@/src/segmentation/masks/labelValue'; +import { inPlaneAxes } from '@/src/segmentation/geometry'; // 4-connected neighbor offsets, shared so the flood-fill loops never allocate // a neighbor array per visited voxel. @@ -110,8 +111,7 @@ export function fillHoles(opts: FillHolesOptions) { const sliceStride = strides[axis]; const sliceCount = dimensions[axis]; - // The two in-plane axes (everything that isn't the slice axis). - const [uAxis, vAxis] = [0, 1, 2].filter((a) => a !== axis); + const [uAxis, vAxis] = inPlaneAxes(axis); const uDim = dimensions[uAxis]; const vDim = dimensions[vAxis]; const uStride = strides[uAxis]; diff --git a/src/segmentation/editing/fillHoles.ts b/src/segmentation/editing/fillHoles.ts index 4dbff6186..dfec7c3cb 100644 --- a/src/segmentation/editing/fillHoles.ts +++ b/src/segmentation/editing/fillHoles.ts @@ -7,7 +7,7 @@ import type { ProcessTarget } from '@/src/segmentation/editing/paintProcess'; import { getEffectiveView } from '@/src/core/views/effectiveView'; import { fillHolesWorker } from '@/src/segmentation/editing/algorithms/fillHoles.worker'; import { createProcessWorkerHost } from '@/src/segmentation/editing/processWorker'; -import { getLPSDirections } from '@/src/utils/lps'; +import { useImageCacheStore } from '@/src/store/image-cache'; import { extentReachesSlice } from '@/src/segmentation/geometry'; export enum FillHolesSliceScope { @@ -72,10 +72,12 @@ export const useFillHolesStore = defineStore('fillHoles', () => { ); } - const labelMapLpsOrientation = getLPSDirections( - Float32Array.from(target.direction) + // A mask sits on its parent's grid, so the parent's axes are the mask's. + const metadata = useImageCacheStore().getImageMetadata( + target.parentImageId ); - const axis = labelMapLpsOrientation[effectiveView.axis]; + if (!metadata) throw new Error('No such parent image'); + const axis = metadata.lpsOrientation[effectiveView.axis]; const { dimensions, scalars: data } = target; const currentSlice = sliceScope.value === FillHolesSliceScope.CurrentSlice; diff --git a/src/segmentation/editing/paintProcess.ts b/src/segmentation/editing/paintProcess.ts index 638ce3414..1f6ced9ca 100644 --- a/src/segmentation/editing/paintProcess.ts +++ b/src/segmentation/editing/paintProcess.ts @@ -16,6 +16,7 @@ import { isEmptyExtent, markedExtent, reframeMaskScalars, + walkExtentRows, type Extent3D, } from '@/src/segmentation/geometry'; import { usePaintToolStore } from '@/src/store/tools/paint'; @@ -174,12 +175,10 @@ export const usePaintProcessStore = defineStore('paintProcess', () => { const before = run.originalScalars; const after = run.processedScalars; - const [ni, nj, nk] = extentSize(extent); - // Rows flat in one loop, as the mask sweeps are: a voxel's parent index is - // then a step along i from the row's own start, not a divide per voxel. - for (let row = 0; row < nj * nk; row += 1) { - const j = extent[2] + (row % nj); - const k = extent[4] + Math.floor(row / nj); + const [ni] = extentSize(extent); + // By rows, as the mask sweeps go: a voxel's parent index is then a step + // along i from the row's own start, not a divide per voxel. + walkExtentRows(extent, (j, k, row) => { const from = row * ni; for (let n = 0; n < ni; n += 1) { const turnedOn = @@ -189,7 +188,7 @@ export const usePaintProcessStore = defineStore('paintProcess', () => { after[from + n] = LABELMAP_BACKGROUND_VALUE; } } - } + }); } /** @@ -235,6 +234,11 @@ export const usePaintProcessStore = defineStore('paintProcess', () => { ); } endRun(); + // After the run ends, since deleting a mask cancels a preview still held. + if (state.step === 'previewing') + segmentationStore.deleteEmptyMasks( + state.runs.map((run) => run.target.maskId) + ); } const segmentationStore = useSegmentationStore(); @@ -371,10 +375,7 @@ export const usePaintProcessStore = defineStore('paintProcess', () => { function resolveEverySegment(imageId: string): ResolvedRun | undefined { const targets = segmentationStore .editableMasks(imageId) - .flatMap(({ maskId }) => { - const target = targetFor(imageId, maskId); - return target ? [target] : []; - }); + .flatMap((maskId) => targetFor(imageId, maskId) ?? []); if (targets.length === 0) { messageStore.addError(nothingEditable(imageId)); return undefined; diff --git a/src/segmentation/editing/rasterizePolygon.ts b/src/segmentation/editing/rasterizePolygon.ts index 78dc400ab..7bc69298b 100644 --- a/src/segmentation/editing/rasterizePolygon.ts +++ b/src/segmentation/editing/rasterizePolygon.ts @@ -15,11 +15,12 @@ import { extentContainsIndex, extentSize, fullExtent, + inPlaneAxes, isEmptyExtent, maskOffset, + sliceExtent, type Extent3D, } from '@/src/segmentation/geometry'; -import { getLPSDirections } from '@/src/utils/lps'; export const rasterizeTargetDisabledReason = (segmentId: Maybe) => useSegmentationStore().editTargetLocked(segmentId) @@ -32,10 +33,7 @@ export const rasterizeTargetDisabledReason = (segmentId: Maybe) => * point that resolves and creates masks: a polygon carrying no segment, or one * whose segment was deleted, lands in the selected segment rather than failing. */ -export function resolveRasterizeTarget( - imageId: string, - segmentId: Maybe -) { +function resolveRasterizeTarget(imageId: string, segmentId: Maybe) { const segmentationStore = useSegmentationStore(); // A locked segment is not editable, the same refusal paint and the processes @@ -79,7 +77,7 @@ function createGridAccessor( const bounds = { extent, mi, mj }; // One scratch voxel with the slice already in place: the setter runs per // filled pixel, so it allocates nothing. - const [axisU, axisV] = [0, 1, 2].filter((axis) => axis !== axisIdx); + const [axisU, axisV] = inPlaneAxes(axisIdx); const ijk = [0, 0, 0]; ijk[axisIdx] = slice; @@ -107,18 +105,10 @@ function polygonBounds( slice: number ) { if (indexPoints.length === 0) return emptyExtent(); - const bounds = [0, 0, 0, 0, 0, 0] as Extent3D; - [0, 1, 2].forEach((axis) => { - if (axis === axisIndex) { - bounds[axis * 2] = slice; - bounds[axis * 2 + 1] = slice; - return; - } + return sliceExtent(axisIndex, slice, (axis) => { const values = indexPoints.map((point) => point[axis]); - bounds[axis * 2] = Math.floor(Math.min(...values)); - bounds[axis * 2 + 1] = Math.ceil(Math.max(...values)); + return [Math.floor(Math.min(...values)), Math.ceil(Math.max(...values))]; }); - return bounds; } /** @@ -128,7 +118,8 @@ function polygonBounds( * pixel swallows it silently, and each filled voxel is claimed from the other * segments under the aimed rule of `voxelClaim`. World points, parent slice * index. Resolving the edit target cancels any competing preview before - * storage changes. + * storage changes. A mask the fill leaves holding nothing, its own or a + * neighbor's, is deleted, and a deleted target is not named back. */ export function rasterizePolygon({ imageId, @@ -144,10 +135,12 @@ export function rasterizePolygon({ viewAxis: LPSAxis; }) { const segmentationStore = useSegmentationStore(); - const parent = useImageCacheStore().getVtkImageData(imageId); - if (!parent) throw new Error('No such parent image'); + const imageCache = useImageCacheStore(); + const parent = imageCache.getVtkImageData(imageId); + const metadata = imageCache.getImageMetadata(imageId); + if (!parent || !metadata) throw new Error('No such parent image'); - const axisIndex = getLPSDirections(parent.getDirection())[viewAxis]; + const axisIndex = metadata.lpsOrientation[viewAxis]; const indexPoints = points.map((point) => [...parent.worldToIndex(point)]); // The part of the image the polygon lands on: what the mask has to grow to @@ -196,8 +189,14 @@ export function rasterizePolygon({ try { fillPoly(grid, points2D, SEGMENT_VALUE); } finally { - claimVoxel?.finish(); + const cleared = claimVoxel?.finish() ?? []; mask.modified(); + segmentationStore.deleteEmptyMasks([target.maskId, ...cleared]); } - return { segmentId: target.segmentId, maskId: target.maskId }; + return { + segmentId: target.segmentId, + maskId: segmentationStore.maskExists(target.maskId) + ? target.maskId + : undefined, + }; } diff --git a/src/segmentation/geometry.ts b/src/segmentation/geometry.ts index 70f82fa25..fde2056e9 100644 --- a/src/segmentation/geometry.ts +++ b/src/segmentation/geometry.ts @@ -60,6 +60,28 @@ export function extentContainsIndex( ); } +/** The two axes a slice along `axis` spans, in index order. */ +export const inPlaneAxes = (axis: number) => + [0, 1, 2].filter((other) => other !== axis) as [number, number]; + +/** + * The box one plane thick at `slice` along `axis`, spanning on each in-plane + * axis what `span` gives it, which is asked in index order. + */ +export function sliceExtent( + axis: number, + slice: number, + span: (inPlane: number, planeIndex: number) => [number, number] +) { + const extent: Extent3D = [0, 0, 0, 0, 0, 0]; + extent[axis * 2] = slice; + extent[axis * 2 + 1] = slice; + inPlaneAxes(axis).forEach((inPlane, planeIndex) => { + [extent[inPlane * 2], extent[inPlane * 2 + 1]] = span(inPlane, planeIndex); + }); + return extent; +} + /** Whether the plane at `slice` along `axis` passes through `extent`. */ export const extentReachesSlice = ( extent: Extent3D, @@ -125,6 +147,23 @@ export function reframeMaskScalars( return values; } +/** + * Walks `extent` one row along i at a time, handing each row its j and k and + * its position among the rows. Stops at the first row answering true and says + * whether one did. One flat loop, so no caller nests a j and a k loop. + */ +export function walkExtentRows( + extent: Extent3D, + visit: (j: number, k: number, row: number) => boolean | void +) { + const [, nj, nk] = extentSize(extent); + for (let row = 0; row < nj * nk; row += 1) { + if (visit(extent[2] + (row % nj), extent[4] + Math.floor(row / nj), row)) + return true; + } + return false; +} + /** * The box the claimed voxels actually occupy inside a mask bounded by * `extent`, empty when it claims nothing. A binding's extent is the @@ -132,7 +171,7 @@ export function reframeMaskScalars( * bounds. Background is 0, so a claimed voxel is a truthy one. */ export function markedExtent(scalars: ArrayLike, extent: Extent3D) { - const [ni, nj, nk] = extentSize(extent); + const [ni] = extentSize(extent); let bounds: Extent3D | undefined; const scanRow = (rowStart: number, j: number, k: number) => { @@ -144,13 +183,18 @@ export function markedExtent(scalars: ArrayLike, extent: Extent3D) { } }; - for (let row = 0; row < nj * nk; row += 1) { - scanRow(row * ni, extent[2] + (row % nj), extent[4] + Math.floor(row / nj)); - } - + walkExtentRows(extent, (j, k, row) => scanRow(row * ni, j, k)); return bounds ?? emptyExtent(); } +/** Whether any voxel is claimed, stopping at the first one found. */ +export function hasMarkedVoxel(scalars: ArrayLike) { + for (let offset = 0; offset < scalars.length; offset += 1) { + if (scalars[offset]) return true; + } + return false; +} + /** The parent-image slice indices holding a claimed voxel, for i, j and k. */ export function markedSlices(scalars: ArrayLike, extent: Extent3D) { const [ni, nj] = extentSize(extent); @@ -217,6 +261,6 @@ export function clipExtent(extent: Extent3D, bounds: Extent3D): Extent3D { ]; } -export function fullExtent(dimensions: number[] | Int32Array): Extent3D { +export function fullExtent(dimensions: ArrayLike): Extent3D { return [0, dimensions[0] - 1, 0, dimensions[1] - 1, 0, dimensions[2] - 1]; } diff --git a/src/segmentation/io/__tests__/export.spec.ts b/src/segmentation/io/__tests__/export.spec.ts index fbbb7a658..18c2ce044 100644 --- a/src/segmentation/io/__tests__/export.spec.ts +++ b/src/segmentation/io/__tests__/export.spec.ts @@ -1,23 +1,27 @@ -import { describe, expect, it } from 'vitest'; +import { beforeEach, describe, expect, it } from 'vitest'; +import { createPinia, setActivePinia } from 'pinia'; import JSZip from 'jszip'; -import { bundleExportFiles, layerFileName } from '@/src/segmentation/io/export'; +import { + bundleExportFiles, + layerFileName, + segmentationFileStem, +} from '@/src/segmentation/io/export'; +import { useDICOMStore } from '@/src/store/datasets-dicom'; -// A labelmap file carries one label per voxel, so a segmentation with overlap -// leaves as several files. One file is the common case and stays the download -// it has always been: same name, same bytes, no archive around it. +// Overlapping segments need separate export parts, bundled in one archive. const bytes = (...values: number[]) => new Uint8Array(values); const readBlob = async (blob: Blob) => Array.from(new Uint8Array(await blob.arrayBuffer())); -describe('naming the file each group of segments writes', () => { - it('gives the first group the plain name', () => { +describe("naming each export part's file", () => { + it('gives the first part the plain stem', () => { expect(layerFileName('Prostate', 'seg.nrrd', 0)).toBe('Prostate.seg.nrrd'); }); - it('numbers every later group after it', () => { + it('numbers every later part', () => { expect(layerFileName('Prostate', 'seg.nrrd', 1)).toBe( 'Prostate_layer1.seg.nrrd' ); @@ -27,6 +31,31 @@ describe('naming the file each group of segments writes', () => { }); }); +describe('the stem a segmentation is saved and staged under', () => { + beforeEach(() => setActivePinia(createPinia())); + + it("drops a file-backed image name's extension, compound ones included", () => { + expect(segmentationFileStem('file-image', 'scan.nii.gz')).toBe('scan'); + }); + + it('keeps every dot of a DICOM series name', () => { + useDICOMStore().volumeInfo['series-1'] = { + NumberOfSlices: 1, + VolumeID: 'series-1', + Modality: 'MR', + SeriesInstanceUID: '1.2.3.4', + SeriesNumber: '1', + SeriesDescription: 'Ax T2 FSE 3.5mm', + WindowLevel: '128', + WindowWidth: '256', + }; + + expect(segmentationFileStem('series-1', 'Ax T2 FSE 3.5mm')).toBe( + 'Ax T2 FSE 3.5mm' + ); + }); +}); + describe('handing the written files to the browser', () => { it('downloads a single file as itself', async () => { const bundle = await bundleExportFiles('Prostate', [ diff --git a/src/segmentation/io/__tests__/labelmap.spec.ts b/src/segmentation/io/__tests__/labelmap.spec.ts new file mode 100644 index 000000000..7c7d52c19 --- /dev/null +++ b/src/segmentation/io/__tests__/labelmap.spec.ts @@ -0,0 +1,119 @@ +import { describe, expect, it } from 'vitest'; +import vtkDataArray from '@kitware/vtk.js/Common/Core/DataArray'; +import vtkImageData from '@kitware/vtk.js/Common/DataModel/ImageData'; +import { + allocateLabelmap, + labelmapScalars, + normalizeLabelmapScalars, +} from '@/src/segmentation/io/labelmap'; +import { toBinaryMask, toLabelMap } from '@/src/segmentation/io/import'; + +describe('labelmap interchange storage', () => { + it.each([255, 256, 65535])('stores label %i without truncation', (count) => { + const parent = vtkImageData.newInstance({ + spacing: [2, 3, 4], + origin: [5, 6, 7], + }); + parent.setDimensions(2, 2, 2); + const image = allocateLabelmap(parent, count); + const values = labelmapScalars(image); + values[7] = count; + expect(values[7]).toBe(count); + expect(values.BYTES_PER_ELEMENT).toBe(count <= 255 ? 1 : 2); + expect(image.indexToWorld([1, 1, 1])).toEqual( + parent.indexToWorld([1, 1, 1]) + ); + }); + + it('rejects a single-file export beyond 16-bit capacity', () => { + expect(() => allocateLabelmap(vtkImageData.newInstance(), 65536)).toThrow( + 'at most 65535' + ); + }); + + it('keeps high input values and excludes invalid values without wrapping', () => { + const input = new Float32Array([ + 0, + 1, + 255, + 256, + 65535, + -1, + 65536, + NaN, + Infinity, + ]); + const { values, excluded } = normalizeLabelmapScalars(input); + expect(values).toBeInstanceOf(Uint16Array); + expect(Array.from(values)).toEqual([0, 1, 255, 256, 65535, 0, 0, 0, 0]); + expect(excluded).toBe(4); + expect(input[6]).toBe(65536); + }); + + it('normalizes a wide binary input to byte mask storage', () => { + expect(normalizeLabelmapScalars(new Uint16Array([0, 1]))).toEqual({ + values: new Uint8Array([0, 1]), + excluded: 0, + }); + }); + + it('excludes invalid values from a plain number array', () => { + const { values, excluded } = normalizeLabelmapScalars([ + 0, + 3, + -2, + NaN, + Infinity, + -Infinity, + 7, + ]); + expect(values).toBeInstanceOf(Uint8Array); + expect(Array.from(values)).toEqual([0, 3, 0, 0, 0, 0, 7]); + expect(excluded).toBe(4); + }); +}); + +describe('reading an imported labelmap', () => { + it('excludes fractional labels instead of merging their voxels', () => { + const input = vtkImageData.newInstance(); + input.setDimensions(4, 1, 1); + input.getPointData().setScalars( + vtkDataArray.newInstance({ + numberOfComponents: 1, + values: new Float32Array([1, 1.9, 2, 2.9]), + }) + ); + + const { labelmap, excluded } = toLabelMap(input); + expect(Array.from(labelmapScalars(labelmap))).toEqual([1, 0, 2, 0]); + expect(excluded).toBe(2); + }); + + it('reads a saved mask entry as binary storage, any nonzero voxel claimed', () => { + const input = vtkImageData.newInstance(); + input.setDimensions(4, 1, 1); + input.getPointData().setScalars( + vtkDataArray.newInstance({ + numberOfComponents: 1, + values: new Uint16Array([256, 1, 2, 0]), + }) + ); + + const mask = toBinaryMask(input)!; + expect(labelmapScalars(mask)).toEqual(new Uint8Array([1, 1, 1, 0])); + expect(mask.getDimensions()).toEqual([4, 1, 1]); + }); + + it('refuses a multi-component saved mask entry', () => { + const input = vtkImageData.newInstance(); + input.setDimensions(2, 2, 1); + input.getPointData().setScalars( + vtkDataArray.newInstance({ + numberOfComponents: 2, + values: new Uint8Array(8), + }) + ); + + expect(toBinaryMask(input)).toBeUndefined(); + }); +}); diff --git a/src/segmentation/io/composition.ts b/src/segmentation/io/composition.ts index b99af741e..c6db7ee52 100644 --- a/src/segmentation/io/composition.ts +++ b/src/segmentation/io/composition.ts @@ -1,101 +1,95 @@ -import { useSegmentationEditsStore } from '@/src/segmentation/editing/coordinator'; +import vtkImageData from '@kitware/vtk.js/Common/DataModel/ImageData'; import { useImageCacheStore } from '@/src/store/image-cache'; import { useSegmentStore } from '@/src/segmentation/segments'; import { useSegmentationStore } from '@/src/segmentation/store'; -import { allocateMask } from '@/src/segmentation/masks/storage'; import { boundScalars, groupByLayer, writeMaskInto, } from '@/src/segmentation/masks/overlap'; import { + allocateLabelmap, + labelmapScalars, LABELMAP_MAX_VALUE, - nextUnusedLabelValue, -} from '@/src/segmentation/masks/labelValue'; -import { - maskScalars, - type SegmentMask, - type LabelmapBinding, - type LabelmapSegment, -} from '@/src/segmentation/model'; -import { fullExtent } from '@/src/segmentation/geometry'; +} from '@/src/segmentation/io/labelmap'; +import { type SegmentMask } from '@/src/segmentation/model'; +import type { SegmentRegistry } from '@/src/segmentation/segmentRegistry'; import { toLabelmapSegment } from '@/src/segmentation/segment'; +import { chunk } from '@/src/utils'; -const boundedMask = (binding?: LabelmapBinding) => - binding && boundScalars(binding.image, binding.extent); +const byRegistryOrder = + (registry: SegmentRegistry) => (a: SegmentMask, b: SegmentMask) => + registry.orderIndexOf(a.segmentId) - registry.orderIndexOf(b.segmentId); -/** - * The given segments as one parent-shaped labelmap, built on demand and never - * stored: what leaves VolView means the whole segmentation, not one segment's - * bounded mask. Earlier in the registry wins where two segments overlap, which - * is the order their actors stack in, so the flattened file resolves an - * overlap the way the screen did. `members` defaults to the image's segments; - * an export passes one group so no overlap is flattened away. - */ -export function compositeLabelmap( +/** Snapshot bounded geometry and appearance without allocating full-volume parts. */ +export function captureLabelmapParts( parentImageId: string, - members?: SegmentMask[] + parts: SegmentMask[][] ) { - useSegmentationEditsStore().beforeRead(); - const imageCacheStore = useImageCacheStore(); - const segmentRegistry = useSegmentStore().segments; - const imageMasks = useSegmentationStore().imageMasks; - const parent = imageCacheStore.getVtkImageData(parentImageId); - if (!parent) throw new Error('No such parent image'); - - const dimensions = parent.getDimensions(); - const labelmap = allocateMask(parent, fullExtent(dimensions)); - const values = maskScalars(labelmap); - - const included = [...(members ?? imageMasks(parentImageId))].sort( - (first, second) => - segmentRegistry.orderIndexOf(first.segmentId) - - segmentRegistry.orderIndexOf(second.segmentId) - ); - // One file carries one label per voxel, so the values are assigned here - // rather than read off the masks, which all hold SEGMENT_VALUE. Callers - // pass a group layeredSegments already sized to fit them. - const used = new Set(); - const segments: LabelmapSegment[] = []; - included.forEach((segment) => { - const labelValue = nextUnusedLabelValue(used, LABELMAP_MAX_VALUE); - used.add(labelValue); - segments.push( - toLabelmapSegment( - segmentRegistry.getSegment(segment.segmentId), - labelValue - ) - ); - }); - [...included].reverse().forEach((segment, index) => { - const bounded = boundedMask(segment.representations.labelmap); - const labelValue = segments[included.length - 1 - index].value; - if (bounded) writeMaskInto(values, dimensions, bounded, labelValue); + const source = useImageCacheStore().getVtkImageData(parentImageId); + if (!source) throw new Error('No such parent image'); + const parent = vtkImageData.newInstance({ + origin: [...source.getOrigin()], + spacing: [...source.getSpacing()], + direction: [...source.getDirection()], }); + parent.setDimensions(source.getDimensions()); + parent.computeTransforms(); + const registry = useSegmentStore().segments; + return { + parent, + parts: parts.map((part) => + [...part].sort(byRegistryOrder(registry)).map((mask, index) => { + const bounded = boundScalars(mask.representations.labelmap); + return { + descriptor: toLabelmapSegment( + registry.getSegment(mask.segmentId), + index + 1 + ), + bounded: bounded && { + ...bounded, + scalars: bounded.scalars.slice(), + }, + }; + }) + ), + }; +} + +type CapturedPart = ReturnType['parts'][number]; - return { labelmap, segments }; +export function composeLabelmapPart(parent: vtkImageData, part: CapturedPart) { + const labelmap = allocateLabelmap(parent, part.length); + const values = labelmapScalars(labelmap); + for (const { descriptor, bounded } of [...part].reverse()) { + if (bounded) + writeMaskInto(values, parent.getDimensions(), bounded, descriptor.value); + } + return { labelmap, segments: part.map(({ descriptor }) => descriptor) }; } -/** - * The image's segments grouped so no group holds an overlap. A labelmap file - * carries one label per voxel, so an export writes a file per group. Always - * at least one group: an image with no segments still exports one file. - */ -export function layeredSegments(parentImageId: string) { - const groups = groupByLayer( - useSegmentationStore().imageMasks(parentImageId), - (segment) => boundedMask(segment.representations.labelmap) +/** Plan overlap-free files and retain why more than one file is necessary. */ +export function planLabelmapExport( + parentImageId: string, + preferredSegmentId?: string +) { + const inRegistryOrder = byRegistryOrder(useSegmentStore().segments); + const masks = [...useSegmentationStore().imageMasks(parentImageId)].sort( + (a, b) => { + if (a.segmentId === preferredSegmentId) return -1; + if (b.segmentId === preferredSegmentId) return 1; + return inRegistryOrder(a, b); + } ); - // One byte per voxel caps a file's segments however little they overlap, - // so a group past the cap is split into files that fit. - const sized = groups.flatMap((group) => - group.length <= LABELMAP_MAX_VALUE - ? [group] - : Array.from( - { length: Math.ceil(group.length / LABELMAP_MAX_VALUE) }, - (_, n) => - group.slice(n * LABELMAP_MAX_VALUE, (n + 1) * LABELMAP_MAX_VALUE) - ) + // A mask holding no voxels still takes a label value and ships as an empty + // segment: a consumer that declared the bin gets to see it came back empty. + const layers = groupByLayer(masks, (mask) => + boundScalars(mask.representations.labelmap) ); - return sized.length ? sized : [[]]; + const parts = layers.flatMap((layer) => chunk(layer, LABELMAP_MAX_VALUE)); + return { + parts: parts.length ? parts : [[]], + hasOverlap: layers.length > 1, + exceedsCapacity: parts.length > layers.length, + }; } diff --git a/src/segmentation/io/export.ts b/src/segmentation/io/export.ts index 08c1a7ed9..6f9a95622 100644 --- a/src/segmentation/io/export.ts +++ b/src/segmentation/io/export.ts @@ -1,4 +1,13 @@ import JSZip from 'jszip'; +import { + captureLabelmapParts, + composeLabelmapPart, +} from '@/src/segmentation/io/composition'; +import { writeSegmentation } from '@/src/io/readWriteImage'; +import { sanitizeSegmentationFileStem } from '@/src/io/state-file/maskArchivePath'; +import type { SegmentMask } from '@/src/segmentation/model'; +import { isRegularImage } from '@/src/utils/dataSelection'; +import { stripExtension } from '@/src/utils/path'; export type ExportFile = { name: string; @@ -6,14 +15,47 @@ export type ExportFile = { }; /** - * The name a group's file carries. The first keeps the plain stem, so a - * segmentation with no overlap saves as the one file it always did. + * The name of one export part's file. Parts past the first come from overlap + * or from the 65535-label capacity; the first keeps the plain stem. */ export const layerFileName = (stem: string, format: string, layer: number) => layer === 0 ? `${stem}.${format}` : `${stem}_layer${layer}.${format}`; export const archiveNameFor = (stem: string) => `${stem}.zip`; +/** + * The stem a segmentation is saved and staged under. Its name starts as its + * image's, so a file-backed one drops the file's extension, and a DICOM series + * name stays whole: its dots are not an extension. + */ +export const segmentationFileStem = (parentImageId: string, name: string) => + sanitizeSegmentationFileStem( + isRegularImage(parentImageId) ? stripExtension(name) : name + ); + +export async function writeLabelmapParts( + { parentId, parts }: { parentId: string; parts: SegmentMask[][] }, + stem: string, + format: string, + { + deliver, + write = writeSegmentation, + }: { + deliver: (file: ExportFile) => Promise | void; + write?: typeof writeSegmentation; + } +) { + // Captured before the first await, so an edit made meanwhile misses the files. + const snapshot = captureLabelmapParts(parentId, parts); + // One at a time: serializing copies the whole buffer, and itk-wasm queues + // the writes on one shared worker whatever the caller does. + for (const [index, part] of snapshot.parts.entries()) { + const { labelmap, segments } = composeLabelmapPart(snapshot.parent, part); + const data = await write(format, labelmap, segments); + await deliver({ name: layerFileName(stem, format, index), data }); + } +} + /** * What a save hands to the browser: the single file itself, or every file in * one archive. A labelmap file carries one label per voxel, so segments that diff --git a/src/segmentation/io/import.ts b/src/segmentation/io/import.ts index c6600a99e..f2e71be12 100644 --- a/src/segmentation/io/import.ts +++ b/src/segmentation/io/import.ts @@ -9,21 +9,23 @@ import { ensureSameSpace } from '@/src/io/resample/resample'; import { overlaySegmentMetadata, parseSegNrrdMetadata, + type ParsedSegment, } from '@/src/io/segNrrdMetadata'; import { useDICOMStore } from '@/src/store/datasets-dicom'; import { useImageCacheStore } from '@/src/store/image-cache'; import { LABELMAP_BACKGROUND_VALUE, makeDefaultSegmentName, - maskScalars, type LabelmapSegment, } from '@/src/segmentation/model'; import { emptyExtent, extentSize, + extentUnion, isEmptyExtent, maskOffset, type Extent3D, + type MaskBounds, growExtent, } from '@/src/segmentation/geometry'; import { @@ -34,56 +36,62 @@ import { import vtkImageExtractComponents from '@/src/utils/imageExtractComponentsFilter'; import vtkLabelMap from '@/src/vtk/LabelMap'; -const LabelmapArrayType = Uint8Array; - -export type ImportedSegment = { sourceValue: number; maskId: string }; - -function convertToUint8(array: number[] | TypedArray): Uint8Array { - const uint8Array = new Uint8Array(array.length); - for (let i = 0; i < array.length; i++) { - const value = array[i]; - uint8Array[i] = value < 0 || value > 255 ? 0 : value; - } - return uint8Array; -} +import { + labelmapScalars, + normalizeLabelmapScalars, +} from '@/src/segmentation/io/labelmap'; +import { SEGMENT_VALUE } from '@/src/segmentation/masks/labelValue'; -function getLabelMapScalars(imageData: vtkImageData) { - const scalars = imageData.getPointData().getScalars(); - let values = scalars.getData(); +type ImportedSegment = { sourceValue: number; maskId: string }; - if (!(values instanceof LabelmapArrayType)) { - values = convertToUint8(values); - } +export const componentCount = (image: vtkImageData) => + image.getPointData().getScalars().getNumberOfComponents(); - return vtkDataArray.newInstance({ - numberOfComponents: scalars.getNumberOfComponents(), - values, - }); -} - -export function toLabelMap(imageData: vtkImageData) { +function labelMapOnGrid( + imageData: vtkImageData, + values: TypedArray, + numberOfComponents: number +) { const labelmap = vtkLabelMap.newInstance( imageData.get('spacing', 'origin', 'direction', 'extent', 'dataDescription') ); labelmap.setDimensions(imageData.getDimensions()); labelmap.computeTransforms(); + labelmap + .getPointData() + .setScalars(vtkDataArray.newInstance({ numberOfComponents, values })); + return labelmap; +} - // outline rendering only supports UInt8Array image types - const scalars = getLabelMapScalars(imageData); - labelmap.getPointData().setScalars(scalars); +export function toLabelMap(imageData: vtkImageData) { + const { values, excluded } = normalizeLabelmapScalars( + imageData.getPointData().getScalars().getData() + ); + const labelmap = labelMapOnGrid(imageData, values, componentCount(imageData)); + return { labelmap, excluded }; +} - return labelmap; +/** + * A saved mask's own archive entry as mask storage, where any nonzero voxel is + * claimed: every reader tests for SEGMENT_VALUE. Undefined for a + * multi-component entry, which bounded storage cannot index. + */ +export function toBinaryMask(imageData: vtkImageData) { + if (componentCount(imageData) > 1) return undefined; + const source = imageData.getPointData().getScalars().getData(); + const values = new Uint8Array(source.length); + for (let index = 0; index < source.length; index += 1) { + if (source[index]) values[index] = SEGMENT_VALUE; + } + return labelMapOnGrid(imageData, values, 1); } function extractEachComponent(input: vtkImageData) { - const numComponents = input - .getPointData() - .getScalars() - .getNumberOfComponents(); + if (componentCount(input) === 1) return [input]; const extractComponentsFilter = vtkImageExtractComponents.newInstance(); extractComponentsFilter.setInputData(input); - return Array.from({ length: numComponents }, (_, i) => { + return Array.from({ length: componentCount(input) }, (_, i) => { extractComponentsFilter.setComponents([i]); extractComponentsFilter.update(); return extractComponentsFilter.getOutputData() as vtkImageData; @@ -101,7 +109,7 @@ function labelValueBounds(labelmap: vtkLabelMap) { const cached = boundsCache.get(labelmap); if (cached?.mTime === labelmap.getMTime()) return cached.bounds; - const scalars = maskScalars(labelmap); + const scalars = labelmapScalars(labelmap); const [di, dj, dk] = labelmap.getDimensions(); const bounds = new Map(); @@ -122,65 +130,51 @@ function labelValueBounds(labelmap: vtkLabelMap) { return bounds; } -type LabelmapSweep = { - scalars: Uint8Array; - dimensions: number[] | Int32Array; - value: number; -}; - -/** Copies one label value's voxels into `mask`, rewritten to `labelValue`. */ -function cropLabelValue( - sweep: LabelmapSweep, - extent: Extent3D, - mask: Uint8Array, - labelValue: number -) { - const [di, dj] = sweep.dimensions; - const [mi, mj] = extentSize(extent); - const bounds = { extent, mi, mj }; - - const copyRow = (j: number, k: number) => { - const sourceStart = (j + k * dj) * di; - const maskStart = maskOffset(bounds, extent[0], j, k); - for (let i = extent[0]; i <= extent[1]; i += 1) { - if (sweep.scalars[sourceStart + i] !== sweep.value) continue; - mask[maskStart + i - extent[0]] = labelValue; - } - }; +type CropTarget = MaskBounds & { mask: Uint8Array }; - for (let k = extent[4]; k <= extent[5]; k += 1) - for (let j = extent[2]; j <= extent[3]; j += 1) copyRow(j, k); -} - -export type MaskMinter = ( - descriptor: LabelmapSegment, - extent: Extent3D -) => { labelValue: number; mask: Uint8Array }; +type MaskMinter = (descriptor: LabelmapSegment, extent: Extent3D) => Uint8Array; /** * Each descriptor gets a mask cropped to the box its value's voxels span, - * filled with the value the minter assigned it. + * holding SEGMENT_VALUE whatever the source called that value. */ export function splitLabelmap( labelmap: vtkLabelMap, descriptors: LabelmapSegment[], mint: MaskMinter ) { - const scalars = maskScalars(labelmap); - const dimensions = labelmap.getDimensions(); + const scalars = labelmapScalars(labelmap); + const [di, dj] = labelmap.getDimensions(); const bounds = labelValueBounds(labelmap); + // Indexed by source value, so the sweep below reads each voxel once however + // many labels there are. Two descriptors may state one value. + const targets: CropTarget[][] = []; descriptors.forEach((descriptor) => { const extent = bounds.get(descriptor.value) ?? emptyExtent(); - const { labelValue, mask } = mint(descriptor, extent); + const mask = mint(descriptor, extent); if (isEmptyExtent(extent)) return; - cropLabelValue( - { scalars, dimensions, value: descriptor.value }, - extent, - mask, - labelValue - ); + const [mi, mj] = extentSize(extent); + const target = { extent, mi, mj, mask }; + (targets[descriptor.value] ??= []).push(target); }); + + const copyRow = (j: number, k: number, i0: number, i1: number) => { + const rowStart = (j + k * dj) * di; + for (let i = i0; i <= i1; i += 1) { + const hits = targets[scalars[rowStart + i]]; + if (!hits) continue; + for (const hit of hits) + hit.mask[maskOffset(hit, i, j, k)] = SEGMENT_VALUE; + } + }; + + // A target's voxels all lie in its extent, so only their union is swept. + const extents = targets.flat().map(({ extent }) => extent); + if (!extents.length) return; + const [i0, i1, j0, j1, k0, k1] = extents.reduce(extentUnion); + for (let k = k0; k <= k1; k += 1) + for (let j = j0; j <= j1; j += 1) copyRow(j, k, i0, i1); } /** DICOM-SEG carries its own catalog; anything else has to be derived. */ @@ -206,7 +200,7 @@ async function segBuildDescriptors( ); } -const distinctLabelValues = (image: vtkLabelMap) => +export const distinctLabelValues = (image: vtkLabelMap) => [...labelValueBounds(image).keys()].sort((first, second) => first - second); export type DecodeOptions = { @@ -216,7 +210,30 @@ export type DecodeOptions = { headerMetadata?: Map; /** What undescribed segments are named after, in place of 'Segment'. */ baseName?: string; - nextColor: () => readonly number[]; + /** + * The values the components read so far carry, and whether this is the last + * component of the file. Reading one component of a multi-component labelmap + * passes it, so a value the header describes but no component carries is + * declared once for the file rather than once per component. + */ + declared?: { covered: Set; last: boolean }; + nextColor: () => RGBAColor; +}; + +/** A declared value is one bin per file, empty if no component carries it. */ +const declaredOncePerFile = ( + merged: ParsedSegment[], + carried: number[], + declared: DecodeOptions['declared'] +) => { + if (!declared) return merged; + const enumerated = new Set(carried); + enumerated.forEach((value) => declared.covered.add(value)); + return merged.filter( + (segment) => + enumerated.has(segment.value) || + (declared.last && !declared.covered.has(segment.value)) + ); }; /** A lone value carries the base name bare: there is nothing to tell apart. */ @@ -257,21 +274,30 @@ export async function decodeLabelmapSegments( const values = distinctLabelValues(image); const nameFor = fallbackNamer(values, options.baseName); - return overlaySegmentMetadata(values, described, (value) => ({ + const merged = overlaySegmentMetadata(values, described, (value) => ({ value, name: nameFor(value), - color: [...options.nextColor()] as RGBAColor, + color: options.nextColor(), visible: true, })); + return declaredOncePerFile(merged, values, options.declared); } export type LabelmapImportHooks = { + resample?: typeof ensureSameSpace; + /** + * Descriptors for one component. On the `last` one, a decode can add the + * descriptors no component's voxels carried, once rather than per component. + */ decode: ( labelmap: vtkLabelMap, - component: number + component: number, + last: boolean ) => Promise; /** Mints the masks for one decoded labelmap, in descriptor order. */ split: (labelmap: vtkLabelMap, descriptors: LabelmapSegment[]) => string[]; + /** Told how many voxels of one component lost an unsupported label. */ + excluded?: (voxels: number) => void; }; /** @@ -279,10 +305,8 @@ export type LabelmapImportHooks = { * run, so the parent is resolved through the cache again after every await: * nothing may be decoded or minted against an image that left the scene. */ -function requireParentImage(parentID: DataSelection) { - const parentImage = getImage(parentID); - if (!parentImage) throw new Error('Parent image is no longer loaded'); - return parentImage; +function assertParentLoaded(parentID: DataSelection) { + if (!getImage(parentID)) throw new Error('Parent image is no longer loaded'); } /** @@ -318,32 +342,41 @@ export async function importLabelmapImage( ); } - const componentCount = childImage - .getPointData() - .getScalars() - .getNumberOfComponents(); - const images = - componentCount === 1 ? [childImage] : extractEachComponent(childImage); - - // Sequential, not fanned out: the splits share one segmentation, and each - // binds its segments against the ones already in it. - const created: ImportedSegment[][] = []; + const images = extractEachComponent(childImage); + + const prepared: Array<{ + labelmap: vtkLabelMap; + descriptors: LabelmapSegment[]; + }> = []; const cache = useImageCacheStore(); for (const [component, image] of images.entries()) { - const matchingParentSpace = await ensureSameSpace(parentImage, image, true); - requireParentImage(parentID); - const labelmapImage = toLabelMap(matchingParentSpace); - const descriptors = await hooks.decode(labelmapImage, component); - requireParentImage(parentID); + const matchingParentSpace = await (hooks.resample ?? ensureSameSpace)( + parentImage, + image, + true + ); + assertParentLoaded(parentID); + const { labelmap: labelmapImage, excluded } = + toLabelMap(matchingParentSpace); + if (excluded) hooks.excluded?.(excluded); + const descriptors = await hooks.decode( + labelmapImage, + component, + component === images.length - 1 + ); + assertParentLoaded(parentID); if (!cache.imageById[imageID]) { throw new Error('Labelmap image is no longer loaded'); } - created.push( - hooks.split(labelmapImage, descriptors).map((maskId, index) => ({ - sourceValue: descriptors[index].value, - maskId, - })) - ); + prepared.push({ labelmap: labelmapImage, descriptors }); } - return created; + + // Finish fallible asynchronous work before creating any masks. The splits + // commit in order without yielding, so each sees the preceding bindings. + return prepared.map(({ labelmap, descriptors }): ImportedSegment[] => + hooks.split(labelmap, descriptors).map((maskId, index) => ({ + sourceValue: descriptors[index].value, + maskId, + })) + ); } diff --git a/src/segmentation/io/labelmap.ts b/src/segmentation/io/labelmap.ts new file mode 100644 index 000000000..dcbf41f7a --- /dev/null +++ b/src/segmentation/io/labelmap.ts @@ -0,0 +1,70 @@ +import vtkDataArray from '@kitware/vtk.js/Common/Core/DataArray'; +import type vtkImageData from '@kitware/vtk.js/Common/DataModel/ImageData'; +import type { TypedArray } from '@kitware/vtk.js/types'; +import vtkLabelMap from '@/src/vtk/LabelMap'; +import { fullExtent } from '@/src/segmentation/geometry'; +import { placeMask } from '@/src/segmentation/masks/storage'; + +export const LABELMAP_MAX_VALUE = 65535; +const LABELMAP_BYTE_MAX_VALUE = 255; +type LabelmapScalars = Uint8Array | Uint16Array; + +export const labelmapScalars = (image: vtkImageData) => + image.getPointData().getScalars().getData() as LabelmapScalars; + +const labelmapArrayType = (maxValue: number) => { + if (maxValue > LABELMAP_MAX_VALUE) { + throw new Error(`A labelmap holds at most ${LABELMAP_MAX_VALUE} segments`); + } + return maxValue > LABELMAP_BYTE_MAX_VALUE ? Uint16Array : Uint8Array; +}; + +/** Interchange storage has distinct labels; editable masks remain binary. */ +export function allocateLabelmap(parent: vtkImageData, count: number) { + const ArrayType = labelmapArrayType(count); + const image = vtkLabelMap.newInstance(); + const dimensions = placeMask( + image, + parent, + fullExtent(parent.getDimensions()) + ); + const values = new ArrayType(dimensions[0] * dimensions[1] * dimensions[2]); + image + .getPointData() + .setScalars(vtkDataArray.newInstance({ numberOfComponents: 1, values })); + return image; +} + +// Number.isInteger also excludes NaN and the infinities. +const isLabelValue = (value: number) => + value >= 0 && value <= LABELMAP_MAX_VALUE && Number.isInteger(value); + +/** + * Unsupported values become background instead of wrapping into another + * segment; `excluded` counts the voxels that lost theirs. + */ +export function normalizeLabelmapScalars(input: number[] | TypedArray) { + if (input instanceof Uint8Array) return { values: input, excluded: 0 }; + // Both passes are hot over whole volumes, so they index the input directly. + // A fresh typed array is already zeroed, so an excluded voxel needs no write. + const { length } = input; + let maximum = 0; + for (let index = 0; index < length; index += 1) { + const value = input[index]; + if (value > maximum && isLabelValue(value)) maximum = value; + } + const ArrayType = labelmapArrayType(maximum); + // Every value a Uint8Array or Uint16Array can hold is a label value. + if (input instanceof ArrayType) return { values: input, excluded: 0 }; + const values = new ArrayType(length); + let excluded = 0; + for (let index = 0; index < length; index += 1) { + const value = input[index]; + if (isLabelValue(value)) values[index] = value; + else excluded += 1; + } + return { values, excluded }; +} + +export const unsupportedLabelsReason = (voxels: number) => + `${voxels} voxels hold labels other than whole numbers 0 to ${LABELMAP_MAX_VALUE}`; diff --git a/src/segmentation/io/restore.ts b/src/segmentation/io/restore.ts index 3fa6e39a1..ddfd7867c 100644 --- a/src/segmentation/io/restore.ts +++ b/src/segmentation/io/restore.ts @@ -1,20 +1,14 @@ import { markRaw } from 'vue'; -import { until } from '@vueuse/core'; -import type { ProgressiveImage } from '@/src/core/progressiveImage'; import type vtkImageData from '@kitware/vtk.js/Common/DataModel/ImageData'; import type { Segmentation } from '@/src/io/state-file/schema'; import { placeMask, setMaskScalars } from '@/src/segmentation/masks/storage'; -import type { ProcessingResultSource } from '@/src/types'; -import { - LABELMAP_BACKGROUND_VALUE, - maskScalars, - type LabelmapBinding, -} from '@/src/segmentation/model'; +import { maskScalars, type LabelmapBinding } from '@/src/segmentation/model'; import { extentContains, extentSize, fullExtent, + hasMarkedVoxel, isEmptyExtent, type Extent3D, } from '@/src/segmentation/geometry'; @@ -23,29 +17,6 @@ import type vtkLabelMap from '@/src/vtk/LabelMap'; export type WireMask = Segmentation['masks'][number]; -export function createLoadedImageReader( - getImage: (id: string) => ProgressiveImage | undefined, - getVtkImageData: (id: string) => vtkImageData | undefined -) { - return async (imageId: string) => { - // A stopped, incomplete load cannot supply the grid for an input. - // Removal also settles the watcher, including removal before it starts. - await until(() => !getImage(imageId)?.loading.value).toBe(true); - if (getImage(imageId)?.status.value !== 'complete') { - throw new Error('Image did not load'); - } - const image = getVtkImageData(imageId); - if (!image) throw new Error('Could not get input image data'); - return image; - }; -} - -export type LoadedLabelmap = { - labelmap: vtkLabelMap; - name: string; - source?: ProcessingResultSource; -}; - export type SkippedRestoreItem = { name: string; reason: string }; type RestoreBindingInput = { @@ -55,37 +26,26 @@ type RestoreBindingInput = { * What each mask's own archive entry held. A mask awaiting an input's * split is absent: its voxels are still inside that input. */ - loaded: Map; + loaded: Map; getParentImage: (id: string) => vtkImageData | undefined; }; -const sameDimensions = (extent: Extent3D, dimensions: number[]) => - arrayEquals(extentSize(extent), dimensions); - -function validExtent( +function extentProblem( extent: Extent3D, labelmap: vtkLabelMap, - parentImage: vtkImageData, - reject: (reason: string) => void + parentImage: vtkImageData ) { - if (isEmptyExtent(extent)) { - const containsForeground = maskScalars(labelmap).some( - (value) => value !== LABELMAP_BACKGROUND_VALUE - ); - if (!containsForeground) return true; - reject('empty extent references a mask with foreground voxels'); - return false; - } - - if (!sameDimensions(extent, labelmap.getDimensions())) { - reject('extent does not match the loaded mask dimensions'); - return false; - } - if (!extentContains(fullExtent(parentImage.getDimensions()), extent)) { - reject('extent leaves the parent image'); - return false; - } - return true; + if (!extent.every(Number.isInteger)) + return 'extent coordinates must be finite integers'; + if (isEmptyExtent(extent)) + return hasMarkedVoxel(maskScalars(labelmap)) + ? 'empty extent references a mask with foreground voxels' + : undefined; + if (!arrayEquals(extentSize(extent), labelmap.getDimensions())) + return 'extent does not match the loaded mask dimensions'; + if (!extentContains(fullExtent(parentImage.getDimensions()), extent)) + return 'extent leaves the parent image'; + return undefined; } /** @@ -101,10 +61,11 @@ export function prepareRestoreBindings(input: RestoreBindingInput) { const place = (wireMask: WireMask, parentImage: vtkImageData | undefined) => { const wireBinding = wireMask.representations.labelmap; - const available = loaded.get(wireMask); - if (!wireBinding || !available) return; + const labelmap = loaded.get(wireMask); + if (!wireBinding || !labelmap) return; - const { name, labelmap, source } = available; + const name = wireBinding.name ?? ''; + const { source } = wireBinding; const reject = (reason: string) => skipped.push({ name: name || wireBinding.path, reason }); @@ -114,7 +75,11 @@ export function prepareRestoreBindings(input: RestoreBindingInput) { } const extent = [...wireBinding.extent] as Extent3D; - if (!validExtent(extent, labelmap, parentImage, reject)) return; + const problem = extentProblem(extent, labelmap, parentImage); + if (problem) { + reject(problem); + return; + } placeMask(labelmap, parentImage, extent); if (isEmptyExtent(extent)) setMaskScalars(labelmap, new Uint8Array(0)); diff --git a/src/segmentation/io/stateFile.ts b/src/segmentation/io/stateFile.ts index 6fc826fd0..ac6c8661b 100644 --- a/src/segmentation/io/stateFile.ts +++ b/src/segmentation/io/stateFile.ts @@ -2,15 +2,14 @@ import type { Ref, ComputedRef } from 'vue'; import type vtkImageData from '@kitware/vtk.js/Common/DataModel/ImageData'; import vtkLabelMap from '@/src/vtk/LabelMap'; +import { untilLoaded } from '@/src/composables/untilLoaded'; import { useSegmentationEditsStore } from '@/src/segmentation/editing/coordinator'; import { allocateMask } from '@/src/segmentation/masks/storage'; -import { SEGMENT_VALUE } from '@/src/segmentation/masks/labelValue'; import { - createLoadedImageReader, orderedWireMasks, prepareRestoreBindings, unlistedWireMasks, - type LoadedLabelmap, + type SkippedRestoreItem, type WireMask, } from '@/src/segmentation/io/restore'; import { readImage, writeSegmentation } from '@/src/io/readWriteImage'; @@ -25,8 +24,17 @@ import type { FileEntry } from '@/src/io/types'; import type { Maybe, ProcessingResultSource } from '@/src/types'; import { toLabelmapSegment } from '@/src/segmentation/segment'; import { cleanUndefined } from '@/src/utils'; -import { normalize } from '@/src/utils/path'; -import { splitLabelmap, toLabelMap } from '@/src/segmentation/io/import'; +import { COMPOUND_EXTENSIONS, normalize } from '@/src/utils/path'; +import { FILE_EXTENSIONS } from '@/src/io/mimeTypes'; +import { + componentCount, + distinctLabelValues, + splitLabelmap, + toBinaryMask, + toLabelMap, + type DecodeOptions, +} from '@/src/segmentation/io/import'; +import { unsupportedLabelsReason } from '@/src/segmentation/io/labelmap'; import { ensureSameSpace } from '@/src/io/resample/resample'; import { useDatasetStore } from '@/src/store/datasets'; import { @@ -61,6 +69,18 @@ export type LabelmapIO = { // ZIP entries are relative; extraction may prefix a root member with a slash. const archivePathKey = (path: string) => normalize(path).replace(/^\/+/, ''); +// Longest first, so 'seg.nrrd' ends a name before 'nrrd' does. +const IMAGE_EXTENSIONS = [...COMPOUND_EXTENSIONS, ...FILE_EXTENSIONS].sort( + (a, b) => b.length - a.length +); + +/** An artifact name is display text, not a path: only a known extension ends it. */ +const stripImageExtension = (name: string) => { + const lower = name.toLowerCase(); + const extension = IMAGE_EXTENSIONS.find((ext) => lower.endsWith(`.${ext}`)); + return extension ? name.slice(0, -(extension.length + 1)) : name; +}; + const defaultLabelmapIO: LabelmapIO = { write: writeSegmentation, read: readImage, @@ -94,15 +114,15 @@ async function mapWithLimit( return results; } -export type SegmentationWireDeps = { +type SegmentationWireDeps = { segmentations: Record; saveFormat: Ref; imageCacheStore: ReturnType; segmentRegistry: SegmentRegistry; labelmapDescriptorByMask: ComputedRef>; createMask: (segmentationId: string, segmentId: string) => SegmentMask; - createBindingForImage: ( - parentImageId: string, + allocateMaskBinding: ( + maskId: string, extent: Extent3D, source?: ProcessingResultSource, name?: string @@ -114,8 +134,8 @@ export type SegmentationWireDeps = { decodeSegments: ( imageId: DataSelection | undefined, image: vtkLabelMap, - options?: { component?: number; headerMetadata?: Map } - ) => Promise & { color: number[] }>>; + options?: Pick + ) => Promise; ensureSegmentationForImage: (parentImageId: string) => Segmentation; getSegmentationForImage: (parentImageId: string) => Segmentation | undefined; maskFor: ( @@ -134,7 +154,7 @@ export type SegmentationWireDeps = { ) => SegmentMask[]; }; -export type DeserializeOptions = { +type DeserializeOptions = { manifest: Manifest; stateFiles: FileEntry[]; dataIDMap: Record; @@ -142,7 +162,7 @@ export type DeserializeOptions = { segmentIdMap?: Record; /** * Per-import restore source, resolved by the restore setup (see - * resolveLabelmapSources in labelmapImports.ts, the single owner of + * planLabelmapSources in labelmapImports.ts, the single owner of * the synthesized-leaf and ownership policy). Mapped through dataIDMap here. */ labelmapSources?: Record; @@ -163,7 +183,7 @@ export function createSegmentationWire(deps: SegmentationWireDeps) { segmentRegistry, labelmapDescriptorByMask, createMask, - createBindingForImage, + allocateMaskBinding, attachMaskBinding, decodeSegments, ensureSegmentationForImage, @@ -275,7 +295,7 @@ export function createSegmentationWire(deps: SegmentationWireDeps) { const restoredImportIds = new Set(); // Non-silent drops: every labelmap left out of the restore is recorded // with a concrete reason so the caller can surface it. - const skipped: Array<{ name: string; reason: string }> = []; + const skipped: SkippedRestoreItem[] = []; // A path-less item's store id: the restore setup already resolved which // STATE id carries its bytes; this only maps that id through dataIDMap. @@ -320,10 +340,13 @@ export function createSegmentationWire(deps: SegmentationWireDeps) { return read; } - const loadedImage = createLoadedImageReader( - (id) => imageCacheStore.imageById[id], - (id) => imageCacheStore.getVtkImageData(id) ?? undefined - ); + // Settles by the rule a live import waits by, so both treat an image alike. + const loadedImage = async (imageId: string) => { + await untilLoaded(imageId); + const image = imageCacheStore.getVtkImageData(imageId); + if (!image) throw new Error('Could not get input image data'); + return image; + }; // Skip before awaiting anything an item whose parent image is unresolved, // whose archive member is missing, or whose datasource never materialized, @@ -370,7 +393,7 @@ export function createSegmentationWire(deps: SegmentationWireDeps) { const { image, headerMetadata } = await readImport(item, storeId); // Bounded conversion indexes a labelmap as single-component, so a // multi-component one would split into shifted, truncated masks. - if (image.getPointData().getScalars().getNumberOfComponents() > 1) { + if (componentCount(image) > 1) { skipped.push({ name: item.name, reason: 'multi-component labelmap artifacts are not supported', @@ -388,18 +411,45 @@ export function createSegmentationWire(deps: SegmentationWireDeps) { }); return undefined; } - const labelmap = toLabelMap( + const { labelmap, excluded } = toLabelMap( await ensureSameSpace(parent, image, true) ); - // A group that carried no descriptors is enumerated here, through + if (excluded) + skipped.push({ + name: item.name, + reason: unsupportedLabelsReason(excluded), + }); + // An import that carried no descriptors is enumerated here, through // the same decode live import uses, while its source image is // still loaded: the temp item dataset is dropped below. - const decoded = item.decode - ? ((await decodeSegments(storeId, labelmap, { - headerMetadata, - })) as LabelmapSegment[]) - : undefined; - return { item, labelmap, decoded }; + const decode = () => + decodeSegments(storeId, labelmap, { + headerMetadata, + // Named after the labelmap, as a live import of its file is. + baseName: + storeId === undefined + ? stripImageExtension(item.name) || undefined + : undefined, + }); + if (item.decode) + return { item, labelmap, decoded: await decode(), unclaimed: [] }; + // Values no saved mask claims keep a default segment, as a live + // import of the same labelmap would give them. Decoding draws + // from the shared color cycle, so it runs only when one exists. + // Only carried values count, so a header's declared-but-empty + // value never depends on whether some other value was unclaimed. + const claimed = new Set(item.masks.map(({ value }) => value)); + const unclaimedValues = new Set( + distinctLabelValues(labelmap).filter( + (value) => !claimed.has(value) + ) + ); + const unclaimed = unclaimedValues.size + ? (await decode()).filter(({ value }) => + unclaimedValues.has(value) + ) + : []; + return { item, labelmap, decoded: undefined, unclaimed }; } catch { // A parse/read failure skips just this item and never rejects the // whole restore; the survivors still attach. @@ -418,12 +468,11 @@ export function createSegmentationWire(deps: SegmentationWireDeps) { // A saved mask names an archive entry of its own, read into a buffer of // its own: masks share no storage, whatever a hand-edited manifest says. - const maskLabelmaps = new Map(); + const maskLabelmaps = new Map(); const wireMasks = wireSegmentations.flatMap(orderedWireMasks); await mapWithLimit(wireMasks, MASK_IO_CONCURRENCY, async (wireMask) => { const binding = wireMask.representations.labelmap; if (binding?.path === undefined) return; - const name = binding.name ?? ''; const file = archiveMember(binding.path); if (!file) { skipped.push({ @@ -433,12 +482,13 @@ export function createSegmentationWire(deps: SegmentationWireDeps) { return; } try { - const { image } = await io.read(file); - maskLabelmaps.set(wireMask, { - labelmap: toLabelMap(image), - name, - ...(binding.source ? { source: binding.source } : {}), - }); + const mask = toBinaryMask((await io.read(file)).image); + if (mask) maskLabelmaps.set(wireMask, mask); + else + skipped.push({ + name: maskLabel(wireMask), + reason: 'multi-component masks are not supported', + }); } catch { // One unreadable mask never rejects the restore; the rest attach. skipped.push({ @@ -450,15 +500,15 @@ export function createSegmentationWire(deps: SegmentationWireDeps) { // Reads, resampling and decoding yield to image deletion. Recheck before // creating any masks, after every asynchronous placement step has settled. - loaded = loaded.filter((result) => { - if (!result) return false; + const placed = loaded.flatMap((result) => { + if (!result) return []; if (imageCacheStore.getVtkImageData(dataIDMap[result.item.parentImage])) - return true; + return [result]; skipped.push({ name: result.item.name, reason: 'parent image is unavailable', }); - return false; + return []; }); const prepared = prepareRestoreBindings({ segmentations: wireSegmentations, @@ -522,27 +572,24 @@ export function createSegmentationWire(deps: SegmentationWireDeps) { // All asynchronous reads have settled. Fill the masks already placed in // wire order; their identities, selection and tool references stay intact. - loaded.forEach((result) => { - if (!result) return; - const { item, labelmap, decoded } = result; + placed.forEach(({ item, labelmap, decoded, unclaimed }) => { const parentImageId = dataIDMap[item.parentImage]; - let restored: SegmentMask[]; - if (decoded) { - const descriptors = decoded.map((descriptor) => ({ - ...descriptor, - ...cleanUndefined({ - fillOpacity: item.display.fillOpacity, - outlineOpacity: item.display.outlineOpacity, - visible: - item.display.visible === undefined - ? undefined - : descriptor.visible && item.display.visible, - }), - })); - restored = splitLabelmapIntoMasks( + // Segments decoded from the labelmap take a migrated group's display. + const splitDecoded = (descriptors: LabelmapSegment[]) => + splitLabelmapIntoMasks( parentImageId, labelmap, - descriptors, + descriptors.map((descriptor) => ({ + ...descriptor, + ...cleanUndefined({ + fillOpacity: item.display.fillOpacity, + outlineOpacity: item.display.outlineOpacity, + visible: + item.display.visible === undefined + ? undefined + : descriptor.visible && item.display.visible, + }), + })), { source: item.source, name: item.name, @@ -553,7 +600,10 @@ export function createSegmentationWire(deps: SegmentationWireDeps) { ), } ); - const activeIndex = descriptors.findIndex( + let restored: SegmentMask[]; + if (decoded) { + restored = splitDecoded(decoded); + const activeIndex = decoded.findIndex( (descriptor) => descriptor.value === item.activeValue ); segmentRegistry.restoreSelection(restored[activeIndex]?.segmentId); @@ -575,20 +625,19 @@ export function createSegmentationWire(deps: SegmentationWireDeps) { targets.map(({ descriptor }) => descriptor), (descriptor, extent) => { const mask = maskByDescriptor.get(descriptor)!; - const binding = createBindingForImage( - parentImageId, + const binding = allocateMaskBinding( + mask.id, extent, item.source, item.name ); - attachMaskBinding(mask.id, binding); - return { - labelValue: SEGMENT_VALUE, - mask: maskScalars(binding.image), - }; + return maskScalars(binding.image); } ); - restored = targets.map(({ mask }) => mask); + restored = [ + ...targets.map(({ mask }) => mask), + ...(unclaimed.length ? splitDecoded(unclaimed) : []), + ]; } if (restored.length) restoredImportIds.add(item.id); else diff --git a/src/segmentation/masks/labelValue.ts b/src/segmentation/masks/labelValue.ts index 5a72c3557..4c19eb433 100644 --- a/src/segmentation/masks/labelValue.ts +++ b/src/segmentation/masks/labelValue.ts @@ -1,6 +1,3 @@ -/** A composed labelmap is written as Uint8, so a label value has to fit in one byte. */ -export const LABELMAP_MAX_VALUE = 255; - /** * The value every mask marks its own voxels with. A mask holds one segment and * nothing else, so the byte says only claimed or not; which segment it belongs @@ -8,26 +5,3 @@ export const LABELMAP_MAX_VALUE = 255; * write time, where one file does have to tell segments apart per voxel. */ export const SEGMENT_VALUE = 1; - -import { LABELMAP_BACKGROUND_VALUE } from '@/src/segmentation/model'; - -export function nextUnusedLabelValue( - used: Set, - maximum: number, - preferred?: number -) { - if ( - preferred !== undefined && - preferred > LABELMAP_BACKGROUND_VALUE && - preferred <= maximum && - !used.has(preferred) - ) { - return preferred; - } - let labelValue = LABELMAP_BACKGROUND_VALUE + 1; - while (used.has(labelValue)) labelValue += 1; - if (labelValue > maximum) { - throw new Error(`An image holds at most ${maximum} segments in a labelmap`); - } - return labelValue; -} diff --git a/src/segmentation/masks/overlap.ts b/src/segmentation/masks/overlap.ts index 2b4b50c07..dd8a1132a 100644 --- a/src/segmentation/masks/overlap.ts +++ b/src/segmentation/masks/overlap.ts @@ -9,8 +9,10 @@ import { extentContainsIndex, extentSize, extentUnion, + fullExtent, isEmptyExtent, maskOffset, + walkExtentRows, type Extent3D, type MaskBounds, } from '@/src/segmentation/geometry'; @@ -27,13 +29,14 @@ export type BoundedScalars = MaskBounds & { * The extent is copied because the callers read it per voxel and a segment's * own copy lives in the reactive tree. */ -export function boundScalars( - mask: vtkLabelMap | undefined, - bounds: Extent3D -): BoundedScalars | undefined { - if (!mask || isEmptyExtent(bounds)) return undefined; - const extent = [...bounds] as Extent3D; +export function boundScalars(binding?: { + image: vtkLabelMap; + extent: Extent3D; +}) { + if (!binding || isEmptyExtent(binding.extent)) return undefined; + const extent = [...binding.extent] as Extent3D; const [mi, mj] = extentSize(extent); + const mask = binding.image; return { mask, scalars: maskScalars(mask), extent, mi, mj }; } @@ -41,7 +44,10 @@ export function boundScalars( * Clipping once here keeps a mask that misses the box out of the per-voxel * containment test, and lets the caller skip the walk when none is left. */ -const masksReaching = (masks: BoundedScalars[], within: Extent3D) => +const masksReaching = ( + masks: T[], + within: Extent3D +) => masks.filter((bounded) => !isEmptyExtent(clipExtent(bounded.extent, within))); /** @@ -74,9 +80,12 @@ export function masksHolding(masks: BoundedScalars[], within: Extent3D) { * Absent when no mask reaches `within`, the box the caller is about to walk. A * mask that does not reach the voxel has nothing there to clear, so nothing * grows. Finish the operation in a finally block to publish each changed mask - * once, including partial writes. + * once, including partial writes; finish returns the masks it changed. */ -export function masksClearing(masks: BoundedScalars[], within: Extent3D) { +export function masksClearing( + masks: T[], + within: Extent3D +) { const reaching = masksReaching(masks, within); if (reaching.length === 0) return undefined; // Flagged by position: a Set would hash a mask per cleared voxel. @@ -98,10 +107,10 @@ export function masksClearing(masks: BoundedScalars[], within: Extent3D) { return { clear, finish: () => { - reaching.forEach((bounded, index) => { - if (changed[index]) bounded.mask.modified(); - }); + const cleared = reaching.filter((_, index) => changed[index]); + cleared.forEach((bounded) => bounded.mask.modified()); changed.fill(0); + return cleared; }, }; } @@ -180,15 +189,14 @@ function maskRows( row: (from: number, to: number, count: number) => boolean ) { const { extent } = bounded; - const [ni, nj, nk] = extentSize(extent); - for (let index = 0; index < nj * nk; index += 1) { - const j = extent[2] + (index % nj); - const k = extent[4] + Math.floor(index / nj); - const from = maskOffset(bounded, extent[0], j, k); - const to = maskOffset(into, extent[0], j, k); - if (row(from, to, ni)) return true; - } - return false; + const [ni] = extentSize(extent); + return walkExtentRows(extent, (j, k) => + row( + maskOffset(bounded, extent[0], j, k), + maskOffset(into, extent[0], j, k), + ni + ) + ); } const occupancyHits = (occupied: Occupancy, bounded: BoundedScalars) => @@ -273,24 +281,20 @@ export function groupByLayer( * writes by precedence. */ export function writeMaskInto( - values: Uint8Array, + values: Uint8Array | Uint16Array, dimensions: readonly number[], bounded: BoundedScalars, labelValue: number ) { - const { extent, scalars } = bounded; - const [dx, dy] = dimensions; - const [ni, nj, nk] = extentSize(extent); - // One flat row loop: nested j and k loops exceed the depth limit. - for (let row = 0; row < nj * nk; row += 1) { - const j = extent[2] + (row % nj); - const k = extent[4] + Math.floor(row / nj); - const to = extent[0] + j * dx + k * dx * dy; - const from = maskOffset(bounded, extent[0], j, k); - for (let n = 0; n < ni; n += 1) { + const { scalars } = bounded; + const [mi, mj] = dimensions; + const parent = { extent: fullExtent(dimensions), mi, mj }; + maskRows(bounded, parent, (from, to, count) => { + for (let n = 0; n < count; n += 1) { // Background is 0, so a voxel this mask leaves unclaimed keeps whatever // the buffer already holds there. if (scalars[from + n]) values[to + n] = labelValue; } - } + return false; + }); } diff --git a/src/segmentation/masks/storage.ts b/src/segmentation/masks/storage.ts index cf486d964..aae1fbac7 100644 --- a/src/segmentation/masks/storage.ts +++ b/src/segmentation/masks/storage.ts @@ -30,15 +30,15 @@ export function placeMask( : parent.indexToWorld([extent[0], extent[2], extent[4]] as Vector3) ); mask.setOrigin(origin as Vector3); + mask.setSpacing(parent.getSpacing()); + mask.setDirection(parent.getDirection()); mask.setDimensions(dimensions as Vector3); mask.computeTransforms(); return dimensions; } export function allocateMask(parent: vtkImageData, extent: Extent3D) { - const mask = vtkLabelMap.newInstance( - parent.get('spacing', 'origin', 'direction') - ); + const mask = vtkLabelMap.newInstance(); const dimensions = placeMask(mask, parent, extent); setMaskScalars( mask, diff --git a/src/segmentation/masks/voxelAccess.ts b/src/segmentation/masks/voxelAccess.ts index 89dfdc867..746155a8f 100644 --- a/src/segmentation/masks/voxelAccess.ts +++ b/src/segmentation/masks/voxelAccess.ts @@ -2,7 +2,7 @@ import type { TypedArray } from '@kitware/vtk.js/types'; import { partition } from '@/src/utils'; import type { VoxelGesture } from '@/src/segmentation/model'; -import type { useImageCacheStore } from '@/src/store/image-cache'; +import type vtkImageData from '@kitware/vtk.js/Common/DataModel/ImageData'; import { regrowMask } from '@/src/segmentation/masks/storage'; import { boundScalars, @@ -29,7 +29,7 @@ import { } from '@/src/segmentation/geometry'; type VoxelAccessDeps = { - imageCacheStore: ReturnType; + parentImageOfMask: (maskId: string) => vtkImageData; findMaskBinding: (maskId: string) => LabelmapBinding | undefined; getMask: (maskId: string) => SegmentMask; segmentationOfMask: (maskId: string) => Segmentation | undefined; @@ -45,7 +45,7 @@ type VoxelAccessDeps = { */ export function createVoxelAccess(deps: VoxelAccessDeps) { const { - imageCacheStore, + parentImageOfMask, findMaskBinding, getMask, segmentationOfMask, @@ -54,14 +54,6 @@ export function createVoxelAccess(deps: VoxelAccessDeps) { overlapAllowed, } = deps; - function requireParentImage(maskId: string) { - const segmentation = segmentationOfMask(maskId); - if (!segmentation) throw new Error('No such mask'); - const parent = imageCacheStore.getVtkImageData(segmentation.parentImageId); - if (!parent) throw new Error('No such parent image'); - return parent; - } - /** * Grows one mask, in place, to cover `extent` in parent index space, with * `padding` voxels of room beyond it when it has to grow at all. @@ -71,7 +63,7 @@ export function createVoxelAccess(deps: VoxelAccessDeps) { const binding = getMask(maskId).representations.labelmap; if (!binding) throw new Error('No storage: call materialize() first'); - const parent = requireParentImage(maskId); + const parent = parentImageOfMask(maskId); // Refused before anything is touched, so a rejected growth leaves the mask // exactly as it was. const parentExtent = fullExtent(parent.getDimensions()); @@ -148,10 +140,6 @@ export function createVoxelAccess(deps: VoxelAccessDeps) { const findMaskVoxels = (maskId: string) => voxelStorage(maskId, 'No such mask'); - /** A bound segment's buffer, absent when it has none or holds nothing. */ - const boundedMask = (binding?: LabelmapBinding) => - binding && boundScalars(binding.image, binding.extent); - /** * The masks of an image's other segments, split by what `gesture` does where * one of them holds a voxel: take the voxel from it, or yield to it and leave @@ -170,7 +158,10 @@ export function createVoxelAccess(deps: VoxelAccessDeps) { others ); const bounded = (masks: SegmentMask[]) => - masks.flatMap((mask) => boundedMask(mask.representations.labelmap) ?? []); + masks.flatMap((mask) => { + const scalars = boundScalars(mask.representations.labelmap); + return scalars ? [{ ...scalars, maskId: mask.id }] : []; + }); return { takeFrom: bounded(takeFrom), yieldTo: bounded(yieldTo) }; } @@ -185,6 +176,8 @@ export function createVoxelAccess(deps: VoxelAccessDeps) { * * `gesture` is the whole of the policy, so see {@link VoxelGesture}. An aimed * operation must call finish in a finally block after its last voxel write. + * Finish returns the ids of the masks it took a voxel from, which the + * operation hands to `deleteEmptyMasks` once it is over. */ function voxelClaim(maskId: string, gesture: VoxelGesture, within: Extent3D) { const { takeFrom, yieldTo } = siblingMasks(maskId, gesture); @@ -197,17 +190,13 @@ export function createVoxelAccess(deps: VoxelAccessDeps) { clearing?.clear(i, j, k); return true; }, - finish: () => clearing?.finish(), + finish: () => clearing?.finish().map((bounded) => bounded.maskId) ?? [], }; } return { - requireParentImage, - ensureMaskContains, maskVoxels, findMaskVoxels, - boundedMask, - siblingMasks, voxelClaim, }; } diff --git a/src/segmentation/model.ts b/src/segmentation/model.ts index 4d069c952..d4efedb9c 100644 --- a/src/segmentation/model.ts +++ b/src/segmentation/model.ts @@ -42,8 +42,11 @@ export type SegmentMask = { /** * Whether a mask holds anything. A record alone is not content, and neither is * storage bound over an empty extent: both mean the segment was resolved on - * this image but never painted. Readers that count or gate on what an image - * actually has ask this instead of whether the record exists. + * this image but never painted. Storage over any other extent holds a voxel, + * since an edit deletes a mask it leaves empty once it ends; only a restored + * file, or an edit that has not ended, can still bind an allocation holding + * none. Readers that count or gate on what an image actually has ask this + * instead of whether the record exists. */ export const maskHasContent = (mask: SegmentMask) => { const binding = mask.representations.labelmap; @@ -143,6 +146,10 @@ export function listMasks(segmentation: Segmentation) { return segmentation.order.map((id) => segmentation.masks[id]); } +/** Whether saving or staging this segmentation would write a voxel. */ +export const segmentationHasContent = (segmentation: Segmentation) => + listMasks(segmentation).some(maskHasContent); + /** * Aimed writes take voxels from unlocked neighbors and go around locked ones, * or leave every neighbor alone while overlap is allowed. Sweeps only grow into diff --git a/src/segmentation/segment.ts b/src/segmentation/segment.ts index f1ff545ab..1f948bf94 100644 --- a/src/segmentation/segment.ts +++ b/src/segmentation/segment.ts @@ -4,8 +4,9 @@ import { STROKE_WIDTH_ANNOTATION_TOOL_DEFAULT, TOOL_COLORS, } from '@/src/config'; +import { NO_NAME } from '@/src/constants'; import type { Maybe } from '@/src/types'; -import { cleanUndefined } from '@/src/utils'; +import { arrayEquals, cleanUndefined, sameFields } from '@/src/utils'; import type { LabelmapSegment } from '@/src/segmentation/model'; import { cssColorToRGBA, rgbaToCssColor } from '@/src/segmentation/color'; @@ -61,7 +62,12 @@ export const resolveSegmentAppearance = (segment: Maybe) => { strokeWidth: stated.strokeWidth, }), }; - return { ...resolved, cssColor: rgbaToCssColor(resolved.color) }; + return { + ...resolved, + // What every list and label shows for a segment whose name is empty. + displayName: resolved.name || NO_NAME, + cssColor: rgbaToCssColor(resolved.color), + }; }; /** @@ -85,11 +91,7 @@ export const toLabelmapSegment = ( }; }; -export const sameLabelmapSegment = (a: LabelmapSegment, b: LabelmapSegment) => - a.value === b.value && - a.name === b.name && - a.visible === b.visible && - a.locked === b.locked && - a.fillOpacity === b.fillOpacity && - a.outlineOpacity === b.outlineOpacity && - a.color.every((channel, index) => channel === b.color[index]); +export const sameLabelmapSegment = ( + { color, ...fields }: LabelmapSegment, + { color: otherColor, ...otherFields }: LabelmapSegment +) => arrayEquals(color, otherColor) && sameFields(fields, otherFields); diff --git a/src/segmentation/segmentRegistry.ts b/src/segmentation/segmentRegistry.ts index 9608f1bca..545215359 100644 --- a/src/segmentation/segmentRegistry.ts +++ b/src/segmentation/segmentRegistry.ts @@ -10,6 +10,7 @@ import { type SegmentInit, } from '@/src/segmentation/segment'; import { cleanUndefined, cycle } from '@/src/utils'; +import { makeDefaultSegmentName } from '@/src/segmentation/model'; import type { ConfiguredSegments } from '@/src/io/import/configSegments'; type SegmentReferenceHolder = { @@ -37,7 +38,7 @@ const configuredAppearance = ({ /** * Identity and shared appearance for a family of segments: one instance backs * paint, rectangles, polygons and rulers together. Explicit order drives the - * picker, shortcuts, serialization and labelmap stacking. + * picker, shortcuts, serialization and export precedence. */ export const createSegmentRegistry = () => { // Stores holding masks or shapes declare them here; the registry knows neither. @@ -114,8 +115,7 @@ export const createSegmentRegistry = () => { const appearanceOf = (id: Maybe) => resolveSegmentAppearance(getSegment(id)); - // Cached: the renderer asks for one index per mask per re-render, and the - // export sort asks twice per comparison. + // Cached: the export sort asks twice per comparison. const orderIndex = computed( () => new Map(segmentOrder.value.map((id, index) => [id, index])) ); @@ -132,21 +132,30 @@ export const createSegmentRegistry = () => { return getSegment(first); }; - const lowestFreeName = (prefix: string, tail: string, first: number) => { - let index = suffixFloors.get(prefix) ?? first; - while (nameTaken(`${prefix}${index}${tail}`)) index += 1; - suffixFloors.set(prefix, index); - return `${prefix}${index}${tail}`; + // `family` keys the floor: every name `nameFor` gives belongs to it. + const lowestFreeName = ( + family: string, + nameFor: (index: number) => string, + first: number + ) => { + let index = suffixFloors.get(family) ?? first; + while (nameTaken(nameFor(index))) index += 1; + suffixFloors.set(family, index); + return nameFor(index); }; // The name index and every lookup ignore surrounding space, so the stem has // to be trimmed as well. const uniqueName = (stem: string) => { const base = stem.trim(); - return nameTaken(base) ? lowestFreeName(`${base} (`, ')', 2) : base; + return nameTaken(base) + ? lowestFreeName(`${base} (`, (index) => `${base} (${index})`, 2) + : base; }; - const defaultName = () => lowestFreeName('Segment ', '', 1); + // A stem family ends in ' (', so this key never collides with one. + const defaultName = () => + lowestFreeName('default', makeDefaultSegmentName, 1); const nextToolColor = cycle(TOOL_COLORS); const nextColor = () => cssColorToRGBA(nextToolColor()); @@ -214,7 +223,9 @@ export const createSegmentRegistry = () => { segmentOrder.value = order; }; - const ensureSelectedSegment = () => selectedSegmentId.value ?? addSegment(); + // Mints without choosing: an automatic seat must not outrank a session's + // restored selection, and the first-row fallback already selects it. + const ensureSelectedSegment = () => selectedSegmentId.value ?? mintSegment(); /** * Exact-name lookup, minting on a miss. Import binds descriptors this way, @@ -232,34 +243,41 @@ export const createSegmentRegistry = () => { // Keep each key's identity and appearance beneath its config contribution. // New segments begin with automatic color and default optional appearance; // a restored segment begins with its session appearance. - const configEntries = new Map< - string, - { - id: string; - appearance: ReturnType; - minted: boolean; - } - >(); + type ConfigEntry = { + id: string; + appearance: ReturnType; + contribution: ReturnType; + minted: boolean; + }; + const configEntries = new Map(); + + const heldByConfig = (id: string) => + [...configEntries.values()].some((entry) => entry.id === id); + + const overlayConfig = (name: string, entry: ConfigEntry) => { + configEntries.set(name, entry); + updateSegment(entry.id, { ...entry.appearance, ...entry.contribution }); + }; + + const seatConfigured = (name: string) => { + const matched = findSegmentByName(name)?.id; + const id = matched ?? mintSegment({ name }); + return { + id, + appearance: configuredAppearance(getSegment(id)!), + minted: !matched, + }; + }; const replaceConfigSegments = (configured: Maybe) => { const next = configured ?? {}; Object.entries(next).forEach(([name, props]) => { - let entry = configEntries.get(name); - if (!entry || !getSegment(entry.id)) { - const matched = findSegmentByName(name)?.id; - const id = matched ?? mintSegment({ name }); - entry = { - id, - appearance: configuredAppearance(getSegment(id)!), - minted: !matched, - }; - } - updateSegment(entry.id, { - ...entry.appearance, - ...fromConfigured(name, props), + const held = configEntries.get(name); + overlayConfig(name, { + ...(held && getSegment(held.id) ? held : seatConfigured(name)), + contribution: fromConfigured(name, props), }); - configEntries.set(name, entry); }); [...configEntries.entries()] @@ -277,10 +295,34 @@ export const createSegmentRegistry = () => { const serialize = () => segmentList.value.map((segment) => ({ ...segment })); + // The session's appearance replaces the reused segment's own and a config + // holding the segment layers over it, as if the session had been restored + // first. + const restoreOnto = (id: string, init: SegmentInit) => { + updateSegment(id, { + fillOpacity: undefined, + outlineOpacity: undefined, + strokeWidth: undefined, + ...init, + }); + const held = [...configEntries].find(([, entry]) => entry.id === id); + // Now the session's segment as well, so dropping the key must not take it. + if (held) + overlayConfig(held[0], { + ...held[1], + appearance: configuredAppearance(getSegment(id)!), + minted: false, + }); + return id; + }; + /** - * Seats restored segments beside the ones already here. Ids are minted fresh - * and every incoming reference is remapped through the returned map, so an - * import into a populated scene overwrites nothing. + * Seats restored segments and returns the idMap every incoming reference is + * remapped through. An incoming segment joins the first existing segment of + * its name when that one holds no mask or shape on any image, restoring its + * own appearance onto it; otherwise it is minted under a unique name, so + * content already here keeps its segment. `repeated` lists the entries left + * unseated because an earlier one claimed their id. */ const adopt = (incoming: Maybe) => { // Prototype-free: a file's ids are its own, so an id spelling an @@ -288,14 +330,32 @@ export const createSegmentRegistry = () => { // already seen here, nor hand a caller an inherited member in place of a // miss when it looks the id up in the returned map. const idMap: Record = Object.create(null); - (incoming ?? []).forEach(({ id, ...init }) => { + const repeated: Segment[] = []; + // Seated by this call, so no later entry of the same name can join it. + const seated = new Set(); + const reusable = (name: string) => { + const existing = findSegmentByName(name)?.id; + return existing && !seated.has(existing) && !hasReferences(existing) + ? existing + : undefined; + }; + (incoming ?? []).forEach((segment) => { + const { id, ...init } = segment; // Nothing makes a file's ids unique, and only one segment can answer for // an id. The first entry wins; minting the rest as well would leave // segments in the sidebar that no mask or shape can ever reference. - if (Object.hasOwn(idMap, id)) return; - idMap[id] = mintSegment(init as SegmentInit); + if (Object.hasOwn(idMap, id)) { + repeated.push(segment); + return; + } + const reused = reusable(init.name); + const target = reused + ? restoreOnto(reused, init) + : mintSegment({ ...init, name: uniqueName(init.name) }); + seated.add(target); + idMap[id] = target; }); - return idMap; + return { idMap, repeated }; }; /** Selects a restored segment unless the user already chose one. */ @@ -322,6 +382,7 @@ export const createSegmentRegistry = () => { ensureSelectedSegment, segmentNamed, replaceConfigSegments, + heldByConfig, serialize, adopt, declareReferences, diff --git a/src/segmentation/segments.ts b/src/segmentation/segments.ts index c09ea7f91..fbb1ea583 100644 --- a/src/segmentation/segments.ts +++ b/src/segmentation/segments.ts @@ -1,8 +1,10 @@ import { defineStore } from 'pinia'; import { markRaw } from 'vue'; +import { onImageDeleted } from '@/src/composables/onImageDeleted'; import type { Manifest, StateFile } from '@/src/io/state-file/schema'; import { createSegmentRegistry } from '@/src/segmentation/segmentRegistry'; +import { useImageCacheStore } from '@/src/store/image-cache'; /** * Paint, rectangles, polygons and rulers share this registry and selection. @@ -10,6 +12,7 @@ import { createSegmentRegistry } from '@/src/segmentation/segmentRegistry'; */ export const useSegmentStore = defineStore('segments', () => { const registry = createSegmentRegistry(); + const imageCacheStore = useImageCacheStore(); function serialize(state: StateFile) { state.manifest.segments = registry.serialize(); @@ -17,15 +20,23 @@ export const useSegmentStore = defineStore('segments', () => { if (selected) state.manifest.selectedSegment = selected; } - /** Fresh ids for the incoming segments; the map remaps every reference. */ + /** Fresh ids for the incoming segments; segmentIdMap remaps every reference. */ function deserialize(manifest: Manifest) { - const segmentIdMap = registry.adopt(manifest.segments); + const { idMap: segmentIdMap, repeated } = registry.adopt(manifest.segments); registry.restoreSelection( manifest.selectedSegment && segmentIdMap[manifest.selectedSegment] ); - return segmentIdMap; + return { segmentIdMap, repeated }; } + // Swept as the last image leaves, never later, so a following load keeps its segments. + onImageDeleted(() => { + if (imageCacheStore.imageIds.length > 0) return; + registry.segmentList.value + .filter(({ id }) => !registry.heldByConfig(id)) + .forEach(({ id }) => registry.deleteSegment(id)); + }); + // Raw: a pinia store is reactive, and a proxy of the registry would unwrap // its refs out from under every consumer that holds them. return { diff --git a/src/segmentation/store.ts b/src/segmentation/store.ts index 734ab41d2..9fe4bfcf6 100644 --- a/src/segmentation/store.ts +++ b/src/segmentation/store.ts @@ -1,14 +1,10 @@ import { defineStore } from 'pinia'; -import { markRaw, reactive, ref } from 'vue'; +import { markRaw, reactive, readonly, ref, shallowReactive } from 'vue'; import type { RGBAColor } from '@kitware/vtk.js/types'; import { CATEGORICAL_COLORS } from '@/src/config'; import { NO_NAME } from '@/src/constants'; import { createMaskFileNamer } from '@/src/segmentation/io/maskFileNaming'; -import { - LABELMAP_MAX_VALUE, - SEGMENT_VALUE, -} from '@/src/segmentation/masks/labelValue'; import { allocateMask } from '@/src/segmentation/masks/storage'; import { createSegmentProjection } from '@/src/segmentation/rendering/projection'; import { createVoxelAccess } from '@/src/segmentation/masks/voxelAccess'; @@ -17,16 +13,16 @@ import { createSegmentationWire, type LabelmapIO, } from '@/src/segmentation/io/stateFile'; - -export type { LabelmapIO }; -export { LABELMAP_MAX_VALUE }; import { onImageDeleted } from '@/src/composables/onImageDeleted'; import { declareManifestRefs } from '@/src/core/manifestRefs'; import { decodeLabelmapSegments, importLabelmapImage, splitLabelmap, + type DecodeOptions, + type LabelmapImportHooks, } from '@/src/segmentation/io/import'; +import { unsupportedLabelsReason } from '@/src/segmentation/io/labelmap'; import { useIdStore } from '@/src/store/id'; import { useImageCacheStore } from '@/src/store/image-cache'; import type { Maybe, ProcessingResultSource } from '@/src/types'; @@ -37,6 +33,8 @@ import { import { DEFAULT_SEGMENTATION_DISPLAY, listMasks, + makeDefaultSegmentName, + maskHasContent, maskScalars, type LabelmapBinding, type LabelmapSegment, @@ -46,9 +44,13 @@ import { } from '@/src/segmentation/model'; import { emptyExtent, - isEmptyExtent, + extentContainsIndex, + hasMarkedVoxel, + maskOffset, type Extent3D, } from '@/src/segmentation/geometry'; +import { boundScalars } from '@/src/segmentation/masks/overlap'; +import { SEGMENT_VALUE } from '@/src/segmentation/masks/labelValue'; import { useSegmentStore } from '@/src/segmentation/segments'; import { useMessageStore } from '@/src/store/messages'; import { @@ -60,7 +62,7 @@ import { } from '@/src/utils'; import vtkLabelMap from '@/src/vtk/LabelMap'; -export type { ImportedSegment } from '@/src/segmentation/io/import'; +export type { LabelmapIO }; // The manifest references this store's remove cascade keeps clean (see the // onImageDeleted registration below), declared for the dev-only save backstop. @@ -97,22 +99,36 @@ declareManifestRefs('segmentations', (manifest) => { }); }); +/** + * What a caller says about one source label value. Only `value` identifies the + * bin; everything else overrides what the labelmap's own metadata decoded. + */ +type SourceDescription = Pick & + Partial>; + +type LabelmapConversionOptions = { + source?: ProcessingResultSource; + descriptions?: SourceDescription[]; + resample?: LabelmapImportHooks['resample']; +}; + export const useSegmentationStore = defineStore('segmentation', () => { const edits = useSegmentationEditsStore(); const imageCacheStore = useImageCacheStore(); const segmentRegistry = useSegmentStore().segments; const segmentations = reactive>({}); - const convertingLabelmaps = reactive(new Set()); // The conversions running for each child image, by parent, so a second // caller for the same pair joins it instead of splitting the same labelmap // twice. The parent belongs in the key: the same child going onto another // parent is other work, and joining it would hand that caller masks made on - // an image it never named. - const conversions = new Map< - DataSelection, - Map> - >(); + // an image it never named. Shallow: readers only ask whether a child has one. + const conversions = shallowReactive( + new Map< + DataSelection, + Map> + >() + ); /** * How many bound masks hold each name, so picking a default name probes this * rather than walking every mask in the scene. A restore attaches the names @@ -168,6 +184,14 @@ export const useSegmentationStore = defineStore('segmentation', () => { return mask; } + /** The image whose grid a mask's voxels sit on. */ + function parentImageOfMask(maskId: string) { + const { parentImageId } = getSegmentationOfMask(maskId); + const image = imageCacheStore.getVtkImageData(parentImageId); + if (!image) throw new Error('No such parent image'); + return image; + } + const getSegmentationForImage = (parentImageId: string) => Object.values(segmentations).find( (segmentation) => segmentation.parentImageId === parentImageId @@ -204,32 +228,6 @@ export const useSegmentationStore = defineStore('segmentation', () => { return segmentation.masks[id]; } - /** - * A fresh binding over voxels on the parent's grid, covering `extent`. `name` - * is the name a manifest carried: it reaches the saved zip's entry path, so a - * restore that generated one instead would rename the file on every round - * trip. Duplicates are fine, serialize resolves the archive path against the - * ones it has already used. - */ - function createBindingForImage( - parentImageId: string, - extent: Extent3D = emptyExtent(), - source?: ProcessingResultSource, - name?: string - ): LabelmapBinding { - const imageData = imageCacheStore.getVtkImageData(parentImageId); - if (!imageData) throw new Error('No such parent image'); - - const baseName = - imageCacheStore.getImageMetadata(parentImageId)?.name ?? NO_NAME; - return { - image: markRaw(allocateMask(imageData, extent)), - extent, - name: name ?? maskFileNamer.pick(parentImageId, baseName), - ...(source ? { source } : {}), - }; - } - /** Attaches prepared storage to a mask without exposing its mutable record to importers. */ function attachMaskBinding(maskId: string, binding: LabelmapBinding) { const mask = getMask(maskId); @@ -244,16 +242,30 @@ export const useSegmentationStore = defineStore('segmentation', () => { return mask.representations.labelmap; } - function detachMask(segmentation: Segmentation, maskId: string) { - edits.beforeEdit(); - const mask = segmentation.masks[maskId]; - const { segmentId } = mask ?? {}; - const boundName = mask?.representations.labelmap?.name; - removeFromArray(segmentation.order, maskId); - delete segmentation.masks[maskId]; - const index = maskIdsBySegment.get(segmentation.id); - if (segmentId && index?.get(segmentId) === maskId) index.delete(segmentId); - if (boundName !== undefined) releaseMaskName(boundName); + /** + * Attaches fresh voxels on the parent's grid, covering `extent`. `name` is + * the name a manifest carried: it reaches the saved zip's entry path, so a + * restore that generated one instead would rename the file on every round + * trip. Duplicates are fine, serialize resolves the archive path against the + * ones it has already used. + */ + function allocateMaskBinding( + maskId: string, + extent: Extent3D = emptyExtent(), + source?: ProcessingResultSource, + name?: string + ) { + const { parentImageId } = getSegmentationOfMask(maskId); + const imageData = parentImageOfMask(maskId); + + const baseName = + imageCacheStore.getImageMetadata(parentImageId)?.name ?? NO_NAME; + return attachMaskBinding(maskId, { + image: allocateMask(imageData, extent), + extent, + name: name ?? maskFileNamer.pick(parentImageId, baseName), + ...(source ? { source } : {}), + }); } // `locked` is required, so a per-stroke read skips resolving the appearance. @@ -324,18 +336,14 @@ export const useSegmentationStore = defineStore('segmentation', () => { bindDescriptorSegment(parentImageId, descriptor, options.ownSegments) ); - const binding = createBindingForImage( - parentImageId, + const binding = allocateMaskBinding( + mask.id, extent, options.source, options.name ); - attachMaskBinding(mask.id, binding); created.push(mask); - - // The copy rewrites the source's value, so the mask holds SEGMENT_VALUE - // whatever the file it came from called this segment. - return { labelValue: SEGMENT_VALUE, mask: maskScalars(binding.image) }; + return maskScalars(binding.image); }); return created; @@ -350,25 +358,48 @@ export const useSegmentationStore = defineStore('segmentation', () => { function decodeSegments( imageId: DataSelection | undefined, image: vtkLabelMap, - options: { component?: number; headerMetadata?: Map } = {} + options: Pick< + DecodeOptions, + 'component' | 'headerMetadata' | 'declared' | 'baseName' + > = {} ) { return decodeLabelmapSegments(imageId, image, { ...options, // A descriptor-less labelmap reads as the file it arrived in, not as // 'Segment N'; the cold restore decodes through here too, so the two - // paths keep naming one labelmap alike. - baseName: imageId === undefined ? undefined : getSelectionStem(imageId), + // paths keep naming one labelmap alike. A caller reading bytes no loaded + // image holds says what they arrived as, since there is no selection + // here to take a name from. + baseName: + options.baseName ?? + (imageId === undefined ? undefined : getSelectionStem(imageId)), nextColor: getNextDecodeColor, }); } + // A declared value no component carried becomes an empty segment, so a + // result that found nothing reads differently from one that never looked. + function withDeclaredEmpties( + decoded: LabelmapSegment[], + bySourceValue: Map>, + covered: Set + ) { + const empties = [...bySourceValue] + .filter(([value]) => value !== 0 && !covered.has(value)) + .map(([value, description]) => ({ + ...description, + value, + name: description.name ?? makeDefaultSegmentName(value), + color: [...(description.color ?? getNextDecodeColor())] as RGBAColor, + visible: description.visible ?? true, + })); + return [...decoded, ...empties]; + } + async function convertImageToLabelmap( imageID: DataSelection, parentID: DataSelection, - source?: ProcessingResultSource, - descriptions: Array< - Pick & Partial> - > = [] + { source, descriptions = [], resample }: LabelmapConversionOptions = {} ) { // A second conversion of an image already converting onto the same parent // would split it again and mint a suffixed duplicate of every segment, and @@ -384,12 +415,26 @@ export const useSegmentationStore = defineStore('segmentation', () => { cleanUndefined(descriptor), ]) ); - convertingLabelmaps.add(imageID); + // Every source value any component of this image carries voxels for. A + // declaration is empty only when none of them did. + const coveredValues = new Set(); const conversion = importLabelmapImage(imageID, parentID, { - decode: (labelmap, component) => - decodeSegments(imageID, labelmap, { component }) as Promise< - LabelmapSegment[] - >, + resample, + // The empties join the descriptor list here, not at the split: the + // import pairs the masks the split returns with these descriptors by + // position, so the two lists have to be the same one. They wait for the + // last component, once every component has said which values it carries. + decode: async (labelmap, component, last) => { + const decoded = await decodeSegments(imageID, labelmap, { + component, + // The file header's own declarations wait for the last component the + // same way, through the same covered values. + declared: { covered: coveredValues, last }, + }); + decoded.forEach((descriptor) => coveredValues.add(descriptor.value)); + if (!last) return decoded; + return withDeclaredEmpties(decoded, bySourceValue, coveredValues); + }, split: (labelmap, descriptors) => { const created = splitLabelmapIntoMasks( parentID, @@ -399,11 +444,22 @@ export const useSegmentationStore = defineStore('segmentation', () => { descriptors.map((descriptor) => ({ ...descriptor, ...bySourceValue.get(descriptor.value), - })), - { source } + })) ); return created.map((mask) => mask.id); }, + excluded: (voxels) => + useMessageStore().addWarning( + 'Some labels could not be imported', + `${unsupportedLabelsReason(voxels)}; they were left empty.` + ), + }).then((created) => { + // A receipt only on a finished import, so a failed one is retried whole. + created.flat().forEach(({ maskId }) => { + const binding = findMask(maskId)?.representations.labelmap; + if (binding && source) binding.source = source; + }); + return created; }); const ontoParents = conversions.get(imageID) ?? new Map(); @@ -413,10 +469,7 @@ export const useSegmentationStore = defineStore('segmentation', () => { } finally { ontoParents.delete(parentID); // The child is still converting while it goes onto another parent. - if (ontoParents.size === 0) { - conversions.delete(imageID); - convertingLabelmaps.delete(imageID); - } + if (ontoParents.size === 0) conversions.delete(imageID); } } @@ -428,9 +481,10 @@ export const useSegmentationStore = defineStore('segmentation', () => { */ function startLabelmapConversion( imageID: DataSelection, - parentID: DataSelection + parentID: DataSelection, + options: LabelmapConversionOptions = {} ) { - return convertImageToLabelmap(imageID, parentID).catch((error) => { + return convertImageToLabelmap(imageID, parentID, options).catch((error) => { useMessageStore().addError('Failed to convert image to a labelmap', { error: ensureError(error), }); @@ -440,16 +494,8 @@ export const useSegmentationStore = defineStore('segmentation', () => { const saveFormat = ref('vti'); /** The single voxel-allocation point: no other operation creates storage. */ - function ensureLabelmapBinding(maskId: string) { - const segmentation = getSegmentationOfMask(maskId); - const mask = segmentation.masks[maskId]; - if (mask.representations.labelmap) return mask.representations.labelmap; - - return attachMaskBinding( - maskId, - createBindingForImage(segmentation.parentImageId) - ); - } + const ensureLabelmapBinding = (maskId: string) => + getMask(maskId).representations.labelmap ?? allocateMaskBinding(maskId); const findMaskBinding = (maskId: string) => findMask(maskId)?.representations.labelmap; @@ -458,7 +504,7 @@ export const useSegmentationStore = defineStore('segmentation', () => { const allowOverlap = ref(false); const { maskVoxels, findMaskVoxels, voxelClaim } = createVoxelAccess({ - imageCacheStore, + parentImageOfMask, findMaskBinding, getMask, segmentationOfMask, @@ -478,14 +524,10 @@ export const useSegmentationStore = defineStore('segmentation', () => { * is not editable, and holding voxels, since an empty mask has no content to * process. */ - function editableMasks(parentImageId: string) { - return imageMasks(parentImageId).flatMap((mask) => { - const binding = mask.representations.labelmap; - if (maskLocked(mask) || !binding || isEmptyExtent(binding.extent)) - return []; - return [{ maskId: mask.id, labelValue: SEGMENT_VALUE }]; - }); - } + const editableMasks = (parentImageId: string) => + imageMasks(parentImageId) + .filter((mask) => !maskLocked(mask) && maskHasContent(mask)) + .map((mask) => mask.id); /** * The ids of the masks an image draws, in `order`. One actor each; they are @@ -497,6 +539,25 @@ export const useSegmentationStore = defineStore('segmentation', () => { .filter((mask) => mask.representations.labelmap) .map((mask) => mask.id); + /** + * The segments whose masks hold the voxel at PARENT indices i, j, k of an + * image, in registry order: what the eyedropper picks from and the probe + * lists. Masks share the parent grid, so one index addresses every mask. + */ + const segmentsAt = (parentImageId: string, i: number, j: number, k: number) => + segmentRegistry.segmentList.value + .filter((segment) => { + const bounded = boundScalars( + maskFor(parentImageId, segment.id)?.representations.labelmap + ); + return ( + !!bounded && + extentContainsIndex(bounded.extent, i, j, k) && + bounded.scalars[maskOffset(bounded, i, j, k)] === SEGMENT_VALUE + ); + }) + .map(({ id }) => id); + const updateSegmentationDisplay = ( segmentationId: string, patch: SegmentationDisplayPatch @@ -504,7 +565,28 @@ export const useSegmentationStore = defineStore('segmentation', () => { /** The mask holds this segment and nothing else, so its voxels go with it. */ function deleteMask(maskId: string) { - detachMask(getSegmentationOfMask(maskId), maskId); + const segmentation = getSegmentationOfMask(maskId); + edits.beforeEdit(); + const { segmentId, representations } = segmentation.masks[maskId]; + removeFromArray(segmentation.order, maskId); + delete segmentation.masks[maskId]; + const index = maskIdsBySegment.get(segmentation.id); + if (index?.get(segmentId) === maskId) index.delete(segmentId); + if (representations.labelmap) + releaseMaskName(representations.labelmap.name); + } + + /** + * Deletes each of these masks that holds no voxel, since an erase never + * shrinks the allocation that content is read from. Edits call it once they + * end, with the masks they wrote or cleared; an id already gone is skipped. + */ + function deleteEmptyMasks(maskIds: Iterable) { + new Set(maskIds).forEach((maskId) => { + const binding = findMaskBinding(maskId); + if (binding && !hasMarkedVoxel(maskScalars(binding.image))) + deleteMask(maskId); + }); } function removeSegmentation(segmentationId: string) { @@ -581,16 +663,17 @@ export const useSegmentationStore = defineStore('segmentation', () => { return ensureMask(imageId, segmentId).id; } - const maskIdsOfSegment = (segmentId: string) => + /** A segment's mask on every image that has one, by the per-image index. */ + const masksOfSegment = (segmentId: string) => Object.values(segmentations).flatMap((segmentation) => { const maskId = maskIdsBySegment.get(segmentation.id)?.get(segmentId); - return maskId ? [maskId] : []; + return maskId ? [segmentation.masks[maskId]] : []; }); segmentRegistry.declareReferences('labelmaps', { - has: (segmentId) => maskIdsOfSegment(segmentId).length > 0, + has: (segmentId) => masksOfSegment(segmentId).length > 0, remove: (segmentId) => - maskIdsOfSegment(segmentId).forEach((maskId) => deleteMask(maskId)), + masksOfSegment(segmentId).forEach((mask) => deleteMask(mask.id)), }); // --- render sync --- // @@ -609,7 +692,7 @@ export const useSegmentationStore = defineStore('segmentation', () => { segmentRegistry, labelmapDescriptorByMask, createMask, - createBindingForImage, + allocateMaskBinding, attachMaskBinding, decodeSegments, ensureSegmentationForImage, @@ -630,9 +713,10 @@ export const useSegmentationStore = defineStore('segmentation', () => { return { segmentations, - convertingLabelmaps, + convertingLabelmaps: readonly(conversions), labelmapDescriptorByMask, maskFor, + masksOfSegment, findEditTarget, resolveEditTarget, editTargetLocked, @@ -655,9 +739,11 @@ export const useSegmentationStore = defineStore('segmentation', () => { saveFormat, allowOverlap, voxelClaim, + deleteEmptyMasks, imageMasks, editableMasks, boundMaskIds, + segmentsAt, serialize, deserialize, }; diff --git a/src/store/__tests__/annotationToolImageDelete.spec.ts b/src/store/__tests__/annotationToolImageDelete.spec.ts index 7f404768c..0e7051bce 100644 --- a/src/store/__tests__/annotationToolImageDelete.spec.ts +++ b/src/store/__tests__/annotationToolImageDelete.spec.ts @@ -11,19 +11,7 @@ import { useRectangleStore } from '@/src/store/tools/rectangles'; import type { Ruler } from '@/src/types/ruler'; import type { RequiredWithPartial } from '@/src/types'; -// --------------------------------------------------------------------------- -// Delete-base-then-save, tool half: removing an -// annotated dataset must remove its annotation tools too — an orphaned -// imageID serialized into the save manifest is exactly the backend's -// intentionally fail-closed 400 ('tool has unresolvable imageID'), turning a -// routine delete gesture into a permanently unsavable session. -// -// The mechanism is the same onImageDeleted subscription segmentGroups already -// uses for its labelmap cascade — so this spec also pins the composable -// itself: it must actually FIRE on image-cache deletion (a watch on the ref -// of the reactive index never triggers on a key delete; the composable -// watches the key SET). -// --------------------------------------------------------------------------- +// Serialized tools must never retain references to removed images. const seatImage = (id: string, name: string) => useImageCacheStore().addVTKImageData(vtkImageData.newInstance(), name, { diff --git a/src/store/__tests__/datasetRemoveCascade.spec.ts b/src/store/__tests__/datasetRemoveCascade.spec.ts index 3c5c6e18f..e6dc8762d 100644 --- a/src/store/__tests__/datasetRemoveCascade.spec.ts +++ b/src/store/__tests__/datasetRemoveCascade.spec.ts @@ -182,8 +182,9 @@ describe('dataset remove — synchronous reference cascade', () => { expect('img-1' in cropStore.croppingByImageID).toBe(false); }); - it('removes the records of a deleted image and keeps their type', () => { + it('removes the records of a deleted image and keeps their segment', () => { seatImage('img-1', 'CT'); + seatImage('img-2', 'PET'); const segmentationStore = useSegmentationStore(); const { maskId } = seatMask('img-1'); const { segmentId } = segmentationStore.getMask(maskId); @@ -192,7 +193,7 @@ describe('dataset remove — synchronous reference cascade', () => { useDatasetStore().remove('img-1'); expect(segmentationStore.maskExists(maskId)).toBe(false); - // A type outlives the images it was painted on, so it stays selected. + // A segment outlives an image it was painted on while another remains. expect(useSegmentStore().segments.selectedSegmentId.value).toBe(segmentId); }); diff --git a/src/store/__tests__/datasets-layers.spec.ts b/src/store/__tests__/datasets-layers.spec.ts index 7fac26bf0..653fd1168 100644 --- a/src/store/__tests__/datasets-layers.spec.ts +++ b/src/store/__tests__/datasets-layers.spec.ts @@ -13,7 +13,10 @@ vi.mock('@/src/io/resample/resample', () => ({ ensureSameSpace })); import { useLayersStore } from '@/src/store/datasets-layers'; import { useImageCacheStore } from '@/src/store/image-cache'; -import { useMessageStore } from '@/src/store/messages'; +import { + messageTitles, + mountMessageCenter, +} from '@/src/components/__tests__/messageDisplay'; import { ParentToLayers, type Manifest, @@ -76,7 +79,7 @@ describe('useLayersStore.addLayer return contract', () => { expect(id).toBeUndefined(); expect(store.getLayers('parent')).toHaveLength(0); expect(cached('parent::source')).toBe(false); - expect(useMessageStore().messages[0].options.details).toContain( + expect(mountMessageCenter().get('.details').text()).toContain( 'no overlap in physical space' ); }); @@ -89,7 +92,7 @@ describe('useLayersStore.addLayer return contract', () => { expect(id).toBeUndefined(); expect(store.getLayers('parent')).toHaveLength(0); - expect(useMessageStore().messages[0].options.details).toContain( + expect(mountMessageCenter().get('.details').text()).toContain( 'Image did not load' ); }); @@ -107,7 +110,7 @@ describe('useLayersStore.addLayer return contract', () => { expect(id).toBeUndefined(); expect(cached('parent::source')).toBe(false); - expect(useMessageStore().messages).toHaveLength(0); + expect(messageTitles()).toHaveLength(0); }); }); @@ -181,7 +184,7 @@ describe('useLayersStore.deserialize with an image that did not load', () => { await settle(); expect(Object.keys(store.parentToLayers)).toEqual([]); - expect(useMessageStore().messages).toHaveLength(0); + expect(messageTitles()).toHaveLength(0); // Before the guard this threw: the failed build left `parentToLayers` with // a key whose value was `undefined`, and serialize mapped over it. expect(resave()).toEqual([]); @@ -197,7 +200,7 @@ describe('useLayersStore.deserialize with an image that did not load', () => { await settle(); expect(store.getLayers('parent')).toHaveLength(0); - expect(useMessageStore().messages).toHaveLength(0); + expect(messageTitles()).toHaveLength(0); // Before the guard the parent's whole relationship was saved with an // `undefined` source key, which the save-time schema rejects outright. expect(ParentToLayers.safeParse(resave()).success).toBe(true); diff --git a/src/store/__tests__/image-stats.spec.ts b/src/store/__tests__/image-stats.spec.ts index 1a3515016..9cc3a170a 100644 --- a/src/store/__tests__/image-stats.spec.ts +++ b/src/store/__tests__/image-stats.spec.ts @@ -2,11 +2,13 @@ import { MessageChannel } from 'node:worker_threads'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { createPinia, disposePinia, setActivePinia } from 'pinia'; import { nextTick } from 'vue'; +import { flushPromises } from '@vue/test-utils'; +import { defer } from '@/src/utils'; import * as Comlink from 'comlink'; import { histogram } from '@/src/utils/histogram'; import { useImageCacheStore } from '@/src/store/image-cache'; import { useImageStatsStore } from '@/src/store/image-stats'; -import { useMessageStore } from '@/src/store/messages'; +import { messageTitles } from '@/src/components/__tests__/messageDisplay'; import { seatImage } from '@/src/segmentation/__tests__/segmentMaskFixtures'; // Real Comlink messages and histogram results, with completion controlled at @@ -26,11 +28,14 @@ class HistogramEndpoint { finish!: (error?: Error) => void; - started = false; + started = defer(); + + terminated = defer(); terminate = vi.fn(() => { this.channel.port1.close(); this.channel.port2.close(); + this.terminated.resolve(); }); constructor() { @@ -40,7 +45,7 @@ class HistogramEndpoint { Comlink.expose( { histogram: async (...args: Parameters) => { - this.started = true; + this.started.resolve(); await completion; return histogram(...args); }, @@ -90,18 +95,22 @@ describe('image statistics worker ownership', () => { ), }); const worker = workers[workers.length - 1]; - await vi.waitFor(() => expect(worker.started).toBe(true)); + await worker.started.promise; return worker; } - async function expectRanges(id: string, offset = 0) { - await vi.waitFor(() => { - expect(useImageStatsStore().getAutoRangeValues(id)).toEqual({ - FullRange: [offset, offset + 511], - LowContrast: [offset + 5, offset + 507], - MediumContrast: [offset + 10, offset + 502], - HighContrast: [offset + 25, offset + 487], - }); + const finish = async (worker: HistogramEndpoint, error?: Error) => { + worker.finish(error); + await worker.terminated.promise; + await flushPromises(); + }; + + function expectRanges(id: string, offset = 0) { + expect(useImageStatsStore().getAutoRangeValues(id)).toEqual({ + FullRange: [offset, offset + 511], + LowContrast: [offset + 5, offset + 507], + MediumContrast: [offset + 10, offset + 502], + HighContrast: [offset + 25, offset + 487], }); } @@ -110,14 +119,14 @@ describe('image statistics worker ownership', () => { const id = `image-${cycle}`; const worker = await startImage(id, cycle * 100 - 300); expect(worker.terminate).not.toHaveBeenCalled(); - worker.finish(); - await expectRanges(id, cycle * 100 - 300); + await finish(worker); + expectRanges(id, cycle * 100 - 300); expect(worker.terminate).toHaveBeenCalledExactlyOnceWith(); useImageCacheStore().removeImage(id); await nextTick(); expect(useImageStatsStore().stats[id]).toBeUndefined(); } - expect(useMessageStore().messages).toEqual([]); + expect(messageTitles()).toEqual([]); }); it('reclaims a rejected worker while other calculations and later loads succeed', async () => { @@ -125,24 +134,21 @@ describe('image statistics worker ownership', () => { const healthy = await startImage('healthy', -1000); const errors = vi.spyOn(console, 'error').mockImplementation(() => {}); - failed.finish(new Error('Histogram failed')); - await vi.waitFor(() => { - expect(useMessageStore().messages).toHaveLength(1); - }); - expect(useMessageStore().messages[0].title).toBe( - 'Auto range computation failed for image failed' - ); + await finish(failed, new Error('Histogram failed')); + expect(messageTitles()).toEqual([ + 'Auto range computation failed for image failed', + ]); expect(errors).toHaveBeenCalled(); expect(failed.terminate).toHaveBeenCalledExactlyOnceWith(); expect(healthy.terminate).not.toHaveBeenCalled(); expect(useImageStatsStore().getAutoRangeValues('failed')).toEqual({}); - healthy.finish(); - await expectRanges('healthy', -1000); + await finish(healthy); + expectRanges('healthy', -1000); expect(healthy.terminate).toHaveBeenCalledExactlyOnceWith(); const later = await startImage('later', 1000); - later.finish(); - await expectRanges('later', 1000); + await finish(later); + expectRanges('later', 1000); expect(later.terminate).toHaveBeenCalledExactlyOnceWith(); }); @@ -151,15 +157,13 @@ describe('image statistics worker ownership', () => { const healthy = await startImage('healthy'); useImageCacheStore().removeImage('removed'); await nextTick(); - removed.finish(); - await vi.waitFor(() => { - expect(removed.terminate).toHaveBeenCalledExactlyOnceWith(); - }); + await finish(removed); + expect(removed.terminate).toHaveBeenCalledExactlyOnceWith(); expect(useImageStatsStore().stats.removed).toBeUndefined(); expect(healthy.terminate).not.toHaveBeenCalled(); - healthy.finish(); - await expectRanges('healthy'); + await finish(healthy); + expectRanges('healthy'); expect(healthy.terminate).toHaveBeenCalledExactlyOnceWith(); - expect(useMessageStore().messages).toEqual([]); + expect(messageTitles()).toEqual([]); }); }); diff --git a/src/store/__tests__/legacyManifestSegmentGroups.spec.ts b/src/store/__tests__/legacyManifestSegmentGroups.spec.ts index 224803fde..bbb1d3f2b 100644 --- a/src/store/__tests__/legacyManifestSegmentGroups.spec.ts +++ b/src/store/__tests__/legacyManifestSegmentGroups.spec.ts @@ -7,7 +7,7 @@ import { useImageCacheStore } from '@/src/store/image-cache'; import { useDatasetStore } from '@/src/store/datasets'; import { ManifestSchema } from '@/src/io/state-file/schema'; import { migrateManifest } from '@/src/io/state-file/migrations'; -import { resolveLabelmapSources } from '@/src/io/import/labelmapImports'; +import { planLabelmapSources } from '@/src/io/import/labelmapImports'; import { listMasks } from '@/src/segmentation/model'; // --------------------------------------------------------------------------- @@ -53,10 +53,8 @@ const legacyManifest = ManifestSchema.parse( ) ); -const makeImage = () => makeSpecImage(); - const seatImage = (id: string, name: string) => - useImageCacheStore().addVTKImageData(makeImage(), name, { id }); + useImageCacheStore().addVTKImageData(makeSpecImage(), name, { id }); describe('migrated legacy manifests without `datasets`', () => { beforeEach(() => { @@ -74,8 +72,8 @@ describe('migrated legacy manifests without `datasets`', () => { stateFiles: [], // Restore keys every fallback dataset by its stringified source id. dataIDMap: { '1': 'store-ct', '3': 'store-seg' }, - segmentIdMap: useSegmentStore().deserialize(legacyManifest), - labelmapSources: resolveLabelmapSources(legacyManifest), + segmentIdMap: useSegmentStore().deserialize(legacyManifest).segmentIdMap, + labelmapSources: planLabelmapSources(legacyManifest).sources, }); expect(skipped).toEqual([]); @@ -106,8 +104,8 @@ describe('migrated legacy manifests without `datasets`', () => { manifest: legacyManifest, stateFiles: [], dataIDMap: { '1': 'store-ct', '3': 'store-seg' }, - segmentIdMap: useSegmentStore().deserialize(legacyManifest), - labelmapSources: resolveLabelmapSources(legacyManifest), + segmentIdMap: useSegmentStore().deserialize(legacyManifest).segmentIdMap, + labelmapSources: planLabelmapSources(legacyManifest).sources, }); const names = useSegmentStore().segments.segmentList.value.map( diff --git a/src/store/tools/__tests__/removeSelectedTools.spec.ts b/src/store/tools/__tests__/removeSelectedTools.spec.ts index e455706c2..de0723b0f 100644 --- a/src/store/tools/__tests__/removeSelectedTools.spec.ts +++ b/src/store/tools/__tests__/removeSelectedTools.spec.ts @@ -1,5 +1,4 @@ import { beforeEach, describe, expect, it } from 'vitest'; -import { useMessageStore } from '@/src/store/messages'; import { setActivePinia, createPinia } from 'pinia'; import { nextTick } from 'vue'; import vtkImageData from '@kitware/vtk.js/Common/DataModel/ImageData'; @@ -89,33 +88,4 @@ describe('removeSelectedTools', () => { expect(useRulerStore().toolByID).toHaveProperty(ruler); }); - - // the selection can hold annotations with no visible cue in the current view - // (hidden, other slices, other axes) and there is no undo - it('reports how many annotations were deleted', () => { - const selectionStore = useToolSelectionStore(); - selectionStore.addSelection(addRuler(), AnnotationToolType.Ruler); - selectionStore.addSelection(addRectangle(), AnnotationToolType.Rectangle); - - removeSelectedTools(); - - const messages = useMessageStore().messages; - expect(messages.at(-1)?.title).toBe('Deleted 2 annotations'); - }); - - it('uses the singular for a single deleted annotation', () => { - useToolSelectionStore().addSelection(addRuler(), AnnotationToolType.Ruler); - - removeSelectedTools(); - - expect(useMessageStore().messages.at(-1)?.title).toBe( - 'Deleted 1 annotation' - ); - }); - - it('says nothing when nothing was deleted', () => { - removeSelectedTools(); - - expect(useMessageStore().messages).toHaveLength(0); - }); }); diff --git a/src/store/tools/index.ts b/src/store/tools/index.ts index 850fa9ae6..9d061507e 100644 --- a/src/store/tools/index.ts +++ b/src/store/tools/index.ts @@ -8,8 +8,6 @@ import { useCrosshairsToolStore } from './crosshairs'; import { usePaintToolStore } from './paint'; import { useRulerStore } from './rulers'; import { useRectangleStore } from './rectangles'; -import { useMessageStore } from '@/src/store/messages'; -import { plural } from '@/src/utils'; import { AnnotationToolType, IToolStore, Tools } from './types'; import { usePolygonStore } from './polygons'; import { useToolSelectionStore } from './toolSelection'; @@ -86,22 +84,9 @@ export function useAnnotationToolStore( export function removeSelectedTools() { const selectionStore = useToolSelectionStore(); - // count what was actually removed: a selection entry can outlive its tool, - // and removeTool is a no-op for one that is already gone - const removed = [...selectionStore.selection].filter(({ id, type }) => { - const store = useAnnotationToolStore(type); - if (!(id in store.toolByID)) return false; - store.removeTool(id); - return true; - }).length; - - // the selection can hold annotations with no visible cue in the current view - // (hidden, other slices, other axes) and there is no undo, so say what went - if (removed > 0) { - useMessageStore().addInfo( - `Deleted ${removed} ${plural(removed, 'annotation')}` - ); - } + [...selectionStore.selection].forEach(({ id, type }) => { + useAnnotationToolStore(type).removeTool(id); + }); // clears any entry whose tool was already gone selectionStore.clearSelection(); diff --git a/src/store/tools/paint.ts b/src/store/tools/paint.ts index fd66855f6..6aba2c873 100644 --- a/src/store/tools/paint.ts +++ b/src/store/tools/paint.ts @@ -13,7 +13,6 @@ import { defineStore } from 'pinia'; import { PaintMode } from '@/src/core/tools/paint'; import { computeEffectiveView } from '@/src/core/views/effectiveView'; import { worldPointToIndex } from '@/src/utils/imageSpace'; -import { maskScalars } from '@/src/segmentation/model'; import { clipExtent, fullExtent, @@ -50,6 +49,9 @@ export const usePaintToolStore = defineStore('paint', () => { const crossPlaneSync = ref(false); const paintPosition = ref([0, 0, 0]); const activePaintViewID = ref>(null); + // Masks strokes wrote or cleared, checked for emptiness when a stroke ends + // rather than per sample. A stroke that never ended leaves them to the next. + const strokeMaskIds = new Set(); const { currentImageID, currentImageMetadata } = useCurrentImage('global'); const imageStatsStore = useImageStatsStore(); @@ -119,7 +121,7 @@ export const usePaintToolStore = defineStore('paint', () => { } /** - * The segment this operation writes into. It is allocated for a stroke that + * The mask this stroke writes into. It is allocated for a stroke that * writes voxels; an erase takes what is already there, so it resolves nothing * into existence and refuses when there is nothing stored to take from. */ @@ -153,27 +155,19 @@ export const usePaintToolStore = defineStore('paint', () => { } function selectSegmentAt(worldPoint: vec3, imageID: string) { + const parent = useImageCacheStore().getVtkImageData(imageID); + if (!parent) return; + // Masks share the parent grid, so one parent index addresses every mask. + const [i, j, k] = [...worldPointToIndex(parent, worldPoint)].map( + Math.round + ); const registry = useSegmentStore().segments; - // Earlier registry entries render in front, including locked segments. - const segments = registry.segmentList.value; - const hit = segments.find((segment) => { - if (!registry.appearanceOf(segment.id).visible) return false; - const binding = segmentationStore.maskFor(imageID, segment.id) - ?.representations.labelmap; - if (!binding || isEmptyExtent(binding.extent)) return false; - const point = [...worldPointToIndex(binding.image, worldPoint)].map( - Math.round - ); - const dims = binding.image.getDimensions(); - if (point.some((value, axis) => value < 0 || value >= dims[axis])) - return false; - const [i, j, k] = point; - return ( - maskScalars(binding.image)[i + dims[0] * (j + dims[1] * k)] === - SEGMENT_VALUE - ); - }); - if (hit) registry.selectSegment(hit.id); + // The eyedropper takes the first visible segment covering the point, + // including locked segments. + const hit = segmentationStore + .segmentsAt(imageID, i, j, k) + .find((segmentId) => registry.appearanceOf(segmentId).visible); + if (hit) registry.selectSegment(hit); } /** @@ -226,7 +220,6 @@ export const usePaintToolStore = defineStore('paint', () => { if (!target) return; const { voxels, maskId } = target; - this.$paint.setBrushValue(SEGMENT_VALUE); const underlyingImagePixels = parentImage .getPointData() .getScalars() @@ -279,7 +272,8 @@ export const usePaintToolStore = defineStore('paint', () => { shouldPaint, }); } finally { - claimVoxel?.finish(); + strokeMaskIds.add(maskId); + claimVoxel?.finish().forEach((cleared) => strokeMaskIds.add(cleared)); } } @@ -320,7 +314,13 @@ export const usePaintToolStore = defineStore('paint', () => { imageID: string ) { strokePoints.value.push(worldPoint); - doPaintStroke.call(this, axisIndex, imageID); + try { + doPaintStroke.call(this, axisIndex, imageID); + } finally { + const touched = [...strokeMaskIds]; + strokeMaskIds.clear(); + segmentationStore.deleteEmptyMasks(touched); + } } const currentImageStats = computed(() => { @@ -353,6 +353,7 @@ export const usePaintToolStore = defineStore('paint', () => { // allocated by the first stroke, so picking up the brush and putting it // down again leaves the image untouched. this.$paint.setBrushSize(this.brushSize); + this.$paint.setBrushValue(SEGMENT_VALUE); isActive.value = true; return true; diff --git a/src/store/tools/useAnnotationTool.ts b/src/store/tools/useAnnotationTool.ts index 62b2d6204..0ee6b50b4 100644 --- a/src/store/tools/useAnnotationTool.ts +++ b/src/store/tools/useAnnotationTool.ts @@ -1,4 +1,4 @@ -import { Ref, computed, markRaw, ref } from 'vue'; +import { Ref, computed, ref } from 'vue'; import type { Vector3 } from '@kitware/vtk.js/types'; import type { Maybe, PartialWithRequired, UnwrapAll } from '@/src/types'; import { isRecord, removeFromArray } from '@/src/utils'; @@ -237,7 +237,6 @@ export const useAnnotationTool = < }); return { - segments: markRaw(registry), appearanceOfTool, toolIDs, toolByID, diff --git a/src/utils/dataSelection.ts b/src/utils/dataSelection.ts index c0eeabd1a..87bfb48f2 100644 --- a/src/utils/dataSelection.ts +++ b/src/utils/dataSelection.ts @@ -26,7 +26,7 @@ const getImageName = (imageID: string) => { return useImageCacheStore().getImageMetadata(imageID)?.name ?? null; }; -export const getSelectionName = (selection: string) => { +const getSelectionName = (selection: string) => { if (isDicomImage(selection)) { return getDisplayName(useDICOMStore().volumeInfo[selection]); } diff --git a/src/utils/index.ts b/src/utils/index.ts index e0023a54a..e0782ad9e 100644 --- a/src/utils/index.ts +++ b/src/utils/index.ts @@ -2,7 +2,6 @@ import { z } from 'zod'; import { TypedArray } from 'itk-wasm'; import { parseUrl } from '@/src/utils/url'; import { EPSILON } from '../constants'; -import { Maybe } from '../types'; export function identity(arg: T) { return arg; @@ -147,6 +146,24 @@ export function arrayEquals(a: ArrayLike, b: ArrayLike) { return true; } +/** + * Whether two records hold identical values under the same keys. Values + * compare by identity, which the constraint keeps meaningful: a record with an + * object or array field would never equal a copy of itself. + */ +export const sameFields = < + R extends Partial>, +>( + one: R, + other: R +) => { + const keys = Object.keys(one) as (keyof R)[]; + return ( + keys.length === Object.keys(other).length && + keys.every((key) => one[key] === other[key]) + ); +}; + type ComparatorFunction = (a: T, b: T) => boolean; export function arrayEqualsWithComparator( a: T[], @@ -238,15 +255,6 @@ export const cleanUndefined = (record: T): Partial => Object.entries(record).filter(([, value]) => value !== undefined) ) as Partial; -// converts named colors (red, antiquewhite, etc) to hex -export function standardizeColor(color: Maybe) { - if (!color) return '#ffffff'; - const ctx = document.createElement('canvas').getContext('2d'); - if (!ctx) throw new Error('Could not create canvas context'); - ctx.fillStyle = color; - return ctx.fillStyle; -} - export function zodEnumFromObjKeys(obj: Record) { const [firstKey, ...otherKeys] = Object.keys(obj) as K[]; return z.enum([firstKey, ...otherKeys]); @@ -268,23 +276,6 @@ export const TypedArrayConstructorNames = [ 'Float64Array', ]; -/** - * Creates a new typed array of the same type as the source array. - * This utility handles the TypeScript typing issues when using array.constructor. - * - * @param sourceArray The source array to match the type of - * @param arrayLength The length of the new array - * @returns A new array of the same type as sourceArray - */ -export function createTypedArrayLike( - sourceArray: T, - arrayLength: number -): T { - return new (sourceArray.constructor as new (length: number) => T)( - arrayLength - ); -} - // https://stackoverflow.com/a/74823834 type Entries = { [K in keyof T]-?: [K, T[K]]; @@ -294,23 +285,6 @@ type Entries = { export const getEntries = (obj: T) => Object.entries(obj) as Entries; -/** - * Normalizes a list of objects to { order, byKey } - * @param objects - * @param key - * @returns - */ -export function normalizeForStore(objects: T[], key: K) { - type KeyType = T[K]; - const order: KeyType[] = objects.map((obj) => obj[key]); - const byKey = objects.reduce>( - (acc, obj) => ({ ...acc, [obj[key] as string | number | symbol]: obj }), - {} as Record - ); - - return { order, byKey }; -} - export function shortenNumber(value: number) { if (Number.isInteger(value)) { return value.toString(); diff --git a/src/vtk/PaintBrushContextRepresentation/index.js b/src/vtk/PaintBrushContextRepresentation/index.js index 79396c9f9..66d70c964 100644 --- a/src/vtk/PaintBrushContextRepresentation/index.js +++ b/src/vtk/PaintBrushContextRepresentation/index.js @@ -122,7 +122,6 @@ function vtkPaintBrushContextRepresentation(publicAPI, model) { const actorProperty = model.pipelines.brush.actor.getProperty(); actorProperty.setLineWidth(2); - actorProperty.setColor([1, 0, 0]); actorProperty.setDisplayLocation(DisplayLocation.FOREGROUND); actorProperty.setRepresentation(Representation.SURFACE); @@ -135,6 +134,7 @@ function vtkPaintBrushContextRepresentation(publicAPI, model) { const stencil = widgetState.getStencil(); const brush = widgetState.getBrush(); + actorProperty.setColor(brush.getColor3().map((channel) => channel / 255)); const { indexToWorld, worldToIndex } = model; if (stencil && brush.getOrigin()) { diff --git a/src/vtk/PaintWidget/state.ts b/src/vtk/PaintWidget/state.ts index b74dfeeb2..6d251a019 100644 --- a/src/vtk/PaintWidget/state.ts +++ b/src/vtk/PaintWidget/state.ts @@ -10,8 +10,8 @@ export interface PaintPointWidgetState extends vtkWidgetState { getScale1(): number; setVisible(visible: boolean): boolean; getVisible(): boolean; - setColor(color: number): boolean; - getColor(): number; + setColor3(color: Vector3): boolean; + getColor3(): Vector3; } export interface PaintWidgetState extends vtkWidgetState { @@ -26,7 +26,7 @@ export default function generateState() { .addStateFromMixin({ labels: ['brush'], name: 'brush', - mixins: ['origin', 'scale1', 'visible', 'color'], + mixins: ['origin', 'scale1', 'visible', 'color3'], initialValues: { scale1: 1, origin: null, diff --git a/tests/baseline/different_direction_labelmap_paint_coronal-chrome-1.png b/tests/baseline/different_direction_labelmap_paint_coronal-chrome-1.png index 008c54b82..50ae7a145 100644 Binary files a/tests/baseline/different_direction_labelmap_paint_coronal-chrome-1.png and b/tests/baseline/different_direction_labelmap_paint_coronal-chrome-1.png differ diff --git a/tests/specs/adaptive-labelmap.e2e.ts b/tests/specs/adaptive-labelmap.e2e.ts new file mode 100644 index 000000000..46bf521e4 --- /dev/null +++ b/tests/specs/adaptive-labelmap.e2e.ts @@ -0,0 +1,124 @@ +import { readFileSync, writeFileSync, rmSync } from 'node:fs'; +import { join } from 'node:path'; +import { gunzipSync } from 'node:zlib'; +import { TEMP_DIR } from '../../wdio.shared.conf'; +import { setValueVueInput, volViewPage } from '../pageobjects/volview.page'; +import { writeManifestToFile, waitForDownload } from './utils'; +import { openAnnotationSegments } from './segmentationTestUtils'; + +const maybeGunzip = (bytes: Buffer) => + bytes.readUInt16BE(0) === 0x1f8b ? gunzipSync(bytes) : bytes; + +const readLabels = (file: string) => { + const bytes = maybeGunzip(readFileSync(join(TEMP_DIR, file))); + const end = bytes.indexOf(Buffer.from('\n\n')); + expect(end).toBeGreaterThan(0); + const header = bytes.subarray(0, end).toString(); + const data = maybeGunzip(bytes.subarray(end + 2)); + const type = header.match(/^type:\s*(.+)$/m)![1].trim(); + const wide = ['unsigned short', 'ushort', 'uint16'].includes(type); + const count = data.length / (wide ? 2 : 1); + const values = Array.from({ length: count }, (_, index) => { + if (!wide) return data[index]; + return header.includes('endian: big') + ? data.readUInt16BE(index * 2) + : data.readUInt16LE(index * 2); + }); + return { + header, + wide, + labels: [...new Set(values.filter(Boolean))].sort((a, b) => a - b), + }; +}; + +const writeCase = (stem: string, labels: number[]) => { + const header = [ + 'NRRD0005', + 'type: unsigned short', + 'dimension: 3', + 'sizes: 16 16 16', + 'space: left-posterior-superior', + 'space origin: (0,0,0)', + 'space directions: (1,0,0) (0,1,0) (0,0,1)', + 'endian: little', + 'encoding: raw', + ]; + const data = Buffer.alloc(4096 * 2); + writeFileSync( + join(TEMP_DIR, `${stem}.nrrd`), + Buffer.concat([Buffer.from(`${header.join('\n')}\n\n`), data]) + ); + labels.forEach((value, index) => { + data.writeUInt16LE(value, index * 2); + header.push( + `Segment${index}_LabelValue:=${value}`, + `Segment${index}_Name:=Region ${value}` + ); + }); + writeFileSync( + join(TEMP_DIR, `${stem}.seg.nrrd`), + Buffer.concat([Buffer.from(`${header.join('\n')}\n\n`), data]) + ); +}; + +const openLabels = async (stem: string, file: string, count: number) => { + await volViewPage.open( + `?urls=[tmp/adaptive-labelmap-config.json,tmp/${stem}.nrrd,tmp/${file}]` + ); + await volViewPage.waitForViews(); + await openAnnotationSegments(); + await browser.waitUntil( + async () => + (await browser.execute( + () => + document.querySelectorAll( + '[data-testid="segment-list"] .item-row .v-list-item-title' + ).length + )) === count, + { timeoutMsg: `Expected ${count} imported segments` } + ); +}; + +describe('Adaptive labelmap export width', function () { + this.timeout(120_000); + for (const labels of [ + Array.from({ length: 255 }, (_, i) => i + 1), + Array.from({ length: 256 }, (_, i) => i + 1), + [256, 65535], + ]) { + it(`round-trips ${labels.length} labels with ${labels.at(-1)} as the highest input value`, async () => { + const stem = `adaptive-${labels.length}`; + writeCase(stem, labels); + await writeManifestToFile( + { io: { segmentationExtension: 'seg' } }, + 'adaptive-labelmap-config.json' + ); + await openLabels(stem, `${stem}.seg.nrrd`, labels.length); + const saveButton = $('button[data-testid="save-segments-button"]'); + await saveButton.waitForEnabled(); + await browser.execute((button) => button.focus(), await saveButton); + await browser.keys(' '); + await volViewPage.activeDialog.waitForDisplayed(); + await volViewPage.saveSegmentsFilenameInput.waitForDisplayed(); + await expect( + $('[data-testid="save-overlap-notice"]') + ).not.toBeDisplayed(); + const output = `${stem}-export.seg.nrrd`; + rmSync(join(TEMP_DIR, output), { force: true }); + await setValueVueInput( + volViewPage.saveSegmentsFilenameInput, + `${stem}-export` + ); + await volViewPage.saveSegmentsConfirmButton.click(); + await waitForDownload(join(TEMP_DIR, output), 40_000); + const saved = readLabels(output); + expect(saved.wide).toBe(labels.length > 255); + expect(saved.labels).toEqual( + Array.from({ length: labels.length }, (_, i) => i + 1) + ); + expect(saved.header).toContain(`Region ${labels.at(-1)}`); + await openLabels(stem, output, labels.length); + expect(await volViewPage.getNotificationsCount()).toBe(0); + }); + } +}); diff --git a/tests/specs/annotations-sidebar.e2e.ts b/tests/specs/annotations-sidebar.e2e.ts index 57bf8d7ed..c37c39d4e 100644 --- a/tests/specs/annotations-sidebar.e2e.ts +++ b/tests/specs/annotations-sidebar.e2e.ts @@ -51,7 +51,7 @@ const DRAWING_TOOLS = [ ]; describe('Annotations sidebar', () => { - it('deletes a segment with its painted mask and measurement and reports what was removed', async () => { + it('deletes a segment with its painted mask and measurement without a notification', async () => { const { axialView } = await placeRectangle(); await volViewPage.activatePaint(); await volViewPage.paintStrokeOnView(axialView); @@ -67,9 +67,11 @@ describe('Annotations sidebar', () => { await expect(axialView.$('svg rect')).not.toExist(); await expect(volViewPage.saveSegmentsButtons[0]).toBeDisabled(); await volViewPage.notifications.click(); - await expect($('.message-center .header > span')).toHaveText( - 'Deleted 1 mask on 1 image and 1 annotation' + await expect($('.message-center')).toBeDisplayed(); + const titles = await $$('.message-center .header > span').map((title) => + title.getText() ); + expect(titles.some((title) => title.startsWith('Deleted '))).toBe(false); }); it('keeps the Segments list in place and selected across tool switches', async () => { diff --git a/tests/specs/clear-scene-segments.e2e.ts b/tests/specs/clear-scene-segments.e2e.ts new file mode 100644 index 000000000..d186f510e --- /dev/null +++ b/tests/specs/clear-scene-segments.e2e.ts @@ -0,0 +1,42 @@ +import path from 'node:path'; +import { TEMP_DIR } from '../../wdio.shared.conf'; +import { volViewPage } from '../pageobjects/volview.page'; +import { + openThroughDialog, + writeManifestToFile, + writeMetaImage, +} from './utils'; +import { + addSegment, + openAnnotationSegments, + segmentNames, + waitForNamedSegments, +} from './segmentationTestUtils'; + +const STEM = 'clear-scene-segments'; + +describe('Clearing the scene', () => { + it('drops every segment the config does not declare', async () => { + const image = writeMetaImage(`${STEM}.mha`); + await writeManifestToFile( + { segments: { Tumor: { color: '#ff0000' } } }, + `${STEM}-config.json` + ); + await volViewPage.open(`?urls=[tmp/${STEM}-config.json,tmp/${image}]`); + await volViewPage.waitForViews(); + await openAnnotationSegments(); + await waitForNamedSegments(); + await addSegment(); + await addSegment(); + expect(await segmentNames()).toEqual(['Tumor', 'Segment 1', 'Segment 2']); + + await browser.keys(['Control', '/']); + await $('[data-testid="segment-list"]').waitForExist({ reverse: true }); + + await openThroughDialog(path.join(TEMP_DIR, image)); + await openAnnotationSegments(); + await waitForNamedSegments(); + expect(await segmentNames()).toEqual(['Tumor']); + expect(await volViewPage.getNotificationsCount()).toEqual(0); + }); +}); diff --git a/tests/specs/different-direction-labelmap.e2e.ts b/tests/specs/different-direction-labelmap.e2e.ts index 5670438d0..6eae69bbe 100644 --- a/tests/specs/different-direction-labelmap.e2e.ts +++ b/tests/specs/different-direction-labelmap.e2e.ts @@ -3,6 +3,7 @@ import { writeManifestToFile } from './utils'; import { volViewPage } from '../pageobjects/volview.page'; import { openAnnotationSegments, + segmentNames, waitForNamedSegments, waitForSegmentContent, } from './segmentationTestUtils'; @@ -40,6 +41,8 @@ describe('Labelmap with different direction matrix', () => { await openAnnotationSegments(); await waitForNamedSegments(); await waitForSegmentContent('Right hip'); + // The manifest describes one of the labelmap's values; the rest keep a row. + expect((await segmentNames()).length).toBeGreaterThan(1); await volViewPage.openLayoutMenu(1); await volViewPage.selectLayoutOption('Coronal Only'); diff --git a/tests/specs/erase-to-empty.e2e.ts b/tests/specs/erase-to-empty.e2e.ts new file mode 100644 index 000000000..c76e6b29e --- /dev/null +++ b/tests/specs/erase-to-empty.e2e.ts @@ -0,0 +1,53 @@ +import type { ChainablePromiseElement } from 'webdriverio'; +import { volViewPage } from '../pageobjects/volview.page'; +import { nudgeTo, pressAtPointer, setupTest } from './annotationTestUtils'; +import { + openAnnotationSegments, + segmentRow, + tooltipOf, + waitForSegmentContent, +} from './segmentationTestUtils'; + +// Vuetify turns off pointer events on a disabled button, so the wrapper it +// sits in is what a hover reaches and what carries the tooltip. +const expectDisabledSaying = async ( + button: ChainablePromiseElement, + reason: string +) => { + await expect(button).toBeDisabled(); + const wrapper = button.$('..'); + await wrapper.moveTo(); + const tooltip = await tooltipOf(wrapper); + await expect(tooltip).toBeDisplayed(); + await expect(tooltip).toHaveText(reason); +}; + +describe('Erasing every voxel a segment holds', () => { + it('leaves the segment nothing to reveal and the image nothing to save', async () => { + const { centerX: x, centerY: y } = await setupTest(); + await volViewPage.activatePaint(); + await openAnnotationSegments(); + + // A press is a stroke of one sample. The eraser presses the same brush at + // the same point, so it takes every voxel the paint stroke wrote. + await nudgeTo(x, y); + await pressAtPointer(); + await waitForSegmentContent('Segment 1'); + const save = $('[data-testid="save-segments-button"]'); + await expect(save).toBeEnabled(); + + const erase = $('button.mode-button*=Erase'); + await erase.waitForClickable(); + await erase.click(); + await expect($('button.mode-button.selected')).toHaveText('Erase'); + await nudgeTo(x, y); + await pressAtPointer(); + + const row = await segmentRow('Segment 1'); + await expectDisabledSaying( + row.$('[data-testid="reveal-segment-button"]'), + 'This segment has nothing on this image' + ); + await expectDisabledSaying(save, 'Nothing is painted on this image yet'); + }); +}); diff --git a/tests/specs/multiple-segmentation-import.e2e.ts b/tests/specs/multiple-segmentation-import.e2e.ts index c88aef3a1..19af564ca 100644 --- a/tests/specs/multiple-segmentation-import.e2e.ts +++ b/tests/specs/multiple-segmentation-import.e2e.ts @@ -45,9 +45,14 @@ const addAsSegmentation = async (name: string) => { const card = $(`.v-card:has([title="${name}"])`); await card.$('button.dataset-menu').click(); const menuItem = $( - `//*[contains(@class,"v-overlay--active")]//*[contains(@class,"v-list-item") and contains(normalize-space(.),"Add as segmentation")]` + '//*[contains(@class,"v-overlay--active")]' + + '//*[contains(concat(" ",normalize-space(@class)," ")," v-list-item ")' + + ' and normalize-space(.)="Add as segmentation"]' ); - await menuItem.$('.v-list-item__content').click(); + await menuItem.waitForDisplayed(); + await menuItem.waitForStable(); + await menuItem.waitForClickable(); + await menuItem.click(); await card .$('[data-testid="segmentation-conversion-progress"]') .waitForDisplayed({ reverse: true }); @@ -108,8 +113,8 @@ describe('Importing overlapping files with the same Slicer segment name', functi await volViewPage.clickSaveSegmentsButton(); const notice = $('[data-testid="save-overlap-notice"]'); await expect(notice).toBeDisplayed(); - expect(await notice.getText()).toBe( - 'Saving 2 files due to overlap, bundled into multi-import-parent.nrrd.zip.' + await expect(notice).toHaveText( + 'Saving 2 files due to overlap, bundled into multi-import-parent.zip.' ); if (process.env.CAPTURE_SEGMENT_IMPORT_DEMO) { const demoDir = path.join(projectRoot(), '.tmp', 'demo'); diff --git a/tests/specs/restore-binds-segment-names.e2e.ts b/tests/specs/restore-binds-segment-names.e2e.ts new file mode 100644 index 000000000..680c25af0 --- /dev/null +++ b/tests/specs/restore-binds-segment-names.e2e.ts @@ -0,0 +1,89 @@ +import fs from 'node:fs'; +import path from 'node:path'; +import JSZip from 'jszip'; +import { TEMP_DIR } from '../../wdio.shared.conf'; +import { volViewPage } from '../pageobjects/volview.page'; +import { openThroughDialog, writeManifestToFile } from './utils'; +import { + openAnnotationSegments, + segmentColor, + segmentNames, + waitForNamedSegments, + waitForSegmentContent, +} from './segmentationTestUtils'; + +const SIZE = 16; +const STEM = 'restore-binds-names'; + +const nrrd = (data: Uint8Array) => + Buffer.concat([ + Buffer.from( + `NRRD0005\ntype: unsigned char\ndimension: 3\nsizes: ${SIZE} ${SIZE} ${SIZE}\nspace: left-posterior-superior\nspace directions: (1,0,0) (0,1,0) (0,0,1)\nspace origin: (0,0,0)\nencoding: raw\n\n` + ), + data, + ]); + +const writeSession = async () => { + const zip = new JSZip(); + zip.file('tumor-mask.nrrd', nrrd(new Uint8Array(SIZE ** 3).fill(1))); + zip.file( + 'manifest.json', + JSON.stringify({ + version: '7.0.0', + dataSources: [{ id: 0, type: 'uri', uri: `/tmp/${STEM}.nrrd` }], + segments: [{ id: 'tumor', name: 'Tumor', color: [0, 0, 255, 255] }], + segmentations: [ + { + id: 'segmentation', + name: 'Session', + parentImage: '0', + order: ['tumor-mask'], + masks: [ + { + id: 'tumor-mask', + segmentId: 'tumor', + representations: { + labelmap: { + path: 'tumor-mask.nrrd', + name: 'Tumor', + extent: [0, SIZE - 1, 0, SIZE - 1, 0, SIZE - 1], + }, + }, + }, + ], + }, + ], + }) + ); + const sessionPath = path.join(TEMP_DIR, `${STEM}.volview.zip`); + fs.writeFileSync( + sessionPath, + await zip.generateAsync({ type: 'nodebuffer' }) + ); + return sessionPath; +}; + +describe('Restoring a session into a configured scene', () => { + it('binds a saved segment to the empty configured segment of its name', async () => { + const parent = new Uint8Array(SIZE ** 3).map((_, offset) => offset % 251); + fs.writeFileSync(path.join(TEMP_DIR, `${STEM}.nrrd`), nrrd(parent)); + await writeManifestToFile( + { segments: { Tumor: { color: '#ff0000' } } }, + `${STEM}-config.json` + ); + await volViewPage.open(`?urls=[tmp/${STEM}-config.json,tmp/${STEM}.nrrd]`); + await volViewPage.waitForViews(); + await openAnnotationSegments(); + await waitForNamedSegments(); + expect(await segmentNames()).toEqual(['Tumor']); + const configuredColor = await segmentColor('Tumor'); + + await openThroughDialog(await writeSession()); + + // Only a session segment bound to the first Tumor row gives it content. + await waitForSegmentContent('Tumor'); + expect(await segmentNames()).toEqual(['Tumor']); + expect(await segmentColor('Tumor')).toEqual(configuredColor); + expect(await volViewPage.getNotificationsCount()).toEqual(0); + }); +}); diff --git a/tests/specs/save-large-labelmap.e2e.ts b/tests/specs/save-large-labelmap.e2e.ts index 801cd3f3d..e3071dde4 100644 --- a/tests/specs/save-large-labelmap.e2e.ts +++ b/tests/specs/save-large-labelmap.e2e.ts @@ -74,24 +74,11 @@ describe('Save large labelmap', function () { await volViewPage.open(`?urls=[tmp/${manifestFileName}]`); await volViewPage.waitForViews(DOWNLOAD_TIMEOUT * 6); - // Activate paint tool — creates a segment group await volViewPage.activatePaint(); // Paint a stroke to allocate the labelmap const views2D = await volViewPage.getViews2D(); - const canvas = await views2D[0].$('canvas'); - const location = await canvas.getLocation(); - const size = await canvas.getSize(); - const cx = Math.round(location.x + size.width / 2); - const cy = Math.round(location.y + size.height / 2); - - await browser - .action('pointer') - .move({ x: cx, y: cy }) - .down() - .move({ x: cx + 20, y: cy }) - .up() - .perform(); + await volViewPage.paintStrokeOnView(views2D[0]); const notificationsBefore = await volViewPage.getNotificationsCount(); diff --git a/tests/specs/seg-nrrd-export.e2e.ts b/tests/specs/seg-nrrd-export.e2e.ts index 2fed7e2f8..3e036e0b8 100644 --- a/tests/specs/seg-nrrd-export.e2e.ts +++ b/tests/specs/seg-nrrd-export.e2e.ts @@ -42,24 +42,11 @@ describe('Slicer-compatible seg.nrrd export', function () { const config = { io: { segmentGroupSaveFormat: 'seg.nrrd' } }; await openConfigAndDataset(config, 'seg-nrrd-export', ONE_CT_SLICE_DICOM); - // Activate paint tool — creates a segment group await volViewPage.activatePaint(); // Paint a stroke so the labelmap has data const views2D = await volViewPage.getViews2D(); - const canvas = await views2D[0].$('canvas'); - const location = await canvas.getLocation(); - const size = await canvas.getSize(); - const cx = Math.round(location.x + size.width / 2); - const cy = Math.round(location.y + size.height / 2); - - await browser - .action('pointer') - .move({ x: cx, y: cy }) - .down() - .move({ x: cx + 20, y: cy }) - .up() - .perform(); + await volViewPage.paintStrokeOnView(views2D[0]); // Save session — downloads a .volview.zip containing the seg.nrrd const sessionFileName = await volViewPage.saveSession(); diff --git a/tests/specs/segment-controls-accessibility.e2e.ts b/tests/specs/segment-controls-accessibility.e2e.ts index 9e1b49053..2918024c2 100644 --- a/tests/specs/segment-controls-accessibility.e2e.ts +++ b/tests/specs/segment-controls-accessibility.e2e.ts @@ -63,10 +63,10 @@ describe('Segment control accessibility', () => { await $('button.v-expansion-panel-title*=Paint').click(); const erase = $('button.mode-button*=Erase'); const sync = $('input[aria-label="Sync Views"]'); + const overlap = $('input[aria-label="Allow Overlap"]'); await expect(erase).toBeDisabled(); await expect(sync).toBeDisabled(); - // It also decides how a polygon rasterizes, which needs no brush. - await expect($('input[aria-label="Allow Overlap"]')).toBeEnabled(); + await expect(overlap).toBeDisabled(); // Reached by keyboard from the panel title, so no pointer position is // involved: the disabled modes first, then the brush parameters. @@ -85,10 +85,21 @@ describe('Segment control accessibility', () => { await expect(reason).toHaveText( 'Select the Paint tool to adjust the brush' ); + await browser.keys('Tab'); + const overlapControls = $('.paint-switches > div:first-child'); + await expect(overlapControls).toBeFocused(); + const overlapReason = await tooltipOf(overlapControls); + await expect(overlapReason).toBeDisplayed(); + await expect(overlapReason).toHaveText( + 'Select the Paint tool to adjust the brush' + ); await AppPage.activatePaint(); await expect(erase).toBeEnabled(); await expect(sync).toBeEnabled(); + await expect(overlap).toBeEnabled(); + await AppPage.activateRectangle(); + await expect(overlap).toBeDisabled(); }); it('keeps the segment editor usable at 375px and side by side on desktop', async () => { diff --git a/tests/specs/segment-overlap.e2e.ts b/tests/specs/segment-overlap.e2e.ts index c3700733f..4eda3a4f4 100644 --- a/tests/specs/segment-overlap.e2e.ts +++ b/tests/specs/segment-overlap.e2e.ts @@ -12,6 +12,7 @@ import { allowOverlap, lockSegment, openAnnotationSegments, + segmentRow, waitForSegmentContent, waitForNamedSegments, } from './segmentationTestUtils'; @@ -140,6 +141,10 @@ describe('Painting one segment over another', function () { expect(baseline.foreground.length).toBeGreaterThan(0); await paintNewSegment(); await waitForSegmentContent('Segment 2'); + const replaced = await segmentRow('Segment 1'); + await expect( + replaced.$('[data-testid="reveal-segment-button"]') + ).toBeDisabled(); await openSaveDialog(); await expect(overlapNotice()).not.toBeDisplayed(); diff --git a/tests/specs/session-state-lifecycle.e2e.ts b/tests/specs/session-state-lifecycle.e2e.ts index 5d56fc117..4f30b21a4 100644 --- a/tests/specs/session-state-lifecycle.e2e.ts +++ b/tests/specs/session-state-lifecycle.e2e.ts @@ -197,8 +197,11 @@ describe('Session state lifecycle', () => { await writeManifestToFile(PROSTATE_610_LABELMAP_MANIFEST, fileName); await openProstateLabelmap(fileName); - // The 6.1.0 labelMaps entry names this segment and colors it red. - expect(await segmentNames()).toEqual(['Right hip']); + // The 6.1.0 labelMaps entry names this segment and colors it red. The + // labelmap's other values restore under default names. + const namesBefore = await segmentNames(); + expect(namesBefore[0]).toEqual('Right hip'); + expect(namesBefore.length).toBeGreaterThan(1); const segmentColorBefore = await segmentColor('Right hip'); const { session, manifest } = await saveAndParseManifest(); @@ -210,8 +213,8 @@ describe('Session state lifecycle', () => { await openAnnotationSegments(); await waitForNamedSegments(); - expect(await segmentNames()).toEqual(['Right hip']); await waitForSegmentContent('Right hip'); + expect(await segmentNames()).toEqual(namesBefore); expect(await segmentColor('Right hip')).toEqual(segmentColorBefore); }); }); diff --git a/tests/specs/utils.ts b/tests/specs/utils.ts index ba3d6a1b0..7270dc753 100644 --- a/tests/specs/utils.ts +++ b/tests/specs/utils.ts @@ -141,3 +141,16 @@ export async function openUrls(datasets: ReadonlyArray) { await writeManifestToFile(manifest, fileName); await openVolViewPage(fileName); } + +/** Loads a file into the page already open, through the Open files dialog. */ +export async function openThroughDialog(filePath: string) { + await browser.sessionSubscribe({ events: ['input.fileDialogOpened'] }); + const opened = new Promise<{ + context: string; + element?: { sharedId: string }; + }>((resolve) => browser.once('input.fileDialogOpened', resolve)); + await $('[data-testid="control-button-Open files"]').click(); + const { context, element } = await opened; + if (!element) throw new Error('The file dialog opened for no input'); + await browser.inputSetFiles({ context, element, files: [filePath] }); +}