← Back to team overview

widelands-dev team mailing list archive

Re: [Merge] lp:~widelands-dev/widelands/bug-1530124 into lp:widelands

 

Added some comments.

IMO the best way to solve the tabs issue is to run clang-format over the whole codebase and remove the conflicting codecheck rules.

I just tend not to run clang-format ATM, because it makes the diff big.

Diff comments:

> 
> === modified file 'src/graphic/text_layout.cc'
> --- src/graphic/text_layout.cc	2016-01-28 21:27:04 +0000
> +++ src/graphic/text_layout.cc	2016-01-29 17:34:45 +0000
> @@ -59,6 +69,16 @@
>  	return as_aligned(txt, UI::Align::kLeft, size, clr);
>  }
>  
> +std::string as_editorfont(const std::string& text, int ptsize, const RGBColor& clr) {
> +	// UI Text is always bold due to historic reasons
> +	static boost::format
> +			f("<rt keep_spaces=1><p><font face=serif size=%i bold=1 shadow=1 color=%s>%s</font></p></rt>");

I copied the code from alread existing format functions like as_uifont, which work just fine.

> +	f % ptsize;
> +	f % clr.hex_value();
> +	f % richtext_escape(text);
> +	return f.str();
> +}
> +
>  std::string as_aligned(const std::string & txt, UI::Align align, int ptsize, const RGBColor& clr) {
>  	std::string alignment = "left";
>  	if ((align & UI::Align::kHorizontal) == UI::Align::kRight) {
> 
> === modified file 'src/graphic/wordwrap.cc'
> --- src/graphic/wordwrap.cc	2016-01-28 21:27:04 +0000
> +++ src/graphic/wordwrap.cc	2016-01-29 17:34:45 +0000
> @@ -33,32 +33,6 @@
>  #include "graphic/rendertarget.h"
>  #include "graphic/text/bidi.h"
>  
> -namespace {
> -std::string as_editorfont(const std::string& text,
> -								  int ptsize = UI_FONT_SIZE_SMALL,
> -								  const RGBColor& clr = UI_FONT_CLR_FG) {
> -	// UI Text is always bold due to historic reasons

I don't know who first made that comment and why it's bold due to historic reasons. I wasn't around when that decision was made - not making it bold would make it harder to read though, especially in the ui_fsmenu screens. We would probably need new backgrounds.

I don't want to redesign anything here for Build 19 anyway - let's finish the font renderer switchover first.

> -	static boost::format
> -			f("<rt keep_spaces=1><p><font face=serif size=%i bold=1 shadow=1 color=%s>%s</font></p></rt>");
> -	f % ptsize;
> -	f % clr.hex_value();
> -	f % richtext_escape(text);
> -	return f.str();
> -}
> -
> -// This is inefficient; only call when we need the exact width.
> -uint32_t text_width(const std::string& text, int ptsize) {
> -	return UI::g_fh1->render(as_editorfont(text, ptsize - UI::g_fh1->fontset().size_offset()))->width();
> -}
> -
> -// This is inefficient; only call when we need the exact height.
> -uint32_t text_height(const std::string& text, int ptsize) {
> -	return UI::g_fh1->render(as_editorfont(text.empty() ? "." : text,
> -														ptsize - UI::g_fh1->fontset().size_offset()))->height();
> -}
> -
> -} // namespace
> -
>  namespace UI {
>  
>  /**
> 
> === modified file 'src/ui_basic/multilinetextarea.cc'
> --- src/ui_basic/multilinetextarea.cc	2016-01-28 21:27:04 +0000
> +++ src/ui_basic/multilinetextarea.cc	2016-01-29 17:34:45 +0000
> @@ -87,8 +87,9 @@
>  	for (int i = 0; i < 2; ++i) {
>  		if (m_text.compare(0, 3, "<rt")) {
>  			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),
> +			std::string text_to_render = richtext_escape(m_text);

Good idea - will do :)

> +			boost::replace_all(text_to_render, "\n", "<br>");
> +			const Image* text_im = UI::g_fh1->render(as_uifont(text_to_render, m_style.font->size(), m_style.fg),
>  																  get_eff_w() - 2 * RICHTEXT_MARGIN);
>  			height = text_im->height();
>  		} else {


-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-1530124/+merge/284356
Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1530124.


References