← Back to team overview

widelands-dev team mailing list archive

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

 


Diff comments:

> 
> === modified file 'src/ui_basic/table.cc'
> --- src/ui_basic/table.cc	2015-12-31 08:36:41 +0000
> +++ src/ui_basic/table.cc	2016-01-06 12:13:29 +0000
> @@ -365,8 +374,25 @@
>  				text_width = text_width + m_scrollbar->get_w();
>  			}
>  			UI::correct_for_align(alignment, text_width, entry_text_im->height(), &point);
> -			// Crop to column width
> -			dst.blitrect(point, entry_text_im, Rect(0, 0, curw - picw, lineheight));
> +
> +			// Crop to column width while blitting
> +			if (((static_cast<int32_t>(curw) + picw) < text_width)) {

What I am suggestion is to always use int when overflows is unlikely. Reason is that errors with arithmetic between signed and unsigned are way more often than overflow errors. This is also coded in the Google c++ style guide here. See here for an explanation https://google.github.io/styleguide/cppguide.html#Integer_Types , On Unsigned Types.

> +				// Fix positioning for BiDi languages.
> +				if (UI::g_fh1->fontset().is_rtl()) {
> +					point.x = alignment & Align_Right ? curx : curx + picw;
> +				}
> +				// We want this always on, e.g. for mixed language savegame filenames
> +				if (i18n::has_rtl_character(entry_string.c_str(), 20)) { // Restrict check for efficiency
> +					dst.blitrect(point,
> +									 entry_text_im,
> +									 Rect(text_width - curw + picw, 0, text_width, lineheight));
> +				}
> +				else {
> +					dst.blitrect(point, entry_text_im, Rect(0, 0, curw - picw, lineheight));
> +				}
> +			} else {
> +				dst.blitrect(point, entry_text_im, Rect(0, 0, curw - picw, lineheight));
> +			}
>  			curx += curw;
>  		}
>  


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


References