Merge lp:~widelands-dev/widelands/multiline_textarea into lp:widelands

[GunChleoc]

MultilineTextarea now uses the new font renderer directly instead of WordWrap.

MultilineTextarea now uses the new font renderer directly instead of WordWrap.

Removed the Impl pattern, but we still need to use Richtext, because the new font handler still has some bugs with richtext strings.

This also fixes the remaining superfluous scrollbar button with Gaelic Spinboxes in the Options window.

[bunnybot (widelandsofficial)]

Hi, I am bunnybot (https://github.com/widelands/bunnybot).

I am keeping the source branch lp:~widelands-dev/widelands/multiline_textarea mirrored to https://github.com/widelands/widelands/tree/_widelands_dev_widelands_multiline_textarea

You can give me commands by starting a line with @bunnybot <command>. I understand:
 merge: Merges the source branch into the target branch, closing the merge proposal. I will use the proposed commit message if it is set.

[kaputtnik (franku)]

Don't know if this is related to richtext, but some fonts are displayed bigger than other fonts. In result the scrollbar button is shown when the font size is bigger. This applies to most RTL languages.

There are several failures in console like:

No corresponding locale found - trying to set it via LANGUAGE=si, LANG=si
Font set 'défaut' for locale 'fr' does not exist; using default instead.
Font set 'défaut' for locale 'fr' does not exist; using default instead.

Don't know Language "si" locale "fr" but with this settings most strings are empty (f.e. no button labels anymore).

I am wondering also about the spelling of the word 'défaut'... the circumflex on the e and missing 'l'. I think this should be 'default' instead of 'défaut'?

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 371. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/104411863.
Appveyor build 278. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_multiline_textarea-278.

[GunChleoc (gunchleoc)]

"défaut" is a screwup by a French translator and I have already fixed it on Transifex.

I know that the scrollbar shows up when the font size is bigger sometimes, this is expected behaviour and not related to this branch. All the screens that show this need to be switched over to Box layout. This will mean a lot of code, so I am planning to fix this up for build 20. We need the bigger font size though, because some fonts show up so small that they are unreadable.

[wl-zocker (wl-zocker)]

Does this "default" has to be translatable at all? I had also translated it to German (it's fixed).

[TiborB (tiborb95)]

Is this supposed to fix the bug-1532279 - because it is linked here but marked as fix committed. Anyway, the bug is no longer here and code looks good to me.

[GunChleoc (gunchleoc)]

There was one remaining instance of that bug for Gaelic language only in the options window, which has now disappeared.

The reason for this is to speed up rendering - you should notice that map descriptions now load faster again.

@wl-zocker: it is "translatable" in the sense that you use it to pick your font set - instructions are on Transifex, but nobody seems to read them. Unfortunately, it is not possible to add comments to JSON files.

[kaputtnik (franku)]

> The reason for this is to speed up rendering - you should notice that map descriptions now load faster again.

Yes indeed... works well :-)

[GunChleoc (gunchleoc)]

@bunnybot merge

[SirVer (sirver)]

Sorry, I am late to the review-party. I try to do one code review per day these days - not more. But I do not always manage.

Just one minor style fix, I suggest to just do it in any other branch you are working on.

Otherwise this is an awesome change. The font handler is really coming together now.

review: Approve

[GunChleoc (gunchleoc)]

Thanks - bookmarked for fixing :)

=== modified file 'src/graphic/text/rt_render.cc'
--- src/graphic/text/rt_render.cc	2016-01-18 19:35:25 +0000
+++ src/graphic/text/rt_render.cc	2016-01-26 07:50:51 +0000
@@ -824,11 +824,11 @@
 		if (a.has("align")) {
 			const std::string align = a["align"].get_string();
 			if (align == "right") {
+				m_ns.halign = UI::Align::Align_Right;
+			} else if (align == "center" || align == "middle") {
+				m_ns.halign = UI::Align::Align_Center;
+			} else {
 				m_ns.halign = UI::Align::Align_Left;
-			} else if (align == "center" || align == "middle") {
-				m_ns.halign = UI::Align::Align_Center;
-			} else {
-				m_ns.halign = UI::Align::Align_Right;
 			}
 		}
 		m_ns.halign = mirror_alignment(m_ns.halign);

