From aeef0ed2f20d97416ab29e06b67f08d68750b106 Mon Sep 17 00:00:00 2001
From: Bela Schaum <schaumb@gmail.com>
Date: Wed, 12 Jun 2024 23:37:56 +0200
Subject: [PATCH] Fix treemap->tablemap

---
 CHANGELOG.md                        |  7 ++++
 src/chart/generator/plotbuilder.cpp |  2 +-
 src/chart/speclayout/tablechart.h   | 21 +++++++++---
 test/e2e/test_cases/test_cases.json |  2 +-
 test/e2e/tests/fixes.json           |  3 ++
 test/e2e/tests/fixes/540.mjs        | 53 +++++++++++++++++++++++++++++
 6 files changed, 81 insertions(+), 7 deletions(-)
 create mode 100644 test/e2e/tests/fixes/540.mjs

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 4bca7357b..816663f37 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,13 @@
 
 ## [Unreleased]
 
+### Fixed
+
+- Only dimension on size (+ color) wrongly displayed as treemap, not tablechart.
+- When markers size is factorisable to 2 numbers product on a tablechart, and the multipliers ratio is less than 16/9,
+  the area fully filled with equal marker rectangles.
+
+
 ## [0.11.1] - 2024-05-31
 
 ### Fixed
diff --git a/src/chart/generator/plotbuilder.cpp b/src/chart/generator/plotbuilder.cpp
index 6976f6c78..0f2adb87b 100644
--- a/src/chart/generator/plotbuilder.cpp
+++ b/src/chart/generator/plotbuilder.cpp
@@ -184,7 +184,7 @@ void PlotBuilder::addSpecLayout(Buckets &buckets)
 	}
 	else if (auto &&size = plot->getOptions()->getChannels().at(
 	             ChannelId::size);
-	         size.isEmpty()) {
+	         size.isDimension()) {
 		Charts::TableChart::setupVector(markers);
 	}
 	else if (!dataCube.empty()) {
diff --git a/src/chart/speclayout/tablechart.h b/src/chart/speclayout/tablechart.h
index 3ec37237b..2dc9ef365 100644
--- a/src/chart/speclayout/tablechart.h
+++ b/src/chart/speclayout/tablechart.h
@@ -18,14 +18,25 @@ template <typename Item>
 void TableChart::setupVector(std::vector<Item> &items,
     bool singleColumn)
 {
-	if (items.empty()) return;
-
 	auto size = 0;
 	for (auto &item : items)
 		if (item.enabled) ++size;
 
-	auto rowsize =
-	    singleColumn ? 1 : static_cast<ssize_t>(ceil(sqrt(size)));
+	if (size == 0) return;
+
+	ssize_t rowsize{1};
+	if (!singleColumn) {
+		rowsize = static_cast<ssize_t>(ceil(sqrt(size)));
+		auto sq = static_cast<ssize_t>(ceil(sqrt(
+		    16.0 * static_cast<double>(rowsize * rowsize) / 9.0)));
+
+		auto rem = rowsize - 1 - (size - 1) % rowsize;
+		for (auto i = rowsize + 1; rem > 0 && i < sq; ++i)
+			if (auto newRem = i - 1 - (size - 1) % i; newRem < rem) {
+				rowsize = i;
+				rem = newRem;
+			}
+	}
 	auto colsize = ceil(
 	    static_cast<double>(size) / static_cast<double>(rowsize));
 
@@ -37,7 +48,7 @@ void TableChart::setupVector(std::vector<Item> &items,
 		if (item.enabled) {
 			item.spacing = {1, 1};
 			auto div = std::div(cnt++, rowsize);
-			item.position = {(1.0 + static_cast<double>(div.rem))
+			item.position = {static_cast<double>(div.rem + 1)
 			                     / static_cast<double>(rowsize),
 			    1.0 - static_cast<double>(div.quot) / colsize};
 			item.size = markerSize;
diff --git a/test/e2e/test_cases/test_cases.json b/test/e2e/test_cases/test_cases.json
index 89af671d8..763de0c36 100644
--- a/test/e2e/test_cases/test_cases.json
+++ b/test/e2e/test_cases/test_cases.json
@@ -2930,7 +2930,7 @@
 			"refs": ["ad12fd0"]
 		},
 		"ww_noFade/wNoFade_cases/2_des_pol-without/rectangle_Ve1/07a_d-w_rec_Ve2_1c": {
-			"refs": ["0b50210"]
+			"refs": ["1fb2de3"]
 		},
 		"ww_noFade/wNoFade_cases/2_des_pol-without/rectangle_Ve1/07a_d-w_rec_Ve2_2c": {
 			"refs": ["fe75086"]
diff --git a/test/e2e/tests/fixes.json b/test/e2e/tests/fixes.json
index e9581bde4..aa94517e6 100644
--- a/test/e2e/tests/fixes.json
+++ b/test/e2e/tests/fixes.json
@@ -34,6 +34,9 @@
 		"536": {
 			"refs": ["05d387c"]
 		},
+		"540": {
+			"refs": ["108b737"]
+		},
 		"32303048": {
 			"refs": ["b5d95ea"]
 		},
diff --git a/test/e2e/tests/fixes/540.mjs b/test/e2e/tests/fixes/540.mjs
new file mode 100644
index 000000000..3cc1cbdd0
--- /dev/null
+++ b/test/e2e/tests/fixes/540.mjs
@@ -0,0 +1,53 @@
+const testSteps = [
+	(chart) =>
+		chart.animate({
+			data: {
+				series: [
+					{ name: 'Foo', type: 'dimension' },
+					{ name: 'Bar', type: 'dimension' }
+				]
+			},
+			config: {
+				size: ['Foo']
+			},
+			style: {
+				plot: {
+					marker: {
+						rectangleSpacing: 0
+					}
+				}
+			}
+		})
+]
+
+testSteps.push(
+	...[...Array(36).keys()].map(
+		(i) => (chart) =>
+			chart.animate(
+				{
+					data: {
+						records: [[`Foo${i}`, `${i}`]]
+					}
+				},
+				0.1
+			)
+	)
+)
+
+testSteps.push((chart) => chart.animate({ config: { color: 'Bar', legend: null } }, 0.5))
+
+testSteps.push(
+	...[...Array(36).keys()].map(
+		(i) => (chart) =>
+			chart.animate(
+				{
+					data: {
+						filter: (record) => parseInt(record.Bar) > i
+					}
+				},
+				0.1
+			)
+	)
+)
+
+export default testSteps