widelands-dev team mailing list archive
-
widelands-dev team
-
Mailing list archive
-
Message #06914
[Merge] lp:~widelands-dev/widelands/scrollbar_fixes into lp:widelands
GunChleoc has proposed merging lp:~widelands-dev/widelands/scrollbar_fixes into lp:widelands.
Commit message:
Various scrollbar-related fixes:
- Improved buttons for short scrollbars
- Fixed height of language list in options.
- Fixed width of MapDetails in Mapselect and missing pixel in Map Details scrollbar.
Requested reviews:
Widelands Developers (widelands-dev)
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/scrollbar_fixes/+merge/290133
The scroll bar just painted an elongated button when there wasn't enough space. Now we have a new strategy:
1. First, try to cut button size in half
2. Leave out the knob
3. Last resort: as before
Test cases are mostly when loading a map, where the SuggestedTeamsBox can get quite high. I tested with Fellowships (enough size), Calvisson (case 1.), Trident of Fire (case 2.) and The Nile (case 3.), at 800x600 resolution.
We still need to do something about The Nile, but I'd rather do that in a separate branch, so we won't lose the test case here.
--
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/scrollbar_fixes into lp:widelands.
=== modified file 'src/ui_basic/box.cc'
--- src/ui_basic/box.cc 2016-02-22 10:23:56 +0000
+++ src/ui_basic/box.cc 2016-03-25 17:49:07 +0000
@@ -105,14 +105,14 @@
if (orientation_ == Horizontal) {
if (totaldepth > max_x_ && scrolling_) {
- maxbreadth += Scrollbar::Size;
+ maxbreadth += Scrollbar::kSize;
}
set_desired_size
(std::min(totaldepth, max_x_), // + get_lborder() + get_rborder(),
std::min(maxbreadth, max_y_)); // + get_tborder() + get_bborder());
} else {
if (totaldepth > max_y_ && scrolling_) {
- maxbreadth += Scrollbar::Size;
+ maxbreadth += Scrollbar::kSize;
}
set_desired_size
(std::min(maxbreadth, max_x_) + get_lborder() + get_rborder(),
@@ -159,16 +159,16 @@
int32_t pagesize;
if (orientation_ == Horizontal) {
sb_x = 0;
- sb_y = get_inner_h() - Scrollbar::Size;
+ sb_y = get_inner_h() - Scrollbar::kSize;
sb_w = get_inner_w();
- sb_h = Scrollbar::Size;
- pagesize = get_inner_w() - Scrollbar::Size;
+ sb_h = Scrollbar::kSize;
+ pagesize = get_inner_w() - Scrollbar::kSize;
} else {
- sb_x = get_inner_w() - Scrollbar::Size;
+ sb_x = get_inner_w() - Scrollbar::kSize;
sb_y = 0;
- sb_w = Scrollbar::Size;
+ sb_w = Scrollbar::kSize;
sb_h = get_inner_h();
- pagesize = get_inner_h() - Scrollbar::Size;
+ pagesize = get_inner_h() - Scrollbar::kSize;
}
if (scrollbar_ == nullptr) {
scrollbar_.reset(
@@ -179,7 +179,7 @@
scrollbar_->set_size(sb_w, sb_h);
}
scrollbar_->set_steps(totaldepth - pagesize);
- scrollbar_->set_singlestepsize(Scrollbar::Size);
+ scrollbar_->set_singlestepsize(Scrollbar::kSize);
scrollbar_->set_pagesize(pagesize);
} else {
scrollbar_.reset();
@@ -216,7 +216,7 @@
uint32_t totaldepth = 0;
uint32_t totalbreadth = orientation_ == Horizontal ? get_inner_h() : get_inner_w();
if (scrollbar_)
- totalbreadth -= Scrollbar::Size;
+ totalbreadth -= Scrollbar::kSize;
for (uint32_t idx = 0; idx < items_.size(); ++idx) {
int depth, breadth;
=== modified file 'src/ui_basic/listselect.cc'
--- src/ui_basic/listselect.cc 2016-02-17 18:20:57 +0000
+++ src/ui_basic/listselect.cc 2016-03-25 17:49:07 +0000
@@ -53,7 +53,7 @@
Panel(parent, x, y, w, h),
lineheight_(UI::g_fh1->render(as_uifont(UI::g_fh1->fontset()->representative_character()))->height()
+ kMargin),
- scrollbar_ (this, get_w() - 24, 0, 24, h, false),
+ scrollbar_ (this, get_w() - Scrollbar::kSize, 0, Scrollbar::kSize, h, false),
scrollpos_ (0),
selection_ (no_selection_index()),
last_click_time_(-10000),
=== modified file 'src/ui_basic/multilineeditbox.cc'
--- src/ui_basic/multilineeditbox.cc 2016-03-01 08:31:21 +0000
+++ src/ui_basic/multilineeditbox.cc 2016-03-25 17:49:07 +0000
@@ -32,8 +32,6 @@
namespace UI {
-static const int32_t ms_scrollbar_w = 24;
-
struct MultilineEditbox::Data {
Scrollbar scrollbar;
@@ -102,7 +100,7 @@
MultilineEditbox::Data::Data(MultilineEditbox & o)
:
-scrollbar(&o, o.get_w() - ms_scrollbar_w, 0, ms_scrollbar_w, o.get_h(), false),
+scrollbar(&o, o.get_w() - Scrollbar::kSize, 0, Scrollbar::kSize, o.get_h(), false),
cursor_pos(0),
maxbytes(0xffff),
ww_valid(false),
@@ -537,13 +535,13 @@
*/
void MultilineEditbox::Data::refresh_ww()
{
- if (int32_t(ww.wrapwidth()) != owner.get_w() - ms_scrollbar_w)
+ if (int32_t(ww.wrapwidth()) != owner.get_w() - Scrollbar::kSize)
ww_valid = false;
if (ww_valid)
return;
ww.set_style(textstyle);
- ww.set_wrapwidth(owner.get_w() - ms_scrollbar_w);
+ ww.set_wrapwidth(owner.get_w() - Scrollbar::kSize);
ww.wrap(text);
ww_valid = true;
=== modified file 'src/ui_basic/multilinetextarea.cc'
--- src/ui_basic/multilinetextarea.cc 2016-02-17 08:48:02 +0000
+++ src/ui_basic/multilinetextarea.cc 2016-03-25 17:49:07 +0000
@@ -42,10 +42,10 @@
text_ (text),
color_(UI_FONT_CLR_FG),
isrichtext(false),
- scrollbar_ (this, get_w() - scrollbar_w(), 0, scrollbar_w(), h, false),
+ scrollbar_ (this, get_w() - Scrollbar::kSize, 0, Scrollbar::kSize, h, false),
scrollmode_(scroll_mode)
{
- assert(scrollmode_ == MultilineTextarea::ScrollMode::kNoScrolling || scrollbar_w() <= w);
+ assert(scrollmode_ == MultilineTextarea::ScrollMode::kNoScrolling || Scrollbar::kSize <= w);
set_thinks(false);
// do not allow vertical alignment as it does not make sense
@@ -134,8 +134,8 @@
recompute();
// Take care about the scrollbar
- scrollbar_.set_pos(Point(get_w() - scrollbar_w(), 0));
- scrollbar_.set_size(scrollbar_w(), get_h());
+ scrollbar_.set_pos(Point(get_w() - Scrollbar::kSize, 0));
+ scrollbar_.set_size(Scrollbar::kSize, get_h());
}
/**
=== modified file 'src/ui_basic/multilinetextarea.h'
--- src/ui_basic/multilinetextarea.h 2016-02-14 10:56:16 +0000
+++ src/ui_basic/multilinetextarea.h 2016-03-25 17:49:07 +0000
@@ -55,9 +55,7 @@
const std::string& get_text() const {return text_;}
void set_text(const std::string&);
-
- uint32_t scrollbar_w() const {return 24;}
- uint32_t get_eff_w() const {return scrollbar_.is_enabled() ? get_w() - scrollbar_w() : get_w();}
+ uint32_t get_eff_w() const {return scrollbar_.is_enabled() ? get_w() - Scrollbar::kSize : get_w();}
void set_color(RGBColor fg) {color_ = fg;}
=== modified file 'src/ui_basic/scrollbar.cc'
--- src/ui_basic/scrollbar.cc 2016-02-03 18:09:15 +0000
+++ src/ui_basic/scrollbar.cc 2016-03-25 17:49:07 +0000
@@ -51,6 +51,7 @@
pos_ (0),
singlestepsize_(1),
pagesize_ (5),
+ buttonsize_ (kSize),
steps_ (100),
pressed_ (None),
time_nextact_ (0),
@@ -68,6 +69,7 @@
pic_buttons_ (g_gr->images().get("images/ui_basic/but3.png"))
{
set_thinks(true);
+ layout();
}
@@ -163,7 +165,7 @@
int32_t knob = get_knob_pos();
int32_t knobsize = get_knob_size();
- if (y < Size)
+ if (y < static_cast<int32_t>(buttonsize_))
return Minus;
if (y < knob - knobsize / 2)
@@ -172,7 +174,7 @@
if (y < knob + knobsize / 2)
return Knob;
- if (y < extent - Size)
+ if (y < extent - static_cast<int32_t>(buttonsize_))
return PlusPage;
return Plus;
@@ -184,7 +186,7 @@
*/
uint32_t Scrollbar::get_knob_pos() {
assert(0 != steps_);
- uint32_t result = Size + get_knob_size() / 2;
+ uint32_t result = buttonsize_ + get_knob_size() / 2;
if (uint32_t const d = steps_ - 1)
result += pos_ * ((horizontal_ ? get_w() : get_h()) - 2 * result) / d;
return result;
@@ -199,8 +201,8 @@
uint32_t knobsize = get_knob_size();
int32_t extent = horizontal_ ? get_w() : get_h();
- extent -= 2 * Size + knobsize;
- pos -= Size + knobsize / 2;
+ extent -= 2 * buttonsize_ + knobsize;
+ pos -= buttonsize_ + knobsize / 2;
pos = (pos * static_cast<int32_t>(steps_)) / extent;
set_scrollpos(pos);
@@ -216,15 +218,15 @@
{
uint32_t extent = horizontal_ ? get_w() : get_h();
- if (extent <= 3 * Size)
- return Size;
+ if (extent <= 3 * buttonsize_)
+ return buttonsize_;
- uint32_t maxhalfsize = extent / 2 - Size;
+ uint32_t maxhalfsize = extent / 2 - buttonsize_;
uint32_t halfsize = (maxhalfsize * get_pagesize()) /
(steps_ + get_pagesize());
uint32_t size = 2 * halfsize;
- if (size < Size)
- size = Size;
+ if (size < buttonsize_)
+ size = buttonsize_;
return size;
}
@@ -262,10 +264,19 @@
pic = pic_plus_;
if (pic) {
- uint16_t cpw = pic->width();
- uint16_t cph = pic->height();
+ double image_scale =
+ std::min(1.,
+ std::min(static_cast<double>(r.w - 4) / pic->width(),
+ static_cast<double>(r.h - 4) / pic->height()));
+ int blit_width = image_scale * pic->width();
+ int blit_height = image_scale * pic->height();
- dst.blit(r.origin() + Point((r.w - cpw) / 2, (r.h - cph) / 2), pic);
+ dst.blitrect_scale(
+ Rect(r.origin() + Point((r.w - blit_width) / 2, (r.h - blit_height) / 2), blit_width, blit_height),
+ pic,
+ Rect(0, 0, pic->width(), pic->height()),
+ 1.,
+ BlendMode::UseAlpha);
}
// Draw border
@@ -321,57 +332,63 @@
}
if (horizontal_) {
- if ((2 * Size + knobsize) > static_cast<uint32_t>(get_w())) {
- // Our owner obviously allocated too little space - draw something
- // stupid
- draw_button(dst, Minus, Rect(Point(0, 0), get_w(), get_h()));
+ if ((2 * buttonsize_ + knobsize) > static_cast<uint32_t>(get_w())) {
+ // Our owner allocated too little space
+ if (static_cast<uint32_t>(get_w()) >= 2 * buttonsize_) {
+ draw_button(dst, Minus, Rect(Point(0, 0), get_w() / 2, get_h()));
+ draw_button(dst, Plus, Rect(Point(get_w() - buttonsize_, 0), get_w() / 2, get_h()));
+ } else {
+ draw_button(dst, Minus, Rect(Point(0, 0), get_w(), get_h()));
+ }
return;
}
- draw_button(dst, Minus, Rect(Point(0, 0), Size, get_h()));
- draw_button(dst, Plus, Rect(Point(get_w() - Size, 0), Size, get_h()));
+ draw_button(dst, Minus, Rect(Point(0, 0), buttonsize_, get_h()));
+ draw_button(dst, Plus, Rect(Point(get_w() - buttonsize_, 0), buttonsize_, get_h()));
draw_button
(dst, Knob, Rect(Point(knobpos - knobsize / 2, 0), knobsize, get_h()));
- assert(Size + knobsize / 2 <= knobpos);
+ assert(buttonsize_ + knobsize / 2 <= knobpos);
draw_area
(dst,
MinusPage,
- Rect(Point(Size, 0), knobpos - Size - knobsize / 2, get_h()));
- assert(knobpos + knobsize / 2 + Size <= static_cast<uint32_t>(get_w()));
+ Rect(Point(buttonsize_, 0), knobpos - buttonsize_ - knobsize / 2, get_h()));
+ assert(knobpos + knobsize / 2 + buttonsize_ <= static_cast<uint32_t>(get_w()));
draw_area
(dst,
PlusPage,
Rect
(Point(knobpos + knobsize / 2, 0),
- get_w() - knobpos - knobsize / 2 - Size, get_h()));
+ get_w() - knobpos - knobsize / 2 - buttonsize_, get_h()));
} else {
- if ((2 * Size + knobsize) > static_cast<uint32_t>(get_h())) {
- // Our owner obviously allocated too little space - draw something
- // stupid
- draw_button(dst, Minus, Rect(Point(0, 0), get_w(), get_h()));
+ if ((2 * buttonsize_ + knobsize) > static_cast<uint32_t>(get_h())) {
+ // Our owner allocated too little space
+ if (static_cast<uint32_t>(get_h()) >= 2 * buttonsize_) {
+ draw_button(dst, Minus, Rect(Point(0, 0), get_w(), get_h() / 2));
+ draw_button(dst, Plus, Rect(Point(0, get_h() - buttonsize_), get_w(), get_h() / 2));
+ } else {
+ draw_button(dst, Minus, Rect(Point(0, 0), get_w(), get_h()));
+ }
return;
}
- draw_button(dst, Minus, Rect(Point(0, 0), get_w(), Size));
- draw_button(dst, Plus, Rect(Point(0, get_h() - Size), get_w(), Size));
+ draw_button(dst, Minus, Rect(Point(0, 0), get_w(), buttonsize_));
+ draw_button(dst, Plus, Rect(Point(0, get_h() - buttonsize_), get_w(), buttonsize_));
draw_button
(dst, Knob, Rect(Point(0, knobpos - knobsize / 2), get_w(), knobsize));
- assert(Size + knobsize / 2 <= knobpos);
+ assert(buttonsize_ + knobsize / 2 <= knobpos);
draw_area
(dst,
MinusPage,
- Rect(Point(0, Size), get_w(), knobpos - Size - knobsize / 2));
- assert
- (knobpos + knobsize / 2 + Size <=
- static_cast<uint32_t>(get_h()));
+ Rect(Point(0, buttonsize_), get_w(), knobpos - buttonsize_ - knobsize / 2));
+ assert(knobpos + knobsize / 2 + buttonsize_ <= static_cast<uint32_t>(get_h()));
draw_area
(dst,
PlusPage,
Rect
(Point(0, knobpos + knobsize / 2),
- get_w(), get_h() - knobpos - knobsize / 2 - Size));
+ get_w(), get_h() - knobpos - knobsize / 2 - buttonsize_));
}
}
@@ -461,4 +478,12 @@
return true;
}
+void Scrollbar::layout() {
+ if ((2 * kSize + get_knob_size()) > static_cast<uint32_t>((horizontal_ ? get_w() : get_h()))) {
+ buttonsize_ = kSize / 2;
+ } else {
+ buttonsize_ = kSize;
+ }
}
+
+} // namespace UI
=== modified file 'src/ui_basic/scrollbar.h'
--- src/ui_basic/scrollbar.h 2016-02-03 18:09:15 +0000
+++ src/ui_basic/scrollbar.h 2016-03-25 17:49:07 +0000
@@ -39,11 +39,9 @@
PlusPage
};
- enum {
- ///< default width for vertical scrollbars,
- // height for horizontal scrollbar
- Size = 24,
- };
+ /// default width for vertical scrollbars,
+ /// or height for horizontal scrollbars
+ static constexpr int kSize = 24;
public:
Scrollbar
@@ -68,6 +66,8 @@
void set_force_draw(bool const t) {force_draw_ = t;}
+ void layout() override;
+
private:
Area get_area_for_point(int32_t x, int32_t y);
uint32_t get_knob_pos();
@@ -92,6 +92,7 @@
uint32_t pos_; ///< from 0 to range_ - 1
uint32_t singlestepsize_;
uint32_t pagesize_;
+ uint32_t buttonsize_;
uint32_t steps_;
Area pressed_; ///< area that the user clicked on (None if mouse is up)
=== modified file 'src/ui_basic/table.cc'
--- src/ui_basic/table.cc 2016-03-10 15:00:32 +0000
+++ src/ui_basic/table.cc 2016-03-25 17:49:07 +0000
@@ -130,8 +130,8 @@
scrollbar_ =
new Scrollbar
(get_parent(),
- get_x() + get_w() - 24, get_y() + headerheight_,
- 24, get_h() - headerheight_,
+ get_x() + get_w() - Scrollbar::kSize, get_y() + headerheight_,
+ Scrollbar::kSize, get_h() - headerheight_,
false);
scrollbar_->moved.connect(boost::bind(&Table::set_scrollpos, this, _1));
scrollbar_->set_steps(1);
=== modified file 'src/ui_fsmenu/mapselect.cc'
--- src/ui_fsmenu/mapselect.cc 2016-02-18 18:27:52 +0000
+++ src/ui_fsmenu/mapselect.cc 2016-03-25 17:49:07 +0000
@@ -51,7 +51,7 @@
table_(this, tablex_, tabley_, tablew_, tableh_, false),
map_details_(this, right_column_x_, tabley_,
- get_right_column_w(right_column_x_ + indent_),
+ get_right_column_w(right_column_x_),
tableh_ - buth_ - 4 * padding_),
basedir_("maps"),
=== modified file 'src/ui_fsmenu/options.cc'
--- src/ui_fsmenu/options.cc 2016-02-16 09:22:53 +0000
+++ src/ui_fsmenu/options.cc 2016-03-25 17:49:07 +0000
@@ -217,7 +217,7 @@
// Language options
label_language_(&box_language_, _("Language"), UI::Align::kLeft),
language_list_(&box_language_, 0, 0, column_width_ / 2,
- get_inner_h() - tab_panel_y_ - buth_ - hmargin_ - 5 * padding_,
+ get_inner_h() - tab_panel_y_ - 2 * buth_ - hmargin_ - 5 * padding_,
true),
os_(opt)
=== modified file 'src/wui/mapdetails.cc'
--- src/wui/mapdetails.cc 2016-02-08 20:19:46 +0000
+++ src/wui/mapdetails.cc 2016-03-25 17:49:07 +0000
@@ -60,7 +60,7 @@
descr_box_(&main_box_, 0, 0, UI::Box::Horizontal,
max_x_, 6 * labelh_ + padding_, padding_ / 2),
descr_label_(&main_box_, 0, 0, max_x_, labelh_, ""),
- descr_(&descr_box_, 0, 0, max_x_ - indent_, 5 * labelh_, "")
+ descr_(&descr_box_, 0, 0, max_x_ - indent_ - 1, 5 * labelh_, "") // -1 to prevent cropping of scrollbar
{
suggested_teams_box_ = new UI::SuggestedTeamsBox(this, 0, 0, UI::Box::Vertical,
padding_, indent_, labelh_, max_x_, 4 * labelh_);
=== modified file 'src/wui/multiplayersetupgroup.cc'
--- src/wui/multiplayersetupgroup.cc 2016-03-08 07:24:38 +0000
+++ src/wui/multiplayersetupgroup.cc 2016-03-25 17:49:07 +0000
@@ -78,7 +78,7 @@
{
set_size(w, h);
name = new UI::Textarea
- (this, 0, 0, w - h - UI::Scrollbar::Size * 11 / 5, h);
+ (this, 0, 0, w - h - UI::Scrollbar::kSize * 11 / 5, h);
add(name, UI::Align::kHCenter);
// Either Button if changeable OR text if not
if (id == settings->settings().usernum) { // Our Client
@@ -417,15 +417,15 @@
labels.push_back
(new UI::Textarea
(this,
- UI::Scrollbar::Size * 6 / 5, buth / 3,
- w / 3 - buth - UI::Scrollbar::Size * 2, buth));
+ UI::Scrollbar::kSize * 6 / 5, buth / 3,
+ w / 3 - buth - UI::Scrollbar::kSize * 2, buth));
labels.back()->set_text(_("Client name"));
labels.back()->set_fontsize(small_font);
labels.push_back
(new UI::Textarea
(this,
- w / 3 - buth - UI::Scrollbar::Size * 6 / 5, buth / 3,
+ w / 3 - buth - UI::Scrollbar::kSize * 6 / 5, buth / 3,
buth * 2, buth));
labels.back()->set_text(_("Role"));
labels.back()->set_fontsize(small_font);
Follow ups