=== modified file 'src/graphic/text_constants.h'
--- src/graphic/text_constants.h	2015-05-30 12:19:57 +0000
+++ src/graphic/text_constants.h	2016-01-26 07:50:51 +0000
@@ -24,7 +24,6 @@
 
 /// Font Sizes
 #define UI_FONT_SIZE_BIG        22
-#define UI_FONT_SIZE_PROSA      18
 #define UI_FONT_SIZE_SMALL      14
 #define UI_FONT_SIZE_ULTRASMALL 10
 
@@ -35,9 +34,6 @@
 #define UI_FONT_CLR_DISABLED RGBColor(127, 127, 127)
 #define UI_FONT_CLR_WARNING  RGBColor(255,  22,  22)
 
-/// Prosa font color
-#define PROSA_FONT_CLR_FG    RGBColor(255, 255,   0)
-
 /// Tooltip font color
 #define UI_FONT_TOOLTIP_CLR  RGBColor(255, 255,   0)
 

=== modified file 'src/graphic/text_layout.cc'
--- src/graphic/text_layout.cc	2015-12-13 18:32:28 +0000
+++ src/graphic/text_layout.cc	2016-01-26 07:50:51 +0000
@@ -54,12 +54,24 @@
 	f % txt;
 	return f.str();
 }
