Merge lp:~widelands-dev/widelands/bug-1687043-multiline-edit into lp:widelands

[Notabilis]

Fixed heavy memory leak in the WordWrap class. The problem was loading the font on every change without freeing it.
Also did some refactoring so the font is only loaded once per class. This fixes a noticeable slowdown while typing in multi-line edit boxes.

[Klaus Halfmann (klaus-halfmann)]

Looks fine for me, will compile this and check some Mutiline text fields....

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 2128. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/227161415.
Appveyor build 1963. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1687043_multiline_edit-1963.

[GunChleoc (gunchleoc)]

Code LGTM too :)

review: Approve

[Klaus Halfmann (klaus-halfmann)]

Multilineedit fields are ok (even with German Umauts),
word wrap is a bit early. Lets go.

@bunnybot merge

review: Approve (compile, test, regressiontest)

=== modified file 'src/graphic/wordwrap.cc'
--- src/graphic/wordwrap.cc	2017-03-25 15:32:49 +0000
+++ src/graphic/wordwrap.cc	2017-04-29 15:04:08 +0000
@@ -25,7 +25,6 @@
 
 #include <SDL_ttf.h>
 #include <boost/format.hpp>
-#include <unicode/uchar.h>
 #include <unicode/unistr.h>
 
 #include "base/log.h"
@@ -35,39 +34,13 @@
 #include "graphic/rendertarget.h"
 #include "graphic/text/bidi.h"
 #include "graphic/text/font_io.h"
-#include "graphic/text/sdl_ttf_font.h"
 #include "graphic/text_layout.h"
 
-namespace {
-
-/**
- * Get a width estimate for text wrapping.
- */
-uint32_t quick_width(const UChar& c, int ptsize) {
-	// Editor font is sans bold.
-	RT::IFont* font = RT::load_font(UI::g_fh1->fontset()->sans_bold(), ptsize);
-	int result = 0;
-	TTF_GlyphMetrics(font->get_ttf_font(), c, nullptr, nullptr, nullptr, nullptr, &result);
-	return result;
-}
-
-uint32_t quick_width(const std::string& text, int ptsize) {
-	int result = 0;
-	const icu::UnicodeString parseme(text.c_str(), "UTF-8");
-	for (int i = 0; i < parseme.length(); ++i) {
-		UChar c = parseme.charAt(i);
-		if (!i18n::is_diacritic(c)) {
-			result += quick_width(c, ptsize);
-		}
-	}
-	return result;
-}
-}
-
 namespace UI {
 
 WordWrap::WordWrap(int fontsize, const RGBColor& color, uint32_t gwrapwidth)
-   : draw_caret_(false), fontsize_(fontsize), color_(color) {
+   : draw_caret_(false), fontsize_(fontsize), color_(color),
+     font_(RT::load_font(UI::g_fh1->fontset()->sans_bold(), fontsize_)) {
 	wrapwidth_ = gwrapwidth;
 
 	if (wrapwidth_ < std::numeric_limits<uint32_t>::max()) {
@@ -101,7 +74,7 @@
 	lines_.clear();
 
 	std::string::size_type line_start = 0;
-	uint32_t margin = quick_width(0x2003, fontsize_);  // Em space
+	uint32_t margin = quick_width(0x2003);  // Em space
 
 	while (line_start <= text.size()) {
 		std::string::size_type next_line_start;
@@ -209,7 +182,7 @@
 		// Diacritics do not add to the line width
 		if (!i18n::is_diacritic(c)) {
 			// This only estimates the width
-			line_width += quick_width(c, fontsize_);
+			line_width += quick_width(c);
 		}
 		unicode_line += c;
 	}
@@ -247,7 +220,7 @@
 bool WordWrap::line_fits(const std::string& text, uint32_t safety_margin) const {
 	// calc_width_for_wrapping is fast, but it will underestimate the width.
 	// So, we test again with text_width to make sure that the line really fits.
-	return quick_width(i18n::make_ligatures(text.c_str()), fontsize_) <=
+	return quick_width(i18n::make_ligatures(text.c_str())) <=
 	          wrapwidth_ - safety_margin &&
 	       text_width(text, fontsize_) <= wrapwidth_ - safety_margin;
 }
@@ -361,4 +334,25 @@
 	}
 }
 
+/**
+ * Get a width estimate for text wrapping.
+ */
+uint32_t WordWrap::quick_width(const UChar& c) const {
+	int result = 0;
+	TTF_GlyphMetrics(font_->get_ttf_font(), c, nullptr, nullptr, nullptr, nullptr, &result);
+	return result;
+}
+
+uint32_t WordWrap::quick_width(const std::string& text) const {
+	int result = 0;
+	const icu::UnicodeString parseme(text.c_str(), "UTF-8");
+	for (int i = 0; i < parseme.length(); ++i) {
+		UChar c = parseme.charAt(i);
+		if (!i18n::is_diacritic(c)) {
+			result += quick_width(c);
+		}
+	}
+	return result;
+}
+
 }  // namespace UI

=== modified file 'src/graphic/wordwrap.h'
--- src/graphic/wordwrap.h	2017-03-25 15:32:49 +0000
+++ src/graphic/wordwrap.h	2017-04-29 15:04:08 +0000
@@ -19,13 +19,16 @@
 #ifndef WL_GRAPHIC_WORDWRAP_H
 #define WL_GRAPHIC_WORDWRAP_H
 
+#include <memory>
 #include <string>
+#include <unicode/uchar.h>
 #include <vector>
 
 #include "base/vector.h"
 #include "graphic/align.h"
 #include "graphic/color.h"
 #include "graphic/text_constants.h"
+#include "graphic/text/sdl_ttf_font.h"
 
 class RenderTarget;
 
@@ -79,13 +82,19 @@
 
 	bool line_fits(const std::string& text, uint32_t safety_margin) const;
 
+	uint32_t quick_width(const UChar& c) const;
+	uint32_t quick_width(const std::string& text) const;
+
 	uint32_t wrapwidth_;
 	bool draw_caret_;
 
 	// TODO(GunChleoc): We can tie these to constexpr once the old font renderer is gone.
-	int fontsize_;
+	const int fontsize_;
 	RGBColor color_;
 
+	// Editor font is sans bold.
+	std::unique_ptr<RT::IFont> font_;
+
 	std::vector<LineData> lines_;
 };