Fix #651. Add title manager to ensure unique displayed titles.

nion/swift/model/DisplayItem.py

@@ -2,6 +2,7 @@
 
 # standard libraries
 import asyncio
+import collections
 import concurrent.futures
 import contextlib
 import copy
@@ -29,7 +30,6 @@
 from nion.swift.model import Changes
 from nion.swift.model import ColorMaps
 from nion.swift.model import DataItem
-from nion.swift.model import DynamicString
 from nion.swift.model import Graphics
 from nion.swift.model import Model
 from nion.swift.model import Persistence
@@ -2346,6 +2346,73 @@ def datum_calibrations(self) -> typing.Sequence[Calibration.Calibration]:
         return [Calibration.Calibration() for c in dimensional_calibrations] if dimensional_calibrations else [Calibration.Calibration()]
 
 
+class DisplayItemTitleManager:
+    """Display item title manager.
+
+    Manage display item titles to ensure they are unique.
+
+    Titles are disambiguated by adding a suffix to the title. The suffix is a short string that is unique among all
+    display items with the same title. The suffix is generated from the display item's UUID. The UUID is used instead
+    of a simple counter to ensure that the suffix is unique even if another display item with lower index is deleted.
+    """
+    def __init__(self) -> None:
+        self.__display_item_title_streams = dict[weakref.ReferenceType["DisplayItem"], Stream.ValueStream[str]]()
+        self.__display_item_title_stream_actions = dict[weakref.ReferenceType["DisplayItem"], Stream.ValueStreamAction[str]]()
+        self.__display_item_suffix_streams = dict[weakref.ReferenceType["DisplayItem"], Stream.ValueStream[str]]()
+
+    def displayed_title_stream_for_display_item(self, display_item: DisplayItem, base_title_stream: Stream.ValueStream[str]) -> Stream.ValueStream[str]:
+        self.__display_item_title_streams[weakref.ref(display_item)] = base_title_stream
+        self.__display_item_title_stream_actions[weakref.ref(display_item)] = Stream.ValueStreamAction(base_title_stream, ReferenceCounting.weak_partial(DisplayItemTitleManager.__displayed_title_changed, self, display_item))
+        self.__display_item_suffix_streams[weakref.ref(display_item)] = Stream.ValueStream[str]()
+
+        def combine_title(base_title: typing.Optional[str], suffix: typing.Optional[str]) -> str:
+            return (base_title or str()) + (" " + suffix if suffix else str())
+
+        return typing.cast(Stream.ValueStream[str], Stream.CombineLatestStream([base_title_stream, self.__display_item_suffix_streams[weakref.ref(display_item)]], combine_title))
+
+    def close_display_item(self, display_item: DisplayItem) -> None:
+        self.__display_item_title_streams.pop(weakref.ref(display_item), None)
+        self.__display_item_title_stream_actions.pop(weakref.ref(display_item), None)
+        self.__display_item_suffix_streams.pop(weakref.ref(display_item), None)
+
+    def __displayed_title_changed(self, display_item: DisplayItem, title: str) -> None:
+        # first make a mapping from the title to the display items with that title.
+        map_title_to_display_items = dict[str, list[weakref.ReferenceType["DisplayItem"]]]()
+        for display_item_ref in self.__display_item_title_stream_actions.keys():
+            display_item_title_stream = self.__display_item_title_streams[display_item_ref]
+            display_item_title = display_item_title_stream.value
+            if display_item_title:
+                map_title_to_display_items[display_item_title] = map_title_to_display_items.get(display_item_title, list()) + [display_item_ref]
+        # next, for any title that has more than one display item, generate a suffix for each display item.
+        for title, display_item_ref_list in map_title_to_display_items.items():
+            if len(display_item_ref_list) > 1:
+                # search for a prefix of the UUID that is unique among all display items with the same title.
+                # start with length 2 and increase until all display items in the subset have a unique prefix.
+                length = 2
+                while length < 32:
+                    uuid_fragments = set[str]()
+                    for display_item_ref in display_item_ref_list:
+                        display_item_ = display_item_ref()
+                        if display_item_:
+                            uuid_fragments.add(display_item_.uuid.hex[:length])
+                    if len(uuid_fragments) < len(display_item_ref_list):
+                        length += 1
+                    else:
+                        break
+                for display_item_ref in display_item_ref_list:
+                    display_item_ = display_item_ref()
+                    if display_item_:
+                        self.__display_item_suffix_streams[display_item_ref].value = "#" + (display_item_.uuid.hex[:length]).upper()
+                # possible implementation of suffixes as indexes; but indexes are not stable during deletions.
+                # for i, display_item_ref in enumerate(display_item_ref_list):
+                #     self.__display_item_suffix_streams[display_item_ref].value = str(i + 1)
+            else:
+                self.__display_item_suffix_streams[display_item_ref_list[0]].value = None
+
+
+display_item_title_manager = DisplayItemTitleManager()
+
+
 class DisplayItem(Persistence.PersistentObject):
     DEFAULT_COLORS = ("#1E90FF", "#FF0000", "#00FF00", "#0000FF", "#FFFF00", "#00FFFF", "#FF00FF", "#888888", "#880000", "#008800", "#000088", "#CCCCCC", "#888800", "#008888", "#880088", "#964B00")
 
