Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Axis refactor v12 - Separate AxisChannel properties from Channel properties #613

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,12 @@

- Fix invalid read/write when animation is contiguous (onFinish callback calls setKeyframe).
- Waterfall chart preset not aligned.
- Split chart count negative values too.
- Split chart count negative values too.

### Changed

- Separate Channel properties to AxisChannel properties at config.
- Channels 'set' rewrite doesn't clear AxisChannel properties.

## [0.16.0] - 2024-11-28

Expand Down
14 changes: 12 additions & 2 deletions src/apps/weblib/typeschema-api/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,12 @@ definitions:
oneOf:
- type: number
- { type: string, enum: [auto] }

AxisChannel:
description: |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Channel props can be inherited:

        $extends: Channel

AxisChannels are special channel settings for the positional channels.
type: object
properties:
axis:
description: Enables the axis line on axis channels.
$ref: AutoBool
Expand Down Expand Up @@ -109,14 +115,18 @@ definitions:
type: object
properties:
x:
$ref: Channel
allOf:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need for allOf if AxisChannel inherits Channel properties

- $ref: Channel
- $ref: AxisChannel
description: |
Parameters for the X-axis, determining the position of the markers on the
x-axis - or their angle when using polar coordinates.
Note: leaving x and y channels empty will result in a
chart "without coordinates" like a Treemap or a Bubble Chart.
y:
$ref: Channel
allOf:
- $ref: Channel
- $ref: AxisChannel
description: |
Parameters for the Y-axis, determining the position of the markers on the
y-axis - or their radius when using polar coordinates) .
Expand Down
13 changes: 8 additions & 5 deletions src/chart/generator/guides.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,19 @@ bool GuidesByAxis::operator==(const GuidesByAxis &other) const

Guides::Guides(const Options &options)
{
const auto &xOpt = options.getChannels().at(AxisId::x);
const auto &yOpt = options.getChannels().at(AxisId::y);
if (xOpt.isEmpty() && yOpt.isEmpty()) return;
const auto &xChannel = options.getChannels().at(AxisId::x);
const auto &yChannel = options.getChannels().at(AxisId::y);
if (xChannel.isEmpty() && yChannel.isEmpty()) return;

auto xIsMeasure = options.isMeasure(ChannelId::x);
auto yIsMeasure = options.isMeasure(ChannelId::y);
auto isPolar = options.coordSystem.get() == CoordSystem::polar;
auto isCircle = options.geometry.get() == ShapeType::circle;
auto isHorizontal = options.isHorizontal();

const auto &xOpt = options.getChannels().axisPropsAt(AxisId::x);
const auto &yOpt = options.getChannels().axisPropsAt(AxisId::y);

x.axis = xOpt.axis.getValue(yIsMeasure);
y.axis = yOpt.axis.getValue(xIsMeasure && !isPolar);

Expand Down Expand Up @@ -58,7 +61,7 @@ Guides::Guides(const Options &options)
x.labels = xOpt.labels.getValue(
(!isPolar || yIsMeasure)
&& ((xIsMeasure && (x.axisSticks || x.interlacings))
|| (!xIsMeasure && !xOpt.isEmpty())));
|| (!xIsMeasure && !xChannel.isEmpty())));

auto stretchedPolar =
isPolar && !yIsMeasure
Expand All @@ -67,7 +70,7 @@ Guides::Guides(const Options &options)
y.labels = yOpt.labels.getValue(
!stretchedPolar
&& ((yIsMeasure && (y.axisSticks || y.interlacings))
|| (!yIsMeasure && !yOpt.isEmpty())));
|| (!yIsMeasure && !yChannel.isEmpty())));
}

