Skip to content
Permalink
Browse files

BUG: avoid crash with LONG images

vtkImageInterpolator explicitly disallows reslicing of
data types that cannot be represented by a double, so
Slicer would crash when a volume of this type was loaded.

Thie change prevents the crash and generates an error message.
I'm not sure there's a better solution at this level of the
code, but it would probably make sense for the application
to generate a warning when long types of volumes are added
to the scene.

From: Steve Pieper <pieper@isomics.com>

git-svn-id: http://svn.slicer.org/Slicer4/trunk@28563 3bd1e089-480b-0410-8dfb-8563597acbee
  • Loading branch information...
pieper committed Oct 21, 2019
1 parent b485c8b commit d596340895ed6e54bc164a6715b3b030654129ee
Showing with 12 additions and 0 deletions.
  1. +12 −0 Libs/MRML/Logic/vtkMRMLSliceLayerLogic.cxx
@@ -645,6 +645,18 @@ void vtkMRMLSliceLayerLogic::UpdateImageDisplay()
return;
}

vtkImageData *imageData = this->VolumeNode->GetImageData();
if (imageData != nullptr &&
(imageData->GetScalarType() == VTK_LONG ||
imageData->GetScalarType() == VTK_UNSIGNED_LONG ||
imageData->GetScalarType() == VTK_LONG_LONG ||
imageData->GetScalarType() == VTK_UNSIGNED_LONG_LONG))
{
vtkErrorMacro("Reslicing can only be done on types representable as double. Node " <<

This comment has been minimized.

Copy link
@lassoan

lassoan Oct 21, 2019

Contributor

This message is good as a source-code comment but as an error message shown to users something like this could work better:

"Image reslicing is not implemented for 'long' voxel types. Node..."

Users would then understand that it could be doable just currently not implemented. Also, it is less confusing if we don't bring up the possibility to convert to double (it is an internal implementation limitation of the image reslice filter).

This comment has been minimized.

Copy link
@pieper

pieper Oct 21, 2019

Author Member

Hmm, yes, but mentioning the double thing gives them something concrete so they can figure out the solution themselves. Anyone who looks in the log file should be given something that can help, like the criteria for data that is supportable.

This comment has been minimized.

Copy link
@lassoan

lassoan Oct 21, 2019

Contributor

Log file contains filename and line number so developers can find all the exact details there (your comment about double, reslice image filter, etc).

Anyway, I don't mind including details in the log that are not useful for users (we have many of these), but it would be nice to make it more clear that reslicing of these voxel types has simply not been implemented (users would then know that it is not some fundamental flaw of the software but they should just ask for it to be implemented).

Since it is an application-level class, we could even make a suggestion to cast the image and/or add a link to the bugtracker where workaround can be described.

It's up to you, the current behavior is already way better than the unexplained crash before.

this->VolumeNode->GetName() << " has image data of type " << imageData->GetScalarTypeAsString());
return;
}

vtkMTimeType oldReSliceMTime = this->Reslice->GetMTime();
vtkMTimeType oldReSliceUVWMTime = this->ResliceUVW->GetMTime();
vtkMTimeType oldAssign = this->AssignAttributeTensorsToScalars->GetMTime();

3 comments on commit d596340

@pieper

This comment has been minimized.

Copy link
Member Author

pieper replied Oct 21, 2019

Issue added here to track: https://issues.slicer.org/view.php?id=4714

@lassoan

This comment has been minimized.

Copy link
Contributor

lassoan replied Oct 21, 2019

These kind of images are so rare that I think this solution should be enough.

@pieper

This comment has been minimized.

Copy link
Member Author

pieper replied Oct 21, 2019

An application level notification about why this data isn't rendered and suggestion about how to workaround would be helpful. But it's not in my critical path.

Please sign in to comment.
You can’t perform that action at this time.