Add a method to clear and then add multiple items to a tree view.

This addresses the lobby performance issue as far as the playerlist updating. Previously for every player an item was added and the size of the tree view recalculated, which led to an increasingly unresponsive UI when the number of players in the lobby got to around 150+. With this change, the replace_children() method adds all the players and then after that recalculates the tree view's size once.
This commit is contained in:
Pentarctagon 2021-06-15 13:28:54 -05:00 committed by Pentarctagon
parent dc813ac693
commit 0e3faee803
6 changed files with 131 additions and 23 deletions

View file

@ -104,7 +104,7 @@ void sub_player_list::update_player_count_label()
void player_list::init(window& w)
{
active_game.init(w, _("Selected Game"), true);
other_rooms.init(w, _("Lobby"), true);
lobby_players.init(w, _("Lobby"), true);
other_games.init(w, _("Other Games"));
tree = find_widget<tree_view>(&w, "player_tree", false, true);
@ -585,35 +585,34 @@ void mp_lobby::update_playerlist()
assert(player_list_.active_game.tree);
assert(player_list_.other_games.tree);
assert(player_list_.other_rooms.tree);
assert(player_list_.lobby_players.tree);
unsigned scrollbar_position = player_list_.tree->get_vertical_scrollbar_item_position();
player_list_.active_game.tree->clear();
player_list_.other_games.tree->clear();
player_list_.other_rooms.tree->clear();
for(auto& user : lobby_info_.users()) {
sub_player_list* target_list(nullptr);
player_list_.lobby_players.tree->clear();
std::map<std::string, std::map<std::string, string_map>> lobby_player_items;
std::map<std::string, std::map<std::string, string_map>> active_game_items;
std::map<std::string, std::map<std::string, string_map>> other_game_items;
for(const auto& user : lobby_info_.users()) {
std::string name = user.name;
std::stringstream icon_ss;
icon_ss << "lobby/status";
switch(user.state) {
case mp::user_info::user_state::LOBBY:
icon_ss << "-lobby";
target_list = &player_list_.other_rooms;
break;
case mp::user_info::user_state::SEL_GAME:
name = colorize(name, {0, 255, 255});
icon_ss << (user.observing ? "-obs" : "-playing");
target_list = &player_list_.active_game;
break;
case mp::user_info::user_state::GAME:
name = colorize(name, font::GRAY_COLOR);
icon_ss << (user.observing ? "-obs" : "-playing");
target_list = &player_list_.other_games;
break;
default:
ERR_LB << "Bad user state in lobby: " << user.name << ": " << static_cast<int>(user.state) << "\n";
@ -639,8 +638,6 @@ void mp_lobby::update_playerlist()
icon_ss << ".png";
assert(target_list->tree);
string_map tree_group_field;
std::map<std::string, string_map> tree_group_item;
@ -652,14 +649,37 @@ void mp_lobby::update_playerlist()
tree_group_field["use_markup"] = "true";
tree_group_item["name"] = tree_group_field;
tree_view_node& player = target_list->tree->add_child("player", tree_group_item);
switch(user.state) {
case mp::user_info::user_state::LOBBY:
lobby_player_items[user.name] = tree_group_item;
break;
case mp::user_info::user_state::SEL_GAME:
active_game_items[user.name] = tree_group_item;
break;
case mp::user_info::user_state::GAME:
other_game_items[user.name] = tree_group_item;
break;
default:
ERR_LB << "Bad user state in lobby: " << user.name << ": " << static_cast<int>(user.state) << "\n";
continue;
}
}
connect_signal_mouse_left_double_click(find_widget<toggle_panel>(&player, "tree_view_node_label", false),
std::bind(&mp_lobby::user_dialog_callback, this, &user));
for(const auto& player : player_list_.active_game.tree->replace_children("player", active_game_items)) {
connect_signal_mouse_left_double_click(find_widget<toggle_panel>(player.second.get(), "tree_view_node_label", false),
std::bind(&mp_lobby::user_dialog_callback, this, lobby_info_.get_user(player.first)));
}
for(const auto& player : player_list_.lobby_players.tree->replace_children("player", lobby_player_items)) {
connect_signal_mouse_left_double_click(find_widget<toggle_panel>(player.second.get(), "tree_view_node_label", false),
std::bind(&mp_lobby::user_dialog_callback, this, lobby_info_.get_user(player.first)));
}
for(const auto& player : player_list_.other_games.tree->replace_children("player", other_game_items)) {
connect_signal_mouse_left_double_click(find_widget<toggle_panel>(player.second.get(), "tree_view_node_label", false),
std::bind(&mp_lobby::user_dialog_callback, this, lobby_info_.get_user(player.first)));
}
player_list_.active_game.update_player_count_label();
player_list_.other_rooms.update_player_count_label();
player_list_.lobby_players.update_player_count_label();
player_list_.other_games.update_player_count_label();
// Don't attempt to restore the scroll position if the window hasn't been laid out yet

View file

@ -54,7 +54,7 @@ struct player_list
void init(window& w);
sub_player_list active_game;
sub_player_list other_rooms;
sub_player_list lobby_players;
sub_player_list other_games;
tree_view* tree;

View file

@ -59,7 +59,7 @@ tree_view_node& tree_view::add_node(
return get_root_node().add_child(id, data, index);
}
std::pair<tree_view_node::ptr_t, int> tree_view::remove_node(tree_view_node* node)
std::pair<std::shared_ptr<tree_view_node>, int> tree_view::remove_node(tree_view_node* node)
{
assert(node && node != root_node_ && node->parent_node_);
const point node_size = node->get_size();

View file

@ -88,7 +88,7 @@ public:
* @returns A pair consisting of a smart pointer managing the removed
* node, and its position before removal.
*/
std::pair<tree_view_node::ptr_t, int> remove_node(tree_view_node* node);
std::pair<std::shared_ptr<tree_view_node>, int> remove_node(tree_view_node* node);
void clear();

View file

@ -126,7 +126,7 @@ void tree_view_node::clear_before_destruct()
}
}
tree_view_node& tree_view_node::add_child_impl(ptr_t&& new_node, const int index)
tree_view_node& tree_view_node::add_child_impl(std::shared_ptr<tree_view_node>&& new_node, const int index)
{
auto itor = children_.end();
@ -186,6 +186,82 @@ tree_view_node& tree_view_node::add_child_impl(ptr_t&& new_node, const int index
return node;
}
std::map<std::string, std::shared_ptr<gui2::tree_view_node>> tree_view_node::replace_children(const std::string& id, const std::map<std::string, std::map<std::string /* widget id */, string_map>>& data)
{
std::map<std::string, std::shared_ptr<gui2::tree_view_node>> nodes;
clear();
if(data.size() == 0)
{
return nodes;
}
int width_modification = 0;
for(const auto& d : data)
{
std::shared_ptr<gui2::tree_view_node> new_node = std::make_shared<tree_view_node>(id, this, get_tree_view(), d.second);
std::shared_ptr<gui2::tree_view_node> node = *children_.insert(children_.end(), std::move(new_node));
// NOTE: we currently don't support moving nodes between different trees, so this
// just ensures that wasn't tried. Remove this if we implement support for that.
assert(node->tree_view_ == tree_view_);
// Safety check. Might only fail if someone accidentally removed the parent_node_ setter in add_child().
assert(node->parent_node_ == this);
nodes[d.first] = node;
if(is_folded()) {
continue;
}
assert(get_tree_view().content_grid());
const point current_size = get_tree_view().content_grid()->get_size();
// Calculate width modification.
// This increases tree width if the width of the new node is greater than the current width.
int best_size = node->get_best_size().x;
best_size += get_indentation_level() * get_tree_view().indentation_step_size_;
int new_width = best_size > current_size.x
? best_size - current_size.x
: 0;
if(new_width > width_modification)
{
width_modification = new_width;
}
}
if(is_folded()) {
return nodes;
}
// Calculate height modification.
// For this, we only increase height if the best size of the tree (that is, the size with the new node)
// is larger than its current size. This prevents the scrollbar being reserved even when there's obviously
// enough visual space.
// Throw away cached best size to force a recalculation.
get_tree_view().layout_initialize(false);
const point current_size = get_tree_view().content_grid()->get_size();
const point tree_best_size = get_tree_view().get_best_size();
const int height_modification = tree_best_size.y > current_size.y && get_tree_view().layout_size() == point()
? tree_best_size.y - current_size.y
: 0;
assert(height_modification >= 0);
// Request new size.
auto& last_node = children_.at(children_.size()-1);
get_tree_view().resize_content(width_modification, height_modification, -1, last_node->calculate_ypos());
return nodes;
}
unsigned tree_view_node::get_indentation_level() const
{
unsigned level = 0;

View file

@ -35,8 +35,7 @@ class tree_view_node : public widget
friend class tree_view;
public:
using ptr_t = std::shared_ptr<tree_view_node>;
using node_children_vector = std::vector<ptr_t>;
using node_children_vector = std::vector<std::shared_ptr<tree_view_node>>;
bool operator==(const tree_view_node& node)
{
@ -73,6 +72,19 @@ public:
return add_child_impl(std::make_shared<tree_view_node>(id, this, get_tree_view(), data), index);
}
/**
* Replaces all children of this tree with new children.
* The motivation here is to provide a way to add multiple children without calculating the trees size for each child added.
* This is a waste of time since the results of that resizing will be immediately thrown out except for the final child added.
*
* @param id The id of the node definition to use for the new nodes.
* @param data data.first is a unique identifying value to be associated to the respective tree_view_node that's returned.
* data.second is the data to provide to the tree_node_view's constructor.
* @return return_value.first is data.first
* return_value.second is the tree_view_node created from data.second
*/
std::map<std::string, std::shared_ptr<gui2::tree_view_node>> replace_children(const std::string& id, const std::map<std::string, std::map<std::string /* widget id */, string_map>>& data);
/**
* Adds a previously-constructed node as a child of this node at the given position.
*
@ -80,7 +92,7 @@ public:
* @param index The item before which to add the new item,
* 0 == begin, -1 == end.
*/
tree_view_node& add_child(ptr_t new_node, const int index = -1)
tree_view_node& add_child(std::shared_ptr<tree_view_node> new_node, const int index = -1)
{
new_node->parent_node_ = this;
return add_child_impl(std::move(new_node), index);
@ -109,7 +121,7 @@ public:
private:
/** Implementation detail for @ref add_child. */
tree_view_node& add_child_impl(ptr_t&& new_node, const int index);
tree_view_node& add_child_impl(std::shared_ptr<tree_view_node>&& new_node, const int index);
public:
/**