DM-54928: Use unique mask planes for each frame

python/lsst/display/firefly/firefly.py

@@ -56,6 +56,18 @@ def firefly_version():
 class DisplayImpl(virtualDevice.DisplayImpl):
     """Device to talk to a firefly display"""
 
+    @staticmethod
+    def _scoped_mask_id(frame, plane_name):
+        """Build a server-side mask layer id that is unique per frame.
+
+        The Firefly server keys mask overlays by ``mask_id`` alone, so two
+        frames that both register a layer named e.g. ``"DETECTED"`` would
+        overwrite each other.  Prefixing the plane name with the frame number
+        keeps each frame's layers independent while remaining human-readable
+        in Firefly's layer panel.
+        """
+        return f"f{frame}__{plane_name}"
+
     @staticmethod
     def __handleCallbacks(event):
         if 'type' in event['data']:
@@ -213,19 +225,30 @@ def _mtv(self, image, mask=None, wcs=None, title="", metadata=None):
                     _fireflyClient.add_mask(bit_number=self._maskDict[k],
                                             image_number=0,
                                             plot_id=str(self.display.frame),
-                                            mask_id=k,
+                                            mask_id=self._scoped_mask_id(self.display.frame, k),
                                             title=k + ' - bit %d'%self._maskDict[k],
                                             color=self._maskPlaneColors[k],
                                             file_on_server=self._fireflyMaskOnServer)
                     if k in self._maskTransparencies:
                         self._setMaskTransparency(self._maskTransparencies[k], k)
-                    self._maskIds.append(k)
+                    self._maskIds.append((self.display.frame, k))
 
     def _remove_masks(self):
