Hi,
Changes are straightforward and make sense. I had a followup on one.
On Wed, Nov 4, 2015 at 10:14 AM, Mike Lothian <m...@fireburn.co.uk> wrote:
> Hi
>
> On Wed, 4 Nov 2015, 8:53 a.m. Samuel Pitoiset <samuel.pitoi...@gmail.com>
> wrote:
>
> Hi Jimmy,
>
> Some comments below.
>
> On 11/04/2015 06:17 AM, Jimmy Berry wrote:
>> - env GALLIUM_HUD_VISIBLE: control default visibility
>> - env GALLIUM_HUD_SIGNAL_TOGGLE: toggle visibility via signal
>> ---
>> Thanks for the feedback.
>>
>> I believe all the suggested changes have been implemented.
>>
>> One note, all the logic except for the toggle was already in hud_create()
>> and
>> not hud_draw().
>>
>> On the subject of allowing the user to specify the signo to use. It was
>> suggested in the original thread that using a fixed signal might end up
>> stealing
>> signals from the parent application. Seems like the user should except
>> funny
>> behavior if they set the signal to something like SIGKILL. I am not
>> opposed to a
>> fixed signo or alternatively providing a default. Something like:
>>
>> GALLIUM_HUD_TOGGLE_SIGNAL=-1 # (results in SIGUSR1)
>>
>>
>> docs/envvars.html | 6 ++++++
>> src/gallium/auxiliary/hud/hud_context.c | 29
>> +++++++++++++++++++++++++++++
>> 2 files changed, 35 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..bffbc2f 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,6 +52,8 @@
>> #include "tgsi/tgsi_text.h"
>> #include "tgsi/tgsi_dump.h"
>>
>> +/* controlls the visibility of all hud contexts */
>
> "Control the visibility of all HUD contexts"
>
>> +static boolean huds_visible = TRUE;
>
> Maybe, hud_is_hidden or something looks like a better name.
>
> I'd have thought keeping the boolean TRUE/1 for the hud being on would make
> more sense
I agree it seems better in the positive rather than a negative.
>
>>
>> struct hud_context {
>> struct pipe_context *pipe;
>> @@ -95,6 +98,11 @@ struct hud_context {
>> } text, bg, whitelines;
>> };
>>
>> +static void
>> +signal_visible_handler(int sig, siginfo_t *siginfo, void *context)
>> +{
>> + huds_visible = !huds_visible;
>> +}
>>
>> static void
>> hud_draw_colored_prims(struct hud_context *hud, unsigned prim,
>> @@ -441,6 +449,9 @@ hud_draw(struct hud_context *hud, struct pipe_resource
>> *tex)
>> struct hud_pane *pane;
>> struct hud_graph *gr;
>>
>> + if (!huds_visible)
>> + return;
>> +
>> hud->fb_width = tex->width0;
>> hud->fb_height = tex->height0;
>> hud->constants.two_div_fb_width = 2.0f / hud->fb_width;
>> @@ -1125,6 +1136,10 @@ hud_create(struct pipe_context *pipe, struct
>> cso_context *cso)
>> struct pipe_sampler_view view_templ;
>> unsigned i;
>> const char *env = debug_get_option("GALLIUM_HUD", NULL);
>> + long signo = debug_get_num_option("GALLIUM_HUD_TOGGLE_SIGNAL", 0);
>> + boolean sig_handled = FALSE;
>> + struct sigaction action;
>> + huds_visible = debug_get_bool_option("GALLIUM_HUD_VISIBLE", TRUE);
>>
>> if (!env || !*env)
>> return NULL;
>> @@ -1267,6 +1282,20 @@ hud_create(struct pipe_context *pipe, struct
>> cso_context *cso)
>>
>> LIST_INITHEAD(&hud->pane_list);
>>
>> + /* setup sig handler once for all hud contexts */
>> + if (!sig_handled) {
>> + memset(&action, 0, sizeof(action));
>
> I think you can get rid of this memset() by doing 'struct sigaction
> action = {};' above.
>
>> + action.sa_sigaction = &signal_visible_handler;
>> + action.sa_flags = SA_SIGINFO;
>> +
>> + if (signo < 1 || signo >= NSIG)
>> + fprintf(stderr, "gallium_hud: invalid signal %ld\n", signo);
>> + else if (sigaction(signo, &action, NULL) < 0)
>> + fprintf(stderr, "gallium_hud: unable to set handler for signal
>> %ld\n", signo);
>> +
>> + sig_handled = TRUE;
>> + }
>> +
>> hud_parse_env_var(hud, env);
>> return hud;
>> }
>>
>
> --
> -Samuel
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
