diff --git a/CHANGELOG.md b/CHANGELOG.md index 536635d1e..b042d4cb1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,12 @@ - From now, it is prohibited to set only aggregator without series name. - Fix series parsing when aggregator comes first. - Fix disappearing dimension when aggregated dimension was already set. +- Fix legend with multiple dimension duplicated markers: + - Markers of color are never merged. + - Markers of size are always merged. + - Markers of lightness are only merged when labelLevel == 0. + - When merge happens, the marker shows the middle value of lightness. +- Remove rare fantom empty marker space on scrollable legend. ## [0.15.0] - 2024-10-28 diff --git a/src/chart/generator/axis.h b/src/chart/generator/axis.h index b92e6e518..806535567 100644 --- a/src/chart/generator/axis.h +++ b/src/chart/generator/axis.h @@ -125,7 +125,7 @@ struct DimensionAxis bool add(const Data::SliceIndex &index, double value, const Math::Range &range, - bool merge = true); + bool merge); [[nodiscard]] bool operator==(const DimensionAxis &other) const; [[nodiscard]] auto begin() diff --git a/src/chart/generator/marker.cpp b/src/chart/generator/marker.cpp index 06479113f..ffef50a1b 100644 --- a/src/chart/generator/marker.cpp +++ b/src/chart/generator/marker.cpp @@ -141,8 +141,6 @@ bool Marker::connectMarkers(bool first, next->polarConnection = polarConnection && first; return true; } - if (next && main) - next->prevMainMarker = RelativeMarkerIndex{next->idx, {}}; return false; } diff --git a/src/chart/generator/marker.h b/src/chart/generator/marker.h index 8c0adf9a6..f91e97bb3 100644 --- a/src/chart/generator/marker.h +++ b/src/chart/generator/marker.h @@ -72,7 +72,8 @@ class Marker return lhs.idx == rhs.idx; } }; - ::Anim::Interpolated prevMainMarker; + ::Anim::Interpolated prevMainMarker{ + RelativeMarkerIndex{idx, {}}}; ::Anim::Interpolated polarConnection{false}; static bool connectMarkers(bool first, diff --git a/src/chart/generator/plotbuilder.cpp b/src/chart/generator/plotbuilder.cpp index fc04a2299..1d4179d9c 100644 --- a/src/chart/generator/plotbuilder.cpp +++ b/src/chart/generator/plotbuilder.cpp @@ -47,21 +47,24 @@ PlotBuilder::PlotBuilder(const Data::DataTable &dataTable, std::size_t mainBucketSize{}; auto &&subBuckets = generateMarkers(mainBucketSize); - if (!plot->options->getChannels().anyAxisSet()) { + if (!plot->options->getChannels().anyAxisSet()) addSpecLayout(subBuckets); - calcLegendAndLabel(dataTable); - normalizeColors(); - if (plot->options->geometry != ShapeType::circle) - normalizeSizes(); - } - else { - addSeparation(subBuckets, mainBucketSize); - calcAxises(dataTable); - calcLegendAndLabel(dataTable); - normalizeSizes(); - normalizeColors(); - addAlignment(subBuckets); - } + else + addAxisLayout(subBuckets, mainBucketSize, dataTable); + + calcLegendAndLabel(dataTable); + normalizeColors(); + normalizeSizes(); +} + +void PlotBuilder::addAxisLayout(Buckets &subBuckets, + const std::size_t &mainBucketSize, + const Data::DataTable &dataTable) +{ + linkMarkers(subBuckets); + addSeparation(subBuckets, mainBucketSize); + calcAxises(dataTable); + addAlignment(subBuckets); } void PlotBuilder::initDimensionTrackers() @@ -103,25 +106,7 @@ Buckets PlotBuilder::generateMarkers(std::size_t &mainBucketSize) plot->markersInfo.insert({first->second, Plot::MarkerInfo{Plot::MarkerInfoContent{marker}}}); - Buckets buckets(plot->markers); - auto &&hasMarkerConnection = - linkMarkers(buckets.sort(&Marker::mainId), true); - std::ignore = linkMarkers(buckets.sort(&Marker::subId), false); - - if (hasMarkerConnection - && plot->getOptions()->geometry.get() == ShapeType::line - && plot->getOptions() - ->getChannels() - .at(AxisId::x) - .isDimension() - && plot->getOptions() - ->getChannels() - .at(AxisId::y) - .isDimension()) { - plot->markerConnectionOrientation.emplace( - *plot->getOptions()->orientation.get()); - } - return buckets; + return Buckets{plot->markers}; } std::vector @@ -137,7 +122,7 @@ PlotBuilder::sortedBuckets(const Buckets &buckets, bool main) const auto it = std::ranges::lower_bound(sorted, idx.itemId, std::less{}, - std::mem_fn(&BucketInfo::index)); + &BucketInfo::index); if (it == sorted.end() || it->index != idx.itemId) it = sorted.emplace(it, idx.itemId, 0.0); @@ -182,6 +167,27 @@ void PlotBuilder::addSpecLayout(Buckets &buckets) } } +void PlotBuilder::linkMarkers(Buckets &buckets) +{ + auto &&hasMarkerConnection = + linkMarkers(buckets.sort(&Marker::mainId), true); + std::ignore = linkMarkers(buckets.sort(&Marker::subId), false); + + if (hasMarkerConnection + && plot->getOptions()->geometry.get() == ShapeType::line + && plot->getOptions() + ->getChannels() + .at(AxisId::x) + .isDimension() + && plot->getOptions() + ->getChannels() + .at(AxisId::y) + .isDimension()) { + plot->markerConnectionOrientation.emplace( + *plot->getOptions()->orientation.get()); + } +} + bool PlotBuilder::linkMarkers(const Buckets &buckets, bool main) const { auto &&sorted = sortedBuckets(buckets, main); @@ -212,19 +218,15 @@ bool PlotBuilder::linkMarkers(const Buckets &buckets, bool main) const for (std::size_t ix{}, max = sorted.size(); ix < max; ++ix) { auto &o = dimOffset[ix]; for (const auto &bucket : buckets) { + auto &&ids = std::ranges::views::values(bucket); auto sIx = sorted[ix].index; - auto it = std::ranges::lower_bound(bucket, + auto it = std::ranges::lower_bound(ids, sIx, std::less{}, - [](const std::pair &p) - { - return p.second.itemId; - }); - if (it == bucket.end() || (*it).second.itemId != sIx) - continue; - - auto &marker = (*it).first; + &Data::MarkerId::itemId); + if (it == ids.end() || (*it).itemId != sIx) continue; + + auto &marker = **it.base().base().base(); if (!marker.enabled) continue; o = std::max(o, marker.size.*coord, @@ -253,33 +255,24 @@ bool PlotBuilder::linkMarkers(const Buckets &buckets, bool main) const double prevPos{}; for (auto i = 0U; i < sorted.size(); ++i) { auto idAct = sorted[i].index; - auto it = std::ranges::lower_bound(bucket, + auto &&ids = std::ranges::views::values(bucket); + auto it = std::ranges::lower_bound(ids, idAct, std::less{}, - [](const std::pair - &p) - { - return p.second.itemId; - }); - Marker *act = - it == bucket.end() || (*it).second.itemId != idAct - ? nullptr - : &(*it).first; + &Data::MarkerId::itemId); + Marker *act = it == ids.end() || (*it).itemId != idAct + ? nullptr + : *it.base().base().base(); auto iNext = (i + 1) % sorted.size(); auto idNext = sorted[iNext].index; - it = std::ranges::lower_bound(bucket, + it = std::ranges::lower_bound(ids, idNext, std::less{}, - [](const std::pair - &p) - { - return p.second.itemId; - }); - Marker *next = - it == bucket.end() || (*it).second.itemId != idNext - ? nullptr - : &(*it).first; + &Data::MarkerId::itemId); + Marker *next = it == ids.end() || (*it).itemId != idNext + ? nullptr + : *it.base().base().base(); if (act) prevPos = act->position.*coord += @@ -394,8 +387,8 @@ void PlotBuilder::calcAxis(const Data::DataTable &dataTable, T type) } else { constexpr bool isTypeAxis = std::is_same_v; - if constexpr (isTypeAxis) { - auto merge = scale.labelLevel == 0; + if constexpr (auto merge = scale.labelLevel == 0; + isTypeAxis) { for (const auto &marker : plot->markers) { if (!marker.enabled) continue; @@ -415,13 +408,17 @@ void PlotBuilder::calcAxis(const Data::DataTable &dataTable, T type) else { const auto &indices = std::get<1>(stats.at(type)); - double count = 0; - for (auto i = 0U; i < indices.size(); ++i) + double count{}; + for (std::size_t i{}; i < indices.size(); ++i) if (const auto &sliceIndex = indices[i]; sliceIndex && axis.dimension.add(*sliceIndex, - i, - {count, count})) + count, + {static_cast(i), + static_cast(i)}, + type == LegendId::size + || (type == LegendId::lightness + && merge))) count += 1; } auto hasLabel = axis.dimension.setLabels( @@ -519,6 +516,10 @@ void PlotBuilder::addSeparation(const Buckets &subBuckets, void PlotBuilder::normalizeSizes() { + if (plot->getOptions()->geometry == ShapeType::circle + && !plot->options->getChannels().anyAxisSet()) + return; + if (plot->getOptions()->geometry == ShapeType::circle || plot->getOptions()->geometry == ShapeType::line) { Math::Range size; @@ -584,8 +585,9 @@ void PlotBuilder::normalizeColors() for (auto &item : plot->axises.at(LegendId::color).dimension) - item.colorBase = - ColorBase(static_cast(item.value), 0.5); + item.colorBase = ColorBase( + static_cast(item.range.middle()), + 0.5); break; case LegendId::lightness: plot->axises.at(LegendId::lightness).measure.range = @@ -593,8 +595,10 @@ void PlotBuilder::normalizeColors() for (auto &item : plot->axises.at(LegendId::lightness).dimension) { - item.value = lightness.rescale(item.value); - item.colorBase = ColorBase(0U, item.value); + item.range = Math::Range::Raw( + lightness.rescale(item.range.getMin()), + lightness.rescale(item.range.getMax())); + item.colorBase = ColorBase(0U, item.range.middle()); } break; default:; diff --git a/src/chart/generator/plotbuilder.h b/src/chart/generator/plotbuilder.h index 2b0243833..8c0554852 100644 --- a/src/chart/generator/plotbuilder.h +++ b/src/chart/generator/plotbuilder.h @@ -35,6 +35,7 @@ class PlotBuilder void initDimensionTrackers(); Buckets generateMarkers(std::size_t &mainBucketSize); + void linkMarkers(Buckets &subBuckets); [[nodiscard]] bool linkMarkers(const Buckets &buckets, bool main) const; void calcAxises(const Data::DataTable &dataTable); @@ -49,6 +50,9 @@ class PlotBuilder [[nodiscard]] std::vector sortedBuckets(const Buckets &buckets, bool main) const; void addSpecLayout(Buckets &buckets); + void addAxisLayout(Buckets &subBuckets, + const std::size_t &mainBucketSize, + const Data::DataTable &dataTable); }; } diff --git a/src/chart/options/autoparam.h b/src/chart/options/autoparam.h index 6175a01cd..545e3c9ec 100644 --- a/src/chart/options/autoparam.h +++ b/src/chart/options/autoparam.h @@ -76,6 +76,8 @@ template struct AutoParam bool operator==(const AutoParam &other) const = default; + const std::optional &getValueOrAuto() { return value; } + private: bool autoSet{}; std::optional value; diff --git a/src/chart/options/channel.cpp b/src/chart/options/channel.cpp index 66ffd4b3e..4672ad75f 100644 --- a/src/chart/options/channel.cpp +++ b/src/chart/options/channel.cpp @@ -154,16 +154,15 @@ bool Channel::isSeriesUsed(const Data::SeriesIndex &index) const void Channel::reset() { - set.measureId = std::nullopt; - set.dimensionIds.clear(); - title = Base::AutoParam{}; + set = {}; + labelLevel = {}; + title = {}; axis = Base::AutoBool(); labels = Base::AutoBool(); ticks = Base::AutoBool(); - interlacing = Base::AutoBool(); guides = Base::AutoBool(); markerGuides = Base::AutoBool(); - labelLevel = 0; + interlacing = Base::AutoBool(); } bool Channel::isEmpty() const diff --git a/src/chart/rendering/drawlegend.cpp b/src/chart/rendering/drawlegend.cpp index f8edac165..ade0621ec 100644 --- a/src/chart/rendering/drawlegend.cpp +++ b/src/chart/rendering/drawlegend.cpp @@ -153,7 +153,7 @@ void DrawLegend::drawDimension(Info &info) const for (const auto &item : info.dimension) { if (item.weight <= 0) continue; - auto itemRect = getItemRect(info, item.range.getMin()); + auto itemRect = getItemRect(info, item.value); if (itemRect.y().getMin() > info.markerWindowRect.y().getMax() || itemRect.y().getMax() @@ -335,8 +335,8 @@ Math::Range DrawLegend::markersLegendRange(const Info &info) if (info.dimensionEnabled) for (const auto &item : info.dimension) - res.include({item.range.getMin() * info.itemHeight, - (item.range.getMax() + 1) * info.itemHeight}); + res.include({item.value * info.itemHeight, + (item.value + 1) * info.itemHeight}); return res; } diff --git a/src/chart/rendering/drawmarkerinfo.cpp b/src/chart/rendering/drawmarkerinfo.cpp index 8b45dc0ea..cd9d02da3 100644 --- a/src/chart/rendering/drawmarkerinfo.cpp +++ b/src/chart/rendering/drawmarkerinfo.cpp @@ -82,17 +82,14 @@ void DrawMarkerInfo::MarkerDC::interpolate(double weight1, void DrawMarkerInfo::MarkerDC::loadMarker(Content &cnt) { const auto &marker = - *std::lower_bound(parent.plot->getMarkers().begin(), - parent.plot->getMarkers().end(), + // NOLINTNEXTLINE(misc-include-cleaner) + *std::ranges::lower_bound(parent.plot->getMarkers(), cnt.markerId.value(), - [](const Gen::Marker &marker, - const Gen::Marker::MarkerIndex &id) - { - return marker.idx < id; - }); + std::less{}, + &Gen::Marker::idx); auto blendedMarker = - Draw::AbstractMarker::createInterpolated(parent.ctx(), + AbstractMarker::createInterpolated(parent.ctx(), marker, ::Anim::first); diff --git a/src/chart/speclayout/sizedependentlayout.h b/src/chart/speclayout/sizedependentlayout.h index 971177d27..6c43f6438 100644 --- a/src/chart/speclayout/sizedependentlayout.h +++ b/src/chart/speclayout/sizedependentlayout.h @@ -18,20 +18,18 @@ template struct SizeDependentLayout std::vector sizes; for (const auto &level : hierarchy) for (auto &sum = sizes.emplace_back(); - const auto &item : level) { - if (auto &&size = item.first.sizeFactor; + const auto &item : level.base()) + if (auto &&size = item->sizeFactor; std::isfinite(size) && size > 0) sum += size; - } const ChartType chart(sizes); for (auto it = chart.markers.data(); const auto &level : hierarchy) { std::vector ssizes(level.size()); - for (std::size_t ix{}; const auto &item : level) { - ssizes[ix++] = item.first.sizeFactor; - } + for (std::size_t ix{}; const auto &item : level.base()) + ssizes[ix++] = item->sizeFactor; const ChartType subChart(ssizes, it++); diff --git a/test/e2e/test_cases/test_cases.json b/test/e2e/test_cases/test_cases.json index 0896aa3d1..86b9f69bc 100644 --- a/test/e2e/test_cases/test_cases.json +++ b/test/e2e/test_cases/test_cases.json @@ -56,7 +56,7 @@ "refs": ["10761b9"] }, "basic_animations/legend_transitions/color_2discrete_anim": { - "refs": ["9c5a5fb"] + "refs": ["4fb1ac9"] }, "basic_animations/legend_transitions/color_conti_anim": { "refs": ["64caf45"] @@ -80,7 +80,7 @@ "refs": ["bdd39d3"] }, "basic_animations/legend_transitions/lightness_2discrete_anim": { - "refs": ["7a83537"] + "refs": ["39bdb26"] }, "basic_animations/legend_transitions/lightness_conti_anim": { "refs": ["02085ea"] @@ -299,7 +299,7 @@ "refs": ["6c42aad"] }, "operations/orientation_tutorial_data/line_orientation": { - "refs": ["c6ddbab"] + "refs": ["1777877"] }, "operations/orientation_tutorial_data/rectangle_orientation": { "refs": ["4c39423"]