Straight forward patch

I updated my initial patch and found something that might work for all mappers.

I propose a method FixupLegacyProperties(DataNode*,BaseRenderer*) that could have the same structure for each mapper (default implementation empty) and would be implemented only when needed.
Whenever a change in a mapper breaks backwards compatibility, a small piece of code could be added to that method to inspect potentially old/legacy properties and update them to the current scheme.

In the specific case of "line width", the method would check for a FloatProperty, and if that one is not found, it would try to find and convert an IntProperty.

I'd like to integrate this change:

• either as in the branch, locally
• or, if preferred, I could add the method interface to Mapper or VtkMapper and adapt other mappers that implement similar behavior

User maleike has pushed new remote branch:

bug-19360-line-width-redefinition

As for testing: I prepared a rendering test for SceneIO that I'd like to integrate together with T18280 (touching MITK-Data only once).

Final patch

CCing Sarina. FixupLegacyProperties is something that should probably be considered as part of your project. Ideally a natural way to deal with legacy/outdated properties should remove the need for this function.

Daniel, is it okay for you to wait a "little" bit longer until we have a decision regarding Sarina's mapper-related project?

(In reply to Stefan Kislinskiy from comment #7)

Daniel, is it okay for you to wait a "little" bit longer until we have a
decision regarding Sarina's mapper-related project?

Sure, take your time. If there is any information available about "Sarina's project" (goal, changes, timeline), I'd be curious to learn about upcoming changes.

(In reply to Daniel Maleike from comment #8)

Sure, take your time. If there is any information available about "Sarina's
project" (goal, changes, timeline), I'd be curious to learn about upcoming
changes.

It's Sarina's introductionary project. Well, it's actually only a part of it: Get rid of the module object factories by basing them on CppMicroServices just like we did with the IO stuff. We're still discussing, if we are going to touch the way Properties are handled by Mappers. However, it is the third and last subproject of Sarina's whole project and she just began with her first subproject.

So we should decide how to proceed here without introducing multiple solutions or solutions that might be declared as deprecated even before they even made it into the next release on one hand, and how we can satisfy you right now on the other hand. :)

@kislinsk I just rebased my changes to the 2016.11 release and created a pull request on github: https://github.com/MITK/MITK/pull/174

Using this occasion: if I remember correctly, you want a "sign-off" on _each_ contributed commit. Is that still the case? Or is one sign-off at the end of a branch sufficient? I cannot find that information in http://mitk.org/wiki/Sign_off_contribution

Thank you! 👍

Sign-off on each commit is preferred.

Pushed new branch T19360-legacy-scene-surface-line-width-incompatible.

The provided pull request does not solve the problem for me. It only replaces the renderer specific property, but in most cases the property will not be renderer specific. node->GetProperty(name, renderer) will return the unspecific one if no specific is found.

One simple fix would be to call FixupLegacyProperties for both property lists. Would this work for you @maleike ?

@goch: very good point! I missed this while rebasing my branch.. sorry for that. I think your solution would work fine. I can adapt the code and test it Monday (wouldn't mind if you do it if you already have this change locally)

@maleike I will just go ahead and add that line myself.

Deleted branch T19360-legacy-scene-surface-line-width-incompatible.