From 651676edf0c9f210cac94e863ee4bcab808d17f4 Mon Sep 17 00:00:00 2001
From: sz <sz@recv.cc>
Date: Wed, 24 Jul 2024 01:25:01 -0500
Subject: [PATCH] Fix long-standing bug in Cell::mean_rgb_continuous()

-- relevant now that things aren't square.
---
 src/lib/cimb_translator/Cell.h            | 10 +++---
 src/lib/cimb_translator/test/CellTest.cpp | 39 ++++++++++++++++++++---
 2 files changed, 40 insertions(+), 9 deletions(-)

diff --git a/src/lib/cimb_translator/Cell.h b/src/lib/cimb_translator/Cell.h
index 3ce65a7a..e2654185 100644
--- a/src/lib/cimb_translator/Cell.h
+++ b/src/lib/cimb_translator/Cell.h
@@ -39,20 +39,20 @@ class Cell
 		const uchar* p = _img.ptr<uchar>(0) + (index * channels);
 
 		int increment = 1 + skip;
-		int toNextCol = channels * (_img.rows - _rows);
+		int toNextRow = channels * (_img.cols - _cols);
 		if (skip)
-			toNextCol += channels * _img.rows;
+			toNextRow += channels * _img.cols;
 
-		for (int i = 0; i < _cols; i+=increment)
+		for (int i = 0; i < _rows; i+=increment)
 		{
-			for (int j = 0; j < _rows; ++j, ++count)
+			for (int j = 0; j < _cols; ++j, ++count)
 			{
 				red += p[0];
 				green += p[1];
 				blue += p[2];
 				p += channels;
 			}
-			p += toNextCol;
+			p += toNextRow;
 		}
 
 		if (!count)
diff --git a/src/lib/cimb_translator/test/CellTest.cpp b/src/lib/cimb_translator/test/CellTest.cpp
index 98689105..e33e54b3 100644
--- a/src/lib/cimb_translator/test/CellTest.cpp
+++ b/src/lib/cimb_translator/test/CellTest.cpp
@@ -108,21 +108,52 @@ TEST_CASE( "CellTest/testRgbCellOffsets.Asymmetric.Contiguous", "[unit]" )
 {
 	cv::Mat img = TestCimbar::loadSample("6bit/4color_ecc30_fountain_0.png");
 
-	cv::Rect crop(125, 8, 4, 6);
+	cv::Rect crop(126, 9, 6, 6);
 	cv::Mat cell = img(crop);
+	cv::Scalar expectedColor = cv::mean(cell);
 
-	auto [r, g, b] = Cell(img, 125, 8, 4, 6).mean_rgb();
+	auto [r, g, b] = Cell(img, 126, 9, 6, 6).mean_rgb();
 
 	DYNAMIC_SECTION( "r" )
 	{
-		assertEquals( 191, (int)r );
+		assertAlmostEquals( expectedColor[0], (int)r );
+		assertEquals( 198, (int)r );
 	}
 	DYNAMIC_SECTION( "g" )
 	{
-		assertEquals( 191, (int)g );
+		assertAlmostEquals( expectedColor[1], (int)g );
+		assertEquals( 198, (int)g );
 	}
 	DYNAMIC_SECTION( "b" )
 	{
+		assertAlmostEquals( expectedColor[2], (int)b );
 		assertEquals( 0, (int)b );
 	}
 }
+
+TEST_CASE( "CellTest/testRgbCellOffsets.WideCanvas", "[unit]" )
+{
+	cv::Mat img = TestCimbar::loadSample("bm/ecc35.png");
+
+	cv::Rect crop(127, 10, 6, 6);
+	cv::Mat cell = img(crop);
+	cv::Scalar expectedColor = cv::mean(cell);
+
+	auto [r, g, b] = Cell(img, 127, 10, 6, 6).mean_rgb();
+
+	DYNAMIC_SECTION( "r" )
+	{
+		assertAlmostEquals( expectedColor[0], (int)r );
+		assertEquals( 148, (int)r );
+	}
+	DYNAMIC_SECTION( "g" )
+	{
+		assertAlmostEquals( expectedColor[1], (int)g );
+		assertEquals( 0, (int)g );
+	}
+	DYNAMIC_SECTION( "b" )
+	{
+		assertAlmostEquals( expectedColor[2], (int)b );
+		assertEquals( 148, (int)b );
+	}
+}