Insert gap pixels only between chips unless module gaps are set

An unset GapPixels::module_gaps defaulted to the chip gap, so every
module boundary received two extra rows or columns. Two stacked
Jungfrau modules read as 1030x1030 instead of 1030x1028. Unset now
means no pixels between modules; only the chip boundaries inside a
module receive a gap.

Also drop the explicit lambda capture in split_edge_counts that clang
reported as unused for floating point pixels.

RELEASE.md

@@ -7,8 +7,9 @@
 - Added gap pixel insertion for Jungfrau and Eiger raw files. ``RawFile``,
   ``File`` (through ``FileConfig::gap_pixels``) and
   ``MultiThreadedFileReader`` take a ``GapPixels`` configuration, in Python
-  the ``gap_pixels`` argument, and assemble frames with 2 pixel chip gaps
-  and optional ``ModuleGaps`` between modules. Gap positions follow
+  the ``gap_pixels`` argument, and assemble frames with 2 pixel gaps at
+  the chip boundaries inside each module and, only when ``ModuleGaps`` is
+  set, gaps between modules. Gap positions follow
   detector coordinates before any ROI. Gap pixels receive a fill value or,
   with ``split_counts``, half of the neighbouring double size pixel. The
   new ``insert_gap_pixels`` and ``gapped_shape`` apply the same transform

docs/src/GapPixels.rst

@@ -4,8 +4,8 @@ GapPixels
 Jungfrau and Eiger modules are built from 256x256 pixel chips. The pixels
 at a chip edge are physically double size, so an image that places the
 chips next to each other is distorted. Gap pixels restore the geometry by
-inserting pixels at every chip boundary and, optionally, wider gaps
-between modules.
+inserting pixels at every chip boundary inside a module and, optionally,
+gaps between modules.
 
 Gap positions are determined in detector coordinates before any ROI.
 A ROI only receives the gaps that fall strictly inside it, so a ROI that
@@ -26,9 +26,9 @@ Field            Default          Meaning
                                   inside a module. Source column 255 expands
                                   to the right and column 256 to the left.
 ``module_gaps``  unset            ``ModuleGaps{x, y}`` inserted at module
-                                  boundaries instead of the chip gap. Unset
-                                  means module boundaries receive the chip
-                                  gap. ``ModuleGaps{8, 36}`` matches the
+                                  boundaries. Unset means no pixels are
+                                  inserted between modules.
+                                  ``ModuleGaps{8, 36}`` matches the
                                   slsDetectorPackage GUI.
 ``fill_value``   0                Value written to gap pixels, converted to
                                   the pixel type. Out of range values wrap
@@ -47,8 +47,8 @@ Image sizes with the default configuration and with ``ModuleGaps{8, 36}``:
 Detector range                         Ungapped   Defaults  ``ModuleGaps{8,36}``
 ====================================== ========== ========= ===================
 One module, columns                    1024       1030      1030
-Two modules side by side, columns      2048       2062      2068
-Two modules stacked, rows              1024       1030      1064
+Two modules side by side, columns      2048       2060      2068
+Two modules stacked, rows              1024       1028      1064
 ROI columns 100 to 400                 300        302       302
 ROI columns 256 to 512                 256        256       256
 ====================================== ========== ========= ===================

docs/src/python/file/pyGapPixels.rst

@@ -5,7 +5,8 @@ GapPixels
 
 Jungfrau and Eiger modules are built from 256x256 pixel chips with double
 size pixels at the chip edges. Gap pixels restore the geometry by inserting
-pixels at every chip boundary and, optionally, wider gaps between modules.
+pixels at every chip boundary inside a module and, optionally, gaps between
+modules.
 Gap positions are determined in detector coordinates before any ROI, so a
 ROI only receives the gaps that fall strictly inside it.
 
@@ -19,9 +20,9 @@ Field            Default          Meaning
 ``chip_gap``     2                Pixels inserted at each chip boundary
                                   inside a module.
 ``module_gaps``  None             ``ModuleGaps(x, y)`` inserted at module
