Fix nan sort

CHANGELOG.md

@@ -5,6 +5,7 @@
 ### Fixed
 
 - next() can be called multiple times from Plugin hooks
+- Fixed nan handling on non-axis charts.
 - Line and circle chats with only dimensions on x, and y axes the markers were off the axis labels.
 - Crash on TreeMap only with negative values
 - On dimension axis where no marker, print the dimension name as default title.

src/base/math/range.h

@@ -2,6 +2,7 @@
 #define MATH_RANGE
 
 #include <algorithm>
+#include <cmath>
 #include <limits>
 #include <string>
 
@@ -48,6 +49,7 @@ template <typename T> struct Range
 
 	void include(const T &value)
 	{
+		if (!std::isfinite(value)) return;
 		max = std::max(max, value);
 		min = std::min(min, value);
 	}

src/chart/generator/plot.cpp

@@ -181,7 +181,16 @@ Plot::sortedBuckets(const Buckets &buckets, bool main) const
 		}
 
 	if (main && options->sort == Sort::byValue)
-		std::sort(sorted.begin(), sorted.end());
+		std::sort(sorted.begin(),
+		    sorted.end(),
+		    [](const std::pair<double, std::size_t> &lhs,
+		        const std::pair<double, std::size_t> &rhs)
+		    {
+			    if (auto ord = std::weak_order(lhs.first, rhs.first);
+			        !std::is_eq(ord))
+				    return std::is_lt(ord);
+			    return lhs.second < rhs.second;
+		    });
 
 	if (main && options->reverse)
 		std::reverse(sorted.begin(), sorted.end());

src/chart/rendering/markers/connectingmarker.cpp