@@ -2412,7 +2479,8 @@ def combine_display_title(specified_title: typing.Optional[str], data_item_title
                 return data_item_title
             return _("Multiple Data Items")
 
-        self.displayed_title_stream = Stream.CombineLatestStream([self.__specified_title_stream, self.__single_data_item_title_stream], combine_display_title)
+        base_title_stream = Stream.CombineLatestStream([self.__specified_title_stream, self.__single_data_item_title_stream], combine_display_title)
+        self.displayed_title_stream = display_item_title_manager.displayed_title_stream_for_display_item(self, typing.cast(Stream.ValueStream[str], base_title_stream))
 
         def displayed_titled_changed(display_item: DisplayItem, displayed_title: typing.Optional[str]) -> None:
             if not display_item._is_reading:
@@ -2451,6 +2519,7 @@ def close(self) -> None:
         with self.__outstanding_condition:
             while self.__outstanding_thread_count:
                 self.__outstanding_condition.wait()
+        display_item_title_manager.close_display_item(self)
         self.__single_data_item_title_stream = typing.cast(typing.Any, None)
         self.__single_data_item_placeholder_title_stream = typing.cast(typing.Any, None)
         self.displayed_title_stream = typing.cast(typing.Any, None)

nion/swift/model/DocumentModel.py

@@ -895,6 +895,9 @@ def current_session_id(self) -> typing.Optional[str]:
         return self.session_id
 
     def copy_data_item(self, data_item: DataItem.DataItem) -> DataItem.DataItem:
+        display_item = self.get_display_item_for_data_item(data_item)
+        assert display_item
+        displayed_title = display_item.displayed_title  # take this here to avoid title manager disambiguation after copy
         computation_copy = copy.deepcopy(self.get_data_item_computation(data_item))
         data_item_copy = copy.deepcopy(data_item)
         self.append_data_item(data_item_copy)
@@ -903,9 +906,7 @@ def copy_data_item(self, data_item: DataItem.DataItem) -> DataItem.DataItem:
             computation_copy._clear_referenced_object("target")
             self.set_data_item_computation(data_item_copy, computation_copy)
         data_item_copy.category = data_item.category
-        display_item = self.get_display_item_for_data_item(data_item_copy)
-        assert display_item
-        data_item_copy.title = display_item.displayed_title + " (" + _("Copy") + ")"
+        data_item_copy.title = displayed_title + " (" + _("Copy") + ")"
         return data_item_copy
 
     def __handle_data_item_inserted(self, data_item: DataItem.DataItem) -> None:
@@ -1903,6 +1904,7 @@ def update_data_item_session(self, data_item: DataItem.DataItem) -> None:
         data_item.update_session(self.session_id)
 
     def get_display_item_snapshot_new(self, display_item: DisplayItem.DisplayItem) -> DisplayItem.DisplayItem:
+        displayed_title = display_item.displayed_title  # take this here to avoid title manager disambiguation after copy
         data_item_copy: typing.Optional[DataItem.DataItem]
         display_item_copy = display_item.snapshot()
         try:
@@ -1927,7 +1929,7 @@ def get_display_item_snapshot_new(self, display_item: DisplayItem.DisplayItem) -
             for i in range(len(display_item.display_layers)):
                 data_index = display_item.display_data_channels.index(display_item.get_display_layer_display_data_channel(i))
                 display_item_copy.add_display_layer_for_display_data_channel(display_item_copy.display_data_channels[data_index], **display_item.get_display_layer_properties(i))
-            display_item_copy.title = display_item.displayed_title + " (" + _("Snapshot") + ")"
+            display_item_copy.title = displayed_title + " (" + _("Snapshot") + ")"
         except Exception:
             display_item_copy.close()
             raise
@@ -1940,6 +1942,7 @@ def get_display_item_copy_new(self, display_item: DisplayItem.DisplayItem) -> Di
         return display_item_copy
 
     def get_display_snapshot_new(self, display_item: DisplayItem.DisplayItem) -> DisplayItem.DisplayItem:
+        displayed_title = display_item.displayed_title  # take this here to avoid title manager disambiguation after copy
         display_item_copy = display_item.snapshot()
         display_data_channels: typing.List[DisplayItem.DisplayDataChannel] = list()
         for display_data_channel in display_item_copy.display_data_channels:
@@ -1966,7 +1969,7 @@ def get_display_snapshot_new(self, display_item: DisplayItem.DisplayItem) -> Dis
         for i in range(len(display_item.display_layers)):
             data_index = display_item.display_data_channels.index(display_item.get_display_layer_display_data_channel(i))
             display_item_copy.add_display_layer_for_display_data_channel(display_item_copy.display_data_channels[data_index], **display_item.get_display_layer_properties(i))
-        display_item_copy.title = display_item.displayed_title + " (" + _("Display Snapshot") + ")"
+        display_item_copy.title = displayed_title + " (" + _("Display Snapshot") + ")"
         self.append_display_item(display_item_copy)
         return display_item_copy
 

nion/swift/resources/changes.json

@@ -2,6 +2,10 @@
   {
     "version": "UNRELEASED",
     "notes": [
+      {
+        "issues": ["https://github.com/nion-software/nionswift/issues/651"],
+        "summary": "Ensure display items have unique, quasi-stable displayed titles."
+      },
       {
         "issues": ["https://github.com/nion-software/nionswift/issues/21"],
         "summary": "Add zoom tool in toolbar. Also add zoom shortcut 'z'."