QIICR / dcmqi

dcmqi (DICOM for Quantitative Imaging) is a free, open source C++ library for conversion between imaging research formats and the standard DICOM representation for image analysis results
https://qiicr.gitbook.io/dcmqi-guide/
BSD 3-Clause "New" or "Revised" License
232 stars 62 forks source link

CIELab conversion appears to be incorrect (in some cases?) #412

Closed fedorov closed 3 years ago

fedorov commented 3 years ago

Original issue identified by the MITK/Kaapana team in https://phabricator.mitk.org/T28039. The input CIELab value appears to be valid, while the converted RGB is not.

It looks like DCMQI CIE color code produces invalid values for attribute (0062,000d) US 0\32768\32768 # 6, 3 RecommendedDisplayCIELabValue

results in RGB 4294967294/0/1

This happens in ImageSEGConverter::dcmSegmentation2itkimage line ~ 603-626. This value is also included in the JSON > produced by that function. The JSON parsing code interprets this as uint but tries to convert it to int and checks the bounds which fails and causes the exception. Best guess is that the CIE color conversion code is somewhat incorrect in dcmqi.

dcmqi conversion is implemented in https://github.com/QIICR/dcmqi/blob/master/libsrc/Helper.cpp.

Related pointers:

pieper commented 3 years ago

not sure if that was "inherited"

Yes, from dcmtk: https://github.com/dcmjs-org/dcmjs/blob/master/src/colors.js#L2-L4

michaelonken commented 3 years ago

@fedorov Sorry for coming back to this after some time but I wanted to make sure to fix the bug in DCMTK if necessary.

You wrote

It looks like DCMQI CIE color code produces invalid values for attribute (0062,000d) US 0\32768\32768 # 6, 3 RecommendedDisplayCIELabValu

I tried this with the DCMTK cielabutil.cc routines and the result looks good to me (code below if you are curious). Does the report above actually apply to DCMTK code? I am not quite sure from the description. Also I looked into the (old unfixed version) Helper.cpp file quoted in the report and it seems it does not use the DCMTK code (or a copy of its related methods). Maybe the generated JavaScript used DCMTK and showed the same bug?

So, do you remember maybe details on how the report applies to DCMTK?

P.S: Code

// (all variables are doubles):
IODCIELabUtil::dicomlab2Lab(cielab_L, cielab_a, cielab_b, 0, 32768, 32768);
IODCIELabUtil::lab2Rgb(r,g,b,cielab_L,cielab_a,cielab_b);
COUT << "cieLab(" << cielab_L << "," << cielab_a << "," << cielab_b << ")" << OFendl;
COUT << "RGB(" << r << "," << g << "," << b << ")" << OFendl;

resulting in output

cieLab(0,-0.498054,-0.498054)
RGB(0,0.00904204,0.0120008)
pieper commented 3 years ago

Hi @michaelonken -- I believe yes dcmtk is correct and the issue was that instead of using the dcmtk implementation dcmqi had a reimplementation with a bug (but that code was removed).

michaelonken commented 3 years ago

Great, thank you @pieper !

fedorov commented 3 years ago

@michaelonken I realized I did not contribute the code I used in testing, but my recollection is (and there is a comment here https://github.com/QIICR/dcmqi/pull/413/commits/ab6a2071c209fbdd9a2f3020de4ef2d114b0d447) that when I looked int DCMTK, its conversion routines do not have round-trip fidelity CIELab->RGB->CIELab (or maybe RGB->CIELab->RGB, I don't remember), and I believe that is the reason I copied the conversion procedure from Pixelmed instead of using DCMTK routines. It was probably a rounding issue somewhere, since the values on round-trip were just a bit off, as I recall.

pieper commented 3 years ago

Spot testing the dcmjs translation of the dcmtk code seems to work fine:

dcmjs.data.Colors.dicomlab2RGB(dcmjs.data.Colors.rgb2DICOMLAB([.5,1.,0]))
(3) [0.5000000000000011, 0.9999999999999999, 4.3032244434471065e-15]

dcmjs.data.Colors.rgb2DICOMLAB(dcmjs.data.Colors.dicomlab2RGB([58909.77583352995, 15439.068757206607, 54947.50986056058]))
(3) [58909.77583352995, 15439.068757206636, 54947.50986056057]
michaelonken commented 3 years ago

Thank you @pieper and @fedorov for the context and even trying this! Now I understand the context (and do not need to fix anything in DCMTK). Hope to see you guys again at a Hackfest or any other occasion :-)