Merge pull request #580 from vizzuhq/Fix-vertical-polar-lines

schaumb · web-flow · commit d4576d6872af · 2024-10-01T14:12:41.000+02:00
Fix vertical polar lines
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,6 +6,12 @@
 
 - Fix again measure axis labels when the axis range and step set too.
 - Fix drilldown regroup strategy on fake-split charts.
+- From now vertical line connections are curved lines.
+- Remove duplicated circles on line-circle transition.
+- Fix area-circle polar connection transition.
+- Fix line-rectangle polar connection linearity.
+- Fix all polar connection interpolation (except fading).
+- Remove unwanted line connections from line-circle + orientation changed anim.
 
 ## [0.13.0] - 2024-09-13
 
diff --git a/src/base/anim/interpolated.h b/src/base/anim/interpolated.h
@@ -132,6 +132,14 @@ template <typename Type> class Interpolated
 		return values[has_second && static_cast<bool>(index)];
 	}
 
+	[[nodiscard]] std::optional<InterpolateIndex> get_index(
+	    const Type &type) const
+	{
+		if (values[0].value == type) return first;
+		if (has_second && values[1].value == type) return second;
+		return {};
+	}
+
 	template <class T>
 	// NOLINTNEXTLINE(cppcoreguidelines-missing-std-forward)
 	[[nodiscard]] auto get(T &&) const = delete;
