Refactor:gui/internal: Add pointer and label when using function "view on map" (#631)

* Factorizing gui_internal_cmd_results_to_map() to also highlight points selected by command "View on map"

* Adding comments on displayed text for type_waypoint and type_route_end

* Adding comments

* Restoring zoom_to_rect in gui_internal_cmd_results_to_map()

* Making zoom_to_rect conditional, moving results_map preparation code to a dedicated function

* Moving "search_results" map preparation into a dedicated function

* Removing unused var

* Adding doxygen comments and disabling widget dealloc (causes segfault)

* Fixing segfault at next do_drawy() when adding pointer for view on map

* Reformatting labels to be displayed in a compact square area rather than all on a line

* Applying astyle to comply to ci_sanity_checks.sh

* Handling newlines in labels associated to element_circle

* Fixing hardcoded nblines

* Moving multi-line label processing to a dedicated function multiline_label_draw()

* Rework on arg naming to line-up with other simular functions. Minor update on doxygen comments

* Moving unsigned int sqrt calculation to utils

* Minor improvement of comment

* Fixing typo on comment

* Also applying size xslt processing to layer itemgra

This allows to also process the size of item_types="found_item", which is used by "View on map" and "Show results on map"

6 files changed, 277 insertions, 66 deletions

diff --git a/navit/graphics.c b/navit/graphics.c
index db94c9ef7..50913eeea 100644
--- a/navit/graphics.c
+++ b/navit/graphics.c
@@ -1459,30 +1459,6 @@ static int fowler(int dy, int dx) {
     }
     return 0;
 }
