ENH: Use color legend instead of scalar bars

[MichaelColonel]

This is a possible solution for #208.

1. Dummy vtkMRMLDisplayableNode and vtkMRMLDisplayNode were added to vtkMRMLIsodoseNode to allow visualization of color legend with vtkMRMLColorLegendDisplayNode;
2. Scalar bars are replaced with color legend;
3. qMRMLColorLegendDisplayNodeWidget was added to control color legend parameters.

[lassoan]

Adding dummy nodes is not very clean and makes it more difficult to synchronize the visibility of the isodose surfaces and the color legend.

Instead, a nicer solution would be to use the subject hierarchy folder's existing vtkMRMLFolderDisplayNode for this.

We could also consider creating a single model node for all the isodose surfaces and color them using point scalars. It would simplify many other things, too. The only thing that would be harder is to quickly toggle visibility of individual lines/surfaces in the data module, but it might not be that important (the levels can be edited in isodose module).

@cpinter what do you think about changing the design to keep all the isodose lines in a single model?

[cpinter]

I think it would lead to simpler code in most places.

Yes the only thing indeed is individual handling of the levels. I'm not sure how often it is needed by actual users, but if @MichaelColonel and @gregsharp say that almost never, then it's probably clear and we don't have to worry about it. Otherwise we could add a feature that separates the surfaces by the scalars.

[MichaelColonel]

I have no objections, but what do you mean by coloring with point scalars? Something like: https://kitware.github.io/vtk-examples/site/Cxx/PolyData/ColoredPoints/, or am i wrong?

[cpinter]

Yes, this is the mechanism. You have an array that assigns some value to each point (or cell - that is also possible) in a way that the point indices match the value indices in the array, then you set the array with GetPointData()->SetScalars. In the models module you can color the polydata using scalars by wnabling it in the Display/Scalars section.

[MichaelColonel]

I've redone it with a single vtkMRMLModelNode.

[MichaelColonel]

I have updated an old PR, so it can be merged with current master. Everything should work out of the box.

[MichaelColonel]

The method int GetDoseUnitsFromString(const char *) has already been added: https://github.com/MichaelColonel/SlicerRT/blob/isodose-cl/Isodose/MRML/vtkMRMLIsodoseNode.h#L113 https://github.com/MichaelColonel/SlicerRT/blob/isodose-cl/Isodose/MRML/vtkMRMLIsodoseNode.cxx#L236-L256

So the scene with should be loaded, but if you recalculate isodose with color legend, the isolines will be too thin and barely visible.

[lassoan]

The method int GetDoseUnitsFromString(const char *) has already been added:

That's good, but for backward compatibility GetDoseUnitsFromString should recognize the old values (0, 1, and -1) and not just the new strings (Gy, Relative, and Unknown).

the isolines will be too thin and barely visible

Why the isolines thickness is related to the dose units?

[MichaelColonel]

That's good, but for backward compatibility GetDoseUnitsFromString should recognize the old values (0, 1, and -1) and not just the new strings (Gy, Relative, and Unknown).

Method modified.

I check the process of loading scene, and the DoseUnits is preserved between an old isodose color bar and a new color legend.

Absolute Dose scene: https://disk.yandex.ru/d/iVwup0TxZoiGXw Relative Dose scene: https://disk.yandex.ru/d/G9q9TB6Z6uDyVA

Why the isolines thickness is related to the dose units? Correct. It isn't related, but the difference in visualization is noticeable.

[MichaelColonel]

Other requested changes will be corrected shortly.

[MichaelColonel]

Doxygen description has been updated.

[cpinter]

Sorry I'm just back from vacation. Anyting to do before integrating this PR?

[MichaelColonel]

I will check everything one more time and let you know.

[MichaelColonel]

It's ready to be merged.

[cpinter]

Thanks @MichaelColonel ! GitHub tells me the branch cannot be rebased due to conflicts, can you please manually rebase the branch?

@lassoan @Sunderlandkyl any requests before merging?

[Sunderlandkyl]

No, lgtm!

[lassoan]

OK to merge for me, too. I would merge it, but, as @cpinter noted, there are conflicts that @MichaelColonel would need to resolve by rebasing on latest master.

[MichaelColonel]

@cpinter could you try to merge it? I hope there are no more conflicts.

[lassoan]

We still have the error message: "This branch cannot be rebased due to conflicts"

• Pull from SlicerRt:master
• Rebase
• Force-push to MichaelColonel:isodose-cl

[MichaelColonel]

I've rebased it.

[lassoan]

It is good now, merged. Thanks a lot for your contribution!