Page MenuHomePhabricator

mitk crashes if a multi component image is loaded that is recognized as binary
Closed, ResolvedPublic

Description

and the binary classification is also not correct because only the ifrst component contains only one value

Event Timeline

Multi component images should not never be handled as binary images.

This might be changed in the future.

User neher has pushed new remote branch:

bug-18667-MultiComponentBinaryCrash

A style issue: please use always parentheses for if-blocks

In general I would prefer a solution that does not change the property but checks for the number of compenents at the location where the crash occurs.

I'd like to chime in here. :)

In my opinion the solution is totally okay. It is at the right place were our heuristic checks if an image is binary or not. There's already deliberate code to circumvent the check, i.e., it is checked if the binary property was already added. This property check is universal. It works both from code and UI.

Hence, +1 for the current solution. Not to mention that it is very clear at this place what the point of the author is even without commenting anything.

I'd like to see a little bit more code convention here as well, especially two spaces for indention and blank lines before and after the if-block. In that case I would even vote against parentheses. AFAIK we did never have this requirement in our style convention and it is uses widely in MITK. This restriction was only for if-else-blocks. :)

If everyone agrees I would vote for the Core Modification flag (given that the code style is improved).

Ok, having a closer look at _where_ the binary property is set in general, it's obviously not the clearest design and we could initialize it anywhere ... But where does the crash happen? There is always the possibility that someone changes the value of the binary property later (maybe even the user) and then it should not crash in my opinion.

You're right, I agree. The current solution does not prevent someone from changing the property later on. I didn't think about that. Where does the crash happen, Peter?

The crash happens in vtk but the last involved mit class is the mitk::VtkMapper in MitkRenderTranslucentGeometry line 87.

stack trace of crash

new approach. see WIKI Specification Page.

User neher has pushed new remote branch:

bug-18667-MultiComponentBinaryCrash2

[df59e3]: Merge branch 'bug-18667-MultiComponentBinaryCrash2'

Merged commits:

2015-03-11 15:14:42 Peter Neher [1f5ecc]
setting Image.Displayed Component property correctly