diff --git a/src/chart/generator/plotbuilder.cpp b/src/chart/generator/plotbuilder.cpp
@@ -184,9 +184,7 @@ PlotBuilder::sortedBuckets(const Buckets &buckets, bool main) const
 
 void PlotBuilder::addSpecLayout(Buckets &buckets)
 {
-	auto geometry = plot->getOptions()
-	                    ->geometry.get_or_first(::Anim::first)
-	                    .value;
+	auto geometry = plot->getOptions()->geometry.values[0].value;
 	if (auto &markers = plot->markers; isConnecting(geometry)) {
 		Charts::TableChart::setupVector(markers, true);
 	}
diff --git a/src/chart/main/style.h b/src/chart/main/style.h
@@ -106,9 +106,7 @@ struct Font
 	[[nodiscard]] std::string calculatedFamily() const
 	{
 		if (fontFamily.has_value())
-			if (auto &&ff =
-			        fontFamily->get_or_first(::Anim::first).value;
-			    !ff.empty())
+			if (auto &&ff = fontFamily->values[0].value; !ff.empty())
 				return ff;
 
 		if (fontParent) return fontParent->calculatedFamily();
diff --git a/src/chart/rendering/colorbuilder.cpp b/src/chart/rendering/colorbuilder.cpp
@@ -24,7 +24,7 @@ ColorBuilder::ColorBuilder(const LightnessRange &lightnessRange,
 Gfx::Color ColorBuilder::render(
     const Anim::Interpolated<Gen::ColorBase> &colorBase) const
 {
-	if (!colorBase.get_or_first(::Anim::first).value.isDiscrete()
+	if (!colorBase.values[0].value.isDiscrete()
 	    && !colorBase.get_or_first(::Anim::second)
 	            .value.isDiscrete()) {
 		auto pos = colorBase.combine(
diff --git a/src/chart/rendering/drawaxes.cpp b/src/chart/rendering/drawaxes.cpp
@@ -368,12 +368,11 @@ void DrawAxes::drawDimensionLabel(bool horizontal,
 			    draw(text.get_or_first(index), position.weight);
 		    if (!labelStyle.position->interpolates()
 		        && !text.interpolates())
-			    draw(text.get_or_first(::Anim::first));
+			    draw(text.values[0]);
 		    else if (labelStyle.position->interpolates())
-			    draw(text.get_or_first(::Anim::first),
-			        position.weight);
+			    draw(text.values[0], position.weight);
 		    else if (text.interpolates()) {
-			    draw(text.get_or_first(::Anim::first));
+			    draw(text.values[0]);
 			    draw(text.get_or_first(::Anim::second));
 		    }
 	    });
diff --git a/src/chart/rendering/drawmarkerinfo.cpp b/src/chart/rendering/drawmarkerinfo.cpp
@@ -195,8 +195,7 @@ void DrawMarkerInfo::draw(Gfx::ICanvas &canvas,
     const Geom::Rect &boundary) const
 {
 	for (const auto &info : plot->getMarkersInfo()) {
-		auto &&[cnt1, weight1] =
-		    info.second.get_or_first(::Anim::first);
+		auto &&[cnt1, weight1] = info.second.values[0];
 		if (!info.second.interpolates() && cnt1) {
 			MarkerDC dc(*this, canvas, boundary, cnt1);
 			dc.draw(weight1);
diff --git a/src/chart/rendering/markerrenderer.cpp b/src/chart/rendering/markerrenderer.cpp
@@ -110,20 +110,31 @@ void MarkerRenderer::drawMarkers(Gfx::ICanvas &canvas,
 
 			draw(canvas, painter, circle, 1, false);
 
-			blended.marker.prevMainMarker.visit(
-			    [this, &blended, &canvas, &painter](
-			        ::Anim::InterpolateIndex index,
-			        const auto &value)
-			    {
-				    draw(canvas,
-				        painter,
-				        ConnectingMarker{ctx(),
-				            blended.marker,
-				            index,
-				            Gen::ShapeType::line},
-				        value.weight,
-				        true);
-			    });
+			auto drawMarker = [this, &blended, &canvas, &painter](
+			                      ::Anim::InterpolateIndex index,
+			                      const auto &value)
+			{
+				draw(canvas,
+				    painter,
+				    ConnectingMarker{ctx(),
+				        blended.marker,
+				        index,
+				        Gen::ShapeType::line},
+				    value.weight,
+				    true);
+			};
+
+			if (blended.marker.prevMainMarker.interpolates())
+				blended.marker.prevMainMarker.visit(drawMarker);
+			else {
+				auto index =
+				    getOptions()
+				        .geometry.get_index(Gen::ShapeType::line)
+				        .value();
+				drawMarker(index,
+				    blended.marker.prevMainMarker.get_or_first(
+				        index));
+			}
 		}
 		else {
 			std::optional<AbstractMarker> other;
@@ -178,9 +189,7 @@ void MarkerRenderer::drawMarkers(Gfx::ICanvas &canvas,
 				if (containsSingle) {
 					auto lineIndex =
 					    isConnecting(
-					        getOptions()
-					            .geometry.get_or_first(::Anim::first)
-					            .value)
+					        getOptions().geometry.values[0].value)
 					        ? ::Anim::first
 					        : ::Anim::second;
 
@@ -195,9 +204,8 @@ void MarkerRenderer::drawMarkers(Gfx::ICanvas &canvas,
 			}
 			else
 				drawMarker(::Anim::first,
-				    ::Anim::Weighted{blended.marker.prevMainMarker
-				                         .get_or_first(::Anim::first)
-				                         .value,
+				    ::Anim::Weighted{
+				        blended.marker.prevMainMarker.values[0].value,
 				        sum_weight});
 		}
 	}
@@ -211,7 +219,7 @@ void MarkerRenderer::drawLabels(Gfx::ICanvas &canvas) const
 		if (blended.marker.enabled == false) continue;
 		drawLabel(canvas,
 		    blended,
-		    axis.unit.get_or_first(::Anim::first).value,
+		    axis.unit.values[0].value,
 		    keepMeasure,
 		    ::Anim::first);
 		drawLabel(canvas,
@@ -270,7 +278,11 @@ void MarkerRenderer::draw(Gfx::ICanvas &canvas,
 		            {Geom::Line(p0, p1), false}))) {
 			painter.drawStraightLine(line,
 			    abstractMarker.lineWidth,
-			    static_cast<double>(abstractMarker.linear));
+			    getOptions().coordSystem.factor(
+			        Gen::CoordSystem::cartesian)
+			            == 1.0
+			        ? 1.0
+			        : static_cast<double>(abstractMarker.linear));
 
 			renderedChart.emplace(
 			    Draw::Marker{abstractMarker.marker.enabled != false,
diff --git a/src/chart/rendering/markers/abstractmarker.cpp b/src/chart/rendering/markers/abstractmarker.cpp
@@ -67,8 +67,7 @@ AbstractMarker AbstractMarker::createInterpolated(
 {
 	const auto &options = ctx.getOptions();
 
-	auto fromShapeType =
-	    options.geometry.get_or_first(::Anim::first).value;
+	auto fromShapeType = options.geometry.values[0].value;
 
 	auto fromMarker = create(ctx, marker, fromShapeType, lineIndex);
 
@@ -84,13 +83,25 @@ AbstractMarker AbstractMarker::createInterpolated(
 	aMarker.labelEnabled =
 	    fromMarker.labelEnabled + toMarker.labelEnabled;
 
-	auto sum =
-	    static_cast<double>(fromMarker.enabled + toMarker.enabled);
+	auto enableFactorBase = static_cast<double>(toMarker.enabled);
+	auto labelEnableFactorBase =
+	    static_cast<double>(toMarker.labelEnabled);
+	auto enableSum = static_cast<double>(aMarker.enabled);
+	auto labelEnableSum = static_cast<double>(aMarker.labelEnabled);
+
+	if (marker.polarConnection.interpolates()) {
+		auto firstWeight = options.geometry.values[0].weight;
+		auto secondWeight = options.geometry.values[1].weight;
+		enableSum = firstWeight + secondWeight;
+		enableFactorBase = secondWeight;
+		labelEnableSum = firstWeight + secondWeight;
+		labelEnableFactorBase = secondWeight;
+	}
 
 	using Math::interpolate;
 	using Vizzu::Draw::interpolate;
-	if (sum > 0.0) {
-		auto factor = static_cast<double>(toMarker.enabled) / sum;
+	if (!Math::Floating::is_zero(enableSum)) {
+		auto factor = enableFactorBase / enableSum;
 		aMarker.morphToCircle = interpolate(fromMarker.morphToCircle,
 		    toMarker.morphToCircle,
 		    factor);
@@ -109,11 +120,8 @@ AbstractMarker AbstractMarker::createInterpolated(
 		aMarker.center =
 		    interpolate(fromMarker.center, toMarker.center, factor);
 	}
-	sum = static_cast<double>(
-	    fromMarker.labelEnabled + toMarker.labelEnabled);
-	if (sum > 0.0) {
-		auto factor =
-		    static_cast<double>(toMarker.labelEnabled) / sum;
+	if (!Math::Floating::is_zero(labelEnableSum)) {
+		auto factor = labelEnableFactorBase / labelEnableSum;
 		aMarker.dataRect = interpolate(fromMarker.dataRect,
 		    toMarker.dataRect,
 		    factor);
diff --git a/src/chart/rendering/markers/connectingmarker.cpp b/src/chart/rendering/markers/connectingmarker.cpp
@@ -29,13 +29,36 @@ ConnectingMarker::ConnectingMarker(const DrawingContext &ctx,
 	auto isLine = type == Gen::ShapeType::line;
 	auto isArea = type == Gen::ShapeType::area;
 
+	auto needConnection =
+	    !ctx.getOptions().geometry.interpolates()
+	    || isConnecting(
+	        ctx.getOptions().geometry.get_or_first(lineIndex).value);
+
+	auto otherNeedConnection = isConnecting(
+	    ctx.getOptions()
+	        .geometry
+	        .get_or_first(lineIndex == ::Anim::first ? ::Anim::second
+	                                                 : ::Anim::first)
+	        .value);
+
 	auto polar = ctx.getOptions().coordSystem.factor<Math::FuzzyBool>(
 	    Gen::CoordSystem::polar);
 	auto horizontal =
 	    ctx.getOptions().orientation.factor<Math::FuzzyBool>(
 	        Gen::Orientation::horizontal);
 
-	linear = !polar || horizontal;
+	auto &&isHorizontal =
+	    ctx.getOptions().orientation.get_or_first(lineIndex).value
+	    == Gen::Orientation::horizontal;
+
+	linear = !polar || horizontal
+	      || Math::FuzzyBool::And(
+	          !isHorizontal
+	              && ctx.plot->axises.at(Gen::Channel::Type::x)
+	                     .dimension.enabled,
+	          ctx.plot->axises.at(Gen::Channel::Type::x)
+	              .measure.enabled.factor(false),
+	          !horizontal);
 
 	lineWidth[0] = lineWidth[1] = 0;
 
@@ -49,7 +72,8 @@ ConnectingMarker::ConnectingMarker(const DrawingContext &ctx,
 	if (prev) {
 		enabled = labelEnabled && prev->enabled;
 		connected = Math::FuzzyBool::And(enabled,
-		    marker.prevMainMarker.get_or_first(lineIndex).weight);
+		    marker.prevMainMarker.get_or_first(lineIndex).weight,
+		    needConnection);
 		if (auto &&pc =
 		        marker.polarConnection.get_or_first(lineIndex);
 		    pc.value) {
@@ -59,6 +83,9 @@ ConnectingMarker::ConnectingMarker(const DrawingContext &ctx,
 			      || Math::FuzzyBool::Or(polar, pc.weight).more();
 			connected = connected && newPolar && horizontal;
 			enabled = enabled && newPolar && horizontal;
+
+			if (polar != false && prev != &marker)
+				linear = linear || Math::FuzzyBool{isHorizontal};
 		}
 	}
 	else {
@@ -92,13 +119,13 @@ ConnectingMarker::ConnectingMarker(const DrawingContext &ctx,
 			auto prevSpacing = prev->spacing * prev->size / 2;
 			auto prevPos = prev->position;
 
-			if (polar != false
-			    && ctx.getOptions()
-			               .orientation.get_or_first(lineIndex)
-			               .value
-			           == Gen::Orientation::horizontal
-			    && prev != &marker && prevPos.x >= 1) {
-				prevPos.x -= 1;
+			if (polar != false && isHorizontal && prev != &marker) {
+				if (!otherNeedConnection
+				    && marker.polarConnection.get_or_first(lineIndex)
+				           .value)
+					prevPos.x = 0;
+				else if (prevPos.x >= 1)
+					prevPos.x -= 1;
 			}
 
 			points[3] = prevPos - prevSpacing;
diff --git a/src/chart/rendering/markers/rectanglemarker.cpp b/src/chart/rendering/markers/rectanglemarker.cpp
@@ -21,6 +21,14 @@ RectangleMarker::RectangleMarker(const Gen::Marker &marker,
     SingleDrawMarker(marker, options, Gen::ShapeType::rectangle)
 {
 	linear = options.coordSystem.factor(Gen::CoordSystem::polar) == 0;
+
+	if (marker.polarConnection.interpolates()
+	    && options.geometry.contains(Gen::ShapeType::line)) {
+		linear = linear
+		      || !marker.polarConnection.factor<Math::FuzzyBool>(true)
+		              .less();
+	}
+
 	border = Math::FuzzyBool(true);
 
 	auto spacing = Geom::Size{
diff --git a/src/chart/rendering/painter/drawline.cpp b/src/chart/rendering/painter/drawline.cpp
@@ -8,6 +8,7 @@
 #include "base/math/interpolation.h"
 
 #include "coordinatesystem.h"
+#include "drawpolygon.h"
 #include "pathsampler.h"
 
 namespace Vizzu::Draw
@@ -22,15 +23,14 @@ DrawLine::DrawLine(const Geom::Line &line,
 
 DrawLine::DrawLine(const Geom::Line &line,
     std::array<double, 2> widths,
-    [[maybe_unused]] double straightFactor,
-    CoordinateSystem &coordSys,
+    const Options &options,
     Gfx::ICanvas &canvas)
 {
-	auto pBeg = coordSys.convert(line.begin);
-	auto pEnd = coordSys.convert(line.end);
+	auto pBeg = options.coordSys.convert(line.begin);
+	auto pEnd = options.coordSys.convert(line.end);
 
-	auto wBeg = widths[0] * coordSys.getRect().size.minSize();
-	auto wEnd = widths[1] * coordSys.getRect().size.minSize();
+	auto wBeg = widths[0] * options.coordSys.getRect().size.minSize();
+	auto wEnd = widths[1] * options.coordSys.getRect().size.minSize();
 
 	const auto &[p0, p1, p2, p3] =
 	    Geom::ConvexQuad::Isosceles(pBeg, pEnd, wBeg * 2, wEnd * 2)
@@ -41,12 +41,25 @@ DrawLine::DrawLine(const Geom::Line &line,
 	if (pBeg != pEnd) {
 		canvas.circle(Geom::Circle(pEnd, wEnd));
 
-		canvas.beginPolygon();
-		canvas.addPoint(p0);
-		canvas.addPoint(p1);
-		canvas.addPoint(p2);
-		canvas.addPoint(p3);
-		canvas.endPolygon();
+		if (options.straightFactor == 1.0) {
+			canvas.beginPolygon();
+			canvas.addPoint(p0);
+			canvas.addPoint(p1);
+			canvas.addPoint(p2);
+			canvas.addPoint(p3);
+			canvas.endPolygon();
+		}
+		else {
+			DrawPolygon({options.coordSys.getOriginal(p0),
+			                options.coordSys.getOriginal(p1),
+			                options.coordSys.getOriginal(p2),
+			                options.coordSys.getOriginal(p3)},
+			    {static_cast<const PathSampler::Options &>(options),
+			        {.toCircleFactor = 0,
+			            .straightFactor = options.straightFactor}},
+			    canvas,
+			    false);
+		}
 	}
 }
 
diff --git a/src/chart/rendering/painter/drawline.h b/src/chart/rendering/painter/drawline.h
diff --git a/src/chart/rendering/painter/painter.cpp b/src/chart/rendering/painter/painter.cpp
diff --git a/test/e2e/test_cases/test_cases.json b/test/e2e/test_cases/test_cases.json
diff --git a/test/e2e/tests/fixes.json b/test/e2e/tests/fixes.json

Original file line number	Diff line number	Diff line change
`@@ -184,9 +184,7 @@ PlotBuilder::sortedBuckets(const Buckets &buckets, bool main) const`
`184`	`184`
`185`	`185`	`void PlotBuilder::addSpecLayout(Buckets &buckets)`
`186`	`186`	`{`
`187`		`- auto geometry = plot->getOptions()`
`188`		`- ->geometry.get_or_first(::Anim::first)`
`189`		`- .value;`
	`187`	`+ auto geometry = plot->getOptions()->geometry.values[0].value;`
`190`	`188`	`if (auto &markers = plot->markers; isConnecting(geometry)) {`
`191`	`189`	`Charts::TableChart::setupVector(markers, true);`
`192`	`190`	`}`
Original file line number	Diff line number	Diff line change
`@@ -24,7 +24,7 @@ ColorBuilder::ColorBuilder(const LightnessRange &lightnessRange,`
`24`	`24`	`Gfx::Color ColorBuilder::render(`
`25`	`25`	`const Anim::Interpolated<Gen::ColorBase> &colorBase) const`
`26`	`26`	`{`
`27`		`- if (!colorBase.get_or_first(::Anim::first).value.isDiscrete()`
	`27`	`+ if (!colorBase.values[0].value.isDiscrete()`
`28`	`28`	`&& !colorBase.get_or_first(::Anim::second)`
`29`	`29`	`.value.isDiscrete()) {`
`30`	`30`	`auto pos = colorBase.combine(`