[Mesa-dev] [PATCH] mesa: Always expose GREMEDY_string_marker.

[Ilia Mirkin]

I don't think that's right. Gremedy is a debugger application that exposes
the extension to allow the app to send info to a debugger. It should not be
exposed under normal circumstances as it may enable debug code paths in
applications.

If the issue is that an application is erroneously calling this function,
the solution is to not make calling the function an error, but leave the
extension off under normal circumstances.

Also in that case, don't call the driver function as it will cause the
driver to do things. Nouveau also emits the contents into the command
stream.

On Feb 15, 2017 7:59 PM, "Kenneth Graunke" <kenneth at whitecape.org> wrote:

> Equivalent marker functionality is already included in KHR_debug, which
> we already expose unconditionally in all drivers (dummy_true).
>
> Grim Fandango Remastered apparently calls glStringMarkerGREMEDY()
> without checking for the extension, spewing GL errors.  Assuming the
> existence of the extension is definitely not valid, but it also seems
> kinda mean to spew GL errors when we could simply expose the feature
> and silently ignore the provided string markers.
>
> This patch enables GREMEDY_string_marker everywhere, and makes the calls
> no-ops if the driver doesn't provide the EmitStringMarker() hook, just
> like we did for the KHR_debug functionality.
>
> This may impact freedreno, which actually puts markers in its command
> buffers.
>
> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> ---
>  src/mesa/main/debug_output.c           | 4 +---
>  src/mesa/main/extensions_table.h       | 2 +-
>  src/mesa/state_tracker/st_debug.c      | 1 -
>  src/mesa/state_tracker/st_debug.h      | 3 +--
>  src/mesa/state_tracker/st_extensions.c | 4 ----
>  5 files changed, 3 insertions(+), 11 deletions(-)
>
> diff --git a/src/mesa/main/debug_output.c b/src/mesa/main/debug_output.c
> index bc933db93d4..1d2dee128b4 100644
> --- a/src/mesa/main/debug_output.c
> +++ b/src/mesa/main/debug_output.c
> @@ -1308,12 +1308,10 @@ void GLAPIENTRY
>  _mesa_StringMarkerGREMEDY(GLsizei len, const GLvoid *string)
>  {
>     GET_CURRENT_CONTEXT(ctx);
> -   if (ctx->Extensions.GREMEDY_string_marker) {
> +   if (ctx->Driver.EmitStringMarker) {
>        /* if length not specified, string will be null terminated: */
>        if (len <= 0)
>           len = strlen(string);
>        ctx->Driver.EmitStringMarker(ctx, string, len);
> -   } else {
> -      _mesa_error(ctx, GL_INVALID_OPERATION, "StringMarkerGREMEDY");
>     }
>  }
> diff --git a/src/mesa/main/extensions_table.h b/src/mesa/main/extensions_
> table.h
> index 7ea56c8422d..ec48aadde3f 100644
> --- a/src/mesa/main/extensions_table.h
> +++ b/src/mesa/main/extensions_table.h
> @@ -285,7 +285,7 @@ EXT(EXT_vertex_array                        ,
> dummy_true
>  EXT(EXT_vertex_array_bgra                   , EXT_vertex_array_bgra
>             , GLL, GLC,  x ,  x , 2008)
>  EXT(EXT_window_rectangles                   , EXT_window_rectangles
>             , GLL, GLC,  x ,  30, 2016)
>
> -EXT(GREMEDY_string_marker                   , GREMEDY_string_marker
>             , GLL, GLC,  x ,  x , 2007)
> +EXT(GREMEDY_string_marker                   , dummy_true
>            , GLL, GLC,  x ,  x , 2007)
>
>  EXT(IBM_multimode_draw_arrays               , dummy_true
>            , GLL, GLC,  x ,  x , 1998)
>  EXT(IBM_rasterpos_clip                      , dummy_true
>            , GLL,  x ,  x ,  x , 1996)
> diff --git a/src/mesa/state_tracker/st_debug.c
> b/src/mesa/state_tracker/st_debug.c
> index d6cb5cd57d8..f2e982c8c7a 100644
> --- a/src/mesa/state_tracker/st_debug.c
> +++ b/src/mesa/state_tracker/st_debug.c
> @@ -58,7 +58,6 @@ static const struct debug_named_value st_debug_flags[] =
> {
>     { "buffer",   DEBUG_BUFFER, NULL },
>     { "wf",       DEBUG_WIREFRAME, NULL },
>     { "precompile",  DEBUG_PRECOMPILE, NULL },
> -   { "gremedy",  DEBUG_GREMEDY, "Enable GREMEDY debug extensions" },
>     { "noreadpixcache", DEBUG_NOREADPIXCACHE, NULL },
>     DEBUG_NAMED_VALUE_END
>  };
> diff --git a/src/mesa/state_tracker/st_debug.h
> b/src/mesa/state_tracker/st_debug.h
> index 6c1e915f68c..4b92a669a37 100644
> --- a/src/mesa/state_tracker/st_debug.h
> +++ b/src/mesa/state_tracker/st_debug.h
> @@ -50,8 +50,7 @@ st_print_current(void);
>  #define DEBUG_BUFFER    0x200
>  #define DEBUG_WIREFRAME 0x400
>  #define DEBUG_PRECOMPILE   0x800
> -#define DEBUG_GREMEDY   0x1000
> -#define DEBUG_NOREADPIXCACHE 0x2000
> +#define DEBUG_NOREADPIXCACHE 0x1000
>
>  #ifdef DEBUG
>  extern int ST_DEBUG;
> diff --git a/src/mesa/state_tracker/st_extensions.c
> b/src/mesa/state_tracker/st_extensions.c
> index 37fe4469c37..d9057c77657 100644
> --- a/src/mesa/state_tracker/st_extensions.c
> +++ b/src/mesa/state_tracker/st_extensions.c
> @@ -1167,10 +1167,6 @@ void st_init_extensions(struct pipe_screen *screen,
>        extensions->ARB_vertex_attrib_64bit = GL_TRUE;
>     }
>
> -   if ((ST_DEBUG & DEBUG_GREMEDY) &&
> -       screen->get_param(screen, PIPE_CAP_STRING_MARKER))
> -      extensions->GREMEDY_string_marker = GL_TRUE;
> -
>     if (screen->get_param(screen, PIPE_CAP_COMPUTE)) {
>        int compute_supported_irs =
>           screen->get_shader_param(screen, PIPE_SHADER_COMPUTE,
> --
> 2.11.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170215/2444d2d7/attachment.html>