Skip to content

Commit

Permalink
BUG: GDCM reporting wrong spacing for some Media Types
Browse files Browse the repository at this point in the history
Noticed and reported by Andriy Federov.  Andriy encountered a dataset
that reported spacing of 1,1,1, when the actual spacing was 0.178038,
0x.174924, 1. I traced the problem to some tortured logic in
gdcmImageHelper.cxx.  Since that is a 3rd party library, I modified
itkGDCMImageIO.cxx to work around the problem rather than fix the
version of GDCM in ITK.

Change-Id: I3ebfc828da36bca86d78123cedb29b0e88aca4c7
  • Loading branch information
Kent Williams committed Jun 5, 2014
1 parent 6f044dc commit 168c457
Show file tree
Hide file tree
Showing 4 changed files with 183 additions and 8 deletions.
111 changes: 103 additions & 8 deletions Modules/IO/GDCM/src/itkGDCMImageIO.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
#include "gdcmUIDGenerator.h"
#include "gdcmAttribute.h"
#include "gdcmGlobal.h"
#include "gdcmMediaStorage.h"

#include <fstream>

Expand Down Expand Up @@ -356,16 +357,110 @@ void GDCMImageIO::InternalReadImageInformation()
m_Dimensions[0] = dims[0];
m_Dimensions[1] = dims[1];

const double *spacing = image.GetSpacing();
m_Spacing[0] = spacing[0];
m_Spacing[1] = spacing[1];
m_Spacing[2] = spacing[2];
double spacing[3];

const double *origin = image.GetOrigin();
m_Origin[0] = origin[0];
m_Origin[1] = origin[1];
m_Origin[2] = origin[2];
//
//
// This is a WORKAROUND for a bug in GDCM -- in
// ImageHeplper::GetSpacingTagFromMediaStorage it was not
// handling some MediaStorage types
// so we have to punt here.
gdcm::MediaStorage ms;

ms.SetFromFile(f);
switch(ms)
{
case gdcm::MediaStorage::HardcopyGrayscaleImageStorage:
case gdcm::MediaStorage::GEPrivate3DModelStorage:
case gdcm::MediaStorage::Philips3D:
case gdcm::MediaStorage::VideoEndoscopicImageStorage:
case gdcm::MediaStorage::UltrasoundMultiFrameImageStorage:
case gdcm::MediaStorage::UltrasoundImageStorage: // ??
case gdcm::MediaStorage::UltrasoundImageStorageRetired:
case gdcm::MediaStorage::UltrasoundMultiFrameImageStorageRetired:
{
std::vector<double> sp;
gdcm::Tag spacingTag(0x0028,0x0030);
if(ds.FindDataElement(spacingTag) &&
!ds.GetDataElement(spacingTag).IsEmpty())
{
gdcm::DataElement de = ds.GetDataElement(spacingTag);
const gdcm::Global &g = gdcm::GlobalInstance;
const gdcm::Dicts &dicts = g.GetDicts();
const gdcm::DictEntry &entry = dicts.GetDictEntry(de.GetTag());
const gdcm::VR & vr = entry.GetVR();
switch(vr)
{
case gdcm::VR::DS:
{
std::stringstream m_Ss;

gdcm::Element<gdcm::VR::DS,gdcm::VM::VM1_n> m_El;

const gdcm::ByteValue * bv = de.GetByteValue();
assert( bv );

std::string s = std::string( bv->GetPointer(), bv->GetLength() );

m_Ss.str( s );
// Stupid file: CT-MONO2-8-abdo.dcm
// The spacing is something like that: [0.2\0\0.200000]
// I would need to throw an expection that VM is not compatible
m_El.SetLength( entry.GetVM().GetLength() * entry.GetVR().GetSizeof() );
m_El.Read( m_Ss );

assert( m_El.GetLength() == 2 );
for(unsigned long i = 0; i < m_El.GetLength(); ++i)
sp.push_back( m_El.GetValue(i) );
std::swap( sp[0], sp[1]);
assert( sp.size() == (unsigned int)entry.GetVM() );
}
break;
case gdcm::VR::IS:
{
std::stringstream m_Ss;

gdcm::Element<gdcm::VR::IS,gdcm::VM::VM1_n> m_El;

const gdcm::ByteValue *bv = de.GetByteValue();
assert( bv );

std::string s = std::string( bv->GetPointer(), bv->GetLength() );
m_Ss.str( s );
m_El.SetLength( entry.GetVM().GetLength() * entry.GetVR().GetSizeof() );
m_El.Read( m_Ss );
for(unsigned long i = 0; i < m_El.GetLength(); ++i)
sp.push_back( m_El.GetValue(i) );
assert( sp.size() == (unsigned int)entry.GetVM() );
}
break;
default:
assert(0);
break;
}
}
spacing[0] = sp[0];
spacing[1] = sp[1];
spacing[2] = 1.0; // punt?
}
break;
default:
{
const double *sp;
sp = image.GetSpacing();
spacing[0] = sp[0];
spacing[1] = sp[1];
spacing[2] = sp[2];
}
break;
}

