Skip to content

Commit

Permalink
Merge #2814
Browse files Browse the repository at this point in the history 
2814: [miral] refactor YamlFileDisplayConfig::apply_to() r=Saviq a=AlanGriffiths

This method contained a lambda over 100 lines long with multiple responsibilities.

This splits the responsibilities between an 11 line lambda and two new methods each with a single responsibility.

Note to reviewers: This is a pure refactoring with no functional changes

Co-authored-by: Alan Griffiths <alan@octopull.co.uk>
  • Loading branch information
@bors @AlanGriffiths
bors[bot] and AlanGriffiths committed Jan 27, 2023
2 parents 3134d61 + 42284a0 commit 7453a53
Show file tree
Hide file tree
Showing 2 changed files with 143 additions and 123 deletions.
261 changes: 138 additions & 123 deletions src/miral/static_display_config.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -301,138 +301,19 @@ void miral::YamlFileDisplayConfig::apply_to(mg::DisplayConfiguration& conf)
conf.for_each_output([&](mg::UserDisplayConfigurationOutput& conf_output)
{
auto& card_data = card_map[conf_output.card_id];
auto& out = card_data.out;
auto const type = static_cast<MirOutputType>(conf_output.type);
auto const index_by_type = ++card_data.output_counts[type];

auto const& conf = (current_config != end(config)) ?
current_config->second[Id{conf_output.card_id, type, index_by_type}] :
Config{};

if (conf_output.connected && conf_output.modes.size() > 0 && !conf.disabled)
if (current_config != end(config))
{
conf_output.used = true;
conf_output.power_mode = mir_power_mode_on;
conf_output.orientation = mir_orientation_normal;

if (conf.position.is_set())
{
conf_output.top_left = conf.position.value();
}
else
{
conf_output.top_left = Point{0, 0};
}

size_t preferred_mode_index{select_mode_index(conf_output.preferred_mode_index, conf_output.modes)};
conf_output.current_mode_index = preferred_mode_index;

if (conf.size.is_set())
{
bool matched_mode = false;

for (auto mode = begin(conf_output.modes); mode != end(conf_output.modes); ++mode)
{
if (mode->size == conf.size.value())
{
if (conf.refresh.is_set())
{
if (std::abs(conf.refresh.value() - mode->vrefresh_hz) < 1.0)
{
conf_output.current_mode_index = distance(begin(conf_output.modes), mode);
matched_mode = true;
}
}
else if (conf_output.modes[conf_output.current_mode_index].size != conf.size.value()
|| conf_output.modes[conf_output.current_mode_index].vrefresh_hz < mode->vrefresh_hz)
{
conf_output.current_mode_index = distance(begin(conf_output.modes), mode);
matched_mode = true;
}
}
}

if (!matched_mode)
{
if (conf.refresh.is_set())
{
mir::log_warning("Display config contains unmatched mode: '%dx%d@%2.1f'",
conf.size.value().width.as_int(), conf.size.value().height.as_int(), conf.refresh.value());
}
else
{
mir::log_warning("Display config contains unmatched mode: '%dx%d'",
conf.size.value().width.as_int(), conf.size.value().height.as_int());
}
}
}

if (conf.scale.is_set())
{
conf_output.scale = conf.scale.value();
}

if (conf.orientation.is_set())
{
conf_output.orientation = conf.orientation.value();
}

if (conf.group_id.is_set())
{
conf_output.logical_group_id = mg::DisplayConfigurationLogicalGroupId{conf.group_id.value()};
}
else
{
conf_output.logical_group_id = mg::DisplayConfigurationLogicalGroupId{};
}
}
else
{
conf_output.used = false;
conf_output.power_mode = mir_power_mode_off;
}

out << "\n " << mir::output_type_name(type);
if (conf_output.card_id.as_value() > 0)
out << '-' << conf_output.card_id.as_value();
out << '-' << index_by_type << ':';

if (conf_output.connected && conf_output.modes.size() > 0)
{
out << "\n # This output supports the following modes:";
for (size_t i = 0; i < conf_output.modes.size(); ++i)
{
if (i) out << ',';
if ((i % 5) != 2) out << ' ';
else out << "\n # ";
out << conf_output.modes[i];
}
out << "\n #"
"\n # Uncomment the following to enforce the selected configuration."
"\n # Or amend as desired."
"\n #"
"\n # state: " << (conf_output.used ? state_enabled : state_disabled)
<< "\t# {enabled, disabled}, defaults to enabled";

if (conf_output.used) // The following are only set when used
{
out << "\n # mode: " << conf_output.modes[conf_output.current_mode_index]
<< "\t# Defaults to preferred mode"
"\n # position: [" << conf_output.top_left.x << ", " << conf_output.top_left.y << ']'
<< "\t# Defaults to [0, 0]"
"\n # orientation: " << as_string(conf_output.orientation)
<< "\t# {normal, left, right, inverted}, defaults to normal"
"\n # scale: " << conf_output.scale
<< "\n # group: " << conf_output.logical_group_id.as_value()
<< "\t# Outputs with the same non-zero value are treated as a single display";
}
apply_to_output(conf_output, current_config->second[Id{conf_output.card_id, type, index_by_type}]);
}
else
{
out << "\n # (disconnected)";
apply_to_output(conf_output, Config{});
}

out << "\n";
serialize_output_configuration(card_data.out, conf_output, index_by_type);
});

