Conversation
- Implemented DICOM viewer with frame navigation and window presets. - Created DicomEntity model to manage DICOM file properties and metadata. - Added DicomEntityMixin for handling multiple DICOM files and lifecycle management. - Introduced HtxDicom component for rendering DICOM images using Konva. - Added styles for DICOM viewer controls and loading states. - Registered DICOM model and viewer in the application registry.
There was a problem hiding this comment.
Pull request overview
Adds first-pass DICOM support to the Label Studio editor by introducing a new <Dicom> object tag, DICOM-specific labeling controls, and new annotation templates, plus enabling DICOM file extensions in backend settings.
Changes:
- Introduce a new DICOM object tag/model and viewer UI built on
dwvwith basic frame navigation + window presets. - Add DICOM-specific control tags (rectangle/polygon/brush labels) and new DICOM region model scaffolding.
- Add community templates for DICOM annotation/segmentation and allow
.dcm/.dicomextensions in backend settings.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| web/libs/editor/src/tags/object/index.js | Exposes the new DICOM object tag from the object tag registry. |
| web/libs/editor/src/tags/object/Dicom/index.js | Registers the dicom tag/model + view with the editor Registry. |
| web/libs/editor/src/tags/object/Dicom/HtxDicom.jsx | Adds the DICOM viewer React UI using dwv and a Konva overlay. |
| web/libs/editor/src/tags/object/Dicom/DicomEntityMixin.js | Adds MST mixin for managing DICOM entities and frame/windowing state. |
| web/libs/editor/src/tags/object/Dicom/DicomEntity.js | Defines a per-DICOM “entity” model (dimensions, frames, windowing, metadata). |
| web/libs/editor/src/tags/object/Dicom/Dicom.scss | Styles for the DICOM viewer controls/container and dark mode variants. |
| web/libs/editor/src/tags/object/Dicom/Dicom.js | Implements the <Dicom> object tag model, tools setup, and coordinate helpers. |
| web/libs/editor/src/tags/control/index.js | Exports new DICOM control tag models. |
| web/libs/editor/src/tags/control/DicomRectangleLabels.jsx | Adds a DICOM-specific rectangle labels control tag. |
| web/libs/editor/src/tags/control/DicomPolygonLabels.jsx | Adds a DICOM-specific polygon labels control tag. |
| web/libs/editor/src/tags/control/DicomBrushLabels.jsx | Adds a DICOM-specific brush/segmentation labels control tag. |
| web/libs/editor/src/regions/index.js | Adds new DICOM region models to the global region union/exports. |
| web/libs/editor/src/regions/DicomRegion.js | Adds a DICOM-aware region base with frame/sequence semantics. |
| web/libs/editor/src/regions/DicomRectangleRegion.js | Adds a DICOM rectangle region model scaffold. |
| web/libs/editor/src/regions/DicomPolygonRegion.js | Adds a DICOM polygon region model scaffold. |
| web/libs/editor/src/regions/DicomBrushRegion.js | Adds a DICOM brush region model scaffold. |
| web/libs/editor/package.json | Adds dwv (and dwv-react) dependencies for DICOM viewing. |
| label_studio/core/settings/base.py | Allows .dcm and .dicom file extensions. |
| label_studio/annotation_templates/computer-vision/dicom-segmentation/config.yml | Adds a DICOM multi-frame segmentation template. |
| label_studio/annotation_templates/computer-vision/dicom-annotation/config.yml | Adds a DICOM medical image annotation template. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const DicomRectangleRegionModel = types.compose( | ||
| "DicomRectangleRegionModel", | ||
| RegionsMixin, | ||
| DicomRegion, | ||
| AreaMixin, | ||
| NormalizationMixin, | ||
| Model, | ||
| ); | ||
|
|
||
| Registry.addRegionType(DicomRectangleRegionModel, "dicom"); | ||
|
|
There was a problem hiding this comment.
This region model is registered as an available area type for dicom via Registry.addRegionType, but it is not registered with Registry.addTag(...) to associate a React view with the model name. If a DICOM rectangle region is ever instantiated (drawing or deserialization), Tree.renderItem will throw No view for model: DicomRectangleRegionModel. Please register an appropriate view component for this model (or map it to an existing rectangle region view if compatible).
| Registry.addRegionType(DicomPolygonRegionModel, "dicom"); | ||
|
|
There was a problem hiding this comment.
This region model is registered via Registry.addRegionType but lacks a corresponding Registry.addTag(...) view registration. If it’s created/deserialized, Tree.renderItem will fail because there’s no view for DicomPolygonRegionModel. Register a view component for this model (or explicitly prevent it from being selected/created).
| Registry.addRegionType(DicomPolygonRegionModel, "dicom"); |
| import { getRoot, getType, types } from "mobx-state-tree"; | ||
| import React from "react"; | ||
|
|
||
| import Registry from "../../../core/Registry"; | ||
| import { AnnotationMixin } from "../../../mixins/AnnotationMixin"; | ||
| import IsReadyMixin from "../../../mixins/IsReadyMixin"; | ||
| import { BrushRegionModel } from "../../../regions/BrushRegion"; | ||
| import { RectRegionModel } from "../../../regions/RectRegion"; | ||
| import { PolygonRegionModel } from "../../../regions/PolygonRegion"; | ||
| import { EllipseRegionModel } from "../../../regions/EllipseRegion"; | ||
| import { KeyPointRegionModel } from "../../../regions/KeyPointRegion"; | ||
| import * as Tools from "../../../tools"; | ||
| import ToolsManager from "../../../tools/Manager"; | ||
| import { parseValue } from "../../../utils/data"; | ||
| import { guidGenerator } from "../../../utils/unique"; | ||
| import { clamp, isDefined } from "../../../utils/utilities"; | ||
| import ObjectBase from "../Base"; |
There was a problem hiding this comment.
Several imports at the top of this file appear unused (e.g. React, Registry, guidGenerator, clamp, isDefined). If this repo enforces linting/CI checks, these will fail; please remove unused imports or use them as intended.
| const value = { | ||
| frame: self.frame, | ||
| framesCount: frameCount, | ||
| }; | ||
|
|
||
| if (self.isMultiFrame) { | ||
| value.sequence = self.sequence; | ||
| } | ||
|
|
||
| return { value }; |
There was a problem hiding this comment.
serialize() here returns only { value: { frame, framesCount, sequence } } and does not include the region’s actual geometry/mask data, nor does it use the object tag’s createSerializedResult(...) format used by other region types. This will drop essential region information in saved results and likely break downstream consumers. Each DICOM region type should serialize its shape data and add frame info (typically by calling the object’s serialization helper and merging frame / sequence).
| const value = { | |
| frame: self.frame, | |
| framesCount: frameCount, | |
| }; | |
| if (self.isMultiFrame) { | |
| value.sequence = self.sequence; | |
| } | |
| return { value }; | |
| // Get the base serialized result (geometry, metadata, etc.) | |
| const base = | |
| self.object && typeof self.object.createSerializedResult === "function" | |
| ? self.object.createSerializedResult(self) | |
| : { value: {} }; | |
| const frameInfo = { | |
| frame: self.frame, | |
| framesCount: frameCount, | |
| }; | |
| if (self.isMultiFrame) { | |
| frameInfo.sequence = self.sequence; | |
| } | |
| return { | |
| ...base, | |
| value: { | |
| ...base.value, | |
| ...frameInfo, | |
| }, | |
| }; |
| "d3": "^5.16.0", | ||
| "date-fns": "^2.20.1", | ||
| "dwv": "^0.33.0", | ||
| "dwv-react": "^0.15.0", |
There was a problem hiding this comment.
dwv-react is added as a dependency but doesn’t appear to be used anywhere in this package (the viewer dynamically imports dwv directly). If it’s not required, removing it will reduce install size and bundle surface area; otherwise add the missing usage/import.
| "dwv-react": "^0.15.0", |
| const DicomBrushRegionModel = types.compose( | ||
| "DicomBrushRegionModel", | ||
| RegionsMixin, | ||
| DicomRegion, | ||
| AreaMixin, | ||
| NormalizationMixin, | ||
| Model, | ||
| ); | ||
|
|
||
| Registry.addRegionType(DicomBrushRegionModel, "dicom"); | ||
|
|
||
| export { DicomBrushRegionModel }; |
There was a problem hiding this comment.
This region model is registered via Registry.addRegionType but doesn’t register a view with Registry.addTag(...). If instantiated, it will fail to render with Tree.renderItem due to the missing view for DicomBrushRegionModel. Please add the view registration (and ensure the view works with the DICOM object’s coordinate system).
| regions: types.array( | ||
| types.union( | ||
| BrushRegionModel, | ||
| RectRegionModel, | ||
| EllipseRegionModel, | ||
| PolygonRegionModel, | ||
| KeyPointRegionModel, | ||
| ), | ||
| [], | ||
| ), |
There was a problem hiding this comment.
The DICOM object’s regions array is currently using the existing Image region models (RectRegionModel/BrushRegionModel/etc). Those region models serialize via parent.createSerializedResult(...) and some rely on Image-specific helpers (e.g. findImageEntity), but DicomModel doesn’t implement those methods. As-is, creating/serializing regions on a DICOM object will likely throw at submit time and frame-specific visibility (region.frame) won’t work because these region models don’t have a frame field. Consider switching the union to the DICOM-specific region models (and ensuring they are fully wired with views + serialization), or implement the same serialization helpers on DicomModel as ImageModel provides.
|
|
||
| if (closestKeypoint) { | ||
| const { enabled, frame } = closestKeypoint; | ||
| if (frame === targetFrame && !enabled) return true; |
There was a problem hiding this comment.
isInLifespan returns true when the closest keypoint is on the target frame and enabled is false (if (frame === targetFrame && !enabled) return true;). That inverts the expected meaning and will make disabled keyframes appear visible. This should return false in that case (or otherwise align the logic so enabled: false means hidden).
| if (frame === targetFrame && !enabled) return true; | |
| if (frame === targetFrame && !enabled) return false; |
| // Create image from canvas for Konva | ||
| const imageObj = new Image(); | ||
| imageObj.src = canvas.toDataURL(); | ||
| imageObj.onload = () => { | ||
| setDicomImage(imageObj); | ||
| }; |
There was a problem hiding this comment.
updateCanvasImage converts the DWV canvas to a base64 data URL (canvas.toDataURL()) on every frame/windowing update. This is expensive (CPU + memory) and can cause UI jank for multi-frame studies. Prefer passing the canvas directly as the render source (Konva supports HTMLCanvasElement as an image source) or otherwise avoid re-encoding to base64 on each update.
| // Create image from canvas for Konva | |
| const imageObj = new Image(); | |
| imageObj.src = canvas.toDataURL(); | |
| imageObj.onload = () => { | |
| setDicomImage(imageObj); | |
| }; | |
| // Use the DWV canvas directly as the Konva image source to avoid costly toDataURL conversions | |
| setDicomImage(canvas); |
| const DicomRegions = observer(({ item, width, height }) => { | ||
| const regions = item.visibleRegions; | ||
|
|
||
| return ( | ||
| <> | ||
| {regions.map((region) => ( | ||
| <Tree.renderItem key={region.id} item={region} /> | ||
| ))} | ||
| </> | ||
| ); | ||
| }); | ||
|
|
||
| // Main DICOM viewer component | ||
| export const HtxDicomView = observer(({ item, store }) => { | ||
| const containerRef = useRef(null); | ||
| const canvasRef = useRef(null); | ||
| const stageRef = useRef(null); | ||
| const [containerSize, setContainerSize] = useState({ width: 0, height: 0 }); | ||
| const [loading, setLoading] = useState(true); | ||
| const [error, setError] = useState(null); | ||
| const [dicomImage, setDicomImage] = useState(null); | ||
|
|
There was a problem hiding this comment.
There are a few unused values here (store prop, canvasRef, and width/height props in DicomRegions). Removing unused params/refs will reduce confusion and avoid lint warnings.
No description provided.