fix(segmentation): should use segment number if provided #357
Conversation
| @@ -1,69 +1,69 @@ | |||
| { | |||
There was a problem hiding this comment.
This is not my fault actually, the prettier config in the repo specifies 4 tabWidth
| SegmentNumber: | ||
| Segment.SegmentNumber || | ||
| (SegmentSequence.length + 1).toString(), |
There was a problem hiding this comment.
Are you sure this is okay with the standard?
https://dicom.innolitics.com/ciods/segmentation/segmentation-image/00620002/00620004
Section C.8.20.2.4
C.8.20.2.4 Segment Number
Segment Number (0062,0004) shall be unique within each Instance, start at a value of 1, and increase monotonically by 1.
There was a problem hiding this comment.
hmmmmm, I encountered this when I was writing the segmentation export and my code was generating a fake segmentation with two segments of 2 and 5 and I saw they get mapped into different color when I re import them (since they were getting mapped to 1 and 2) but now that you sent that link maybe you are right. So does this mean there is no way to specifically export a segmentation object that number the segments arbitrary? interesting I guess
There was a problem hiding this comment.
there is no way to specifically export a segmentation object that specifically number the segments arbitrary?
Correct. But why this would be an issue? Segment number is just a pointer within the segmentation object to connect semantics of the segment and the color of the segment with the corresponding pixel data.
There was a problem hiding this comment.
The issue I encountered initially was the color lut for segments (different colors before export VS after import), I think we have a bug in OHIF too ...
Something strange I saw in SEG export was this, not sure if there is an explanation for it
My metadata for dcmjs to export SEG using this rgb2DICOMLAB
const RecommendedDisplayCIELabValue = dcmjs.data.Colors.rgb2DICOMLAB(
color.slice(0, 3)
);
But later looking at the DICOM again uing the DICOM dump here
As you see the encoded results are different, so my parser with dicomlab2RGB returns a different color.
Am I missing an obvious thing here? is there any scaling happening ? (but I didn't see any other scale handling code anywhere before in dcmjs nor in OHIF)
Event inside the dcmjs it has correct value but it gets rewritten
There was a problem hiding this comment.
I don't think I was involved in writing that conversion function.
But in dcmqi we did spend a bit of time debugging it, and in the end the conversion was implemented in DCMTK. Maybe this can help debug OHIF conversion routines: https://github.com/DCMTK/dcmtk/blob/master/dcmiod/libsrc/cielabutil.cc.
There was a problem hiding this comment.
The display values are clearly out of range - from the spec:
Attributes such as Graphic Layer Recommended Display CIELab Value (0070,0401) consist of three unsigned short values:
An L value linearly scaled to 16 bits, such that 0x0000 corresponds to an L of 0.0, and 0xFFFF corresponds to an L of 100.0.
An a* then a b* value, each linearly scaled to 16 bits and offset to an unsigned range, such that 0x0000 corresponds to an a* or b* of -128.0, 0x8080 corresponds to an a* or b* of 0.0 and 0xFFFF corresponds to an a* or b* of 127.0
So, the actual values of L* a* and b* for unencoded are:
0...100, -128...127, -128...127
or the encoded values are:
0...65535
for PCS encoding. The provided values are completely outside the range, so something is converting things incorrectly before being passed into dcmjs
There was a problem hiding this comment.
Ok silly mistake, apparently the RGB that rgb2DICOMLAB accepts is scaled to 0-1 RGB, my bad
|
The dcmjs version is the dcmtk version converted to js. I might have made a mistake in the conversion but it worked for the tests I had tried. https://github.com/dcmjs-org/dcmjs/blob/master/src/colors.js |
When exporting segmentation if segment number is specified it the copy should use that instead of deriving it from the length similar to SegmentLabel and SegmentAlgorithm which is used to copy