Commit 1a889f6d authored by Andras Lasso's avatar Andras Lasso

BUG: Fix crash in vtkImageHistogram if input contains NaN

vtkImageHistogram caused application crash when input image contained NaN value
using Intel TBB backend, Visual Studio 2017, x64.

The problem was that when the image contained a NaN value then the computed scalar range was set to (Nan, Nan),
which later got converted to invalid bin min/max indexes. The safety check that should have forced
the indices to the valid range did not work, as it relied on comparison with a NaN value.

Fixed computation of scalar range (similar to how it is done here:
vtk/vtk@06d553fb), by changing
the order of operands in the ternary operator.

To make sure that similar error do not cause crash in the future, added a NaN-value check in the bin range computation, too.

Added test case to verify that the results are correct and the application does not crash anymore if input image contains NaN.
parent 16f7bbdf
Pipeline #171233 failed with stage
......@@ -17,9 +17,13 @@
// The command line arguments are:
// -I => run in interactive mode
#include "vtkFloatingPointExceptions.h"
#include "vtkImageAccumulate.h"
#include "vtkImageCast.h"
#include "vtkImageData.h"
#include "vtkImageHistogramStatistics.h"
#include "vtkMath.h"
#include "vtkNew.h"
#include "vtkPNGReader.h"
#include "vtkTestUtilities.h"
......@@ -28,7 +32,7 @@
int ImageHistogramStatistics(int argc, char* argv[])
{
vtkPNGReader* reader = vtkPNGReader::New();
vtkNew<vtkPNGReader> reader;
char* fname = vtkTestUtilities::ExpandDataFileName(argc, argv, "Data/fullhead15.png");
......@@ -36,7 +40,7 @@ int ImageHistogramStatistics(int argc, char* argv[])
delete[] fname;
// Use float data to get the most code coverage
vtkImageCast* imageCast = vtkImageCast::New();
vtkNew<vtkImageCast> imageCast;
imageCast->SetOutputScalarTypeToFloat();
imageCast->SetInputConnection(reader->GetOutputPort());
......@@ -47,7 +51,7 @@ int ImageHistogramStatistics(int argc, char* argv[])
double stdevTest = 660.9126299774935;
double tol = 1e-6;
vtkImageHistogramStatistics* statistics = vtkImageHistogramStatistics::New();
vtkNew<vtkImageHistogramStatistics> statistics;
statistics->SetInputConnection(imageCast->GetOutputPort());
statistics->GenerateHistogramImageOff();
statistics->Update();
......@@ -60,7 +64,7 @@ int ImageHistogramStatistics(int argc, char* argv[])
// uncomment to test vtkImageAccumulate instead
/*
vtkImageAccumulate *accumulate = vtkImageAccumulate::New();
vtkNew<vtkImageAccumulate> accumulate;
accumulate->SetInputConnection(reader->GetOutputPort());
accumulate->Update();
......@@ -104,9 +108,74 @@ int ImageHistogramStatistics(int argc, char* argv[])
retVal = false;
}
reader->Delete();
imageCast->Delete();
statistics->Delete();
// Make sure histogram computation does not crash if image has NaN pixel
// Clone the input image
vtkNew<vtkImageData> imageDataWitNaN;
imageDataWitNaN->DeepCopy(imageCast->GetOutput());
double rangeOriginal[2];
imageDataWitNaN->GetScalarRange(rangeOriginal);
// Set pixel value at (1,1,0) position to NaN
imageDataWitNaN->SetScalarComponentFromDouble(1, 1, 0, 0, vtkMath::Nan());
// Verify that scalar range is still computed correctly
// GetScalarRange() would crash due to invalid floating-point operation, therefore we
// need to disable floating-point exceptions here.
vtkFloatingPointExceptions::Disable();
double rangeWithNaN[2];
imageDataWitNaN->GetScalarRange(rangeWithNaN);
if (fabs((rangeOriginal[0] - rangeWithNaN[0]) / rangeOriginal[0]) > tol)
{
cout.precision(16);
cout << "rangeWithNaN[0] " << rangeWithNaN[0] << " should be " << rangeOriginal[0] << endl;
retVal = false;
}
if (fabs((rangeOriginal[1] - rangeWithNaN[1]) / rangeOriginal[1]) > tol)
{
cout.precision(16);
cout << "rangeWithNaN[1] " << rangeWithNaN[1] << " should be " << rangeOriginal[1] << endl;
retVal = false;
}
// Verify that the filter does not crash
statistics->SetInputData(imageDataWitNaN);
statistics->Update();
// Verify that the results are still the same (one pixel should not cause perceivable difference)
// (exact same tests as above)
if (fabs((minVal - minValTest) / maxValTest) > tol)
{
cout.precision(16);
cout << "minVal " << minVal << " should be " << minValTest << endl;
retVal = false;
}
if (fabs((maxVal - maxValTest) / maxValTest) > tol)
{
cout.precision(16);
cout << "maxVal " << maxVal << " should be " << maxValTest << endl;
retVal = false;
}
if (fabs((meanVal - meanValTest) / maxValTest) > tol)
{
cout.precision(16);
cout << "meanVal " << meanVal << " should be " << meanValTest << endl;
retVal = false;
}
if (fabs((median - medianTest) / maxValTest) > tol)
{
cout.precision(16);
cout << "median " << median << " should be " << medianTest << endl;
retVal = false;
}
if (fabs((stdev - stdevTest) / maxValTest) > tol)
{
cout.precision(16);
cout << "stdev " << stdev << " should be " << stdevTest << endl;
retVal = false;
}
return !retVal;
}
......@@ -331,8 +331,13 @@ void vtkImageHistogramExecuteRange(vtkImageData* inData, vtkImageStencilData* st
{
T x = *inPtr;
xmin = (xmin < x ? xmin : x);
xmax = (xmax > x ? xmax : x);
// x may be NaN but we don't want xmin and xmax
// to be ever end up being set to NaN.
// Therefore x must be used as a second operand
// of the ternary operator (because result of comparing
// NaN to any other value is always false).
xmin = (xmin >= x ? x : xmin);
xmax = (xmax <= x ? x : xmax);
inPtr += nc;
} while (--n);
......@@ -388,6 +393,11 @@ void vtkImageHistogramExecute(vtkImageHistogram* self, vtkImageData* inData,
x += xshift;
x *= xscale;
// x may be NaN but we don't want xmin and xmax
// to be ever end up being set to NaN.
// Therefore x must be used as a second operand
// of the ternary operator (because result of comparing
// NaN to any other value is always false).
x = (x > xmin ? x : xmin);
x = (x < xmax ? x : xmax);
......@@ -868,12 +878,12 @@ void vtkImageHistogram::ThreadedRequestData(vtkInformation* vtkNotUsed(request),
double scale = 1.0 / binSpacing;
double minBinRange = (scalarRange[0] - binOrigin) * scale;
double maxBinRange = (scalarRange[1] - binOrigin) * scale;
if (minBinRange < 0)
if (minBinRange < 0 || vtkMath::IsNan(minBinRange))
{
minBinRange = 0;
useFastExecute = false;
}
if (maxBinRange > maxBin)
if (maxBinRange > maxBin || vtkMath::IsNan(maxBinRange))
{
maxBinRange = maxBin;
useFastExecute = false;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment