GunChleoc has proposed merging
lp:~widelands-dev/widelands/bug-1653460-panel-init-width into lp:widelands.
Commit message:
Fixed assertion failure in int UI::Panel::get_inner_h():
- Fixed bug with automatic column sizing for tables where the columns were
already being resized before all of them were added.
- Allow getting inner width/height for panels that have their borders set but
w/h == 0. Added assertions and fallbacks to set_size and set_desired_size to
make sure that width/height will never be negative.
- Explicitly initialize all variables in ui_basic.
Requested reviews:
Widelands Developers (widelands-dev)
Related bugs:
Bug #1653460 in widelands: "UI::Panel::get_inner_h() const: Assertion
`tborder_ + bborder_ <= h_' failed."
https://bugs.launchpad.net/widelands/+bug/1653460
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/bug-1653460-panel-init-width/+merge/318358
The assertion in the bug should now be fixed for all panels containing tables:
* Fullscreen menus:
- campaign_select
- internet_lobby
- launch_mpg
- launch_spg
- loadgame
- mapselect
- netsetup_lan
* In-game:
- game_message_menu
- game_summary
- productionsitewindow
- trainingsitewindow
- tribal_encyclopedia
* Editor:
- help
- main_menu_load_map
- main_menu_save_map
I don't know why this happened only sometimes and not all the time though.
--
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/bug-1653460-panel-init-width into lp:widelands.
=== modified file 'src/ui_basic/box.cc'
--- src/ui_basic/box.cc 2017-02-25 13:27:40 +0000
+++ src/ui_basic/box.cc 2017-02-27 14:05:30 +0000
@@ -98,7 +98,7 @@
int maxbreadth = mindesiredbreadth_;
for (uint32_t idx = 0; idx < items_.size(); ++idx) {
- int depth, breadth;
+ int depth, breadth = 0;
get_item_desired_size(idx, &depth, &breadth);
totaldepth += depth;
@@ -137,7 +137,7 @@
int totaldepth = 0;
for (size_t idx = 0; idx < items_.size(); ++idx) {
- int depth, unused;
+ int depth, unused = 0;
get_item_desired_size(idx, &depth, &unused);
totaldepth += depth;
}
@@ -221,7 +221,7 @@
totalbreadth -= Scrollbar::kSize;
for (uint32_t idx = 0; idx < items_.size(); ++idx) {
- int depth, breadth;
+ int depth, breadth = 0;
get_item_size(idx, &depth, &breadth);
if (items_[idx].type == Item::ItemPanel) {
@@ -371,7 +371,7 @@
switch (it.type) {
case Item::ItemPanel: {
- int32_t breadth, maxbreadth;
+ int32_t breadth, maxbreadth = 0;
if (orientation_ == Horizontal) {
breadth = it.u.panel.panel->get_inner_h();
=== modified file 'src/ui_basic/listselect.cc'
--- src/ui_basic/listselect.cc 2017-02-23 19:38:51 +0000
+++ src/ui_basic/listselect.cc 2017-02-27 14:05:30 +0000
@@ -69,10 +69,9 @@
scrollbar_.moved.connect(boost::bind(&BaseListselect::set_scrollpos, this, _1));
if (selection_mode_ == ListselectLayout::kShowCheck) {
- int pic_h;
check_pic_ = g_gr->images().get("images/ui_basic/list_selected.png");
max_pic_width_ = check_pic_->width();
- pic_h = check_pic_->height();
+ int pic_h = check_pic_->height();
if (pic_h > lineheight_)
lineheight_ = pic_h;
} else {
=== modified file 'src/ui_basic/multilineeditbox.cc'
--- src/ui_basic/multilineeditbox.cc 2017-02-23 17:58:25 +0000
+++ src/ui_basic/multilineeditbox.cc 2017-02-27 14:05:30 +0000
@@ -303,7 +303,7 @@
if (d_->cursor_pos < d_->text.size()) {
d_->refresh_ww();
- uint32_t cursorline, cursorpos;
+ uint32_t cursorline, cursorpos = 0;
d_->ww.calc_wrapped_pos(d_->cursor_pos, cursorline, cursorpos);
if (cursorline + 1 < d_->ww.nrlines()) {
@@ -332,7 +332,7 @@
if (d_->cursor_pos > 0) {
d_->refresh_ww();
- uint32_t cursorline, cursorpos;
+ uint32_t cursorline, cursorpos = 0;
d_->ww.calc_wrapped_pos(d_->cursor_pos, cursorline, cursorpos);
if (cursorline > 0) {
@@ -361,7 +361,7 @@
} else {
d_->refresh_ww();
- uint32_t cursorline, cursorpos;
+ uint32_t cursorline, cursorpos = 0;
d_->ww.calc_wrapped_pos(d_->cursor_pos, cursorline, cursorpos);
d_->set_cursor_pos(d_->ww.line_offset(cursorline));
@@ -379,7 +379,7 @@
} else {
d_->refresh_ww();
- uint32_t cursorline, cursorpos;
+ uint32_t cursorline, cursorpos = 0;
d_->ww.calc_wrapped_pos(d_->cursor_pos, cursorline, cursorpos);
if (cursorline + 1 < d_->ww.nrlines())
@@ -488,7 +488,7 @@
void MultilineEditbox::Data::scroll_cursor_into_view() {
refresh_ww();
- uint32_t cursorline, cursorpos;
+ uint32_t cursorline, cursorpos = 0;
ww.calc_wrapped_pos(cursor_pos, cursorline, cursorpos);
int32_t lineheight = textstyle.font->height();
=== modified file 'src/ui_basic/multilinetextarea.cc'
--- src/ui_basic/multilinetextarea.cc 2017-02-23 19:38:51 +0000
+++ src/ui_basic/multilinetextarea.cc 2017-02-27 14:05:30 +0000
@@ -79,7 +79,7 @@
* and adjust scrollbar settings accordingly.
*/
void MultilineTextarea::recompute() {
- uint32_t height;
+ int height = 0;
// We wrap the text twice. We need to do this to account for the presence/absence of the
// scollbar.
=== modified file 'src/ui_basic/panel.cc'
--- src/ui_basic/panel.cc 2017-01-25 18:55:59 +0000
+++ src/ui_basic/panel.cc 2017-02-27 14:05:30 +0000
@@ -240,8 +240,12 @@
if (nw == w_ && nh == h_)
return;
- w_ = nw;
- h_ = nh;
+ assert(nw >= 0);
+ assert(nh >= 0);
+
+ // Make sure that we never get negative width/height in release builds.
+ w_ = std::max(0, nw);
+ h_ = std::max(0, nh);
if (parent_)
move_inside_parent();
@@ -282,9 +286,12 @@
assert(w < 3000);
assert(h < 3000);
+ assert(w >= 0);
+ assert(h >= 0);
- desired_w_ = w;
- desired_h_ = h;
+ // Make sure that we never get negative width/height in release builds.
+ desired_w_ = std::max(0, w);
+ desired_h_ = std::max(0, h);
if (!get_layout_toplevel() && parent_) {
parent_->update_desired_size();
} else {
@@ -351,6 +358,7 @@
* Set the size of the inner area (total area minus border)
*/
void Panel::set_inner_size(int const nw, int const nh) {
+ assert(nw >= 0 && nh >= 0);
set_size(nw + lborder_ + rborder_, nh + tborder_ + bborder_);
}
@@ -366,6 +374,15 @@
bborder_ = b;
}
+int Panel::get_inner_w() const {
+ assert(w_ == 0 || lborder_ + rborder_ <= w_);
+ return (w_ == 0 ? 0 : w_ - (lborder_ + rborder_));
+}
+int Panel::get_inner_h() const {
+ assert(h_ == 0 || tborder_ + bborder_ <= h_);
+ return (h_ == 0 ? 0 : h_ - (tborder_ + bborder_));
+}
+
/**
* Make this panel the top-most panel in the parent's Z-order.
*/
=== modified file 'src/ui_basic/panel.h'
--- src/ui_basic/panel.h 2017-01-25 18:55:59 +0000
+++ src/ui_basic/panel.h 2017-02-27 14:05:30 +0000
@@ -180,14 +180,8 @@
return bborder_;
}
- int get_inner_w() const {
- assert(lborder_ + rborder_ <= w_);
- return w_ - (lborder_ + rborder_);
- }
- int get_inner_h() const {
- assert(tborder_ + bborder_ <= h_);
- return h_ - (tborder_ + bborder_);
- }
+ int get_inner_w() const;
+ int get_inner_h() const;
const Panel* get_next_sibling() const {
return next_;
=== modified file 'src/ui_basic/scrollbar.cc'
--- src/ui_basic/scrollbar.cc 2017-01-25 18:55:59 +0000
+++ src/ui_basic/scrollbar.cc 2017-02-27 14:05:30 +0000
@@ -130,7 +130,7 @@
}
Scrollbar::Area Scrollbar::get_area_for_point(int32_t x, int32_t y) {
- int32_t extent;
+ int32_t extent = 0;
// Out of panel
if (x < 0 || x >= static_cast<int32_t>(get_w()) || y < 0 || y >= static_cast<int32_t>(get_h()))
=== modified file 'src/ui_basic/table.cc'
--- src/ui_basic/table.cc 2017-02-26 11:57:15 +0000
+++ src/ui_basic/table.cc 2017-02-27 14:05:30 +0000
@@ -108,7 +108,7 @@
// If there would be existing entries, they would not get the new column.
assert(size() == 0);
- uint32_t complete_width = 0;
+ int complete_width = 0;
for (const Column& col : columns_) {
complete_width += col.width;
}
@@ -133,7 +133,6 @@
flexible_column_ = columns_.size() - 1;
}
}
- layout();
}
void Table<void*>::set_column_title(uint8_t const col, const std::string& title) {
@@ -209,8 +208,7 @@
if (entries == 0) {
entries = size();
}
- int tablewidth;
- int tableheight;
+ int tablewidth, tableheight = 0;
get_desired_size(&tablewidth, &tableheight);
tableheight = headerheight_ + 2 + get_lineheight() * entries;
set_desired_size(tablewidth, tableheight);
@@ -606,15 +604,13 @@
// Find a column to resize
size_t resizeable_column = std::numeric_limits<size_t>::max();
- if (flexible_column_ != std::numeric_limits<size_t>::max()) {
+ if (flexible_column_ < columns_.size()) {
resizeable_column = flexible_column_;
} else {
// Use the widest column
- int all_columns_width = scrollbar_ && scrollbar_->is_enabled() ? scrollbar_->get_w() : 0;
uint32_t widest_width = columns_[resizeable_column].width;
for (size_t i = 1; i < columns_.size(); ++i) {
const uint32_t width = columns_[i].width;
- all_columns_width += width;
if (width > widest_width) {
widest_width = width;
resizeable_column = i;
@@ -630,8 +626,9 @@
}
if (all_columns_width != get_w()) {
Column& column = columns_.at(resizeable_column);
- column.width = column.width + get_w() - all_columns_width;
+ column.width = std::max(0, column.width + get_w() - all_columns_width);
column.btn->set_size(column.width, column.btn->get_h());
+
int offset = 0;
for (const auto& col : columns_) {
col.btn->set_pos(Vector2i(offset, col.btn->get_y()));
=== modified file 'src/ui_basic/table.h'
--- src/ui_basic/table.h 2017-02-26 11:57:15 +0000
+++ src/ui_basic/table.h 2017-02-27 14:05:30 +0000
@@ -184,6 +184,8 @@
void set_column_title(uint8_t col, const std::string& title);
void set_column_compare(uint8_t col, const CompareFn& fn);
+ void layout() override;
+
void clear();
void set_sort_column(uint8_t const col) {
assert(col < columns_.size());
@@ -280,11 +282,10 @@
private:
bool default_compare_string(uint32_t column, uint32_t a, uint32_t b);
bool sort_helper(uint32_t a, uint32_t b);
- void layout() override;
struct Column {
Button* btn;
- uint32_t width;
+ int width;
Align alignment;
CompareFn compare;
};
@@ -293,8 +294,8 @@
static const int32_t ms_darken_value = -20;
Columns columns_;
- uint32_t total_width_;
- uint32_t headerheight_;
+ int total_width_;
+ const uint32_t headerheight_;
int32_t lineheight_;
const Image* button_background_;
Scrollbar* scrollbar_;
=== modified file 'src/ui_basic/tabpanel.cc'
--- src/ui_basic/tabpanel.cc 2017-02-23 19:38:51 +0000
+++ src/ui_basic/tabpanel.cc 2017-02-27 14:05:30 +0000
@@ -139,7 +139,7 @@
// size of contents
if (active_ < tabs_.size()) {
Panel* const panel = tabs_[active_]->panel;
- int panelw, panelh;
+ int panelw, panelh = 0;
panel->get_desired_size(&panelw, &panelh);
// TODO(unknown): the panel might be bigger -> add a scrollbar in that case
=== modified file 'src/ui_basic/textarea.cc'
--- src/ui_basic/textarea.cc 2017-02-23 19:38:51 +0000
+++ src/ui_basic/textarea.cc 2017-02-27 14:05:30 +0000
@@ -163,7 +163,7 @@
int32_t y = get_y();
update_desired_size();
- int w, h;
+ int w, h = 0;
get_desired_size(&w, &h);
switch (align_) {
=== modified file 'src/ui_basic/window.cc'
--- src/ui_basic/window.cc 2017-02-24 10:21:37 +0000
+++ src/ui_basic/window.cc 2017-02-27 14:05:30 +0000
@@ -128,7 +128,7 @@
*/
void Window::update_desired_size() {
if (center_panel_ && !is_minimal_) {
- int innerw, innerh;
+ int innerw, innerh = 0;
center_panel_->get_desired_size(&innerw, &innerh);
set_desired_size(
innerw + get_lborder() + get_rborder(), innerh + get_tborder() + get_bborder());
=== modified file 'src/ui_fsmenu/campaign_select.cc'
--- src/ui_fsmenu/campaign_select.cc 2017-02-26 11:57:15 +0000
+++ src/ui_fsmenu/campaign_select.cc 2017-02-27 14:05:30 +0000
@@ -358,6 +358,7 @@
void FullscreenMenuCampaignMapSelect::layout() {
// TODO(GunChleoc): Implement when we have box layout for the details.
+ table_.layout();
}
std::string FullscreenMenuCampaignMapSelect::get_map() {
=== modified file 'src/ui_fsmenu/internet_lobby.cc'
--- src/ui_fsmenu/internet_lobby.cc 2017-02-23 19:38:51 +0000
+++ src/ui_fsmenu/internet_lobby.cc 2017-02-27 14:05:30 +0000
@@ -153,6 +153,7 @@
void FullscreenMenuInternetLobby::layout() {
// TODO(GunChleoc): Box layout and then implement
+ clientsonline_list_.layout();
}
/// think function of the UI (main loop)
=== modified file 'src/ui_fsmenu/loadgame.cc'
--- src/ui_fsmenu/loadgame.cc 2017-02-26 11:57:15 +0000
+++ src/ui_fsmenu/loadgame.cc 2017-02-27 14:05:30 +0000
@@ -188,6 +188,7 @@
ta_mapname_.set_tooltip(_("The map that this game is based on"));
delete_.set_tooltip(_("Delete this game"));
}
+ set_thinks(false);
minimap_icon_.set_visible(false);
back_.sigclicked.connect(boost::bind(&FullscreenMenuLoadGame::clicked_back, boost::ref(*this)));
@@ -240,6 +241,7 @@
void FullscreenMenuLoadGame::layout() {
// TODO(GunChleoc): Implement when we have box layout for the details.
+ table_.layout();
}
void FullscreenMenuLoadGame::think() {
=== modified file 'src/ui_fsmenu/netsetup_lan.cc'
--- src/ui_fsmenu/netsetup_lan.cc 2017-02-23 19:38:51 +0000
+++ src/ui_fsmenu/netsetup_lan.cc 2017-02-27 14:05:30 +0000
@@ -126,6 +126,7 @@
void FullscreenMenuNetSetupLAN::layout() {
// TODO(GunChleoc): Box layout and then implement
+ opengames.layout();
}
void FullscreenMenuNetSetupLAN::think() {
=== modified file 'src/wui/game_message_menu.cc'
--- src/wui/game_message_menu.cc 2017-02-26 11:57:15 +0000
+++ src/wui/game_message_menu.cc 2017-02-27 14:05:30 +0000
@@ -152,6 +152,7 @@
ColTimeSent, boost::bind(&GameMessageMenu::compare_time_sent, this, _1, _2));
list->set_sort_column(ColTimeSent);
+ list->layout();
set_can_focus(true);
focus();
=== modified file 'src/wui/game_summary.cc'
--- src/wui/game_summary.cc 2017-02-25 13:27:40 +0000
+++ src/wui/game_summary.cc 2017-02-27 14:05:30 +0000
@@ -208,6 +208,7 @@
if (!players_status.empty()) {
players_table_->select(current_player_position);
}
+ players_table_->layout();
}
void GameSummaryScreen::continue_clicked() {
=== modified file 'src/wui/maptable.cc'
--- src/wui/maptable.cc 2017-02-26 11:57:15 +0000
+++ src/wui/maptable.cc 2017-02-27 14:05:30 +0000
@@ -76,4 +76,5 @@
}
}
sort();
+ layout();
}
_______________________________________________
Mailing list: https://launchpad.net/~widelands-dev
Post to : widelands-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~widelands-dev
More help : https://help.launchpad.net/ListHelp
