Review

Author: schaumb

src/chart/generator/marker.cpp

@@ -12,8 +12,7 @@ Marker::Marker(const Options &options,
     ChannelsStats &stats,
     const Data::MultiIndex &index,
     MarkerIndex idx,
-    bool needMarkerInfo,
-    bool rectangleSpacing) :
+    bool needMarkerInfo) :
     enabled(data.empty() || !index.isEmpty()),
     cellInfo(enabled || needMarkerInfo
                  ? data.cellInfo(index, idx, needMarkerInfo)
@@ -76,16 +75,18 @@ Marker::Marker(const Options &options,
 	    index,
 	    horizontal ? &mainId->value : subAxisId);
 
-	spacing.x = (horizontal || (lineOrCircle && !polar)
-	                || (channels.at(ChannelId::y).isDimension()
-	                    && channels.at(ChannelId::y).hasDimension()
-	                    && options.geometry == ShapeType::rectangle
-	                    && options.align != Base::Align::Type::stretch
-	                    && rectangleSpacing))
-	                 && options.getChannels().anyAxisSet()
-	                 && channels.at(ChannelId::x).isDimension()
-	              ? 1
-	              : 0;
+	auto yChannelRectDim =
+	    channels.at(ChannelId::y).isDimension()
+	    && channels.at(ChannelId::y).hasDimension()
+	    && options.geometry == ShapeType::rectangle
+	    && options.align != Base::Align::Type::stretch;
+
+	spacing.x =
+	    (horizontal || (lineOrCircle && !polar) || yChannelRectDim)
+	            && options.getChannels().anyAxisSet()
+	            && channels.at(ChannelId::x).isDimension()
+	        ? 1
+	        : 0;
 
 	position.y = size.y = getValueForChannel(channels,
 	    ChannelId::y,
@@ -94,12 +95,13 @@ Marker::Marker(const Options &options,
 	    index,
 	    !horizontal ? &mainId->value : subAxisId);
 
-	spacing.y = (!horizontal || lineOrCircle
-	                || (channels.at(ChannelId::x).isDimension()
-	                    && channels.at(ChannelId::x).hasDimension()
-	                    && options.geometry == ShapeType::rectangle
-	                    && options.align != Base::Align::Type::stretch
-	                    && rectangleSpacing))
+	auto xChannelRectDim =
+	    channels.at(ChannelId::x).isDimension()
+	    && channels.at(ChannelId::x).hasDimension()
+	    && options.geometry == ShapeType::rectangle
+	    && options.align != Base::Align::Type::stretch;
+
+	spacing.y = (!horizontal || lineOrCircle || xChannelRectDim)
 	                 && options.getChannels().anyAxisSet()
 	                 && channels.at(ChannelId::y).isDimension()
 	              ? 1

src/chart/generator/marker.h

@@ -29,8 +29,7 @@ class Marker
 	    ChannelsStats &stats,
 	    const Data::MultiIndex &index,
 	    MarkerIndex idx,
-	    bool needMarkerInfo,
-	    bool rectangleSpacing);
+	    bool needMarkerInfo);
 
 	::Anim::Interpolated<ColorBase> colorBase;
 	Geom::Point position;

src/chart/generator/plot.cpp

@@ -129,11 +129,6 @@ Buckets Plot::generateMarkers()
 	for (auto &&[ix, mid] : options->markersInfo)
 		map.emplace(mid, ix);
 
-	bool rectangleSpacing{true};
-	if (auto &&sp = style.plot.marker.rectangleSpacing;
-	    sp && sp->get().has_value())
-		rectangleSpacing = *sp->get() > 0.0;
-
 	for (auto first = map.begin(), last = map.end();
 	     auto &&index : getDataCube()) {
 		auto &&markerId = markers.size();
@@ -144,8 +139,7 @@ Buckets Plot::generateMarkers()
 		    getStats(),
 		    index,
 		    markerId,
-		    needInfo,
-		    rectangleSpacing);
+		    needInfo);
 
 		mainBuckets[marker.mainId.get().seriesId]
 		           [marker.mainId.get().itemId] = ▮