-                                  boundaries instead of the chip gap. None
-                                  means module boundaries receive the chip
-                                  gap. ``ModuleGaps(8, 36)`` matches the
+                                  boundaries. None means no pixels are
+                                  inserted between modules.
+                                  ``ModuleGaps(8, 36)`` matches the
                                   slsDetectorPackage GUI.
 ``fill_value``   0                Value written to gap pixels, converted to
                                   the pixel dtype. Module gaps always
@@ -34,7 +35,7 @@ Field            Default          Meaning
 ================ ================ ===========================================
 
 A single Jungfrau module reads as ``(514, 1030)`` with the defaults. Two
-stacked modules read as ``(1030, 1030)`` with the defaults and
+stacked modules read as ``(1028, 1030)`` with the defaults and
 ``(1064, 1030)`` with ``ModuleGaps(8, 36)``.
 
 .. code-block:: python

include/aare/GapPixels.hpp

@@ -18,8 +18,7 @@
 namespace aare {
 
 /**
- * @brief Pixels inserted at each boundary between modules, replacing the
- * chip gap there.
+ * @brief Pixels inserted at each boundary between modules.
  */
 struct ModuleGaps {
     ssize_t x{};
@@ -34,7 +33,9 @@ struct ModuleGaps {
 /**
  * @brief Configuration for inserting gap pixels into Jungfrau and Eiger
  * images. Chip and module sizes are fixed by the detector type: 256x256
- * pixel chips in 1024x512 pixel modules (512x512 for an Eiger quad).
+ * pixel chips in 1024x512 pixel modules (512x512 for an Eiger quad). Gaps
+ * are inserted at the chip boundaries inside each module and, only when
+ * module_gaps is set, between modules.
  *
  * Gap positions are determined in detector coordinates before any ROI, so
  * a ROI only receives the gaps that fall strictly inside it.
@@ -44,8 +45,8 @@ struct GapPixels {
     /// column 255 expands to the right and column 256 to the left.
     ssize_t chip_gap{2};
 
-    /// @brief Pixels inserted at module boundaries instead of the chip gap.
-    /// Unset means module boundaries receive the chip gap.
+    /// @brief Pixels inserted at module boundaries. Unset means no pixels
+    /// are inserted between modules.
     std::optional<ModuleGaps> module_gaps{};
 
     /// @brief Value written to gap pixels, converted to the pixel type.

python/src/bind_GapPixels.hpp

@@ -32,8 +32,7 @@ py::array insert_gap_pixels_typed(const py::array &image, const ROI &roi,
 
 void define_gap_pixels_bindings(py::module &m) {
     py::class_<ModuleGaps>(m, "ModuleGaps", R"(
-        Pixels inserted at each boundary between modules, replacing the chip
-        gap there.
+        Pixels inserted at each boundary between modules.
         )")
         .def(py::init<>())
         .def(py::init<ssize_t, ssize_t>(), py::arg("x"), py::arg("y"))
@@ -55,8 +54,8 @@ void define_gap_pixels_bindings(py::module &m) {
             Pixels inserted at each chip boundary inside a module. Source
             column 255 expands to the right and column 256 to the left.
         module_gaps : ModuleGaps, optional
-            Pixels inserted at module boundaries instead of the chip gap.
-            None means module boundaries receive the chip gap.
+            Pixels inserted at module boundaries. None means no pixels are
+            inserted between modules.
         fill_value : float
             Value written to gap pixels, converted to the pixel type. Module
             gaps always receive this value.

python/tests/test_GapPixels.py

@@ -34,7 +34,7 @@ def test_configuration_defaults_and_equality():
 
 def test_gapped_shape():
     assert gapped_shape(ROI(0, 1024, 0, 512), DetectorType.Jungfrau) == (514, 1030)
-    assert gapped_shape(ROI(0, 1024, 0, 1024), DetectorType.Jungfrau) == (1030, 1030)
+    assert gapped_shape(ROI(0, 1024, 0, 1024), DetectorType.Jungfrau) == (1028, 1030)
     assert gapped_shape(ROI(0, 1024, 0, 1024), DetectorType.Jungfrau, GapPixels(module_gaps=ModuleGaps(8, 36))) == (1064, 1030)
     assert gapped_shape(ROI(0, 512, 0, 512), DetectorType.Eiger, quad=True) == (514, 514)
     assert gapped_shape(ROI(256, 512, 0, 256), DetectorType.Jungfrau) == (256, 256)
@@ -54,6 +54,16 @@ def test_insert_gap_pixels_matches_numpy(dtype):
     assert (result == expected).all()
 
 
+def test_insert_gap_pixels_without_module_gaps():
+    # two stacked modules: chip gaps at rows 256 and 768, none at the module
+    # boundary at row 512
+    image = np.arange(1024 * 1024, dtype=np.uint32).reshape(1024, 1024)
+    result = insert_gap_pixels(image, DetectorType.Jungfrau)
+    expected = reference(image, [(256, 2), (512, 2), (768, 2)], [(256, 2), (768, 2)])
+    assert result.shape == (1028, 1030)
+    assert (result == expected).all()
+
+
 def test_insert_gap_pixels_with_module_gaps():
     image = np.arange(1024 * 1024, dtype=np.uint32).reshape(1024, 1024)
     gaps = GapPixels(module_gaps=ModuleGaps(8, 36), fill_value=9)

python/tests/test_RawFile.py

@@ -527,8 +527,9 @@ def test_raw_file_gap_pixels(tmp_path, layout):
     gaps = GapPixels(fill_value=5)
     with RawFile(master_path, gap_pixels=gaps) as reader:
         assert reader.gap_pixels == gaps
-        assert (reader.rows(), reader.cols()) == (512 * layout_rows + 2 * (layout_rows * 2 - 1),
-                                                  1024 * layout_cols + 2 * (layout_cols * 4 - 1))
+        # one chip boundary per module row, three per module column and no
+        # gap between modules
+        assert (reader.rows(), reader.cols()) == ((512 + 2) * layout_rows, (1024 + 6) * layout_cols)
         header, image = reader.read_frame()
         assert header.size == layout_rows * layout_cols
         assert (image == insert_gap_pixels(expected, DetectorType.Jungfrau, gaps)).all()

src/GapPixels.cpp

@@ -15,8 +15,7 @@ GapLayout gap_layout(DetectorType detector, bool quad, const GapPixels &gaps) {
     if (gaps.chip_gap < 0) {
         throw std::invalid_argument(LOCATION + "Chip gap must not be negative");
     }
-    const ModuleGaps module_gaps =
-        gaps.module_gaps.value_or(ModuleGaps{gaps.chip_gap, gaps.chip_gap});
+    const ModuleGaps module_gaps = gaps.module_gaps.value_or(ModuleGaps{});
     if (module_gaps.x < 0 || module_gaps.y < 0) {
         throw std::invalid_argument(LOCATION +
                                     "Module gaps must not be negative");

src/GapPixels.test.cpp

@@ -76,11 +76,11 @@ TEST_CASE("Gap layout follows the detector type", "[GapPixels]") {
         CHECK(layout.x.chip == 256);
         CHECK(layout.x.module == 1024);
         CHECK(layout.x.chip_gap == 2);
-        CHECK(layout.x.module_gap == 2);
+        CHECK(layout.x.module_gap == 0);
         CHECK(layout.y.chip == 256);
         CHECK(layout.y.module == 512);
         CHECK(layout.y.chip_gap == 2);
-        CHECK(layout.y.module_gap == 2);
+        CHECK(layout.y.module_gap == 0);
     }
     SECTION("Eiger quad has a 512 pixel wide module") {
         CHECK(detail::gap_layout(DetectorType::Eiger, true, GapPixels{})
@@ -135,11 +135,11 @@ TEST_CASE("Gapped shape of detector ROIs", "[GapPixels]") {
     CHECK(gapped_shape(ROI{0, 1024, 0, 512}, DetectorType::Jungfrau, false,
                        module_8_36) == Shape{514, 1030});
     CHECK(gapped_shape(ROI{0, 2048, 0, 512}, DetectorType::Jungfrau, false) ==
-          Shape{514, 2062});
+          Shape{514, 2060});
     CHECK(gapped_shape(ROI{0, 2048, 0, 512}, DetectorType::Jungfrau, false,
                        module_8_36) == Shape{514, 2068});
     CHECK(gapped_shape(ROI{0, 1024, 0, 1024}, DetectorType::Jungfrau, false) ==
-          Shape{1030, 1030});
+          Shape{1028, 1030});
     CHECK(gapped_shape(ROI{0, 1024, 0, 1024}, DetectorType::Jungfrau, false,
                        module_8_36) == Shape{1064, 1030});
     CHECK(gapped_shape(ROI{0, 512, 0, 512}, DetectorType::Eiger, true) ==
@@ -155,7 +155,7 @@ TEST_CASE("Gapped shape of detector ROIs", "[GapPixels]") {
         CHECK(gapped_shape(ROI{255, 257, 0, 1}, DetectorType::Jungfrau,
                            false) == Shape{1, 4});
         CHECK(gapped_shape(ROI{200, 301, 300, 601}, DetectorType::Jungfrau,
-                           false) == Shape{303, 103});
+                           false) == Shape{301, 103});
         CHECK(gapped_shape(ROI{200, 301, 300, 601}, DetectorType::Jungfrau,
                            false, module_8_36) == Shape{337, 103});
         CHECK(gapped_shape(ROI{512, 768, 0, 256}, DetectorType::Jungfrau,
@@ -369,6 +369,35 @@ TEST_CASE("insert_gap_pixels on ROIs", "[GapPixels]") {
     }
 }
 
+TEST_CASE("Module boundaries receive no gap unless module gaps are set",
+          "[GapPixels]") {
+    // rows 510..513 of two stacked modules hold the module boundary at 512
+    const ROI roi{0, 2, 510, 514};
+    auto source = pattern<uint16_t>(4, 2);
+    GapPixels gaps;
+    gaps.fill_value = 9;
+    SECTION("fill value") {}
+    SECTION("split counts") {
+        gaps.split_counts = true;
+        gaps.seed = 1;
+    }
+    auto result = insert_gap_pixels(NDView<const uint16_t, 2>(source.view()),
+                                    roi, DetectorType::Jungfrau, false, gaps);
+    REQUIRE(result.shape() == std::array<ssize_t, 2>{4, 2});
+    CHECK(result == source);
+
+    gaps.module_gaps = ModuleGaps{0, 3};
+    auto with_gap = insert_gap_pixels(NDView<const uint16_t, 2>(source.view()),
+                                      roi, DetectorType::Jungfrau, false, gaps);
+    REQUIRE(with_gap.shape() == std::array<ssize_t, 2>{7, 2});
+    for (ssize_t row = 2; row < 5; ++row) {
+        CHECK(with_gap(row, 0) == 9);
+        CHECK(with_gap(row, 1) == 9);
+    }
+    CHECK(with_gap(1, 0) == source(1, 0));
+    CHECK(with_gap(5, 0) == source(2, 0));
+}
+
 TEST_CASE("Split counts between edge pixels and their gaps", "[GapPixels]") {
     GapPixels gaps;
     gaps.split_counts = true;

src/RawFile.test.cpp

@@ -476,12 +476,12 @@ TEST_CASE("Gap pixels on recorded Jungfrau and Eiger files",
          {514, 514},
          {}},
         {"raw/virtual/920/jungfrau/two_modules_master_0.json",
-         {1030, 1030},
+         {1028, 1030},
          {}},
         {"raw/virtual/920/jungfrau/two_modules_master_0.json",
          {1064, 1030},
          ModuleGaps{8, 36}},
-        {"raw/ROITestData/MultipleChipROI/run_master_2.json", {303, 103}, {}},
+        {"raw/ROITestData/MultipleChipROI/run_master_2.json", {301, 103}, {}},
         {"raw/ROITestData/MultipleChipROI/run_master_2.json",
          {337, 103},
          ModuleGaps{8, 36}},