[Mesa-dev] gallium/hud: control visibility at startup and runtime.

[Eero Tamminen]

On 11/03/2015 05:43 PM, Brian Paul wrote:
> On 11/03/2015 03:43 AM, boombatower wrote:
>> - env GALLIUM_HUD_VISIBLE: control default visibility
>> - env GALLIUM_HUD_SIGNAL_TOGGLE: toggle visibility via signal
>> ---
>>   docs/envvars.html                       |  6 ++++++
>>   src/gallium/auxiliary/hud/hud_context.c | 23 +++++++++++++++++++++++
>>   2 files changed, 29 insertions(+)
>>
>> diff --git a/docs/envvars.html b/docs/envvars.html
>> index bdfe999..530bbb7 100644
>> --- a/docs/envvars.html
>> +++ b/docs/envvars.html
>> @@ -179,6 +179,12 @@ Mesa EGL supports different sets of environment
>> variables.  See the
>>   <li>GALLIUM_HUD - draws various information on the screen, like
>> framerate,
>>       cpu load, driver statistics, performance counters, etc.
>>       Set GALLIUM_HUD=help and run e.g. glxgears for more info.
>> +<li>GALLIUM_HUD_VISIBLE - control default visibility, defaults to true.
>> +<li>GALLIUM_HUD_TOGGLE_SIGNAL - toggle visibility via user specified
>> signal.
>> +    Especially useful to toggle hud at specific points of application
>> and
>> +    disable for unencumbered viewing the rest of the time. For
>> example, set
>> +    GALLIUM_HUD_VISIBLE to false and GALLIUM_HUD_SIGNAL_TOGGLE to 10
>> (SIGUSR1).
>> +    Use kill -10 <pid> to toggle the hud as desired.
>>   <li>GALLIUM_LOG_FILE - specifies a file for logging all errors,
>> warnings, etc.
>>       rather than stderr.
>>   <li>GALLIUM_PRINT_OPTIONS - if non-zero, print all the Gallium
>> environment
>> diff --git a/src/gallium/auxiliary/hud/hud_context.c
>> b/src/gallium/auxiliary/hud/hud_context.c
>> index ffe30b8..f6bfa80 100644
>> --- a/src/gallium/auxiliary/hud/hud_context.c
>> +++ b/src/gallium/auxiliary/hud/hud_context.c
>> @@ -33,6 +33,7 @@
>>    * Set GALLIUM_HUD=help for more info.
>>    */
>>
>> +#include <signal.h>
>>   #include <stdio.h>
>>
>>   #include "hud/hud_context.h"
>> @@ -51,8 +52,10 @@
>>   #include "tgsi/tgsi_text.h"
>>   #include "tgsi/tgsi_dump.h"
>>
>> +static boolean __visible_toggle = false;
>
> There's no need for the double underscore prefix.  Maybe "toggle_hud"
> would be a better name.  And there should probably be a comment on it
> explaining how it works.  It's not obvious.

Regarding Marek's comment, IMHO it's better to have this as 
"huds_visible", see below...


> Let's either use gallium's boolean/TRUE/FALSE or stdbool's
> bool/true/false rather than mix them.
>
>
>>
>>   struct hud_context {
>> +   boolean visible;
>>      struct pipe_context *pipe;
>>      struct cso_context *cso;
>>      struct u_upload_mgr *uploader;
>> @@ -95,6 +98,11 @@ struct hud_context {
>>      } text, bg, whitelines;
>>   };
>>
>> +static void
>> +signal_visible_handler(int signo)
>> +{
>> +   __visible_toggle = true;
>> +}
>>
>>   static void
>>   hud_draw_colored_prims(struct hud_context *hud, unsigned prim,
>> @@ -431,8 +439,15 @@ hud_alloc_vertices(struct hud_context *hud,
>> struct vertex_queue *v,
>>   void
>>   hud_draw(struct hud_context *hud, struct pipe_resource *tex)
>>   {
>> +   if (__visible_toggle) {
>> +      hud->visible = !hud->visible;
>> +      __visible_toggle = false;
>> +   }
>> +   if (!hud->visible) return;

	if (hud->visible != huds_visible)
		hud->visible = huds_visible;
	if (!hud->visible)
		return


> Please put 'return' on the next line.
>
> I keep forgetting, but I think some compilers complain about
> declarations after code, so this should be moved after the declarations
> below.
 >
>> +
>>      struct cso_context *cso = hud->cso;
>>      struct pipe_context *pipe = hud->pipe;
>> +
>
> No need to introduce a new empty line here.
>
>>      struct pipe_framebuffer_state fb;
>>      struct pipe_surface surf_templ, *surf;
>>      struct pipe_viewport_state viewport;
>> @@ -1124,7 +1139,9 @@ hud_create(struct pipe_context *pipe, struct
>> cso_context *cso)
>>      struct hud_context *hud;
>>      struct pipe_sampler_view view_templ;
>>      unsigned i;
>> +   boolean visible = debug_get_bool_option("GALLIUM_HUD_VISIBLE", true);
>>      const char *env = debug_get_option("GALLIUM_HUD", NULL);
>> +   long signo = debug_get_num_option("GALLIUM_HUD_TOGGLE_SIGNAL", 0);
>
> Can we avoid calling debug_get_bool/num_option() for every draw call?
> Wouldn't hud_create() be a better place for this?

Signal checking and setup should be done only once in program startup. 
  Is hud_create() run for each context (hud) separately?


	- Eero


>>
>>      if (!env || !*env)
>>         return NULL;
>> @@ -1138,6 +1155,7 @@ hud_create(struct pipe_context *pipe, struct
>> cso_context *cso)
>>      if (!hud)
>>         return NULL;
>>
>> +   hud->visible = visible;
>>      hud->pipe = pipe;
>>      hud->cso = cso;
>>      hud->uploader = u_upload_create(pipe, 256 * 1024, 16,
>> @@ -1267,6 +1285,11 @@ hud_create(struct pipe_context *pipe, struct
>> cso_context *cso)
>>
>>      LIST_INITHEAD(&hud->pane_list);
>>
>> +   if (signo < 1 || signo >= NSIG)
>> +      fprintf(stderr, "gallium_hud: invalid signal %ld\n", signo);
>> +   else if (signal(signo, signal_visible_handler) == SIG_ERR)
>> +      fprintf(stderr, "gallium_hud: unable to set handler for signal
>> %ld\n", signo);
>
> Can that go into hud_create() too?
>
> Finally, I'll need to check if any of this can work on Windows.  If not,
> we'll need some #ifndef PIPE_OS_WINDOWS tests...
>
> -Brian
>
>> +
>>      hud_parse_env_var(hud, env);
>>      return hud;
>>   }
>>
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev