From 58da96d6f096aedca968e3a414e8a5e9d835fe40 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20Fr=C3=B6jdh?= Date: Fri, 9 Oct 2026 21:59:07 +0200 Subject: [PATCH] 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 | 5 ++-- docs/src/GapPixels.rst | 14 +++++----- docs/src/python/file/pyGapPixels.rst | 11 ++++---- include/aare/GapPixels.hpp | 11 ++++---- python/src/bind_GapPixels.hpp | 7 +++-- python/tests/test_GapPixels.py | 12 ++++++++- python/tests/test_RawFile.py | 5 ++-- src/GapPixels.cpp | 3 +-- src/GapPixels.test.cpp | 39 ++++++++++++++++++++++++---- src/RawFile.test.cpp | 4 +-- 10 files changed, 76 insertions(+), 35 deletions(-) diff --git a/RELEASE.md b/RELEASE.md index 3bf40b65..fcfb76ad 100644 --- a/RELEASE.md +++ b/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 diff --git a/docs/src/GapPixels.rst b/docs/src/GapPixels.rst index 9e49d075..34a2acce 100644 --- a/docs/src/GapPixels.rst +++ b/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 ====================================== ========== ========= =================== diff --git a/docs/src/python/file/pyGapPixels.rst b/docs/src/python/file/pyGapPixels.rst index 73e36ecf..862c8d72 100644 --- a/docs/src/python/file/pyGapPixels.rst +++ b/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 diff --git a/include/aare/GapPixels.hpp b/include/aare/GapPixels.hpp index b9e831e5..f187cf78 100644 --- a/include/aare/GapPixels.hpp +++ b/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 module_gaps{}; /// @brief Value written to gap pixels, converted to the pixel type. diff --git a/python/src/bind_GapPixels.hpp b/python/src/bind_GapPixels.hpp index b8af904f..0fc19bcb 100644 --- a/python/src/bind_GapPixels.hpp +++ b/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_(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(), 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. diff --git a/python/tests/test_GapPixels.py b/python/tests/test_GapPixels.py index 0eb8e03d..147b9947 100644 --- a/python/tests/test_GapPixels.py +++ b/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) diff --git a/python/tests/test_RawFile.py b/python/tests/test_RawFile.py index 85353393..d421ac34 100644 --- a/python/tests/test_RawFile.py +++ b/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() diff --git a/src/GapPixels.cpp b/src/GapPixels.cpp index 38f94aa5..3fb6c7e0 100644 --- a/src/GapPixels.cpp +++ b/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"); diff --git a/src/GapPixels.test.cpp b/src/GapPixels.test.cpp index 8ccfebd5..b1859924 100644 --- a/src/GapPixels.test.cpp +++ b/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(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(source.view()), + roi, DetectorType::Jungfrau, false, gaps); + REQUIRE(result.shape() == std::array{4, 2}); + CHECK(result == source); + + gaps.module_gaps = ModuleGaps{0, 3}; + auto with_gap = insert_gap_pixels(NDView(source.view()), + roi, DetectorType::Jungfrau, false, gaps); + REQUIRE(with_gap.shape() == std::array{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; diff --git a/src/RawFile.test.cpp b/src/RawFile.test.cpp index f4dc29e0..d6ebf744 100644 --- a/src/RawFile.test.cpp +++ b/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}},