From e40756df54073c53d63f731d8e826024f6f08f9b Mon Sep 17 00:00:00 2001 From: Bela Schaum Date: Thu, 30 May 2024 22:11:28 +0200 Subject: [PATCH] self-review --- CHANGELOG.md | 1 + src/chart/animator/morph.cpp | 4 +- src/chart/animator/planner.cpp | 3 +- src/chart/generator/marker.cpp | 12 ++---- src/chart/generator/marker.h | 5 +-- src/chart/generator/plotbuilder.cpp | 6 +-- .../rendering/markers/connectingmarker.cpp | 5 +-- test/e2e/tests/fixes.json | 3 ++ test/e2e/tests/fixes/536.mjs | 37 +++++++++++++++++++ 9 files changed, 57 insertions(+), 19 deletions(-) create mode 100644 test/e2e/tests/fixes/536.mjs diff --git a/CHANGELOG.md b/CHANGELOG.md index 003187198..70d111528 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ - Add missing canvas change function in htmlcanvas plugin. - On split charts the first range was not part of the separation calculation. - When the first marker was disabled it was calculated as an enabled marker on the XY normalization. +- Sorted or reversed marker connections behaved chaotic at coordinate system change. ## [0.11.0] - 2024-05-23 diff --git a/src/chart/animator/morph.cpp b/src/chart/animator/morph.cpp index 40d318eba..79818f598 100644 --- a/src/chart/animator/morph.cpp +++ b/src/chart/animator/morph.cpp @@ -176,7 +176,9 @@ void Connection::transform(const Marker &source, target.prevMainMarkerIdx, factor); - actual.mainId = interpolate(source.mainId, target.mainId, factor); + actual.polarConnection = interpolate(source.polarConnection, + target.polarConnection, + factor); } void Vertical::transform(const Gen::Plot &source, diff --git a/src/chart/animator/planner.cpp b/src/chart/animator/planner.cpp index beb36471e..42a0789c4 100644 --- a/src/chart/animator/planner.cpp +++ b/src/chart/animator/planner.cpp @@ -292,7 +292,8 @@ void Planner::calcNeeded() { return source.prevMainMarkerIdx != target.prevMainMarkerIdx - || source.mainId != target.mainId; + || source.polarConnection + != target.polarConnection; }) || srcOpt->isHorizontal() != trgOpt->isHorizontal(); } diff --git a/src/chart/generator/marker.cpp b/src/chart/generator/marker.cpp index 8491fc8d1..1e1057a96 100644 --- a/src/chart/generator/marker.cpp +++ b/src/chart/generator/marker.cpp @@ -73,7 +73,7 @@ Marker::Marker(const Options &options, data, stats, index, - horizontal ? &mainId->value : subAxisId); + horizontal ? &mainId : subAxisId); auto yChannelRectDim = channels.at(ChannelId::y).isDimension() @@ -93,7 +93,7 @@ Marker::Marker(const Options &options, data, stats, index, - !horizontal ? &mainId->value : subAxisId); + !horizontal ? &mainId : subAxisId); auto xChannelRectDim = channels.at(ChannelId::x).isDimension() @@ -127,8 +127,6 @@ void Marker::setNextMarker(bool first, bool horizontal, bool main) { - (main ? nextMainMarkerIdx : nextSubMarkerIdx) = marker.idx; - if (main) marker.prevMainMarkerIdx = idx; if (!first) { @@ -136,6 +134,8 @@ void Marker::setNextMarker(bool first, horizontal ? &Geom::Point::x : &Geom::Point::y; marker.position.*coord += position.*coord; } + else if (main && this != &marker) + marker.polarConnection = true; } void Marker::resetSize(bool horizontal) @@ -150,10 +150,6 @@ void Marker::setIdOffset(size_t offset) { if (prevMainMarkerIdx.hasOneValue()) prevMainMarkerIdx->value += offset; - if (nextMainMarkerIdx.hasOneValue()) - nextMainMarkerIdx->value += offset; - if (nextSubMarkerIdx.hasOneValue()) - nextSubMarkerIdx->value += offset; } Conv::JSONObj &&Marker::appendToJSON(Conv::JSONObj &&jsonObj) const diff --git a/src/chart/generator/marker.h b/src/chart/generator/marker.h index a175f1d61..2c13510fc 100644 --- a/src/chart/generator/marker.h +++ b/src/chart/generator/marker.h @@ -54,14 +54,13 @@ class Marker using Id = Data::MarkerId; - ::Anim::Interpolated mainId; + Id mainId; Id subId; Id sizeId; MarkerIndex idx; ::Anim::Interpolated prevMainMarkerIdx; - ::Anim::Interpolated nextMainMarkerIdx; - ::Anim::Interpolated nextSubMarkerIdx; + ::Anim::Interpolated polarConnection; void setNextMarker(bool first, Marker &marker, diff --git a/src/chart/generator/plotbuilder.cpp b/src/chart/generator/plotbuilder.cpp index 019262037..6976f6c78 100644 --- a/src/chart/generator/plotbuilder.cpp +++ b/src/chart/generator/plotbuilder.cpp @@ -97,8 +97,8 @@ Buckets PlotBuilder::generateMarkers(std::size_t &mainBucketSize) markerId, needInfo); - mainBuckets[marker.mainId.get().seriesId] - [marker.mainId.get().itemId] = ▮ + mainBuckets[marker.mainId.seriesId][marker.mainId.itemId] = + ▮ subBuckets[marker.subId.seriesId][marker.subId.itemId] = ▮ @@ -339,7 +339,7 @@ void PlotBuilder::calcDimensionAxis(ChannelId type) const auto &id = (type == ChannelId::x) == plot->getOptions()->isHorizontal() - ? marker.mainId.get() + ? marker.mainId : marker.subId; if (const auto &slice = id.label) diff --git a/src/chart/rendering/markers/connectingmarker.cpp b/src/chart/rendering/markers/connectingmarker.cpp index 903b8f479..2baa3846f 100644 --- a/src/chart/rendering/markers/connectingmarker.cpp +++ b/src/chart/rendering/markers/connectingmarker.cpp @@ -43,9 +43,8 @@ ConnectingMarker::ConnectingMarker(const DrawingContext &ctx, enabled && (marker.enabled || prev->enabled); connected = connected && (prev->enabled || marker.enabled); - if (prev->mainId.get_or_first(lineIndex).value.itemId - > marker.mainId.get_or_first(lineIndex) - .value.itemId) { + if (marker.polarConnection.get_or_first(lineIndex) + .value) { linear = linear || polar.more(); connected = connected && polar.more() && horizontal; enabled = enabled && polar && horizontal; diff --git a/test/e2e/tests/fixes.json b/test/e2e/tests/fixes.json index 7f13e0c15..e9581bde4 100644 --- a/test/e2e/tests/fixes.json +++ b/test/e2e/tests/fixes.json @@ -31,6 +31,9 @@ "530": { "refs": ["1ad4684"] }, + "536": { + "refs": ["05d387c"] + }, "32303048": { "refs": ["b5d95ea"] }, diff --git a/test/e2e/tests/fixes/536.mjs b/test/e2e/tests/fixes/536.mjs new file mode 100644 index 000000000..7ece60622 --- /dev/null +++ b/test/e2e/tests/fixes/536.mjs @@ -0,0 +1,37 @@ +const testSteps = [ + (chart) => + chart.animate({ + data: { + series: [ + { name: 'Foo', values: ['Alice', 'Bob', 'Ted', 'Jonas'] }, + { name: 'Bar', values: [15, 32, 12, 55] } + ] + } + }), + (chart) => + chart.animate({ + channels: { + x: 'Foo', + y: 'Bar' + }, + geometry: 'area', + reverse: true + }), + + (chart) => + chart.animate({ + coordSystem: 'polar' + }), + (chart) => + chart.animate({ + coordSystem: 'cartesian', + geometry: 'line', + sort: 'byValue' + }), + (chart) => + chart.animate({ + reverse: false + }) +] + +export default testSteps