Page MenuHomePhabricator

Pointset interaction crashes if large numbers are used as coordinates
Closed, ResolvedPublic

Description

Insert a manually a point with coordinates of e.g. (-1000000000000000000,0,0)
Click in 3D renderwindow or do a reinit

> Crash

Event Timeline

I could not reproduce this crash

I followed Michael Brehler's steps, adding the point (-1000000000000000000,0,0) manually through the PointSet Interaction plugin, clicking or reiniting the 3D renderwindow.

Everything seems to work well.

Tested under Linux Ubuntu 14.04

I can reproduce the crash for two cases:

  1. Load image (Pic3D.nrrd), create a PointSet and add point (-100000000000000000,0,0). Do a reinit on the PointSet -> crash
  2. Create a PointSet, add point (-100000000000000000,0,0). Load image (Pic3D.nrrd) -> crash

Second crash is related to mitk::DisplayGeometry Class that should be solved if the recent major geometry changes are integrated into the master branch.

Crashes in PlaneGeometry::CheckBounds due to bound[1] (witdh) = 0.
Value range is 19 digits like long data type.
Problem suspected in QmitkEditPointDialog::SetPoint in QmitkEditPointDialog.cpp

case: pointset contains a single point

  • after reinit on this pointset a boundingbox with bounds [-0.5, 0.5] for single point is calculated (mitkPointSet.cpp, l. 765). Due to double accuracy this is not possible for large values and leads to a bounding box of size 0 which causes an assert (mitkPlaneGeometry.cpp, l.81) to fail. -> possible work around for pointsets that are created in GUI: std::min, std::max query in mitkPointSet.cpp to limit range of coordinates

Note:

  1. mitkPointSet.cpp checks itkBounds first: if (width == 0) -> bounds +-0.5
  2. mitkRenderingManager.cpp checks BaseGeometry::BoundsArrayTyp newBounds afterwards: if(bound 1 equal bound 2) -> second bound +1

User reuterv has pushed new remote branch:

bug-18976-PoinstSet-Interaction-crashes-large-coordinates

crash 1:
adding a point manually to a pointset with large value outside of double precision and afterwards performing a global reinit
-> only occurs in debug mode, not in release mode
-> fixed problem concerning double precision for bounding boxes with 0 extent:

  • added throw exception in mitkRenderingManager.cpp if bounding box has 0 extent due to double precision exceeded
  • added QMessageBox with warning that input point coordinate exceeds double precision in QmitkEditPointDialog.cpp

crash 2: add poinset with large value and load an image:
std::bad_alloc error due to too large bounding box
-> bug is related to T18332 and work will continue there

Two issues before I am happy to set Needs_Core_Modification to '+'. :)

  1. You throw an exception now but I guess it isn't catched and handled gracefully yet.
  1. Please refactor your code in QmitkEditPointDialog.cpp as you wrote rather duplicate code here, e.g., add short static functions (in sense of local, not static member functions) to QmitkEditPointDialog.cpp:

static void EmitWarning(const QString& message, const QString& title)
{

MITK_WARN << message.toStdString();
QMessageBox::warning(nullptr, title, message);

}

static bool ValidatePrecision(double value)
{

return std::log10(std::abs(value)) + 1.0 > std::numeric_limits<double>::digits10;

}

static bool ValidateCoordinate(const QString& name, double value)
{

auto hasValidPrecision = ValidatePrecision(value);

if (!hasValidPrecision)
  EmitWarning(QString("Point set %1 coordinate is outside double precision range.").arg(name), "Invalid point set input");

return hasValidPrecision;

}

This would reduce the complete if/else block to:

auto x = d->m_XCoord->text().toDouble();
auto y = d->m_YCoord->text().toDouble();
auto z = d->m_ZCoord->text().toDouble();

if (ValidateCoordinate("X", x) && ValidateCoordinate("Y", y) && ValidateCoordinate("Z", z))
{

auto point = d->m_PointSet->GetPoint(d->m_PointId, d->m_Timestep);
point.SetElement(0, x);
point.SetElement(1, y);
point.SetElement(2, z);
d->m_PointSet->SetPoint(d->PointId, point);
this->accept();

}

[465fe4]: Merge branch 'bug-18976-PoinstSet-Interaction-crashes-large-coordinate

Merged commits:

2015-08-19 15:35:54 Vincent Reuter [64556b]

  • changed static function declaration

2015-08-19 14:57:31 Vincent Reuter [264d7e]

  • added static functions in QmitkEditPointDialog.cpp for checking double precision

2015-08-12 17:36:25 Vincent Reuter [d6c9c6]
added documentation in mitkRenderingManager.h for function initializeViews: exception thrown due to bounding box extent = 0 because of exceeding double precision range


2015-08-12 16:52:28 Vincent Reuter [cf2043]

  • added throw exception in mitkRenderingManager.cpp if bounding box has 0 extent due to double precision exceeded
  • added QMessageBox with warning that input point coordinate exceeds double precision in QmitkEditPointDialog.cpp