From 0675962ed77e024f478b1fd9788efd661f63fcab Mon Sep 17 00:00:00 2001
From: Erik Frojdh <erik.frojdh@gmail.com>
Date: Fri, 5 Jun 2026 14:04:14 +0200
Subject: [PATCH] added bounds check

---
 include/aare/hist/PixelHistogramImpl.hpp    |  6 ++
 src/hist/PedestalTrackingPixelHistogram.cpp |  2 +-
 src/hist/PixelHistogramImpl.test.cpp        | 71 ++++++++++++++++++++-
 3 files changed, 77 insertions(+), 2 deletions(-)

diff --git a/include/aare/hist/PixelHistogramImpl.hpp b/include/aare/hist/PixelHistogramImpl.hpp
index d0bb39c..f324eaa 100644
--- a/include/aare/hist/PixelHistogramImpl.hpp
+++ b/include/aare/hist/PixelHistogramImpl.hpp
@@ -11,6 +11,7 @@ silently dropped.
 #include "aare/NDView.hpp"
 
 #include <cstddef>
+#include <limits>
 #include <stdexcept>
 #include <type_traits>
 
@@ -106,6 +107,11 @@ void PixelHistogramImpl<T, StorageType>::fill_unchecked(int row, int col,
     if (bin >= m_n_bins) {
         bin = m_n_bins - 1;
     }
+    if constexpr (std::is_integral_v<StorageType>) {
+        if (m_values(row, col, bin) >= std::numeric_limits<StorageType>::max()) {
+            return;
+        }
+    }
     ++m_values(row, col, bin);
 }
 
diff --git a/src/hist/PedestalTrackingPixelHistogram.cpp b/src/hist/PedestalTrackingPixelHistogram.cpp
index 129fc17..6684fd0 100644
--- a/src/hist/PedestalTrackingPixelHistogram.cpp
+++ b/src/hist/PedestalTrackingPixelHistogram.cpp
@@ -18,7 +18,7 @@ PedestalTrackingPixelHistogram::PedestalTrackingPixelHistogram(
     : rows_(rows), cols_(cols), n_threads_(n_threads), xmin_(xmin), xmax_(xmax),
       current_work_kind_(WorkKind::FillWithThreshold), current_image_(nullptr),
       current_images_(nullptr), completed_threads_(0), stop_workers_(false),
-      work_generation_(0), max_batch_size_(max_pending / 2), n_sigma_(n_sigma) {
+      work_generation_(0), max_batch_size_(max_pending/3), n_sigma_(n_sigma) {
     if (rows_ < 1 || cols_ < 1 || n_bins < 1) {
         throw std::invalid_argument("PedestalTrackingPixelHistogram requires "
                                     "positive rows, cols and bins");
diff --git a/src/hist/PixelHistogramImpl.test.cpp b/src/hist/PixelHistogramImpl.test.cpp
index 52a3e7e..d2fc88b 100644
--- a/src/hist/PixelHistogramImpl.test.cpp
+++ b/src/hist/PixelHistogramImpl.test.cpp
@@ -70,4 +70,73 @@ TEST_CASE("Fill a small histogram from an NDArray") {
     REQUIRE(v(0, 2, 2) == 1);
     REQUIRE(v(1, 0, 3) == 1);
     REQUIRE(v(1, 1, 4) == 1);
-}
\ No newline at end of file
+}
+
+TEST_CASE("Check that pixel histogram does not overflow"){
+    int rows = 1;
+    int cols = 1;
+    int n_bins = 10;
+    double xmin = 0.0;
+    double xmax = 1.0;
+    aare::PixelHistogramImpl<double, uint8_t> hist_u8(rows, cols, n_bins, xmin,
+                                                   xmax);
+
+    for (int i = 0; i < 255; ++i) {
+        hist_u8.fill(0, 0, 0.05);
+    }
+
+    auto v0 = hist_u8.view();
+    REQUIRE(v0(0, 0, 0) == 255);
+
+    hist_u8.fill(0, 0, 0.05);
+    REQUIRE(v0(0, 0, 0) == 255);
+
+    aare::PixelHistogramImpl<double, int8_t> hist_i8(rows, cols, n_bins, xmin,
+                                                   xmax);
+
+    for (int i = 0; i < 350; ++i) {
+        hist_i8.fill(0, 0, 0.05);
+    }
+
+    auto v1 = hist_i8.view();
+    REQUIRE(v1(0, 0, 0) == 127);
+
+}
+
+TEST_CASE("Check that values outside the range do not affect the histogram") {
+    int rows = 1;
+    int cols = 1;
+    int n_bins = 10;
+    double xmin = 0.0;
+    double xmax = 1.0;
+    aare::PixelHistogramImpl<double, double> hist(rows, cols, n_bins, xmin,
+                                                   xmax);
+    hist.fill(0, 0, -0.1); // below range
+    hist.fill(0, 0, 1.0);  // at upper edge (should be out of range)
+    hist.fill(0, 0, 1.1);  // above range
+   
+    auto v = hist.view();
+    auto total = std::accumulate(v.begin(), v.end(), 0);
+    REQUIRE(total == 0);
+
+}
+
+TEST_CASE("Check that row and column bounds are checked") {
+    int rows = 1;
+    int cols = 1;
+    int n_bins = 10;
+    double xmin = 0.0;
+    double xmax = 1.0;
+    aare::PixelHistogramImpl<double, double> hist(rows, cols, n_bins, xmin,
+                                                   xmax);
+    REQUIRE_THROWS_AS(hist.fill(0, 1, -0.1), std::out_of_range); // col out of range
+    REQUIRE_THROWS_AS(hist.fill(1, 0, 1.0), std::out_of_range);  // row out of range
+    REQUIRE_THROWS_AS(hist.fill(58, -1, 1.1), std::out_of_range);  // both out of range
+
+    auto v = hist.view();
+    auto total = std::accumulate(v.begin(), v.end(), 0);
+    REQUIRE(total == 0);
+
+}
+
+