From 7f9e90bbb4ae5f7c71eda5a90c873f0bfd24fda2 Mon Sep 17 00:00:00 2001
From: "Brett M. Morris" <bmmorris@stsci.edu>
Date: Wed, 27 Nov 2024 13:54:20 -0500
Subject: [PATCH 1/2] add WFI footprint

---
 CHANGES.rst                                   |  2 +
 .../imviz/plugins/footprints/footprints.py    | 18 ++++-
 .../imviz/plugins/footprints/footprints.vue   |  2 +-
 .../plugins/footprints/preset_regions.py      | 79 +++++++++++++------
 4 files changed, 73 insertions(+), 28 deletions(-)

diff --git a/CHANGES.rst b/CHANGES.rst
index 296f82eb5a..68229bb9a0 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -14,6 +14,8 @@ Imviz
 
 * Orientation plugin API now exposes create_north_up_east_left and create_north_up_east_right methods. [#3308]
 
+* Add Roman WFI and CGI footprints to the Footprints plugin. [#3322]
+
 Mosviz
 ^^^^^^
 
diff --git a/jdaviz/configs/imviz/plugins/footprints/footprints.py b/jdaviz/configs/imviz/plugins/footprints/footprints.py
index 9b0b50d31d..862b58b9c9 100644
--- a/jdaviz/configs/imviz/plugins/footprints/footprints.py
+++ b/jdaviz/configs/imviz/plugins/footprints/footprints.py
@@ -20,6 +20,13 @@
 __all__ = ['Footprints']
 
 
+_available_instruments = {
+    display_name: {'label': display_name, 'siaf_name': siaf_name, 'observatory': observatory}
+    for observatory, instruments in preset_regions._instruments.items()
+    for display_name, siaf_name in instruments.items()
+}
+
+
 @tray_registry('imviz-footprints', label="Footprints")
 class Footprints(PluginTemplateMixin, ViewerSelectMixin, HasFileImportSelect):
     """
@@ -119,11 +126,13 @@ def __init__(self, *args, **kwargs):
                                                      on_remove=self._on_overlay_remove)
 
         if self.has_pysiaf:
-            preset_options = list(preset_regions._instruments.keys())
+            preset_options = list(_available_instruments.keys())
         else:
             preset_options = ['None']
+
         if not self.app.state.settings.get('server_is_remote', False):
             preset_options.append('From File...')
+
         self.preset = FileImportSelectPluginComponent(self,
                                                       items='preset_items',
                                                       selected='preset_selected',
@@ -480,8 +489,11 @@ def overlay_regions(self):
                     regs = [regs]
                 overlay['regions'] = regs
             regs = overlay.get('regions', [])
-        elif self.has_pysiaf and self.preset_selected in preset_regions._instruments:
-            regs = preset_regions.jwst_footprint(self.preset_selected, **callable_kwargs)
+        elif self.has_pysiaf and self.preset_selected in _available_instruments.keys():
+            regs = preset_regions.instrument_footprint(
+                _available_instruments[self.preset_selected]['observatory'],
+                self.preset_selected, **callable_kwargs
+            )
         else:  # pragma: no cover
             regs = []
         return regs
diff --git a/jdaviz/configs/imviz/plugins/footprints/footprints.vue b/jdaviz/configs/imviz/plugins/footprints/footprints.vue
index a9009792cf..19cc450b96 100644
--- a/jdaviz/configs/imviz/plugins/footprints/footprints.vue
+++ b/jdaviz/configs/imviz/plugins/footprints/footprints.vue
@@ -91,7 +91,7 @@
 
       <j-plugin-section-header>Footprint Definition</j-plugin-section-header>
       <v-alert v-if="!has_pysiaf" type="warning" style="margin-left: -12px; margin-right: -12px">
-        To use JWST footprints, install pysiaf and restart jdaviz.
+        To use JWST or Roman footprints, install pysiaf and restart jdaviz.
       </v-alert>
 
       <plugin-file-import-select
diff --git a/jdaviz/configs/imviz/plugins/footprints/preset_regions.py b/jdaviz/configs/imviz/plugins/footprints/preset_regions.py
index e885abd395..030dea9290 100644
--- a/jdaviz/configs/imviz/plugins/footprints/preset_regions.py
+++ b/jdaviz/configs/imviz/plugins/footprints/preset_regions.py
@@ -16,23 +16,30 @@
     "_instruments",
     "_full_apertures",
     "_all_apertures",
-    "jwst_footprint"
+    "instrument_footprint"
 ]
 
-_instruments = {'NIRSpec': 'NIRSpec',
-                'NIRCam:short': 'NIRCam',
-                'NIRCam:long': 'NIRCam',
-                'NIRISS': 'NIRISS',
-                'MIRI': 'MIRI',
-                'FGS': 'FGS'
-                }
+_instruments = {
+    'JWST': {
+        'NIRSpec': 'NIRSpec',
+        'NIRCam:short': 'NIRCam',
+        'NIRCam:long': 'NIRCam',
+        'NIRISS': 'NIRISS',
+        'MIRI': 'MIRI',
+        'FGS': 'FGS',
+    },
+    'Roman': {
+        'WFI': 'Roman',
+        'WFI+CGI': 'Roman',
+    }
+}
 
 _full_apertures = {'NIRSpec': 'NRS_FULL_MSA',
                    'NIRCam:short': 'NRCALL_FULL',
                    'NIRCam:long': 'NRCALL_FULL',
                    'NIRISS': 'NIS_AMIFULL',
                    'MIRI': 'MIRIM_FULL',
-                   'FGS': 'FGS1_FULL'
+                   'FGS': 'FGS1_FULL',
                    }
 
 _all_apertures = {'NIRSpec': ["NRS_FULL_MSA1",
@@ -56,18 +63,27 @@
                   'NIRCam:long': ["NRCA5_FULL", "NRCB5_FULL"],
                   'NIRISS': ['NIS_AMIFULL'],
                   'MIRI': ['MIRIM_FULL'],
-                  'FGS': ['FGS1_FULL', 'FGS2_FULL']
+                  'FGS': ['FGS1_FULL', 'FGS2_FULL'],
+                  'WFI': [f'WFI{i+1:02d}_FULL' for i in range(18)],
+                  'WFI+CGI': [f'WFI{i+1:02d}_FULL' for i in range(18)] + ['CGI_CEN'],
                   }
 
 
-def jwst_footprint(instrument, ra, dec, pa, v2_offset=0.0, v3_offset=0.0, apertures=None):
+def instrument_footprint(
+        observatory, instrument, ra, dec, pa,
+        v2_offset=0.0, v3_offset=0.0, apertures=None
+):
     """
     Create footprint regions in sky coordinates from a jwst instrument.
 
     Parameters
     ----------
+    observatory : string
+        Observatory, one of {"JWST", "Roman"}.
     instrument : string
-        Instrument, one of 'nirspec', 'nircam:short', 'nircam:long'.
+        Instrument. For JWST, must be one of
+        {"NIRSpec", "NIRCam:short", "NIRCam:long", "NIRISS", "MIRI", "FGS"};
+        for Roman, must be one of {"WFI", "CGI"}.
     ra : float
         RA of NIRCam center, in degrees.
     dec : float
@@ -92,17 +108,30 @@ def jwst_footprint(instrument, ra, dec, pa, v2_offset=0.0, v3_offset=0.0, apertu
     if not _has_pysiaf:
         raise ImportError('jwst_footprint requires pysiaf to be installed')
 
-    if instrument not in _instruments:  # pragma: no cover
-        raise ValueError(f"instrument must be one of {', '.join(_instruments.keys())}")
-
-    siaf_interface = pysiaf.Siaf(_instruments.get(instrument))
-
-    # Get center and PA offset from full aperture
-    full = siaf_interface.apertures[_full_apertures.get(instrument)]
-    corners = full.corners("tel", rederive=False)
-    v2 = np.mean(corners[0]) - v2_offset
-    v3 = np.mean(corners[1]) + v3_offset
-    pa_offset = full.V3IdlYAngle
+    if observatory not in _instruments:
+        raise ValueError(f"observatory must be one of {', '.join(_instruments.keys())}")
+
+    if instrument not in _instruments[observatory]:  # pragma: no cover
+        raise ValueError(f"instrument must be one of {', '.join(_instruments[observatory].keys())}")
+
+    siaf_interface = pysiaf.Siaf(_instruments.get(observatory).get(instrument))
+
+    # use different definitions of the center for JWST and Roman:
+    if observatory == "Roman":
+        if instrument.startswith("WFI"):
+            center = siaf_interface.apertures['WFI_CEN']
+        else:
+            center = siaf_interface.apertures['CGI_CEN']
+        v2 = center.V2Ref - v2_offset
+        v3 = center.V3Ref + v3_offset
+        pa_offset = center.V3IdlYAngle
+    else:
+        # Get center and PA offset from full aperture
+        full = siaf_interface.apertures[_full_apertures.get(instrument)]
+        corners = full.corners("tel", rederive=False)
+        v2 = np.mean(corners[0]) - v2_offset
+        v3 = np.mean(corners[1]) + v3_offset
+        pa_offset = full.V3IdlYAngle
 
     # Attitude matrix for sky coordinates
     attmat = pysiaf.utils.rotations.attitude(v2, v3, ra, dec, pa - pa_offset)
@@ -115,7 +144,9 @@ def jwst_footprint(instrument, ra, dec, pa, v2_offset=0.0, v3_offset=0.0, apertu
     for aperture_name in apertures:
         aperture = siaf_interface.apertures[aperture_name]
         aperture.set_attitude_matrix(attmat)
-        poly_points = aperture.closed_polygon_points("sky")
+        poly_points = aperture.closed_polygon_points(
+            "sky", rederive='CGI' not in instrument
+        )
 
         sky_coord = coordinates.SkyCoord(*poly_points, unit="deg")
         reg = regions.PolygonSkyRegion(sky_coord)

From 8a20a2b7edb4483b969932582fea13f00d329013 Mon Sep 17 00:00:00 2001
From: "Brett M. Morris" <bmmorris@stsci.edu>
Date: Thu, 5 Dec 2024 09:02:52 -0500
Subject: [PATCH 2/2] addressing review comment from Ricky

---
 jdaviz/configs/imviz/plugins/footprints/preset_regions.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/jdaviz/configs/imviz/plugins/footprints/preset_regions.py b/jdaviz/configs/imviz/plugins/footprints/preset_regions.py
index 030dea9290..9e9200eea5 100644
--- a/jdaviz/configs/imviz/plugins/footprints/preset_regions.py
+++ b/jdaviz/configs/imviz/plugins/footprints/preset_regions.py
@@ -118,7 +118,7 @@ def instrument_footprint(
 
     # use different definitions of the center for JWST and Roman:
     if observatory == "Roman":
-        if instrument.startswith("WFI"):
+        if 'CGI' not in instrument:
             center = siaf_interface.apertures['WFI_CEN']
         else:
             center = siaf_interface.apertures['CGI_CEN']