GuidesByAxis &Guides::at(AxisId channel)
Expand Down
11 changes: 7 additions & 4 deletions src/chart/generator/plotbuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ void PlotBuilder::calcLegendAndLabel(const Data::DataTable &dataTable)
calcLegend.measure = {std::get<0>(stats.at(type)),
meas->getColIndex(),
dataTable.getUnit(meas->getColIndex()),
scale.step.getValue()};
{1}};
}
}
else if (!scale.isEmpty()) {
Expand Down Expand Up @@ -409,6 +409,9 @@ void PlotBuilder::calcAxis(const Data::DataTable &dataTable,
const auto &scale = plot->getOptions()->getChannels().at(type);
if (scale.isEmpty()) return;

auto &axisProps =
plot->getOptions()->getChannels().axisPropsAt(type);

auto &axis = plot->axises.at(type);

auto isAutoTitle = scale.title.isAuto();
Expand All @@ -424,12 +427,12 @@ void PlotBuilder::calcAxis(const Data::DataTable &dataTable,
axis.measure = {{0, 100},
meas.getColIndex(),
"%",
scale.step.getValue()};
axisProps.step.getValue()};
else
axis.measure = {std::get<0>(stats.at(type)),
meas.getColIndex(),
dataTable.getUnit(meas.getColIndex()),
scale.step.getValue()};
axisProps.step.getValue()};
}
else {
for (auto merge =
Expand All @@ -454,7 +457,7 @@ void PlotBuilder::calcAxis(const Data::DataTable &dataTable,
merge);
}
if (auto &&series = plot->getOptions()->labelSeries(type);
!axis.dimension.setLabels(scale.step.getValue(1.0))
!axis.dimension.setLabels(axisProps.step.getValue(1.0))
&& series && isAutoTitle)
axis.title = series.value().getColIndex();
for (std::uint32_t pos{};
Expand Down
7 changes: 1 addition & 6 deletions src/chart/options/channel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -148,12 +148,7 @@ void Channel::reset()
set = {};
labelLevel = {};
title = {};
axis = Base::AutoBool();
labels = Base::AutoBool();
ticks = Base::AutoBool();
guides = Base::AutoBool();
markerGuides = Base::AutoBool();
interlacing = Base::AutoBool();
range = {};
}

bool Channel::isEmpty() const
Expand Down
24 changes: 15 additions & 9 deletions src/chart/options/channel.h
Original file line number Diff line number Diff line change
Expand Up @@ -118,9 +118,22 @@ struct ChannelSeriesList
bool removeSeries(const Data::SeriesIndex &index);
};

class Channel
struct AxisChannelProperties
schaumb marked this conversation as resolved.
Show resolved Hide resolved
{
Base::AutoBool axis{};
Base::AutoBool labels{};
Base::AutoBool ticks{};
Base::AutoBool interlacing{};
Base::AutoBool guides{};
Base::AutoBool markerGuides{};
Base::AutoParam<double> step{};

[[nodiscard]] bool operator==(
const AxisChannelProperties &) const = default;
};

struct Channel
{
public:
using OptionalIndex = ChannelSeriesList::OptionalIndex;
using IndexSet = std::set<Data::SeriesIndex>;
using DimensionIndices = ChannelSeriesList::DimensionIndices;
Expand Down Expand Up @@ -161,13 +174,6 @@ class Channel
ChannelRange range{};
Base::AutoParam<std::size_t> labelLevel{};
Base::AutoParam<std::string, true> title{};
Base::AutoBool axis{};
Base::AutoBool labels{};
Base::AutoBool ticks{};
Base::AutoBool guides{};
Base::AutoBool markerGuides{};
Base::AutoBool interlacing{};
Base::AutoParam<double> step{};
};

