Notabilis has proposed merging
lp:~widelands-dev/widelands/bug-1687043-multiline-edit into lp:widelands.
Requested reviews:
Widelands Developers (widelands-dev)
Related bugs:
Bug #1687043 in widelands: "Memory leak in Multilineeditbox"
https://bugs.launchpad.net/widelands/+bug/1687043
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/bug-1687043-multiline-edit/+merge/323428
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.
--
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/bug-1687043-multiline-edit into lp:widelands.
=== 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_;
};
_______________________________________________
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