-        """Remove mask layers"""
-        for k in self._maskIds:
-            _fireflyClient.remove_mask(plot_id=str(self.display.frame), mask_id=k)
-        self._maskIds = []
+        """Remove mask layers for the current frame.
+
+        Layers belonging to other frames are left in place; otherwise
+        loading a new image in one frame would clear the mask overlays
+        of every other displayed frame.
+        """
+        frame = self.display.frame
+        kept = []
+        for f, name in self._maskIds:
+            if f == frame:
+                _fireflyClient.remove_mask(plot_id=str(frame),
+                                           mask_id=self._scoped_mask_id(f, name))
+            else:
+                kept.append((f, name))
+        self._maskIds = kept
 
     def _buffer(self, enable=True):
         """!Enable or disable buffering of writes to the display
@@ -429,16 +452,24 @@ def _scale(self, algorithm, min, max, unit=None, *args, **kwargs):
             self._lastStretch = rval['rv_string']
 
     def _setMaskTransparency(self, transparency, maskName):
-        """Specify mask transparency (percent); or None to not set it when loading masks"""
+        """Specify mask transparency (percent); or None to not set it when loading masks.
+
+        Operates on the current frame.  ``_maskIds`` holds ``(frame, name)``
+        tuples (recorded by ``_mtv``); we filter those to the current frame.
+        Plane names from ``_defaultMaskPlaneColor`` are also included, scoped
+        to the current frame.
+        """
+        frame = self.display.frame
         if maskName is not None:
-            masklist = [maskName]
+            names = [maskName]
         else:
-            masklist = set(self._maskIds + list(self.display._defaultMaskPlaneColor.keys()))
-        for k in masklist:
+            names = {name for f, name in self._maskIds if f == frame}
+            names.update(self.display._defaultMaskPlaneColor.keys())
+        for k in names:
             self._maskTransparencies[k] = transparency
             _fireflyClient.dispatch(action_type='ImagePlotCntlr.overlayPlotChangeAttributes',
-                                    payload={'plotId': str(self.display.frame),
-                                             'imageOverlayId': k,
+                                    payload={'plotId': str(frame),
+                                             'imageOverlayId': self._scoped_mask_id(frame, k),
                                              'attributes': {'opacity': 1.0 - transparency/100.},
                                              'doReplot': False})
 
@@ -450,15 +481,17 @@ def _getMaskTransparency(self, maskName):
         return transparency
 
     def _setMaskPlaneColor(self, maskName, color):
-        """Specify mask color """
-        _fireflyClient.remove_mask(plot_id=str(self.display.frame),
-                                   mask_id=maskName)
+        """Specify mask color for the current frame.
+        """
+        frame = self.display.frame
+        scoped_id = self._scoped_mask_id(frame, maskName)
+        _fireflyClient.remove_mask(plot_id=str(frame), mask_id=scoped_id)
         self._maskPlaneColors[maskName] = color
         if (color.lower() != 'ignore'):
             _fireflyClient.add_mask(bit_number=self._maskDict[maskName],
                                     image_number=1,
-                                    plot_id=str(self.display.frame),
-                                    mask_id=maskName,
+                                    plot_id=str(frame),
+                                    mask_id=scoped_id,
                                     color=self.display.getMaskPlaneColor(maskName),
                                     file_on_server=self._fireflyFitsID)
 

tests/test_mask_scoping.py

@@ -0,0 +1,168 @@
+#
+# LSST Data Management System
+# Copyright 2026 LSST Corporation.
+#
+# This product includes software developed by the
+# LSST Project (http://www.lsst.org/).
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the LSST License Statement and
+# the GNU General Public License along with this program.  If not,
+# see <http://www.lsstcorp.org/LegalNotices/>.
+#
+
+"""Tests that mask overlays sent to the Firefly server are keyed by both
+frame and plane name, so that multiple frames carrying same-named planes
+(e.g. ``DETECTED`` on three different exposures) do not overwrite each
+other on the server.
+"""
+
+import unittest
+from types import SimpleNamespace
+from unittest import mock
+
+import lsst.utils.tests
+from lsst.display.firefly import firefly as firefly_mod
+
+
+def _make_impl(frame, mask_ids=None, mask_dict=None, mask_plane_colors=None,
+               default_mask_plane_color=None, mask_transparencies=None):
+    """Construct a ``DisplayImpl`` without running ``__init__``.
+
+    ``DisplayImpl.__init__`` requires a live Firefly server; we sidestep it
+    via ``__new__`` and inject only the attributes the methods under test
+    read.  ``display`` is stubbed as a SimpleNamespace exposing the small
+    surface those methods touch (``frame``, ``getMaskPlaneColor``,
+    ``_defaultMaskPlaneColor``).
+    """
+    impl = firefly_mod.DisplayImpl.__new__(firefly_mod.DisplayImpl)
+    impl.display = SimpleNamespace(
+        frame=frame,
+        getMaskPlaneColor=lambda name: (mask_plane_colors or {}).get(name, "red"),
+        _defaultMaskPlaneColor=default_mask_plane_color or {},
+    )
+    impl._maskIds = list(mask_ids) if mask_ids is not None else []
+    impl._maskDict = dict(mask_dict) if mask_dict is not None else {}
+    impl._maskPlaneColors = dict(mask_plane_colors) if mask_plane_colors is not None else {}
+    impl._maskTransparencies = dict(mask_transparencies) if mask_transparencies is not None else {}
+    impl._fireflyFitsID = "fits-id-stub"
+    # ``__del__`` -> ``_close()`` reads these attributes; satisfy it
+    # since we are bypassing ``__init__``.
+    impl.verbose = False
+    impl._client = None
+    return impl
+
+
+class ScopedMaskIdTest(unittest.TestCase):
+    """The helper itself is pure -- verify it produces distinct ids per
+    frame while remaining human-readable in the layer panel."""
+
+    def test_distinct_per_frame(self):
+        self.assertNotEqual(firefly_mod.DisplayImpl._scoped_mask_id(0, "DETECTED"),
+                            firefly_mod.DisplayImpl._scoped_mask_id(1, "DETECTED"))
+
+    def test_includes_plane_name(self):
+        self.assertIn("DETECTED", firefly_mod.DisplayImpl._scoped_mask_id(0, "DETECTED"))
+        self.assertIn("0", firefly_mod.DisplayImpl._scoped_mask_id(0, "DETECTED"))
+
+
+class RemoveMasksTest(unittest.TestCase):
+    """``_remove_masks`` is invoked when a new image is loaded into a
+    frame; it must only clear *that* frame's overlays."""
+
+    def test_only_removes_current_frame(self):
+        impl = _make_impl(
+            frame=1,
+            mask_ids=[(0, "DETECTED"), (1, "DETECTED"), (1, "BAD"), (2, "SAT")],
+        )
+        with mock.patch.object(firefly_mod, "_fireflyClient") as client:
+            impl._remove_masks()
+            removed = [(c.kwargs["plot_id"], c.kwargs["mask_id"])
+                       for c in client.remove_mask.call_args_list]
+        # Frame 1 layers removed, frames 0 and 2 left alone.
+        self.assertEqual(set(removed),
+                         {("1", "f1__DETECTED"), ("1", "f1__BAD")})
+        self.assertEqual(set(impl._maskIds), {(0, "DETECTED"), (2, "SAT")})
+
+
+class SetMaskPlaneColorTest(unittest.TestCase):
+    """``setMaskPlaneColor`` should retarget only the current frame's
+    layer, leaving sibling frames' layers in place."""
+
+    def test_scopes_mask_id(self):
+        impl = _make_impl(
+            frame=2,
+            mask_dict={"DETECTED": 5},
+            mask_plane_colors={"DETECTED": "red"},
+        )
+        with mock.patch.object(firefly_mod, "_fireflyClient") as client:
+            impl._setMaskPlaneColor("DETECTED", "cyan")
+            (remove_call,) = client.remove_mask.call_args_list
+            (add_call,) = client.add_mask.call_args_list
+        self.assertEqual(remove_call.kwargs["plot_id"], "2")
+        self.assertEqual(remove_call.kwargs["mask_id"], "f2__DETECTED")
+        self.assertEqual(add_call.kwargs["plot_id"], "2")
+        self.assertEqual(add_call.kwargs["mask_id"], "f2__DETECTED")
+        self.assertEqual(impl._maskPlaneColors["DETECTED"], "cyan")
+
+    def test_ignore_color_skips_add(self):
+        impl = _make_impl(
+            frame=0,
+            mask_dict={"DETECTED": 5},
+            mask_plane_colors={"DETECTED": "red"},
+        )
+        with mock.patch.object(firefly_mod, "_fireflyClient") as client:
+            impl._setMaskPlaneColor("DETECTED", "ignore")
+            self.assertEqual(client.remove_mask.call_count, 1)
+            self.assertEqual(client.add_mask.call_count, 0)
+
+
+class SetMaskTransparencyTest(unittest.TestCase):
+    """``setMaskTransparency`` dispatches per-layer attribute changes;
+    the dispatched ``imageOverlayId`` must be the frame-scoped id."""
+
+    def test_named_plane_uses_scoped_overlay_id(self):
+        impl = _make_impl(frame=3)
+        with mock.patch.object(firefly_mod, "_fireflyClient") as client:
+            impl._setMaskTransparency(40, "DETECTED")
+            (call,) = client.dispatch.call_args_list
+        payload = call.kwargs["payload"]
+        self.assertEqual(payload["plotId"], "3")
+        self.assertEqual(payload["imageOverlayId"], "f3__DETECTED")
+        self.assertAlmostEqual(payload["attributes"]["opacity"], 0.6)
+
+    def test_all_planes_filters_by_frame(self):
+        # ``maskName=None`` means "all of this frame's planes".  Layers
+        # registered against other frames must not be touched.
+        impl = _make_impl(
+            frame=1,
+            mask_ids=[(0, "DETECTED"), (1, "DETECTED"), (1, "BAD")],
+            default_mask_plane_color={},
+        )
+        with mock.patch.object(firefly_mod, "_fireflyClient") as client:
+            impl._setMaskTransparency(0, None)
+            ids = {c.kwargs["payload"]["imageOverlayId"]
+                   for c in client.dispatch.call_args_list}
+        self.assertEqual(ids, {"f1__DETECTED", "f1__BAD"})
+
+
+class TestMemory(lsst.utils.tests.MemoryTestCase):
+    pass
+
+
+def setup_module(module):
+    lsst.utils.tests.init()
+
+
+if __name__ == "__main__":
+    lsst.utils.tests.init()
+    unittest.main()