From d572fc4c76558342e70598bf45844e7a5453cb43 Mon Sep 17 00:00:00 2001 From: Bela Schaum Date: Fri, 18 Oct 2024 21:20:42 +0200 Subject: [PATCH 1/2] Do not interpolate hiding/showing legend --- CHANGELOG.md | 2 +- src/base/refl/auto_enum.h | 11 +++--- src/base/refl/auto_struct.h | 13 ++++--- src/chart/animator/planner.cpp | 22 ++++-------- src/chart/animator/planner.h | 2 -- src/chart/generator/plotbuilder.cpp | 53 ++++++++++++++++++++--------- src/chart/main/events.h | 2 +- src/chart/options/options.cpp | 17 ++------- src/chart/options/options.h | 16 ++++++--- src/chart/rendering/drawchart.cpp | 2 +- src/chart/rendering/drawlegend.cpp | 11 +++--- src/chart/rendering/drawlegend.h | 2 +- test/e2e/test_cases/test_cases.json | 24 ++++++------- 13 files changed, 94 insertions(+), 83 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 563b55fdd..556d8f039 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,7 @@ - Markers are the same even if new record added. - Axis line hide/show at same time with axis labels/ticks/title. - Do not draw invisible axis line. - +- Do not interpolate hiding/showing legend ### Changed diff --git a/src/base/refl/auto_enum.h b/src/base/refl/auto_enum.h index 671529878..8e501c539 100644 --- a/src/base/refl/auto_enum.h +++ b/src/base/refl/auto_enum.h @@ -196,28 +196,31 @@ template constexpr E get_enum(const std::string_view &data) template struct EnumArray : std::array)> { + constexpr static auto first = Detail::from_to().first; using base_array = std::array)>; [[nodiscard]] constexpr V &operator[](E value) noexcept { return base_array::operator[]( - static_cast(value)); + static_cast(value) - first); } [[nodiscard]] constexpr const V &operator[]( E value) const noexcept { return base_array::operator[]( - static_cast(value)); + static_cast(value) - first); } [[nodiscard]] constexpr V &at(E value) { - return base_array::at(static_cast(value)); + return base_array::at( + static_cast(value) - first); } [[nodiscard]] constexpr const V &at(E value) const { - return base_array::at(static_cast(value)); + return base_array::at( + static_cast(value) - first); } bool operator==(const EnumArray &) const = default; diff --git a/src/base/refl/auto_struct.h b/src/base/refl/auto_struct.h index a3353f191..9d04f19af 100644 --- a/src/base/refl/auto_struct.h +++ b/src/base/refl/auto_struct.h @@ -593,6 +593,13 @@ template struct Applier [[maybe_unused]] Visitor &v) noexcept { if constexpr (!std::is_empty_v) { + constexpr auto members = + Members::get_member_functors(nullptr); + + static_assert( + std::tuple_size_v> > 0 + || std::tuple_size_v > 0, + "Unable to run reflection"); if constexpr (std::tuple_size_v> > 0) { std::invoke( [&v](std::tuple *) @@ -604,11 +611,7 @@ template struct Applier }, std::add_pointer_t>{}); } - - if constexpr (constexpr auto members = - Members::get_member_functors( - nullptr); - std::tuple_size_v > 0) { + if constexpr (std::tuple_size_v > 0) { std::apply( [&v](MF...) { diff --git a/src/chart/animator/planner.cpp b/src/chart/animator/planner.cpp index 741000361..a9251f3fc 100644 --- a/src/chart/animator/planner.cpp +++ b/src/chart/animator/planner.cpp @@ -351,12 +351,10 @@ bool Planner::positionMorphNeeded() const bool Planner::needColor() const { - return (isAnyLegend(Gen::ChannelId::color) - && source->axises.at(Gen::ChannelId::color) - != target->axises.at(Gen::ChannelId::color)) - || (isAnyLegend(Gen::ChannelId::lightness) - && source->axises.at(Gen::ChannelId::lightness) - != target->axises.at(Gen::ChannelId::lightness)) + return source->axises.at(Gen::ChannelId::color) + != target->axises.at(Gen::ChannelId::color) + || source->axises.at(Gen::ChannelId::lightness) + != target->axises.at(Gen::ChannelId::lightness) || anyMarker(+[](const Gen::Marker &source, const Gen::Marker &target) -> bool { @@ -406,9 +404,8 @@ bool Planner::needVertical() const != target->axises.at(Gen::AxisId::y) || source->guides.at(Gen::AxisId::y) != target->guides.at(Gen::AxisId::y) - || (isAnyLegend(Gen::ChannelId::size) - && source->axises.at(Gen::ChannelId::size) - != target->axises.at(Gen::ChannelId::size)) + || source->axises.at(Gen::ChannelId::size) + != target->axises.at(Gen::ChannelId::size) || (source->markerConnectionOrientation != target->markerConnectionOrientation && (source->markerConnectionOrientation.value_or( @@ -461,13 +458,6 @@ bool Planner::needHorizontal() const }); } -bool Planner::isAnyLegend(Gen::ChannelId type) const -{ - const auto &src = source->getOptions()->legend.get(); - const auto &trg = target->getOptions()->legend.get(); - return (src && *src == type) || (trg && *trg == type); -} - void Planner::addMorph(SectionId sectionId, ::Anim::Duration duration, ::Anim::Duration delay, diff --git a/src/chart/animator/planner.h b/src/chart/animator/planner.h index 28bb4d61b..a9c6b9e69 100644 --- a/src/chart/animator/planner.h +++ b/src/chart/animator/planner.h @@ -52,8 +52,6 @@ class Planner : public ::Anim::Group static size_t dimensionCount(const Gen::Plot *plot, Gen::AxisId type); - [[nodiscard]] bool isAnyLegend(Gen::ChannelId type) const; - ::Anim::Options getOptions(SectionId sectionId, ::Anim::Duration duration, ::Anim::Duration delay = ::Anim::Duration(0), diff --git a/src/chart/generator/plotbuilder.cpp b/src/chart/generator/plotbuilder.cpp index 37dc3dc41..976f66664 100644 --- a/src/chart/generator/plotbuilder.cpp +++ b/src/chart/generator/plotbuilder.cpp @@ -357,9 +357,12 @@ void PlotBuilder::normalizeXY() void PlotBuilder::calcMeasureAxises(const Data::DataTable &dataTable) { - for (const Channel &ch : - plot->getOptions()->getChannels().getChannels()) - calcMeasureAxis(dataTable, ch.type); + for (const ChannelId &ch : + {ChannelId::x, ChannelId::y, ChannelId::label}) + calcMeasureAxis(dataTable, ch); + + if (auto &&legend = plot->options->legend.get()) + calcMeasureAxis(dataTable, asChannel(*legend)); } void PlotBuilder::calcMeasureAxis(const Data::DataTable &dataTable, @@ -395,9 +398,11 @@ void PlotBuilder::calcMeasureAxis(const Data::DataTable &dataTable, void PlotBuilder::calcDimensionAxises() { - for (const Channel &ch : - plot->getOptions()->getChannels().getChannels()) - calcDimensionAxis(ch.type); + for (const ChannelId &ch : {ChannelId::x, ChannelId::y}) + calcDimensionAxis(ch); + + if (auto &&legend = plot->options->legend.get()) + calcDimensionAxis(asChannel(*legend)); } void PlotBuilder::calcDimensionAxis(ChannelId type) @@ -590,17 +595,31 @@ void PlotBuilder::normalizeColors() cbase.setPos(color.rescale(cbase.getPos())); } - plot->axises.at(ChannelId::color).measure.range = color; - plot->axises.at(ChannelId::lightness).measure.range = lightness; - - for (auto &value : plot->axises.at(ChannelId::color).dimension) - value.second.colorBase = - ColorBase(static_cast(value.second.value), 0.5); - - for (auto &value : - plot->axises.at(ChannelId::lightness).dimension) { - value.second.value = lightness.rescale(value.second.value); - value.second.colorBase = ColorBase(0U, value.second.value); + if (auto &&legend = plot->options->legend.get()) { + switch (*legend) { + case Options::LegendId::color: + plot->axises.at(ChannelId::color).measure.range = color; + + for (auto &value : + plot->axises.at(ChannelId::color).dimension) + value.second.colorBase = ColorBase( + static_cast(value.second.value), + 0.5); + break; + case Options::LegendId::lightness: + plot->axises.at(ChannelId::lightness).measure.range = + lightness; + + for (auto &value : + plot->axises.at(ChannelId::lightness).dimension) { + value.second.value = + lightness.rescale(value.second.value); + value.second.colorBase = + ColorBase(0U, value.second.value); + } + break; + default:; + } } } diff --git a/src/chart/main/events.h b/src/chart/main/events.h index 999f610d0..062b5b195 100644 --- a/src/chart/main/events.h +++ b/src/chart/main/events.h @@ -207,7 +207,7 @@ class Events struct LegendProperties { - Gen::ChannelId channel; + Gen::Options::LegendId channel; double scrollTop{}; double scrollHeight{}; }; diff --git a/src/chart/options/options.cpp b/src/chart/options/options.cpp index 013fe8fb7..bcc148294 100644 --- a/src/chart/options/options.cpp +++ b/src/chart/options/options.cpp @@ -27,12 +27,6 @@ ChannelExtrema operator"" _perc(long double percent) } } -[[nodiscard]] bool operator==(const Options::LegendId &l, - const ChannelId &c) -{ - return Options::toChannel(l) == c; -} - void Options::reset() { channels.reset(); @@ -283,14 +277,14 @@ std::optional Options::getAutoLegend() const series.erase(*id); for (auto channelId : {LegendId::color, LegendId::lightness}) - if (channels.at(toChannel(channelId)) + if (channels.at(asChannel(channelId)) .dimensions() .contains_any(series.begin(), series.end())) return channelId; for (auto channelId : {LegendId::color, LegendId::lightness, LegendId::size}) - if (auto &&mid = channels.at(toChannel(channelId)).measureId) + if (auto &&mid = channels.at(asChannel(channelId)).measureId) if (series.contains(*mid)) return channelId; return std::nullopt; @@ -341,7 +335,7 @@ bool Options::labelsShownFor(const Data::SeriesIndex &series) const return channels.at(AxisId::x).labelSeries() == series || channels.at(AxisId::y).labelSeries() == series || (legend.get() - && channels.at(toChannel(*legend.get())).labelSeries() + && channels.at(asChannel(*legend.get())).labelSeries() == series); } @@ -365,9 +359,4 @@ void Options::showTooltip(std::optional marker) } } -[[nodiscard]] ChannelId Options::toChannel(const LegendId &l) -{ - return static_cast(l); -} - } \ No newline at end of file diff --git a/src/chart/options/options.h b/src/chart/options/options.h index 241a1e690..88aaf14bf 100644 --- a/src/chart/options/options.h +++ b/src/chart/options/options.h @@ -52,10 +52,6 @@ class Options using Orientation = ::Anim::Interpolated; using MarkersInfoMap = std::map; - friend bool operator==(const LegendId &, const ChannelId &); - - [[nodiscard]] static ChannelId toChannel(const LegendId &); - Options() = default; [[nodiscard]] const Channels &getChannels() const @@ -193,6 +189,18 @@ class Options ChannelExtrema max); }; +[[nodiscard]] constexpr ChannelId asChannel( + const Options::LegendId &l) +{ + return static_cast(l); +} + +[[nodiscard]] constexpr bool operator==(const Options::LegendId &l, + const ChannelId &c) +{ + return asChannel(l) == c; +} + using PlotOptionsPtr = std::shared_ptr; } diff --git a/src/chart/rendering/drawchart.cpp b/src/chart/rendering/drawchart.cpp index 11f52e186..9ce990db0 100644 --- a/src/chart/rendering/drawchart.cpp +++ b/src/chart/rendering/drawchart.cpp @@ -50,7 +50,7 @@ void DrawChart::drawLegend(Gfx::ICanvas &canvas, if (legend.value) legendObj.draw(canvas, bounds, - Gen::Options::toChannel(*legend.value), + *legend.value, legend.weight); }); } diff --git a/src/chart/rendering/drawlegend.cpp b/src/chart/rendering/drawlegend.cpp index 970989a61..ad4369e18 100644 --- a/src/chart/rendering/drawlegend.cpp +++ b/src/chart/rendering/drawlegend.cpp @@ -31,7 +31,7 @@ namespace Vizzu::Draw void DrawLegend::draw(Gfx::ICanvas &canvas, const Geom::Rect &legendLayout, - Gen::ChannelId channelType, + Gen::Options::LegendId channelType, double weight) const { auto markerWindowRect = @@ -58,8 +58,9 @@ void DrawLegend::draw(Gfx::ICanvas &canvas, .weight = weight, .itemHeight = itemHeight, .markerSize = markerSize, - .measure = plot->axises.at(channelType).measure, - .dimension = plot->axises.at(channelType).dimension, + .measure = plot->axises.at(asChannel(channelType)).measure, + .dimension = + plot->axises.at(asChannel(channelType)).dimension, .properties = {.channel = channelType}, .fadeBarGradient = {markerWindowRect.leftSide(), {.line = {}, @@ -120,7 +121,7 @@ const Gfx::LinearGradient &DrawLegend::FadeBarGradient::operator()( void DrawLegend::drawTitle(const Info &info) const { - plot->axises.at(info.properties.channel) + plot->axises.at(asChannel(info.properties.channel)) .title.visit( [this, &info, @@ -286,7 +287,7 @@ void DrawLegend::drawMeasure(const Info &info) const auto bar = getBarRect(info); - using ST = Gen::ChannelId; + using ST = Gen::Options::LegendId; switch (info.properties.channel) { case ST::color: colorBar(info, bar); break; case ST::lightness: lightnessBar(info, bar); break; diff --git a/src/chart/rendering/drawlegend.h b/src/chart/rendering/drawlegend.h index 268b55303..027762c35 100644 --- a/src/chart/rendering/drawlegend.h +++ b/src/chart/rendering/drawlegend.h @@ -13,7 +13,7 @@ class DrawLegend : public DrawingContext public: void draw(Gfx::ICanvas &canvas, const Geom::Rect &legendLayout, - Gen::ChannelId channelType, + Gen::Options::LegendId channelType, double weight) const; const Events::DrawEvents::Legend &events = rootEvents.draw.legend; diff --git a/test/e2e/test_cases/test_cases.json b/test/e2e/test_cases/test_cases.json index a8a07116c..0896aa3d1 100644 --- a/test/e2e/test_cases/test_cases.json +++ b/test/e2e/test_cases/test_cases.json @@ -2,19 +2,19 @@ "suite": "/test/e2e/test_cases", "test": { "basic_animations/anim_order/circle_without_2_carte_horizontal": { - "refs": ["3d44d8e"] + "refs": ["cbdfce7"] }, "basic_animations/anim_order/circle_without_2_carte_vertical": { - "refs": ["c5c0df5"] + "refs": ["8d79239"] }, "basic_animations/anim_order/rectangle_without_2_carte_bar": { - "refs": ["2a487b0"] + "refs": ["f55a535"] }, "basic_animations/anim_order/rectangle_without_2_carte_column": { - "refs": ["e88e517"] + "refs": ["6b48769"] }, "basic_animations/anim_order/rectangle_without_2_polar_bar": { - "refs": ["06f37e3"] + "refs": ["d4f69ca"] }, "basic_animations/anim_order/rectangle_without_2_polar_column": { "refs": ["6ed4a66"] @@ -62,7 +62,7 @@ "refs": ["64caf45"] }, "basic_animations/legend_transitions/color_conti_changes_anim": { - "refs": ["7872a7f"] + "refs": ["e13376e"] }, "basic_animations/legend_transitions/color_conti_discrete_anim": { "refs": ["c209ca4"] @@ -71,7 +71,7 @@ "refs": ["0ebaa76"] }, "basic_animations/legend_transitions/color_discrete_changes_anim": { - "refs": ["c36beaa"] + "refs": ["133ff31"] }, "basic_animations/legend_transitions/color_off_on_anim": { "refs": ["55f74b0"] @@ -2180,7 +2180,7 @@ "refs": ["db6403b"] }, "ww_noFade/wNoFade_Tests/1_des_pol/circle/05a_cir_1c": { - "refs": ["216fee7"] + "refs": ["8f7bbda"] }, "ww_noFade/wNoFade_Tests/1_des_pol/circle/05b_cir_1c": { "refs": ["8c9bec9"] @@ -2537,7 +2537,7 @@ "refs": ["c5f9b0c"] }, "ww_noFade/wNoFade_cases/1_des_pol/circle/05a_cir_1c": { - "refs": ["0627807"] + "refs": ["1d179cc"] }, "ww_noFade/wNoFade_cases/1_des_pol/circle/05b_cir_1c": { "refs": ["edf0ce6"] @@ -2567,7 +2567,7 @@ "refs": ["6ee8c09"] }, "ww_noFade/wNoFade_cases/1_des_pol/line/05b_lin": { - "refs": ["a0a7cec"] + "refs": ["b391702"] }, "ww_noFade/wNoFade_cases/1_des_pol/line/06a_lin": { "refs": ["8813bdf"] @@ -2831,10 +2831,10 @@ "refs": ["1e1175c"] }, "ww_noFade/wNoFade_cases/2_des_pol-without/line_V1/05a_d-w_lin_V1": { - "refs": ["bcbbb71"] + "refs": ["53af9c5"] }, "ww_noFade/wNoFade_cases/2_des_pol-without/line_V1/05b_d-w_lin_V1": { - "refs": ["6788f87"] + "refs": ["1589dde"] }, "ww_noFade/wNoFade_cases/2_des_pol-without/line_V1/06a_d-w_lin_V1": { "refs": ["f9998fb"] From d16cfdc83e83c9a0fb3c97efb8492319e2fe4ada Mon Sep 17 00:00:00 2001 From: Bela Schaum Date: Fri, 18 Oct 2024 21:36:14 +0200 Subject: [PATCH 2/2] Fix clang tidy + changelog --- CHANGELOG.md | 2 +- src/chart/rendering/drawlegend.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 556d8f039..e15b6fc9d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,6 @@ ### Fixed -- Removed 'min' align property from the API which equivalent with the 'none'. - Markers are the same even if new record added. - Axis line hide/show at same time with axis labels/ticks/title. - Do not draw invisible axis line. @@ -12,6 +11,7 @@ ### Changed +- Removed 'min' align property from the API which equivalent with the 'none'. - Changed MarkerId to be a string instead of a number. diff --git a/src/chart/rendering/drawlegend.cpp b/src/chart/rendering/drawlegend.cpp index ad4369e18..45ea3cb62 100644 --- a/src/chart/rendering/drawlegend.cpp +++ b/src/chart/rendering/drawlegend.cpp @@ -21,7 +21,7 @@ #include "base/text/smartstring.h" #include "chart/generator/plot.h" // NOLINT(misc-include-cleaner) #include "chart/main/events.h" -#include "chart/options/channel.h" +#include "chart/options/options.h" #include "chart/rendering/colorbuilder.h" #include "chart/rendering/drawbackground.h" #include "chart/rendering/drawlabel.h"