auto print_template_config = [&card_map](std::ostream& out)
Expand All @@ -457,6 +338,140 @@ void miral::YamlFileDisplayConfig::apply_to(mg::DisplayConfiguration& conf)
dump_config(print_template_config);
}

void miral::YamlFileDisplayConfig::serialize_output_configuration(
std::ostream& out, mg::UserDisplayConfigurationOutput& conf_output, int index_by_type)
{
auto const type = static_cast<MirOutputType>(conf_output.type);

out << "\n " << mir::output_type_name(type);
if (conf_output.card_id.as_value() > 0)
out << '-' << conf_output.card_id.as_value();
out << '-' << index_by_type << ':';

if (conf_output.connected && conf_output.modes.size() > 0)
{
out << "\n # This output supports the following modes:";
for (size_t i = 0; i < conf_output.modes.size(); ++i)
{
if (i) out << ',';
if ((i % 5) != 2) out << ' ';
else out << "\n # ";
out << conf_output.modes[i];
}
out << "\n #"
"\n # Uncomment the following to enforce the selected configuration."
"\n # Or amend as desired."
"\n #"
"\n # state: " << (conf_output.used ? state_enabled : state_disabled)
<< "\t# {enabled, disabled}, defaults to enabled";

if (conf_output.used) // The following are only set when used
{
out << "\n # mode: " << conf_output.modes[conf_output.current_mode_index]
<< "\t# Defaults to preferred mode"
"\n # position: [" << conf_output.top_left.x << ", " << conf_output.top_left.y << ']'
<< "\t# Defaults to [0, 0]"
"\n # orientation: " << as_string(conf_output.orientation)
<< "\t# {normal, left, right, inverted}, defaults to normal"
"\n # scale: " << conf_output.scale
<< "\n # group: " << conf_output.logical_group_id.as_value()
<< "\t# Outputs with the same non-zero value are treated as a single display";
}
}
else
{
out << "\n # (disconnected)";
}

out << "\n";
}

void miral::YamlFileDisplayConfig::apply_to_output(mg::UserDisplayConfigurationOutput& conf_output, Config const& conf)
{
if (conf_output.connected && conf_output.modes.size() > 0 && !conf.disabled)
{
conf_output.used = true;
conf_output.power_mode = mir_power_mode_on;
conf_output.orientation = mir_orientation_normal;

if (conf.position.is_set())
{
conf_output.top_left = conf.position.value();
}
else
{
conf_output.top_left = Point{0, 0};
}

size_t preferred_mode_index{select_mode_index(conf_output.preferred_mode_index, conf_output.modes)};
conf_output.current_mode_index = preferred_mode_index;

if (conf.size.is_set())
{
bool matched_mode = false;

for (auto mode = begin(conf_output.modes); mode != end(conf_output.modes); ++mode)
{
if (mode->size == conf.size.value())
{
if (conf.refresh.is_set())
{
if (std::abs(conf.refresh.value() - mode->vrefresh_hz) < 1.0)
{
conf_output.current_mode_index = distance(begin(conf_output.modes), mode);
matched_mode = true;
}
}
else if (conf_output.modes[conf_output.current_mode_index].size != conf.size.value()
|| conf_output.modes[conf_output.current_mode_index].vrefresh_hz < mode->vrefresh_hz)
{
conf_output.current_mode_index = distance(begin(conf_output.modes), mode);
matched_mode = true;
}
}
}

if (!matched_mode)
{
if (conf.refresh.is_set())
{
mir::log_warning("Display config contains unmatched mode: '%dx%d@%2.1f'",
conf.size.value().width.as_int(), conf.size.value().height.as_int(), conf.refresh.value());
}
else
{
mir::log_warning("Display config contains unmatched mode: '%dx%d'",
conf.size.value().width.as_int(), conf.size.value().height.as_int());
}
}
}

if (conf.scale.is_set())
{
conf_output.scale = conf.scale.value();
}

if (conf.orientation.is_set())
{
conf_output.orientation = conf.orientation.value();
}

if (conf.group_id.is_set())
{
conf_output.logical_group_id = mg::DisplayConfigurationLogicalGroupId{conf.group_id.value()};
}
else
{
conf_output.logical_group_id = mg::DisplayConfigurationLogicalGroupId{};
}
}
else
{
conf_output.used = false;
conf_output.power_mode = mir_power_mode_off;
}
}

void miral::YamlFileDisplayConfig::dump_config(std::function<void(std::ostream&)> const& print_template_config)
{
std::ostringstream out;
Expand Down
5 changes: 5 additions & 0 deletions src/miral/static_display_config.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,11 @@ class YamlFileDisplayConfig : public mir::graphics::DisplayConfigurationPolicy
using Id2Config = std::map<Id, Config>;
using Layout2Id2Config = std::map<std::string, Id2Config>;
Layout2Id2Config config;

static void apply_to_output(mir::graphics::UserDisplayConfigurationOutput& conf_output, Config const& conf);

static void serialize_output_configuration(
std::ostream& out, mir::graphics::UserDisplayConfigurationOutput& conf_output, int index_by_type);
};

class ReloadingYamlFileDisplayConfig : public YamlFileDisplayConfig
Expand Down

0 comments on commit 7453a53

Please sign in to comment.