[[nodiscard]] constexpr ChannelId operator+(const AxisId &axis)
Expand Down
12 changes: 12 additions & 0 deletions src/chart/options/channels.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ namespace Vizzu::Gen
struct Channels : Refl::EnumArray<ChannelId, Channel>
{
using IndexSet = std::set<Data::SeriesIndex>;
EnumArray<AxisId, AxisChannelProperties> axisProps;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't something like this would be more straight forward:

struct Channels
{
	Refl::EnumArray<NonAxisId, Channel> nonAxisChannels;
	EnumArray<AxisId, AxisChannel /* inherits Channel */> axisChannels;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Later I'll fix that.


[[nodiscard]] bool anyAxisSet() const;
[[nodiscard]] bool isEmpty() const;
Expand All @@ -37,6 +38,17 @@ struct Channels : Refl::EnumArray<ChannelId, Channel>
return at(+id);
}

[[nodiscard]] const AxisChannelProperties &axisPropsAt(
const AxisId &id) const
{
return axisProps[id];
}

[[nodiscard]] AxisChannelProperties &axisPropsAt(const AxisId &id)
{
return axisProps[id];
}

void removeSeries(const Data::SeriesIndex &index);

[[nodiscard]] bool isSeriesUsed(
Expand Down
35 changes: 25 additions & 10 deletions src/chart/options/config.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,12 @@ std::string Config::paramsJson()
for (const auto &param : channelParams)
arr << "channels." + std::string{channelName} + "."
+ std::string{param};

if (channelName == "x" || channelName == "y")
for (const auto &param :
getAccessorNames<AxisChannelProperties>())
arr << "channels." + std::string{channelName} + "."
+ std::string{param};
}
return res;
}
Expand Down Expand Up @@ -83,35 +89,39 @@ void Config::setChannelParam(const std::string &path,

if (property == "set") {
if (parts.size() == 3) {
channel.reset();
channel.set = {};
options.markersInfo.clear();
return;
}

if (auto &listParser = ChannelSeriesList::Parser::instance();
parts.size() == 4) {
auto &listParser = ChannelSeriesList::Parser::instance();
if (parts.size() == 4) {
if (parts[3] == "begin") {
if (parts[2] == "set") channel.reset();

if (parts[2] == "set") channel.set = {};
options.markersInfo.clear();
listParser.table = &table.get();
listParser.channels.resize(std::stoull(value));
return;
}

listParser.current = std::nullopt;
listParser.path = parts;
}
else {
else
listParser.current = std::stoull(parts.at(3));
listParser.path = parts;
}
listParser.path = parts;
}

if (property == "range") property += "." + parts.at(3);

if (auto &&accessor = getAccessor<Channel>(property).set)
accessor(channel, value);
else if (auto &&axisAccessor =
getAccessor<AxisChannelProperties>(property).set;
(channelId == AxisId::x || channelId == AxisId::y)
&& axisAccessor)
axisAccessor(
options.getChannels().axisPropsAt(
channelId == AxisId::x ? AxisId::x : AxisId::y),
value);
else
throw std::logic_error(
path + "/" + value
Expand All @@ -130,6 +140,11 @@ std::string Config::getChannelParam(const std::string &path) const

if (auto &&accessor = getAccessor<Channel>(property).get)
return accessor(channel);
if (auto &&axisAccessor =
getAccessor<AxisChannelProperties>(property).get;
(id == AxisId::x || id == AxisId::y) && axisAccessor)
return axisAccessor(options.get().getChannels().axisPropsAt(
id == AxisId::x ? AxisId::x : AxisId::y));

throw std::logic_error(path + ": invalid channel parameter");
}
Expand Down
4 changes: 3 additions & 1 deletion src/chart/options/options.h
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,9 @@ class Options : public OptionProperties
[]<std::size_t... Ix>(std::index_sequence<Ix...>)
{
return decltype(channels){
Channel::makeChannel(static_cast<ChannelId>(Ix))...};
{{Channel::makeChannel(
static_cast<ChannelId>(Ix))...}},
{}};
}(std::make_index_sequence<
std::tuple_size_v<decltype(channels)::base_array>>{})};

Expand Down
2 changes: 1 addition & 1 deletion test/e2e/tests/config_tests.json
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
"refs": ["2ac83c5"]
},
"dimension_axis_title": {
"refs": ["33278a6"]
"refs": ["0f4b203"]
},
"dimension_axis_density": {
"refs": ["0de86a3"]
Expand Down
4 changes: 2 additions & 2 deletions test/e2e/tests/config_tests/dimension_axis_title.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ const testSteps = [
}),
(chart) =>
chart.animate({
x: { set: 'Baz' },
size: { set: 'Baz' }
x: { set: 'Baz', title: 'Who' },
size: { set: 'Baz', title: 'Bigness' }
})
]

Expand Down