widelands-dev team mailing list archive

[GunChleoc <fios@xxxxxxxxxxxxxxxxxxx>]

GunChleoc has proposed merging lp:~widelands-dev/widelands/table_align into lp:widelands.

Requested reviews:
  Widelands Developers (widelands-dev)
Related bugs:
  Bug #1307356 in widelands: "Implement RTL support"
  https://bugs.launchpad.net/widelands/+bug/1307356

For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/table_align/+merge/279685

This is a bugfix for RTL languages.

When text is too long to fit a table cell, it is now cropped and positioned correctly. This will work for all language settings.

Also, if the table cell has a picture, it is shown on the right if the table cell is right-aligned.

I have attached a bogus savegame to the bug with a long name in Arabic script (I don't speak Arabic, so I hope it doesn't say anything offensive :P).
-- 
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/table_align into lp:widelands.

=== modified file 'src/graphic/text/bidi.cc'
--- src/graphic/text/bidi.cc	2015-10-11 15:00:53 +0000
+++ src/graphic/text/bidi.cc	2015-12-06 09:39:12 +0000
@@ -555,11 +555,12 @@
 namespace i18n {
 
 
-// True if a string does not contain Latin characters
-bool has_rtl_character(const char* input) {
+// True if a string does not contain Latin characters.
+// Checks for the first 'limit' characters maximum.
+bool has_rtl_character(const char* input, int32_t limit) {
 	bool result = false;
 	const icu::UnicodeString parseme(input);
-	for (int32_t i = 0; i < parseme.length(); ++i) {
+	for (int32_t i = 0; i < parseme.length() && i < limit; ++i) {
 		if (is_rtl_character(parseme.char32At(i))) {
 			result = true;
 			break;

=== modified file 'src/graphic/text/bidi.h'
--- src/graphic/text/bidi.h	2015-09-28 18:47:57 +0000
+++ src/graphic/text/bidi.h	2015-12-06 09:39:12 +0000
@@ -32,7 +32,7 @@
 	std::string make_ligatures(const char* input);
 	std::string line2bidi(const char* input);
 	std::vector<std::string> split_cjk_word(const char* input);
-	bool has_rtl_character(const char* input);
+	bool has_rtl_character(const char* input, int32_t limit = std::numeric_limits<int32_t>::max());
 	bool has_rtl_character(std::vector<std::string> input);
 	bool has_cjk_character(const char* input);
 	bool cannot_start_line(const UChar& c);

=== modified file 'src/ui_basic/table.cc'
--- src/ui_basic/table.cc	2015-11-21 11:34:10 +0000
+++ src/ui_basic/table.cc	2015-12-06 09:39:12 +0000
@@ -25,6 +25,7 @@
 #include "graphic/font_handler1.h"
 #include "graphic/graphic.h"
 #include "graphic/rendertarget.h"
+#include "graphic/text/bidi.h"
 #include "graphic/text/font_set.h"
 #include "graphic/text_constants.h"
 #include "graphic/text_layout.h"
@@ -310,6 +311,10 @@
 						}
 					}
 
+					if (alignment & Align_Right) {
+						draw_x += curw - blit_width;
+					}
+
 					dst.blitrect_scale(
 								// Center align if text is empty
 								Rect(draw_x,
@@ -328,12 +333,16 @@
 						} else {
 							draw_x = point.x + (curw - picw) / 2;
 						}
+					} else if (alignment & Align_Right) {
+						draw_x += curw - picw;
 					}
 					dst.blit(Point(draw_x, point.y + (lineheight - pich) / 2), entry_picture);
 				}
 				point.x += picw;
 			}
 
+			++picw; // A bit or margin between image and text
+
 			if (entry_string.empty()) {
 				curx += curw;
 				continue;
@@ -341,7 +350,7 @@
 			const Image* entry_text_im = UI::g_fh1->render(as_uifont(entry_string, m_fontsize));
 
 			if (alignment & Align_Right) {
-				point.x += curw - picw;
+				point.x += curw - 2 * picw;
 			} else if (alignment & Align_HCenter) {
 				point.x += (curw - picw) / 2;
 			}
@@ -352,8 +361,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)) {
+				// 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;
 		}