@@ -69,7 +69,7 @@ ConnectingMarker::ConnectingMarker(const DrawingContext &ctx,
 		    isLine ? *ctx.rootStyle.plot.marker.lineMaxWidth : 0;
 
 		lineWidth[1] =
-		    std::max(maxWidth * marker.sizeFactor, minWidth);
+		    std::max(minWidth, maxWidth * marker.sizeFactor);
 
 		auto horizontalFactor =
 		    isArea ? fabs(2 * static_cast<double>(horizontal) - 1)
@@ -96,10 +96,9 @@ ConnectingMarker::ConnectingMarker(const DrawingContext &ctx,
 
 			points[3] = prevPos - prevSpacing;
 
-			lineWidth[0] =
-			    isLine
-			        ? std::max(maxWidth * prev->sizeFactor, minWidth)
-			        : 0;
+			lineWidth[0] = isLine ? std::max(minWidth,
+			                   maxWidth * prev->sizeFactor)
+			                      : 0;
 
 			points[0] =
 			    prevPos - prevSpacing

src/chart/speclayout/bubblechart.cpp

@@ -11,9 +11,10 @@ BubbleChart::BubbleChart(const std::vector<double> &circleAreas,
 {
 	markers.reserve(circleAreas.size());
 
-	for (auto j = 0U; j < circleAreas.size(); ++j)
-		markers.emplace_back(j,
-		    std::sqrt(std::max(0.0, circleAreas[j])));
+	for (auto j = 0U; const auto &circleArea : circleAreas)
+		markers.emplace_back(j++,
+		    std::sqrt(std::abs(circleArea)),
+		    std::signbit(circleArea));
 
 	std::sort(markers.begin(), markers.end(), SpecMarker::sizeOrder);
 
@@ -25,101 +26,94 @@ BubbleChart::BubbleChart(const std::vector<double> &circleAreas,
 
 void BubbleChart::generate()
 {
-	auto baseIndex = 0U;
-
-	auto firstMarkerSize = markers.empty() ? 0.0 : markers[0].size();
-	for (auto i = 0U; i < markers.size(); ++i) {
-		auto &marker = markers[i];
-		auto markerSize = marker.size();
-
-		switch (i) {
-		case 0:
-			marker.emplaceCircle(Geom::Point{0, 0}, markerSize);
-			break;
-
-		case 1:
-			marker.emplaceCircle(
-			    Geom::Point{firstMarkerSize + markerSize, 0},
-			    markerSize);
-			break;
-
-		default:
-			if (markerSize == 0.0) {
-				marker.emplaceCircle(Geom::Point{0, 0}, 0);
-				continue;
-			}
-
-			auto candidate0 =
-			    getTouchingCircle(marker, baseIndex, i - 1);
-			auto candidate1 =
-			    getTouchingCircle(marker, baseIndex + 1, i - 1);
-
-			if (candidate1
-			    && !candidate1->overlaps(markers[baseIndex].circle(),
-			        0.00001)) {
-				marker.emplaceCircle(*candidate1);
-				++baseIndex;
-			}
-			else if (candidate0
-			         && !candidate0->overlaps(
-			             markers[baseIndex + 1].circle(),
-			             0.00001)) {
-				marker.emplaceCircle(*candidate0);
-			}
-			else
-				throw std::logic_error(
-				    "Cannot generate bubble chart");
-
-			break;
+	auto baseMarker = markers.begin();
+	while (baseMarker != markers.end()
+	       && !std::isfinite(baseMarker->size()))
+		baseMarker++->emplaceCircle(Geom::Point{0, 0}, 0);
+	while (baseMarker != markers.end() && baseMarker->negative)
+		baseMarker++->emplaceCircle(Geom::Point{0, 0}, 0);
+	if (baseMarker == markers.end()) return;
+
+	auto firstMarkerSize = baseMarker->size();
+	baseMarker->emplaceCircle(Geom::Point{0, 0}, firstMarkerSize);
+
+	auto currMarker = baseMarker + 1;
+	while (currMarker != markers.end() && currMarker->negative)
+		currMarker++->emplaceCircle(Geom::Point{0, 0}, 0);
+	if (currMarker == markers.end()) return;
+
+	auto markerSize = currMarker->size();
+	currMarker->emplaceCircle(
+	    Geom::Point{firstMarkerSize + markerSize, 0},
+	    markerSize);
+
+	for (auto preMarker = currMarker++; currMarker != markers.end();
+	     ++currMarker) {
+		if (currMarker->negative) {
+			currMarker->emplaceCircle(Geom::Point{0, 0}, 0);
+			continue;
 		}
+
+		markerSize = currMarker->size();
+		if (markerSize == 0.0) break;
+
+		if (auto &&candidate1 = getTouchingCircle(markerSize,
+		        baseMarker[1],
+		        *preMarker);
+		    candidate1
+		    && !candidate1->overlaps(baseMarker->circle(), 0.00001)) {
+			currMarker->emplaceCircle(*candidate1);
+			++baseMarker;
+		}
+		else if (auto &&candidate0 = getTouchingCircle(markerSize,
+		             *baseMarker,
+		             *preMarker);
+		         candidate0
+		         && !candidate0->overlaps(baseMarker[1].circle(),
+		             0.00001))
+			currMarker->emplaceCircle(*candidate0);
+		else
+			throw std::logic_error("Cannot generate bubble chart");
+		preMarker = currMarker;
 	}
+
+	while (currMarker != markers.end())
+		currMarker++->emplaceCircle(Geom::Point{0, 0}, 0);
 }
 
 void BubbleChart::normalize(const Geom::Rect &rect)
 {
 	if (markers.empty()) return;
 
-	Geom::Rect bound = markers[0].circle().boundary();
-
-	for (auto &marker : markers)
+	auto bound = markers[0].circle().boundary();
+	for (const auto &marker : markers)
 		bound = bound.boundary(marker.circle().boundary());
 
 	auto maxSize = std::max(bound.width(), bound.height());
-
-	auto center = rect.center();
-
-	for (auto &marker : markers) {
-		auto normCenter =
-		    center
-		    + rect.size * (marker.circle().center - bound.center())
-		          / maxSize;
-
-		auto normRadius =
-		    marker.circle().radius * rect.size.minSize() / maxSize;
-
-		marker.emplaceCircle(Geom::Circle(normCenter, normRadius));
-	}
+	auto &&cMul = rect.size / maxSize;
+	auto &&radMul = rect.size.minSize() / maxSize;
+	for (auto &&center = rect.center(); auto &marker : markers)
+		marker.emplaceCircle(
+		    center + cMul * (marker.circle().center - bound.center()),
+		    marker.circle().radius * radMul);
 }
 
 std::optional<Geom::Circle> BubbleChart::getTouchingCircle(
-    const SpecMarker &act,
-    size_t firstIdx,
-    size_t lastIdx) const
+    double newMarkerSize,
+    const SpecMarker &firstMarker,
+    const SpecMarker &lastMarker)
 {
-	if (firstIdx == lastIdx) return std::nullopt;
-
-	auto first = markers[firstIdx].circle();
-	auto last = markers[lastIdx].circle();
-
-	auto &&size = act.size();
-	first.radius += size;
-	last.radius += size;
+	if (&firstMarker == &lastMarker) return {};
+	auto first = firstMarker.circle();
+	auto last = lastMarker.circle();
 
-	auto newCenter = last.intersection(first)[0];
+	first.radius += newMarkerSize;
+	last.radius += newMarkerSize;
 
-	if (!newCenter) return std::nullopt;
+	if (const auto newCenter = last.intersection(first)[0])
+		return Geom::Circle(*newCenter, newMarkerSize);
 
-	return Geom::Circle(*newCenter, size);
+	return {};
 }
 
 }

src/chart/speclayout/bubblechart.h

@@ -27,10 +27,10 @@ class BubbleChart
 
 	void normalize(const Geom::Rect &rect);
 
-	[[nodiscard]] std::optional<Geom::Circle> getTouchingCircle(
-	    const SpecMarker &act,
-	    size_t firstIdx,
-	    size_t lastIdx) const;
+	[[nodiscard]] static std::optional<Geom::Circle>
+	getTouchingCircle(double newMarkerSize,
+	    const SpecMarker &firstMarker,
+	    const SpecMarker &lastMarker);
 };
 
 }

src/chart/speclayout/bubblechartbuilder.h

@@ -26,7 +26,9 @@ void BubbleChartBuilder::setupVector(double maxRadius,
 	std::vector<double> sizes(hierarchy.size());
 	for (std::size_t ix{}; const auto &level : hierarchy)
 		for (auto &sum = sizes[ix++]; const auto &item : level)
-			if (item->sizeFactor > 0) sum += item->sizeFactor;
+			if (auto &&size = item->sizeFactor;
+			    std::isfinite(size) && size > 0)
+				sum += size;
 
 	const BubbleChart chart(sizes);
 

src/chart/speclayout/specmarker.h

@@ -12,12 +12,16 @@ namespace Vizzu::Charts
 
 struct SpecMarker
 {
-	size_t index;
 	std::variant<double, Geom::Rect, Geom::Circle> sizeOrShape;
+	size_t index;
+	bool negative;
 
-	SpecMarker(size_t index, double radiusOrAreaFactorSize) :
+	SpecMarker(size_t index,
+	    double radiusOrAreaFactorSize,
+	    bool negative) :
+	    sizeOrShape(radiusOrAreaFactorSize),
 	    index(index),
-	    sizeOrShape(radiusOrAreaFactorSize)
+	    negative(negative)
 	{}
 
 	template <typename... T> void emplaceCircle(T &&...params)
@@ -51,7 +55,7 @@ struct SpecMarker
 
 	static bool sizeOrder(const SpecMarker &a, const SpecMarker &b)
 	{
-		return b.size() < a.size();
+		return std::is_lt(std::weak_order(b.size(), a.size()));
 	}
 };
 

src/chart/speclayout/treemap.cpp

@@ -14,18 +14,17 @@ TreeMap::TreeMap(const std::vector<double> &sizes,
 	markers.reserve(sizes.size());
 
 	for (auto j = 0U; j < sizes.size(); ++j)
-		markers.emplace_back(j, sizes[j]);
+		markers.emplace_back(j,
+		    std::abs(sizes[j]),
+		    std::signbit(sizes[j]));
 
 	std::ranges::sort(markers, SpecMarker::sizeOrder);
 
-	auto it = std::ranges::upper_bound(markers,
-	    SpecMarker{0, 0.0},
-	    SpecMarker::sizeOrder);
-
-	divide(markers.begin(), it, p0, p1);
-
-	while (it != markers.end()) it++->emplaceRect({0, 0}, {0, 0});
+	auto start = markers.begin();
+	while (start != markers.end() && !std::isfinite(start->size()))
+		start++->emplaceRect({0, 0}, {0, 0});
 
+	divide(start, markers.end(), p0, p1);
 	std::ranges::sort(markers, SpecMarker::indexOrder);
 }
 
@@ -35,13 +34,21 @@ void TreeMap::divide(It begin,
     const Geom::Point &p1,
     bool horizontal)
 {
+	while (begin != end && begin->negative)
+		begin++->emplaceRect({0, 0}, {0, 0});
+	while (begin != end && std::prev(end)->negative)
+		(--end)->emplaceRect({0, 0}, {0, 0});
+
+	if (begin == end) return;
+
 	if (begin + 1 == end) {
 		begin->emplaceRect(p0, p1);
 		return;
 	}
 
 	auto sum = 0.0;
-	for (auto it = begin; it != end; ++it) sum += it->size();
+	for (auto it = begin; it != end; ++it)
+		if (!it->negative) sum += it->size();
 
 	if (sum == 0) {
 		for (auto it = begin; it != end; ++it)
@@ -51,8 +58,12 @@ void TreeMap::divide(It begin,
 
 	double factor{};
 	auto it = begin;
-	while (it != end)
-		if (factor += it++->size() / sum; factor > 0.4) break;
+	for (; it != end; ++it)
+		if (!it->negative)
+			if (factor += it->size() / sum; factor > 0.4) {
+				++it;
+				break;
+			}
 
 	auto &&[px, py] = Math::interpolate(p0, p1, factor);
 

src/chart/speclayout/treemap.h

@@ -42,7 +42,9 @@ void TreeMap::setupVector(const Hierarchy &hierarchy)
 	std::vector<double> sizes(hierarchy.size());
 	for (std::size_t ix{}; const auto &level : hierarchy)
 		for (auto &sum = sizes[ix++]; const auto &item : level)
-			if (item->sizeFactor > 0) sum += item->sizeFactor;
+			if (auto &&size = item->sizeFactor;
+			    std::isfinite(size) && size > 0)
+				sum += size;
 
 	TreeMap chart(sizes);