[Mesa-dev] [PATCH 1/5] gallium/hud: add an option to rename each data source

[Nicolai Hähnle]

On 01.01.2017 01:05, Marek Olšák wrote:
> From: Marek Olšák <marek.olsak at amd.com>
>
> useful for radeonsi performance counters
> ---
>  src/gallium/auxiliary/hud/hud_context.c | 40 ++++++++++++++++++++++++---------
>  1 file changed, 30 insertions(+), 10 deletions(-)
>
> diff --git a/src/gallium/auxiliary/hud/hud_context.c b/src/gallium/auxiliary/hud/hud_context.c
> index 779c116..4c65af3 100644
> --- a/src/gallium/auxiliary/hud/hud_context.c
> +++ b/src/gallium/auxiliary/hud/hud_context.c
> @@ -782,20 +782,31 @@ hud_pane_create(unsigned x1, unsigned y1, unsigned x2, unsigned y2,
>     pane->max_num_vertices = (x2 - x1 + 2) / 2;
>     pane->ceiling = ceiling;
>     pane->dyn_ceiling = dyn_ceiling;
>     pane->dyn_ceil_last_ran = 0;
>     pane->initial_max_value = max_value;
>     hud_pane_set_max_value(pane, max_value);
>     LIST_INITHEAD(&pane->graph_list);
>     return pane;
>  }
>
> +/* replace '-' with a space */
> +static void
> +strip_hyphens(char *s)
> +{
> +   while (*s) {
> +      if (*s == '-')
> +         *s = ' ';
> +      s++;
> +   }
> +}
> +
>  /**
>   * Add a graph to an existing pane.
>   * One pane can contain multiple graphs over each other.
>   */
>  void
>  hud_pane_add_graph(struct hud_pane *pane, struct hud_graph *gr)
>  {
>     static const float colors[][3] = {
>        {0, 1, 0},
>        {1, 0, 0},
> @@ -885,21 +896,21 @@ hud_graph_set_dump_file(struct hud_graph *gr)
>  /**
>   * Read a string from the environment variable.
>   * The separators "+", ",", ":", and ";" terminate the string.
>   * Return the number of read characters.
>   */
>  static int
>  parse_string(const char *s, char *out)
>  {
>     int i;
>
> -   for (i = 0; *s && *s != '+' && *s != ',' && *s != ':' && *s != ';';
> +   for (i = 0; *s && *s != '+' && *s != ',' && *s != ':' && *s != ';' && *s != '=';
>          s++, out++, i++)
>        *out = *s;
>
>     *out = 0;
>
>     if (*s && !i) {
>        fprintf(stderr, "gallium_hud: syntax error: unexpected '%c' (%i) while "
>                "parsing a string\n", *s, *s);
>        fflush(stderr);
>     }
> @@ -1164,41 +1175,48 @@ hud_parse_env_var(struct hud_context *hud, const char *env)
>           /* driver queries */
>           if (!processed) {
>              if (!hud_driver_query_install(&hud->batch_query, pane, hud->pipe,
>                                            name)) {
>                 fprintf(stderr, "gallium_hud: unknown driver query '%s'\n", name);
>                 fflush(stderr);
>              }
>           }
>        }
>
> -      if (*env == ':') {
> +      if (*env == ':' || *env == '=') {
> +         char key = *env;
>           env++;
>
>           if (!pane) {
>              fprintf(stderr, "gallium_hud: syntax error: unexpected ':', "
>                      "expected a name\n");
>              fflush(stderr);
>              break;
>           }
>
>           num = parse_string(env, s);
>           env += num;
>
> -         if (num && sscanf(s, "%u", &i) == 1) {
> -            hud_pane_set_max_value(pane, i);
> -            pane->initial_max_value = i;
> -         }
> -         else {
> -            fprintf(stderr, "gallium_hud: syntax error: unexpected '%c' (%i) "
> -                    "after ':'\n", *env, *env);
> -            fflush(stderr);
> +         if (key == ':') {
> +            if (num && sscanf(s, "%u", &i) == 1) {
> +               hud_pane_set_max_value(pane, i);
> +               pane->initial_max_value = i;
> +            }
> +            else {
> +               fprintf(stderr, "gallium_hud: syntax error: unexpected '%c' (%i) "
> +                       "after ':'\n", *env, *env);
> +               fflush(stderr);
> +            }
> +         } else if (key == '=') {
> +            strip_hyphens(s);
> +            strcpy(LIST_ENTRY(struct hud_graph,
> +                              pane->graph_list.prev, head)->name, s);
>           }
>        }

I don't see why one shouldn't be able to put both :[max] and =[name]. 
Since the : is per-pane and the = is per-data-source, I think it makes 
sense to have

if (*env == '=') {
   ...
}

if (*env == ':') {
   ...
}

I suppose a while-loop would also work and would be simpler.

Apart from this, the series is

Reviewed-by: Nicolai Hähnle <nicolai.haehnle at amd.com>

Nicolai

>
>        if (*env == 0)
>           break;
>
>        /* parse a separator */
>        switch (*env) {
>        case '+':
>           env++;
> @@ -1264,20 +1282,22 @@ print_help(struct pipe_screen *screen)
>     puts("");
>     puts("  Names are identifiers of data sources which will be drawn as graphs");
>     puts("  in panes. Multiple graphs can be drawn in the same pane.");
>     puts("  There can be multiple panes placed in rows and columns.");
>     puts("");
>     puts("  '+' separates names which will share a pane.");
>     puts("  ':[value]' specifies the initial maximum value of the Y axis");
>     puts("             for the given pane.");
>     puts("  ',' creates a new pane below the last one.");
>     puts("  ';' creates a new pane at the top of the next column.");
> +   puts("  '=' followed by a string, changes the name of the last data source");
> +   puts("      to that string");
>     puts("");
>     puts("  Example: GALLIUM_HUD=\"cpu,fps;primitives-generated\"");
>     puts("");
>     puts("  Additionally, by prepending '.[identifier][value]' modifiers to");
>     puts("  a name, it is possible to explicitly set the location and size");
>     puts("  of a pane, along with limiting overall maximum value of the");
>     puts("  Y axis and activating dynamic readjustment of the Y axis.");
>     puts("  Several modifiers may be applied to the same pane simultaneously.");
>     puts("");
>     puts("  'x[value]' sets the location of the pane on the x axis relative");
>