test/e2e/test_cases/test_cases.json

@@ -1775,7 +1775,7 @@
 			"refs": ["2352b28"]
 		},
 		"ww_next_steps/next_steps/38_C_L_line": {
-			"refs": ["b8ffd23"]
+			"refs": ["d03e139"]
 		},
 		"ww_next_steps/next_steps_Tests/02_C_R": {
 			"refs": ["c550e7b"]
@@ -1802,7 +1802,7 @@
 			"refs": ["feed828"]
 		},
 		"ww_next_steps/next_steps_Tests/38_C_L_line": {
-			"refs": ["a19aa52"]
+			"refs": ["3b273c7"]
 		},
 		"ww_next_steps/next_steps_Tests/axisLabel_problem": {
 			"refs": ["d33f180"]
@@ -2624,7 +2624,7 @@
 			"refs": ["9f2a6ba"]
 		},
 		"ww_noFade/wNoFade_cases/1_des_pol/rectangle_Ve1/04a_rec_Ve1_1c": {
-			"refs": ["aa58cd6"]
+			"refs": ["cbc5606"]
 		},
 		"ww_noFade/wNoFade_cases/1_des_pol/rectangle_Ve1/04a_rec_Ve1_2c": {
 			"refs": ["40b7f79"]
@@ -2633,7 +2633,7 @@
 			"refs": ["819fdce"]
 		},
 		"ww_noFade/wNoFade_cases/1_des_pol/rectangle_Ve1/06a_rec_Ve1_1c": {
-			"refs": ["21ba8df"]
+			"refs": ["5586c4d"]
 		},
 		"ww_noFade/wNoFade_cases/1_des_pol/rectangle_Ve1/07a_rec_Ve1_2c": {
 			"refs": ["0d9d4eb"]
@@ -2888,7 +2888,7 @@
 			"refs": ["0e97ae5"]
 		},
 		"ww_noFade/wNoFade_cases/2_des_pol-without/rectangle_V1/04a_d-w_rec_Ve1_1c_V1": {
-			"refs": ["7164b5b"]
+			"refs": ["537c8d4"]
 		},
 		"ww_noFade/wNoFade_cases/2_des_pol-without/rectangle_V1/04a_d-w_rec_Ve1_2c_V1": {
 			"refs": ["fe2656a"]
@@ -2909,7 +2909,7 @@
 			"refs": ["7008537"]
 		},
 		"ww_noFade/wNoFade_cases/2_des_pol-without/rectangle_Ve1/04a_d-w_rec_Ve1_1c": {
-			"refs": ["f9144aa"]
+			"refs": ["a1d4c96"]
 		},
 		"ww_noFade/wNoFade_cases/2_des_pol-without/rectangle_Ve1/04a_d-w_rec_Ve1_2c": {
 			"refs": ["3e15bfc"]
@@ -2918,7 +2918,7 @@
 			"refs": ["990dc60"]
 		},
 		"ww_noFade/wNoFade_cases/2_des_pol-without/rectangle_Ve1/06a_d-w_rec_Ve1_1c": {
-			"refs": ["a685430"]
+			"refs": ["c6fbce3"]
 		},
 		"ww_noFade/wNoFade_cases/2_des_pol-without/rectangle_Ve1/06a_d-w_rec_Ve1_2c": {
 			"refs": ["ac1ed71"]
@@ -2930,7 +2930,7 @@
 			"refs": ["27d694a"]
 		},
 		"ww_noFade/wNoFade_cases/2_des_pol-without/rectangle_Ve1/07a_d-w_rec_Ve2_1c": {
-			"refs": ["bcc4e6f"]
+			"refs": ["0b50210"]
 		},
 		"ww_noFade/wNoFade_cases/2_des_pol-without/rectangle_Ve1/07a_d-w_rec_Ve2_2c": {
 			"refs": ["fe75086"]