Page MenuHomePhabricator

Planar figures crash if drawn with empty data storage
Closed, ResolvedPublic

Description

How to reproduce:

  1. Start workbench
  2. Select an measurment option and start drawing with an empty storage

crash.

Reason:

  • mitk::PlanarFigureInteractor::MoveCurrentPoint() is retrieves a planargeometry which is a nullptr
  • then this->TransformPositionEventToPoint2D(positionEvent, planarFigureGeometry, point2D) is invoked with the nullptr and crashes.

What todo:

  • Introduce a nullptr check in PlanarFigureInteractor to avoid nullptr exception (see above) but throw meaning full exceptions it it is a violated precondition.

Revisions and Commits

rMITK MITK
Restricted Differential Revision
Restricted Differential Revision
Restricted Differential Revision

Event Timeline

kislinsk triaged this task as Normal priority.Feb 7 2020, 9:03 AM
kislinsk added a subscriber: kislinsk.

While there should be a nullptr check I guess, this is another regression in the first place as it wasn't possible with the Measurement view to draw with an empty storage until recently. Is this possibly related to your work on selections?

Here's a screenshot from the last release. All the tools are deactivated as long as there isn't any visible object.

I guess @floca mentioned this because it was introduced by refactoring the Measurement plugin. Still working on this.

Yup. And it is remarked as a regression in the review. I wrote this task while reviewing, before I chacked the baseline. Sorry for that.
Even if the regression test is fixed. I think a nullptr check and a reasonable error reaction or throw, wouldn't hurt ;)

@kalali isn't this task obsolete after your rework of the rework measurement view?

@kalali Could you tell if this is obsolete now?

As noted, this was never a real issue since the broken code was never merged.
As D238#5458 stated, measurement options are only enabled if an image is selected.

However another task should be opened to add a nullptr check somewhere in the classes mentioned in the description to prohibit planar figure measurements without an image. This is out of scope of the Measurement-plugin.
Additionally, in https://phabricator.mitk.org/source/mitk/browse/master/Plugins/org.mitk.gui.qt.measurementtoolbox/src/internal/QmitkMeasurementView.cpp;4b80587b8479b1eec289231b4a317fb1ff1d03ca$357 there was an option to work without an image. That is not possible anymore and I don't know what that meant so we should clarify this.

@kalali OK, could you please open task where needed or tag with RequestForDiscussion. Thanks

As noted, this was never a real issue since the broken code was never merged.
As D238#5458 stated, measurement options are only enabled if an image is selected.

However another task should be opened to add a nullptr check somewhere in the classes mentioned in the description to prohibit planar figure measurements without an image. This is out of scope of the Measurement-plugin.
Additionally, in https://phabricator.mitk.org/source/mitk/browse/master/Plugins/org.mitk.gui.qt.measurementtoolbox/src/internal/QmitkMeasurementView.cpp;4b80587b8479b1eec289231b4a317fb1ff1d03ca$357 there was an option to work without an image. That is not possible anymore and I don't know what that meant so we should clarify this.

Is that a yes or a no to Ralf's question? I do not understand if was fixed since I created this task or not but I experienced it in the master branch.

People who experience this bug in master should state it here and leave the task open. I never experienced this bug and I did not open this task so please go ahead and see if the error can be reproduced with your systems.
As I wrote, the root of this error is still not fixed (missing check for nullptr) so I could add an issue for that - but that was not the scope of my refactoring, related to this issue.

Just checked, seems to be fixed in the current master branch. Set this task's status to Resolved?

floca lowered the priority of this task from Normal to Low.EditedApr 15 2020, 10:55 AM
floca updated the task description. (Show Details)
floca edited projects, added Restricted Project; removed Request for Discussion.

It depends on the scope of the task. I have altered the description. I would reuse this task for the nullptr issue. But there is no accute release relevant issue anymore, like Amir said.

kalali added a revision: Restricted Differential Revision.Aug 11 2020, 4:51 PM
kalali claimed this task.