-static int int_sqrt(unsigned int n) {
-    unsigned int h, p= 0, q= 1, r= n;
-
-    /* avoid q rollover */
-    if(n >= (1<<(sizeof(n)*8-2))) {
-        q = 1<<(sizeof(n)*8-2);
-    } else {
-        while ( q <= n ) {
-            q <<= 2;
-        }
-        q >>= 2;
-    }
-
-    while ( q != 0 ) {
-        h = p + q;
-        p >>= 1;
-        if ( r >= h ) {
-            p += q;
-            r -= h;
-        }
-        q >>= 2;
-    }
-    return p;
-}
 
 struct draw_polyline_shape {
     int wi;
@@ -1530,9 +1506,9 @@ static void draw_shape(struct draw_polyline_context *ctx, struct point *pnt, int
     dys=shape->dy*shape->dy;
     lscales=lscale*lscale;
     if (dxs + dys > lscales)
-        l = int_sqrt(dxs+dys)*lscale;
+        l = uint_sqrt(dxs+dys)*lscale;
     else
-        l = int_sqrt((dxs+dys)*lscales);
+        l = uint_sqrt((dxs+dys)*lscales);
 
     shape->fow=fowler(-shape->dy, shape->dx);
     dbg(lvl_debug,"fow=%d",shape->fow);
@@ -1991,7 +1967,70 @@ static int limit_count(struct coord *c, int count) {
     return count;
 }
 
+/**
+ * @brief Draw a multi-line text next to a specified point @p pref
+ *
+ * @param gra The graphics instance on which to draw
+ * @param fg The graphics color to use to draw the text
+ * @param bg The graphics background color to use to draw the text
+ * @param font The font to use to draw the text
+ * @param pref The position to draw the text (draw at the right and vertically aligned relatively to this point)
+ * @param label The text to draw (may contain '\n' for multiline text, if so lines will be stacked vertically)
+ * @param line_spacing The delta between each line (set its value at to least the font text size, to be readable)
+ */
+static void multiline_label_draw(struct graphics *gra, struct graphics_gc *fg, struct graphics_gc *bg,
+                                 struct graphics_font *font, struct point pref, const char *label, int line_spacing) {
+
+    char *input_label=g_strdup(label);
+    char *label_lines[10];	/* Max 10 lines of text */
+    int label_nblines=0;
+    int label_linepos=0;
+    char *startline=input_label;
+    char *endline=startline;
+    while (endline && *endline!='\0') {
+        while (*endline!='\0' && *endline!='\n') { /* Search for new line */
+            endline=g_utf8_next_char(endline);
+        }
+        if (*endline=='\0')
+            endline=NULL;	/* This means we reached the end of string */
+        if (endline) /* Test if we got a new line character ('\n') */
+            *endline='\0';	/* Terminate string at line ('\n') and print this line */
+        label_lines[label_nblines++]=startline;
+        if (endline==NULL)	/* endline is NULL, this was the last line of the multi-line string */
+            break;
+        endline++;	/* No need for g_utf8_next_char() here, as we know '\n' is a single byte UTF-8 char */
+        startline=endline;	/* Start processing next line, by setting startline to its first character */
+    }
+    if (label_nblines>(sizeof(label_lines)/sizeof(char
+                       *))) {	/* Does label_nblines overflows the number of entries in array label_lines? */
+        dbg(lvl_warning,"Too many lines (%d) in label \"%s\", truncating to %lu", label_nblines, label,
+            sizeof(label_lines)/sizeof(char *));
+        label_nblines=sizeof(label_lines)/sizeof(char *);
+    }
+    /* Horizontally, we position the label next to the specified point (on the right handside) */
+    pref.x+=1;
+    /* Vertically, we center the text with respect to specified point */
+    pref.y-=(label_nblines*line_spacing)/2;
+
+    /* Parse all stored lines, and display them */
+    for (label_linepos=0; label_linepos<label_nblines; label_linepos++) {
+        gra->meth.draw_text(gra->priv, fg->priv, bg?bg->priv:NULL, font->priv, label_lines[label_linepos],
+                            &pref, 0x10000, 0);
+        pref.y+=line_spacing;
+    }
+    g_free(input_label);
+}
+
 
+/**
+ * @brief Draw a displayitem element
+ *
+ * This function will invoke the appropriate draw primitive depending on the type of the element to draw
+ *
+ * @brief di The displayitem to draw
+ * @brief dummy Unused
+ * @brief dc The display_context to use to draw items
+ */
 static void displayitem_draw(struct displayitem *di, void *dummy, struct display_context *dc) {
     int *width=g_alloca(sizeof(int)*dc->maxlen);
     struct point *pa=g_alloca(sizeof(struct point)*dc->maxlen);
@@ -2048,11 +2087,12 @@ static void displayitem_draw(struct displayitem *di, void *dummy, struct display
                         graphics_gc_set_foreground(gc_background, &e->u.circle.background_color);
                         dc->gc_background=gc_background;
                     }
-                    p.x=pa[0].x+3;
-                    p.y=pa[0].y+10;
-                    if (font)
-                        gra->meth.draw_text(gra->priv, gc->priv, gc_background?gc_background->priv:NULL, font->priv, di->label, &p, 0x10000, 0);
-                    else
+                    if (font) {
+                        /* Set p to the center of the circle */
+                        p.x=pa[0].x+(e->u.circle.radius/2);
+                        p.y=pa[0].y+(e->u.circle.radius/2);
+                        multiline_label_draw(gra, gc, gc_background, font, p, di->label, e->text_size+1);
+                    } else
                         dbg(lvl_error,"Failed to get font with size %d",e->text_size);
                 }
             }
diff --git a/navit/gui/internal/gui_internal.c b/navit/gui/internal/gui_internal.c
index 119d71dc2..a3df6c235 100644
--- a/navit/gui/internal/gui_internal.c
+++ b/navit/gui/internal/gui_internal.c
@@ -112,6 +112,7 @@ static struct gui_config_settings config_profiles[]= {
 };
 
 static void gui_internal_cmd_view_in_browser(struct gui_priv *this, struct widget *wm, void *data);
+static void gui_internal_prepare_search_results_map(struct gui_priv *this, struct widget *table, struct coord_rect *r);
 
 static int gui_internal_is_active_vehicle(struct gui_priv *this, struct vehicle *vehicle);
 
@@ -706,7 +707,12 @@ static void gui_internal_cmd_delete_bookmark(struct gui_priv *this, struct widge
 
 
 /**
- *  Get a utf-8 string, return the same prepared for case insensitive search. Result should be g_free()d after use.
+ * @brief Remove the case in a string
+ *
+ * @warning Result should be g_free()d after use.
+ *
+ * @param s The input utf-8 string
+ * @return An equivalent string prepared for case insensitive search
  */
 char *removecase(char *s) {
     char *r;
@@ -714,7 +720,21 @@ char *removecase(char *s) {
     return r;
 }
 
+/**
+ * @brief Apply the command "View on Map", centers the map on the selected point and highlight this point using
+ * type_found_item style
+ *
+ * @param this The GUI context
+ * @param wm The widget that points to this function as a callback
+ * @param data Private data provided during callback (unused)
+ */
 static void gui_internal_cmd_view_on_map(struct gui_priv *this, struct widget *wm, void *data) {
+
+    struct widget *w;
+    struct widget *wr;
+    struct widget *wi;
+    char *label;
+
     if (wm->item.type != type_none) {
         enum item_type type;
         if (wm->item.type < type_line)
@@ -725,6 +745,22 @@ static void gui_internal_cmd_view_on_map(struct gui_priv *this, struct widget *w
             type=type_selected_area;
         graphics_clear_selection(this->gra, NULL);
         graphics_add_selection(this->gra, &wm->item, type, NULL);
+    } else {
+        if (wm->item.priv_data)
+            label = wm->item.priv_data;	/* Use the label of the point to view on map */
+        else
+            label = g_strdup("");
+        w = gui_internal_widget_table_new(this, 0, 0);	/* Create a basic table */
+        gui_internal_widget_append(w,wr=gui_internal_widget_table_row_new(this,0));	/* In this table, add one row */
+        gui_internal_widget_append(wr,wi=gui_internal_box_new_with_label(this,0,
+                                         label));	/* That row contains a widget of type widget_box */
+        wi->name = label;	/* Use the label of the point to view on map */
+        wi->c.x=wm->c.x;	/* Use the coordinates of the point to place it on the map */
+        wi->c.y=wm->c.y;
+        gui_internal_prepare_search_results_map(this, w, NULL);
+        g_free(label);
+        wi->name = NULL;
+        gui_internal_widget_destroy(this, w);
     }
     navit_set_center(this->nav, &wm->c, 1);
     gui_internal_prune_menu(this, NULL);
@@ -854,29 +890,22 @@ static void gui_internal_cmd_view_in_browser(struct gui_priv *this, struct widge
     }
 }
 
-
-/*
- * @brief Transfers search results to a map.
+/**
+ * @brief Get the search result map (and create it if it does not exist)
  *
- * @param this The graphics context.
- * @param wm called widget.
- * @param data event data (pointer to the table widget containing results, or NULL to clean the result map without adding any new data).
+ * @param this The GUI context
+ *
+ * @return A pointer to the map named "search_results" or NULL if there wasa failure
  */
-static void gui_internal_cmd_results_to_map(struct gui_priv *this, struct widget *wm, void *data) {
-    struct widget *w;
+static struct map *get_search_results_map(struct gui_priv *this) {
+
     struct mapset *ms;
     struct map *map;
-    struct map_rect *mr;
-    struct item *item;
-    GList *l;
-    struct coord_rect r;
-    struct attr a;
-    int count;
 
     ms=navit_get_mapset(this->nav);
 
     if(!ms)
-        return;
+        return NULL;
 
     map=mapset_get_map_by_name(ms, "search_results");
     if(!map) {
@@ -912,9 +941,94 @@ static void gui_internal_cmd_results_to_map(struct gui_priv *this, struct widget
 
         for(i=0; attrs[i]; i++)
             g_free(attrs[i]);
+    }
+    return map;
+}
 
+/**
+ * @brief Optimizes the format of a string, adding carriage returns so that when displayed, the result text zone is roughly as wide as high
+ *
+ * @param[in,out] s The string to proces (will be modified by this function, but length will be unchanged)
+ */
+static void square_shape_str(char *s) {
+    char *c;
+    char *last_break;
+    unsigned int max_cols = 0;
+    unsigned int cur_cols = 0;
+    unsigned int max_rows = 0;
+    unsigned int surface;
+    unsigned int target_cols;
+
+    if (!s)
+        return;
+    for (c=s; *c!='\0'; c++) {
+        if (*c==' ') {
+            if (max_cols < cur_cols)
+                max_cols = cur_cols;
+            cur_cols = 0;
+            max_rows++;
+        } else
+            cur_cols++;
+    }
+    if (max_cols < cur_cols)
+        max_cols = cur_cols;
+    if (cur_cols)	/* If last line does not end with CR, add it to line numbers anyway */
+        max_rows++;
+    /* Give twice more room for rows (hence the factor 2 below)
+     * This will render as a rectangular shape, taking more horizontal space than vertical */
+    surface = max_rows * 2 * max_cols;
+    target_cols = uint_sqrt(surface);
+
+    if (target_cols < max_cols)
+        target_cols = max_cols;
+
+    target_cols = target_cols + target_cols/10;	/* Allow 10% extra on columns */
+    dbg(lvl_debug, "square_shape_str(): analyzing input text=\"%s\". max_rows=%u, max_cols=%u, surface=%u, target_cols=%u",
+        s, max_rows, max_cols, max_rows * 2 * max_cols, target_cols);
+
+    cur_cols = 0;
+    last_break = NULL;
+    for (c=s; *c!='\0'; c++) {
+        if (*c==' ') {
+            if (cur_cols>=target_cols) {	/* This line is too long, break at the previous non alnum character */
+                if (last_break) {
+                    *last_break =
+                        '\n';	/* Replace the previous non alnum character with a line break, this creates a new line and prevents the previous line from being too long */
+                    cur_cols = c-last_break;
+                }
+            }
+            last_break = c;	/* Record this position as a candidate to insert a line break */
+        }
+        cur_cols++;
     }
+    if (cur_cols>=target_cols && last_break) {
+        *last_break =
+            '\n';	/* Replace the previous non alnum character with a line break, this creates a new line and prevents the previous line from being too long */
+    }
+
+    dbg(lvl_debug, "square_shape_str(): output text=\"%s\"", s);
+}
+
+/**
+ * @brief Create a map rect highlighting one of multiple points provided in argument @data and displayed using
+ *        the style type_found_item (name for each point will also be displayed aside)
+ *
+ * @param this The GUI context
+ * @param table A table widget or any of its descendants. The table contain results to place on the map.
+ *              Providing NULL here will remove all previous results from the map.
+ * @param[out] r The minimum rect focused to contain all results placed  on the map (or unchanged if r==NULL)
+ */
+static void gui_internal_prepare_search_results_map(struct gui_priv *this, struct widget *table, struct coord_rect *r) {
+    struct widget *w;
+    struct map *map;
+    struct map_rect *mr;
+    struct item *item;
+    GList *l;
+    struct attr a;
+    int count;
+    char *name_label;
 
+    map = get_search_results_map(this);
     if(!map)
         return;
 
@@ -931,8 +1045,8 @@ static void gui_internal_cmd_results_to_map(struct gui_priv *this, struct widget
 
     this->results_map_population=0;
 
-    /* Find the table to pupulate the map */
-    for(w=data; w && w->type!=widget_table; w=w->parent);
+    /* Find the table to populate the map */
+    for(w=table; w && w->type!=widget_table; w=w->parent);
 
     if(!w) {
         map_rect_destroy(mr);
@@ -957,12 +1071,16 @@ static void gui_internal_cmd_results_to_map(struct gui_priv *this, struct widget
                 c.y=wi->c.y;
                 item_coord_set(it, &c, 1, change_mode_modify);
                 a.type=attr_label;
-                a.u.str=wi->name;
+                name_label = g_strdup(wi->name);
+                square_shape_str(name_label);
+                a.u.str=name_label;
                 item_attr_set(it, &a, change_mode_modify);
-                if(!count++)
-                    r.lu=r.rl=c;
-                else
-                    coord_rect_extend(&r,&c);
+                if (r) {
+                    if(!count++)
+                        r->lu=r->rl=c;
+                    else
+                        coord_rect_extend(r,&c);
+                }
             }
         }
     }
@@ -971,18 +1089,35 @@ static void gui_internal_cmd_results_to_map(struct gui_priv *this, struct widget
         return;
     a.type=attr_orientation;
     a.u.num=0;
-    navit_set_attr(this->nav,&a);
-    navit_zoom_to_rect(this->nav,&r);
-    gui_internal_prune_menu(this, NULL);
+    navit_set_attr(this->nav,&a);	/* Set orientation to North */
+    if (r) {
+        navit_zoom_to_rect(this->nav,r);
+        gui_internal_prune_menu(this, NULL);
+    }
     this->results_map_population=count;
 }
 
+/**
+ * @brief Apply the command "Show results on the map", highlighting one of multiple points using
+ *        type_found_item style (with their respective name placed aside)
+ *
+ * @param this The GUI context
+ * @param wm The widget that called us
+ * @param data Private data provided during callback (should be a pointer to the table widget containing results,
+ *             or NULL to remove all previous results from the map).
+ */
+static void gui_internal_cmd_results_to_map(struct gui_priv *this, struct widget *wm, void *data) {
+    struct coord_rect r;
+
+    gui_internal_prepare_search_results_map(this, (struct widget *)data, &r);
+}
+
 /*
- * @brief Removes search results from a map.
+ * @brief Removes all existing search results from a map.
  *
- * @param this The graphics context.
- * @param wm called widget.
- * @param data event data
+ * @param this The GUI context
+ * @param wm The widget that called us
+ * @param data Private data (unused).
  */
 static void gui_internal_cmd_results_map_clean(struct gui_priv *this, struct widget *wm, void *data) {
     gui_internal_cmd_results_to_map(this,wm,NULL);
@@ -1025,10 +1160,10 @@ static void gui_internal_cmd_delete_waypoint(struct gui_priv *this, struct widge
  * argument or in WGS84 coordinates (i.e. latitude and longitude in degrees) via the {@code g_in}
  * argument. One of these must be supplied, the other should be {@code NULL}.
  *
- * @param this The internal GUI instance
+ * @param this The GUI context
  * @param pc_in Projected coordinates of the position
  * @param g_in WGS84 coordinates of the position
- * @param wm
+ * @param wm The widget that points to this function as a callback
  * @param name The display name for the position
  * @param flags Flags specifying the operations available from the GUI
  */
@@ -1204,10 +1339,12 @@ void gui_internal_cmd_position_do(struct gui_priv *this, struct pcoord *pc_in, s
                                            image_new_xs(this, "gui_active"), gravity_left_center|orientation_horizontal|flags_fill,
                                            gui_internal_cmd_view_on_map, NULL));
         wbc->c=pc;
-        if ((flags & 4) && wm)
+        if ((flags & 4) && wm) {
             wbc->item=wm->item;
-        else
+        } else {
             wbc->item.type=type_none;
+            wbc->item.priv_data = g_strdup(name); /* Will be freed up by gui_internal_cmd_view_on_map() */
+        }
     }
     if(flags & 256 && this->results_map_population) {
         gui_internal_widget_append(wtable,row=gui_internal_widget_table_row_new(this,
diff --git a/navit/route.c b/navit/route.c
index 116040857..453afa0d1 100644
--- a/navit/route.c
+++ b/navit/route.c
@@ -3267,6 +3267,8 @@ static int rm_attr_get(void *priv_data, enum attr_type attr_type, struct attr *a
         if(mr->item.type==type_waypoint || mr->item.type == type_route_end) {
             if(mr->str)
                 g_free(mr->str);
+            /* Build the text displayed close to the destination cursor.
+             * It will contain the sequence number of the waypoint (1, 2...) */
             mr->str=g_strdup_printf("%d",route->reached_destinations_count+g_list_position(route->destinations,mr->dest)+1);
             attr->u.str=mr->str;
             return 1;
diff --git a/navit/util.c b/navit/util.c
index 707b7c4f7..51bb4db8d 100644
--- a/navit/util.c
+++ b/navit/util.c
@@ -50,6 +50,37 @@ void strtolower(char *dest, const char *src) {
     *dest='\0';
 }
 
+/**
+ * @brief Fast compute of square root for unsigned ints
+ *
+ * @param n The input number
+ * @return sqrt(n)
+ */
+unsigned int uint_sqrt(unsigned int n) {
+    unsigned int h, p= 0, q= 1, r= n;
+
+    /* avoid q rollover */
+    if(n >= (1<<(sizeof(n)*8-2))) {
+        q = 1<<(sizeof(n)*8-2);
+    } else {
+        while ( q <= n ) {
+            q <<= 2;
+        }
+        q >>= 2;
+    }
+
+    while ( q != 0 ) {
+        h = p + q;
+        p >>= 1;
+        if ( r >= h ) {
+            p += q;
+            r -= h;
+        }
+        q >>= 2;
+    }
+    return p;
+}
+
 int navit_utf8_strcasecmp(const char *s1, const char *s2) {
     char *s1_folded,*s2_folded;
     int cmpres;
diff --git a/navit/util.h b/navit/util.h
index b7b60b533..7aa6449e6 100644
--- a/navit/util.h
+++ b/navit/util.h
@@ -25,6 +25,7 @@
 
 void strtoupper(char *dest, const char *src);
 void strtolower(char *dest, const char *src);
+unsigned int uint_sqrt(unsigned int n);
 int navit_utf8_strcasecmp(const char *s1, const char *s2);
 GList * g_hash_to_list(GHashTable *h);
 GList * g_hash_to_list_keys(GHashTable *h);
diff --git a/navit/xslt/android.xslt b/navit/xslt/android.xslt
index 68e731be0..9ef181a88 100644
--- a/navit/xslt/android.xslt
+++ b/navit/xslt/android.xslt
@@ -68,7 +68,7 @@
          <xsl:apply-templates/>
       </xsl:copy>
    </xsl:template>
-   <xsl:template match="/config/navit/layout/layer/itemgra/child::*">
+   <xsl:template match="/config/navit/layout/layer/itemgra/child::*|/config/navit/layer/itemgra/child::*">
       <xsl:copy>
          <xsl:copy-of select="@*[not(name()='text_size') and not(name()='width') and not(name()='radius') and not(name()='w') and not(name()='h') and not(name()='x') and not(name()='y') and not(name()='dash')]"/>
          <xsl:if test="@text_size">