const double *origin = image.GetOrigin();
for(unsigned i = 0; i < 3; ++i)
{
m_Spacing[i] = spacing[i];
m_Origin[i] = origin[i];
}
if ( image.GetNumberOfDimensions() == 3 )
{
m_Dimensions[2] = dims[2];
Expand Down
6 changes: 6 additions & 0 deletions Modules/IO/GDCM/test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ itkGDCMSeriesStreamReadImageWriteTest.cxx
itkGDCMImagePositionPatientTest.cxx
itkGDCMImageIOOrthoDirTest.cxx
itkGDCMImageOrientationPatientTest.cxx
itkGDCMLoadImageSpacingTest.cxx
)

CreateTestDriver(ITKIOGDCM "${ITKIOGDCM-Test_LIBRARIES}" "${ITKIOGDCMTests}")
Expand Down Expand Up @@ -80,3 +81,8 @@ set_property(TEST itkGDCMSeriesMissingDicomTagTest APPEND PROPERTY DEPENDS ITKDa

itk_add_test(NAME itkGDCMImageIONoCrashTest
COMMAND ITKIOGDCMTestDriver itkGDCMImageIONoCrashTest DATA{${ITK_DATA_ROOT}/Input/OT-PAL-8-face.dcm})

itk_add_test(NAME itkGDCMLoadImageSpacingTest
COMMAND ITKIOGDCMTestDriver itkGDCMLoadImageSpacingTest
DATA{Input/gdcmSpacingTest.dcm}
)
1 change: 1 addition & 0 deletions Modules/IO/GDCM/test/Input/gdcmSpacingTest.dcm.md5
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
6772e7ad0331fed4832ee98ecae4c70d
73 changes: 73 additions & 0 deletions Modules/IO/GDCM/test/itkGDCMLoadImageSpacingTest.cxx
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
/*=========================================================================
*
* Copyright Insight Software Consortium
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0.txt
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
*=========================================================================*/
#include <iostream>
#include "itkGDCMSeriesFileNames.h"
#include "itkGDCMImageIO.h"
#include "itkImageFileReader.h"
#include "gdcmImageHelper.h"

typedef itk::Image<unsigned short, 2> ImageType;
typedef itk::ImageFileReader<ImageType> ReaderType;

//
// This test is specifically for a problem detected in GDCMImageIO
// by Andriy Fedorov, who noticed that the DCMTK reader
// reported the correct spacing for this DICOM dataset, but GDCM
// report spacing of 1,1,1 ... looking through the gdcm library
// code in Modules/ThirdParty/GDCM, I noticed that in
// gdcmImageHelper.cxx there was a bunch of elaborate code that in the
// case of Andriy's dataset, used the wrong tag to find the spacing,
// ignored the error when the tag wasn't found, and silenetly returned
// 1,1,1 for the spacing.
// The patch that this test is part of has a fallback for the case of
// the MediaStorageTypes for which gdcm has trouble with the spacing tag.

template <typename TIO, typename TSeriesNames>
int
ReadFile(const char *fname)
{
typename TIO::Pointer imageIO = TIO::New();
typename ReaderType::Pointer reader = ReaderType::New();

reader->SetImageIO(imageIO);
reader->SetFileName(fname);

reader->Update();
typename ImageType::Pointer image = reader->GetOutput();
std::cout << image << std::endl;
typename ImageType::SpacingType spacing = image->GetSpacing();
if(vcl_abs(spacing[0]-0.178038) >= 0.000001 ||
vcl_abs(spacing[1]-0.174924) >= 0.000001)
{
return EXIT_FAILURE;
}

return EXIT_SUCCESS;
}

int itkGDCMLoadImageSpacingTest(int argc, char *argv[])
{
if(argc < 2)
{
std::cerr << "Missing image filename argument" << std::endl;
return EXIT_FAILURE;
}

const char *fname = argv[1];
return ReadFile<itk::GDCMImageIO,itk::GDCMSeriesFileNames>(fname);
}

0 comments on commit 168c457

Please sign in to comment.