BUG: Ensure RemoveAllMarkups also invoke MarkupRemovedEvent

Created by: jcfr

From previous discussion:

@naucoin mentionned:

The Texts api was ported over from the Annotations module, and isn't currently used in the Slicer core. The call to init the TextList in remove all markups is to deal with any texts that were added outside of the module, for example through python. The setting of the Locked ivar was done specifically to avoid another modified event while removing all markups. The way the events are managed were to ensure that only one markup removed event would get triggered at the very end, unless events are disabled - it's a workaround for cases where there are lists with 100's of markups and the events were slowing things to a crawl. If you run the py_AddManyMarkupsFiducialTest test before and after your changes, do you see a significant rise in total execution time?

.. and @jcfr replied:

The Texts api was ported over from the Annotations module, and isn't currently used in the Slicer core.

What do you think if the remove it ? There is already the Description associated with markups. It seems this doesn't apply anymore.

The call to init the TextList in remove all markups is to deal with any texts that were added outside of the module, for example through python.

If we keep this API arround, similarly through python, the developer should now call RemoveTexts.

I think we need to keep things consistent, it will make our respective code and project easier to maintain.

The setting of the Locked ivar was done specifically to avoid another modified event while removing all markups.

Hum ... the issue is that anybody developing an interface or some code would have no way to know that the lock state changed. It would be implicit knowledge. If we go along that route, we should document it clearly.

The way the events are managed were to ensure that only one markup removed event would get triggered at the very end, unless events are disabled - it's a workaround for cases where there are lists with 100's of markups and the events were slowing things to a crawl.

The normal case should behave as expected. Meaning that for client of the API, catching the MarkupRemovedEvent should mean the samething each time. If dealing with 100's of markups has performance problem, we should probably introduce (or generalize) the concept of BatchProcess already in place for the NodeAdded/NodeRemoved

Or simpler, an other event named AllMarkupRemovedEvent should be added. Otherwise, it is hard to know if one or all markup have been removed. Thinking about .. if the call data is null .. we could by convention says it applies to all list. I just think the cost of adding an extra event is fairly low and would make things clearer ?

Or generalize the concept of StartModify/EndModify to any event associated with a given object. That would be an easy way to "compress" event and avoid the overhead of having 100's of similar event...

If you run the py_AddManyMarkupsFiducialTest test before and after your changes, do you see a significant rise in total execution time?

With: 24.17 sec, 26.65 sec, 24.01 sec (3 runs) Without: 26.59 sec, 27.04 sec, 24.72 sec (3 runs)