+
 std::string as_uifont(const std::string & txt, int size, const RGBColor& clr) {
+	return as_aligned(txt, UI::Align::Align_Left, size, clr);
+}
+
+std::string as_aligned(const std::string & txt, UI::Align align, int ptsize, const RGBColor& clr) {
+	std::string alignment = "left";
+	if ((align & UI::Align_Horizontal) == UI::Align::Align_Right) {
+		alignment = "right";
+	} else if ((align & UI::Align_Horizontal) == UI::Align::Align_HCenter) {
+		alignment = "center";
+	}
+
 	// UI Text is always bold due to historic reasons
 	static boost::format
-			f("<rt><p><font face=serif size=%i bold=1 shadow=1 color=%s>%s</font></p></rt>");
-
-	f % size;
+			f("<rt><p align=%s><font face=serif size=%i bold=1 shadow=1 color=%s>%s</font></p></rt>");
+	f % alignment;
+	f % ptsize;
 	f % clr.hex_value();
 	f % txt;
 	return f.str();

=== modified file 'src/graphic/text_layout.h'
--- src/graphic/text_layout.h	2015-10-09 08:35:26 +0000
+++ src/graphic/text_layout.h	2016-01-26 07:50:51 +0000
@@ -23,6 +23,7 @@
 #include <string>
 #include <unicode/uchar.h>
 
+#include "graphic/align.h"
 #include "graphic/font.h"
 #include "graphic/color.h"
 #include "graphic/text_constants.h"
@@ -45,6 +46,9 @@
  */
 std::string as_uifont
 	(const std::string&, int ptsize = UI_FONT_SIZE_SMALL, const RGBColor& clr = UI_FONT_CLR_FG);
+std::string as_aligned(const std::string & txt, UI::Align align, int ptsize = UI_FONT_SIZE_SMALL,
+							  const RGBColor& clr = UI_FONT_CLR_FG);
+
 std::string as_tooltip(const std::string&);
 std::string as_waresinfo(const std::string&);
 std::string as_window_title(const std::string&);

=== modified file 'src/graphic/wordwrap.cc'
--- src/graphic/wordwrap.cc	2016-01-23 09:18:49 +0000
+++ src/graphic/wordwrap.cc	2016-01-26 07:50:51 +0000
@@ -66,12 +66,12 @@
  * and a default-constructed text style.
  */
 WordWrap::WordWrap() :
-	m_wrapwidth(std::numeric_limits<uint32_t>::max()), m_draw_caret(false), mode_(WordWrap::Mode::kDisplay)
+	m_wrapwidth(std::numeric_limits<uint32_t>::max()), m_draw_caret(false)
 {
 }
 
 WordWrap::WordWrap(const TextStyle & style, uint32_t gwrapwidth) :
-	m_style(style), m_draw_caret(false), mode_(WordWrap::Mode::kDisplay)
+	m_style(style), m_draw_caret(false)
 {
 	m_wrapwidth = gwrapwidth;
 
@@ -112,9 +112,8 @@
  * Perform the wrapping computations for the given text and fill in
  * the private data containing the wrapped results.
  */
-void WordWrap::wrap(const std::string & text, WordWrap::Mode mode)
+void WordWrap::wrap(const std::string & text)
 {
-	mode_ = mode;
 	m_lines.clear();
 
 	std::string::size_type line_start = 0;
@@ -378,17 +377,13 @@
 		}
 
 		const Image* entry_text_im =
-				UI::g_fh1->render(mode_ == WordWrap::Mode::kDisplay ?
-											as_uifont(m_lines[line].text,
-														 m_style.font->size() - UI::g_fh1->fontset().size_offset(),
-														 m_style.fg) :
-											as_editorfont(m_lines[line].text,
-															  m_style.font->size() - UI::g_fh1->fontset().size_offset(),
-															  m_style.fg));
+				UI::g_fh1->render(as_editorfont(m_lines[line].text,
+														  m_style.font->size() - UI::g_fh1->fontset().size_offset(),
+														  m_style.fg));
 		UI::correct_for_align(alignment, entry_text_im->width(), fontheight, &point);
 		dst.blit(point, entry_text_im);
 
-		if (mode_ == WordWrap::Mode::kEditor && m_draw_caret && line == caretline) {
+		if (m_draw_caret && line == caretline) {
 			std::string line_to_caret = m_lines[line].text.substr(0, caretpos);
 			// TODO(GunChleoc): Arabic: Fix cursor position for BIDI text.
 			int caret_x = text_width(line_to_caret, m_style.font->size());

=== modified file 'src/graphic/wordwrap.h'
--- src/graphic/wordwrap.h	2015-12-13 09:09:21 +0000
+++ src/graphic/wordwrap.h	2016-01-26 07:50:51 +0000
@@ -33,11 +33,6 @@
  * Helper struct that provides word wrapping and related functionality.
  */
 struct WordWrap {
-	enum class Mode {
-		kDisplay,
-		kEditor
-	};
-
 	WordWrap();
 	WordWrap(const TextStyle & style, uint32_t wrapwidth = std::numeric_limits<uint32_t>::max());
 
@@ -46,7 +41,7 @@
 
 	uint32_t wrapwidth() const;
 
-	void wrap(const std::string & text, WordWrap::Mode mode = WordWrap::Mode::kDisplay);
+	void wrap(const std::string & text);
 
 	uint32_t width() const;
 	uint32_t height() const;
@@ -82,8 +77,6 @@
 	uint32_t m_wrapwidth;
 	bool m_draw_caret;
 
-	WordWrap::Mode mode_;
-
 	std::vector<LineData> m_lines;
 };
 

=== modified file 'src/ui_basic/messagebox.cc'
--- src/ui_basic/messagebox.cc	2015-12-13 09:46:58 +0000
+++ src/ui_basic/messagebox.cc	2016-01-26 07:50:51 +0000
@@ -21,6 +21,7 @@
 
 #include "base/i18n.h"
 #include "graphic/font_handler.h"
+#include "graphic/font_handler1.h"
 #include "graphic/graphic.h"
 #include "ui_basic/button.h"
 #include "ui_basic/multilinetextarea.h"
@@ -61,8 +62,8 @@
 		 text.c_str(), align);
 
 	uint32_t width, height;
-	std::string font = d->textarea->get_font_name();
-	int32_t fontsize = d->textarea->get_font_size();
+	std::string font = UI::g_fh1->fontset().serif();
+	int32_t fontsize = UI_FONT_SIZE_SMALL;
 
 	UI::g_fh->get_size(font, fontsize, text, width, height, maxwidth);
 	// stupid heuristic to avoid excessively long lines

=== modified file 'src/ui_basic/multilineeditbox.cc'
--- src/ui_basic/multilineeditbox.cc	2015-10-23 12:05:36 +0000
+++ src/ui_basic/multilineeditbox.cc	2016-01-26 07:50:51 +0000
@@ -566,7 +566,7 @@
 	ww.set_style(textstyle);
 	ww.set_wrapwidth(owner.get_w() - ms_scrollbar_w);
 
-	ww.wrap(text, WordWrap::Mode::kEditor);
+	ww.wrap(text);
 	ww_valid = true;
 
 	int32_t textheight = ww.height();

=== modified file 'src/ui_basic/multilinetextarea.cc'
--- src/ui_basic/multilinetextarea.cc	2015-10-09 09:30:12 +0000
+++ src/ui_basic/multilinetextarea.cc	2016-01-26 07:50:51 +0000
@@ -19,28 +19,18 @@
 
 #include "ui_basic/multilinetextarea.h"
 
+#include <boost/algorithm/string.hpp>
 #include <boost/bind.hpp>
 
-#include "graphic/font_handler.h"
 #include "graphic/font_handler1.h"
-#include "graphic/richtext.h"
+#include "graphic/rendertarget.h"
 #include "graphic/text/font_set.h"
 #include "graphic/text_constants.h"
-#include "graphic/text_layout.h"
-#include "graphic/wordwrap.h"
 
 namespace UI {
 
 static const uint32_t RICHTEXT_MARGIN = 2;
 
-struct MultilineTextarea::Impl {
-	bool isrichtext;
-	WordWrap ww;
-	RichText rt;
-
-	Impl() : isrichtext(false) {}
-};
-
 MultilineTextarea::MultilineTextarea
 	(Panel * const parent,
 	 const int32_t x, const int32_t y, const uint32_t w, const uint32_t h,
@@ -49,8 +39,9 @@
 	 const bool always_show_scrollbar)
 	:
 	Panel       (parent, x, y, w, h),
-	m(new Impl),
 	m_text      (text),
+	m_style(UI::TextStyle::ui_small()),
+	isrichtext(false),
 	m_scrollbar (this, get_w() - scrollbar_w(), 0, scrollbar_w(), h, false),
 	m_scrollmode(ScrollNormal)
 {
@@ -62,44 +53,18 @@
 
 	m_scrollbar.moved.connect(boost::bind(&MultilineTextarea::scrollpos_changed, this, _1));
 
-	UI::FontSet fontset = UI::g_fh1->fontset();
-	m_scrollbar.set_singlestepsize(g_fh->get_fontheight(fontset.serif(), UI_FONT_SIZE_SMALL));
-	m_scrollbar.set_pagesize(h - 2 * g_fh->get_fontheight(fontset.serif(), UI_FONT_SIZE_BIG));
+	m_scrollbar.set_singlestepsize(UI::g_fh1->render(as_uifont(".", UI_FONT_SIZE_SMALL))->height());
+	m_scrollbar.set_pagesize(h - 2 * UI::g_fh1->render(as_uifont(".", UI_FONT_SIZE_BIG))->height());
 	m_scrollbar.set_steps(1);
 	m_scrollbar.set_force_draw(always_show_scrollbar);
 
-	set_font(fontset.serif(), UI_FONT_SIZE_SMALL, UI_FONT_CLR_FG);
+	recompute();
 
 	update(0, 0, get_eff_w(), get_h());
 }
 
 
 /**
- * Free allocated resources
-*/
-MultilineTextarea::~MultilineTextarea()
-{
-}
-
-/**
- * Change the font used for non-richtext text.
- */
-void MultilineTextarea::set_font(std::string name, int32_t size, RGBColor fg)
-{
-	m_fontname = name;
-	m_fontsize = size;
-	m_fcolor = fg;
-
-	TextStyle style;
-	style.font = Font::get(m_fontname, m_fontsize);
-	style.fg = m_fcolor;
-	style.bold = true; // for historic reasons
-
-	m->ww.set_style(style);
-	recompute();
-}
-
-/**
  * Replace the current text with a new one.
  * Fix up scrolling state if necessary.
  */
@@ -110,7 +75,7 @@
 }
 
 /**
- * Recompute the word wrapping or rich-text layouting,
+ * Recompute the text rendering or rich-text layouting,
  * and adjust scrollbar settings accordingly.
  */
 void MultilineTextarea::recompute()
@@ -121,15 +86,16 @@
 	bool scroolbar_was_enabled = m_scrollbar.is_enabled();
 	for (int i = 0; i < 2; ++i) {
 		if (m_text.compare(0, 3, "<rt")) {
-			m->isrichtext = false;
-			m->ww.set_wrapwidth(get_eff_w());
-			m->ww.wrap(m_text);
-			height = m->ww.height();
+			isrichtext = false;
+			boost::replace_all(m_text, "\n", "<br>");
+			const Image* text_im = UI::g_fh1->render(as_uifont(m_text, m_style.font->size(), m_style.fg),
+																  get_eff_w() - 2 * RICHTEXT_MARGIN);
+			height = text_im->height();
 		} else {
-			m->isrichtext = true;
-			m->rt.set_width(get_eff_w() - 2 * RICHTEXT_MARGIN);
-			m->rt.parse(m_text);
-			height = m->rt.height() + 2 * RICHTEXT_MARGIN;
+			isrichtext = true;
+			rt.set_width(get_eff_w() - 2 * RICHTEXT_MARGIN);
+			rt.parse(m_text);
+			height = rt.height() + 2 * RICHTEXT_MARGIN;
 		}
 
 		bool setbottom = false;
@@ -183,22 +149,37 @@
  */
 void MultilineTextarea::draw(RenderTarget & dst)
 {
-	if (m->isrichtext) {
-		m->rt.draw(dst, Point(RICHTEXT_MARGIN, RICHTEXT_MARGIN - m_scrollbar.get_scrollpos()));
+	if (isrichtext) {
+		rt.draw(dst, Point(RICHTEXT_MARGIN, RICHTEXT_MARGIN - m_scrollbar.get_scrollpos()));
 	} else {
-		int32_t anchor = 0;
-
-		switch (m_align & Align_Horizontal) {
-		case Align_HCenter:
-			anchor = get_eff_w() / 2;
-			break;
-		case Align_Right:
-			anchor = get_eff_w();
-			break;
-		default:
-			break;
+		const Image* text_im = UI::g_fh1->render(as_aligned(m_text, m_align, m_style.font->size(), m_style.fg),
+															  get_eff_w() - 2 * RICHTEXT_MARGIN);
+
+		uint32_t blit_width = std::min(text_im->width(), static_cast<int>(get_eff_w()));
+		uint32_t blit_height = std::min(text_im->height(), static_cast<int>(get_inner_h()));
+
+		if (blit_width > 0 && blit_height > 0) {
+			int32_t anchor = 0;
+			Align alignment = mirror_alignment(m_align);
+			switch (alignment & Align_Horizontal) {
+			case Align_HCenter:
+				anchor = (get_eff_w() - blit_width) / 2;
+				break;
+			case Align_Right:
+				anchor = get_eff_w() - blit_width - RICHTEXT_MARGIN;
+				break;
+			default:
+				anchor = RICHTEXT_MARGIN;
+				break;
+			}
+
+			dst.blitrect_scale(
+				Rect(anchor, 0, blit_width, blit_height),
+				text_im,
+				Rect(0, m_scrollbar.get_scrollpos(), blit_width, blit_height),
+				1.,
+				BlendMode::UseAlpha);
 		}
-		m->ww.draw(dst, Point(anchor, -m_scrollbar.get_scrollpos()), m_align);
 	}
 }
 

=== modified file 'src/ui_basic/multilinetextarea.h'
--- src/ui_basic/multilinetextarea.h	2015-09-27 09:03:23 +0000
+++ src/ui_basic/multilinetextarea.h	2016-01-26 07:50:51 +0000
@@ -24,6 +24,8 @@
 
 #include "graphic/align.h"
 #include "graphic/color.h"
+#include "graphic/richtext.h"
+#include "graphic/text_layout.h"
 #include "ui_basic/panel.h"
 #include "ui_basic/scrollbar.h"
 
@@ -46,7 +48,6 @@
 		 const std::string & text         = std::string(),
 		 const Align                      = Align_Left,
 		 const bool always_show_scrollbar = false);
-	~MultilineTextarea();
 
 	const std::string & get_text() const {return m_text;}
 	ScrollMode get_scrollmode() const {return m_scrollmode;}
@@ -54,12 +55,10 @@
 	void set_text(const std::string &);
 	void set_scrollmode(ScrollMode mode);
 
-	void set_font(std::string name, int32_t size, RGBColor fg);
-
 	uint32_t scrollbar_w() const {return 24;}
 	uint32_t get_eff_w() const {return m_scrollbar.is_enabled() ? get_w() - scrollbar_w() : get_w();}
 
-	void set_color(RGBColor fg) {m_fcolor = fg;}
+	void set_color(RGBColor fg) {m_style.fg = fg;}
 
 	// Drawing and event handlers
 	void draw(RenderTarget &) override;
@@ -67,29 +66,22 @@
 	bool handle_mousewheel(uint32_t which, int32_t x, int32_t y) override;
 	void scroll_to_top();
 
-	const char *  get_font_name() {return m_fontname.c_str();}
-	int32_t       get_font_size() {return m_fontsize;}
-	RGBColor &    get_font_clr () {return m_fcolor;}
+protected:
+	void layout() override;
 
 private:
-	struct Impl;
-
-	std::unique_ptr<Impl> m;
-
 	void recompute();
 	void scrollpos_changed(int32_t pixels);
 
 	std::string m_text;
+	UI::TextStyle m_style;
+	Align m_align;
+
+	bool isrichtext;
+	RichText rt;
+
 	Scrollbar   m_scrollbar;
 	ScrollMode  m_scrollmode;
-
-protected:
-	void layout() override;
-
-	Align        m_align;
-	std::string  m_fontname;
-	int32_t  m_fontsize;
-	RGBColor m_fcolor;
 };
 
 }

=== modified file 'src/ui_fsmenu/fileview.cc'
--- src/ui_fsmenu/fileview.cc	2015-08-06 17:14:34 +0000
+++ src/ui_fsmenu/fileview.cc	2016-01-26 07:50:51 +0000
@@ -77,8 +77,6 @@
 	title.set_font(ui_fn(), fs_big(), UI_FONT_CLR_FG);
 	title.set_pos
 		(Point((get_inner_w() - title.get_w()) / 2, get_h() * 167 / 1000));
-
-	textview.set_font(UI::g_fh1->fontset().serif(), UI_FONT_SIZE_PROSA, PROSA_FONT_CLR_FG);
 }
 
 void FullscreenMenuTextView::set_text(const std::string & text)
@@ -117,8 +115,6 @@
 	UI::UniqueWindow(&parent, "file_view", &reg, 0, 0, ""),
 	textview(this, 0, 0, 560, 240)
 {
-	textview.set_font(UI::g_fh1->fontset().serif(), UI_FONT_SIZE_PROSA, PROSA_FONT_CLR_FG);
-
 	set_inner_size(560, 240);
 
 	if (get_usedefaultpos())

=== modified file 'src/ui_fsmenu/launch_mpg.cc'
--- src/ui_fsmenu/launch_mpg.cc	2016-01-23 16:00:40 +0000
+++ src/ui_fsmenu/launch_mpg.cc	2016-01-26 07:50:51 +0000
@@ -209,8 +209,6 @@
 	m_clients    .set_font(m_fn, m_fs, RGBColor(0, 255, 0));
 	m_players    .set_font(m_fn, m_fs, RGBColor(0, 255, 0));
 	m_map        .set_font(m_fn, m_fs, RGBColor(0, 255, 0));
-	m_client_info.set_font(m_fn, m_fs, UI_FONT_CLR_FG);
-	m_map_info   .set_font(m_fn, m_fs, UI_FONT_CLR_FG);
 
 	m_mapname .set_text(_("(no map)"));
 	m_map_info.set_text(_("The host has not yet selected a map or saved game."));
@@ -480,13 +478,13 @@
 
 	if (settings.mapfilename != m_filename_proof) {
 		if (!g_fs->file_exists(settings.mapfilename)) {
-			m_client_info.set_font(m_fn, m_fs, UI_FONT_CLR_WARNING);
+			m_client_info.set_color(UI_FONT_CLR_WARNING);
 			m_client_info.set_text
 				(_("The selected file can not be found. If it is not automatically "
 				   "transferred to you, please write to the host about this problem."));
 		} else {
 			// Reset font color
-			m_client_info.set_font(m_fn, m_fs, UI_FONT_CLR_FG);
+			m_client_info.set_color(UI_FONT_CLR_FG);
 
 			// Update local nr of players - needed for the client UI
 			m_nr_players = settings.players.size();