Signed vs. unsigned pixel data in DICOMAppHelper

I’m attempting to use the vtkDICOMImageReader class to read in some DICOM files. The pixel data in the DICOM file headers is written as signed shorts and is to be rescaled as floats. In the DICOMAppHelper class with the callback for reading pixel data (DICOMAppHelper.cxx line 869) from the DICOM header, there is logic to determine whether or not the rescaled output is float, and then if the input data (from the DICOM file) is written as 8- or 16-bit. At this point, I have 16-bit data to be rescaled as floats, but the code does not check to see if the input 16-bit data is signed or unsigned and just assumes the input is unsigned short (line 931). I believe this is a bug because the DICOM standard supports signed short pixel data as far as I know. Is there any way to add support for checking signed vs unsigned input data?

My best suggestion is to stop using vtkDICOMImageReader and start using David Gobbi’s vtkDICOM.

There are many issues with the old image reader that are resolved by it, and this is probably one of them. Others are lack of ability to use compressed image formats, inability to handle nested item groups, VR_UT is mis-classified, it has file handle leaks and so on. I hacked around those bugs for 7-8 years before adopting David’s vastly superior classes and have never looked back.

Note that @dgobb’s vtkDICOM library is already available as a VTK remote module, so you can get it as part of vtk by simply setting CMake variable Module_vtkDICOM:BOOL=ON when you configure your VTK build.

Should we move out current DICOM reader (vtkDICOMImageReader, DICOMParser library, and tests) to a remote module? Maybe something like “vtkLegacyDICOMReader”. It would reduce confusion for people who are new to VTK.

That sounds like overkill, but if vtkDICOMImageReader is so bad, it should probably be marked deprecated, which would be easy, fast, and not disruptive.

Thanks for the suggestion. I added the CMake variable but I get a CMake warning that the variable was not used. The same warning seen here. I’m building VTK 8.2.0 as well.

There is a bug in VTK-8.2 in remote file parsing. You need to use latest VTK or backport the very simple fix.

This is still an issue for me and is why I haven’t upgraded to VTK8.2 despite many attempts. vtkModule_DICOM is not visible in the cmake gui and does nothing when added anyways? I tried both VTK 8.2 and the latest master on github.