QIICR
Issues related to segments surface rendering
There are few issues that could be related to the recently revisited implementation of segment surface support:
- surface no longer shows up automatically in 3d viewer upon loading of the segments
- surface generation seems to take much longer than it used to, for a rather small dataset (on the order of several minutes for this dataset: https://qiicr.gitbooks.io/dicom4qi/content/instructions/seg.html#test-dataset-3)
- 3d viewer is very slow interacting with a relatively moderate dataset (tested with this dataset of brain segmentations: http://slicer.kitware.com/midas3/item/324959, around 40 segments)
- Loading how? What do you mean no longer? When did it work?
- I'm not aware of any change affecting conversion speed. Do you know when it was faster?
- Yes, surface rendering is unreasonably slow, however it is the same with models (export the segmentation to model hierarchy and see). VTK8+OpenGL2 will be a big improvement.
@lassoan Do you have any comment especially about the second issue?
I converted the segmentation to closed surface using both 4.8 and the latest build, and both were practically instantaneous.
I will need more details once RSNA is over and you have time to help me reproduce these.
@cpinter I just checked with the nightly build on Windows. It takes 1 minutes until the surfaces appear in the 3D view.
When using SegmentEditor only, this issue doesn't occur. I need to investigate to see where it gets stuck for a minute.
I figured that it has to do with observing events of the segmentation
Commenting the following code out, fixes the performance issue.
https://github.com/QIICR/QuantitativeReporting/blob/905436c0c2939e77e7de4fe480e32e5d209214a8/QuantitativeReporting/QuantitativeReporting.py#L513-L522
I will investigate a bit more to see which event is invoked and how often.
@cpinter Not sure why, but for showing the 3D surface, the SegmentModified event is invoked for each segment to be displayed in 3D. Is that on purpose and if so, why?
Showing surface should not invoke SegmentModified events. However creating it actually changes the data contained in a segment (add a whole new representation), so I think that's why this event is invoked. Andrey mentioned this is something new. Can you confirm? Is this event related to the slowdown?
The thing is, that our SegmentStatistics get updated upon SegmentModified (which absolutely makes sense). The issue here is, that it would get called 11 times (11 segments). Maybe when displaying in 3D there should only the event RepresentationModified be invoked instead of both SegmentModified and RepresentationModified.
QuantitativeReportingWidget.onSegmentModified called 11 times
QuantitativeReportingWidget.onRepresentationModified called 11 times
You can call StartModify() before you call the method that emits those events and then call EndModify(). As a result, only one modified event will be invoked, when you call EndModify().
If you cannot control when modified events are called then you can do a restteable delayed update: instead of directly connecting recalculation method to the modified event callback, connect a timer instead. If the timer expires (you can set it to expire in a second or so) then you call recalculation method. The timer is reset each time modified event is called, so for events that arrive in rapid succession, you'll only get one timer elapsed event. This technique is used in "Grow from seeds" Segment editor effect.
@lassoan Thanks! I will take a look into that. Regarding the StartModify() and EndModify() I think that should be done on side of SegmentEditor, right? QuantitativeReporting has nothing to do with invoking those events, since it just observes them.
When you are interested in only a particular segment, then squashing all modified events into one may result in slower performance, as you would redo your processing when unrelated segments are changed. So, it would not be a good solution to squash all events in the Segmentation module.
There are several options:
-
Recompute metrics for only those segments that are changed.
-
Observe MasterRepresentationModified event and detect changes in representation conversion parameters (such as smoothing). Creating 3D representation would not count as a modification then. For easy comparison of conversion parameters, you can retrieve all conversion parameters as a string using SerializeAllConversionParameters.
-
Delayed recomputation of metrics.
Probably delayed recomputation is the simplest option.
Auto recompute for the metrics is not really a hard requirement. We could mark the table as out of date to make this clear to the user when they are out of data, add a button to trigger recompute explicitly, and always prompt the user and recompute before they are exported to DICOM.
Interesting is also that when clicking Show 3D three times (show, hide, show) the number of SegmentModified and RepresentationModified events invoked ist unequal.
When hiding the segments from the 3D view, only SegmentModified is invoked.
QuantitativeReportingWidget.onSegmentModified called 33 times
QuantitativeReportingWidget.onRepresentationModified called 22 times
Simple show/hide shouldn't trigger SegmentModified. I'll look into this.
Once the 3D surfaces are displayed and show/hide segments from the segment table view, no such events are invoked. Only when using the Show 3D button those events are invoked.
Makes sense. If Show 3D is turned off, then the representation is actually removed, then re-converted when enabled again. The reason for this is that we didn't want to slow editing down when there is no 3D visible. The user would wonder why it became so slow. I know the name of the button does not suggest this, but here's the difference between user centeredness and technically correctness.
@cpinter sorry for the delay in replying
surface no longer shows up automatically in 3d viewer upon loading of the segments
- Loading how? What do you mean no longer? When did it work?
In the past (I believe before the 3d surface button was reworked), when segmentation was loaded, 3d surface would show up in 3d viewer automatically. Now it has to be triggered by the user clicking the button. This is confusing some users, since it changed the behavior.
From @cpinter
When a segmentation is created, then a master representation is set (labelmap in this case), and the way a representation can be created is calling
vtkSegmentation::CreateRepresentation(reprName). This call should be in the DICOM SEG import plugin, after it loaded the SEG into a segmentation.
More from Csaba:
This seems to be the root of the problem https://github.com/QIICR/QuantitativeReporting/commit/6ffabe1d255e94e74bb5b3eff70348d376599224#diff-8df200e5defa8405424843c46be1ad61R258
My hunch is that the call is too early.
It is possible that it needs to be called after the observations are done, but it is yet only an idea.
@cpinter I just figured out, that no SegmentModified event is invoked when painting something. Is that on purpose?
That event in vtkSegmentation and the segmentation node is invoked when the Modified event of the vtkSegment object is invoked (when changing name, color, tags). If you want to get notified about data change, then you need to use the MasterRepresentationModified event.