Re: [Mesa-dev] [PATCH] RFC: mesa: remove MESA_VERBOSE env variable

2017-04-26 Thread Emil Velikov 
On 21 April 2017 at 17:46, Jordan Justen  wrote:
> On 2017-04-21 04:02:11, Emil Velikov wrote:
>> On 21 April 2017 at 07:18, Jordan Justen  wrote:
>> > On 2017-04-20 18:49:57, Timothy Arceri wrote:
>> >> On 21/04/17 11:37, Jordan Justen wrote:
>> >> > On 2017-04-20 12:33:45, Samuel Pitoiset wrote:
>> >> >> I have used it sometimes, but since VERBOSE_API is missing in a bunch 
>> >> >> of
>> >> >> places, that's quite useless. :)
>> >> >>
>> >> >
>> >> > I also use MESA_VERBOSE=api every few months or so. I've found it
>> >> > pretty useful, but frustratingly I've nearly always ended up having to
>> >> > add new calls for it become useful.
>> >> >
>> >> > Something that automates wrapping the API probably is perhaps more
>> >> > maintainable.
>> >>
>> >> As Eric mentioned it would probably be best to add something to dispatch
>> >> generation if we want this to be reliable. It shouldn't be too difficult
>> >> to do.
>> >>
>> >
>> > Could that version could be implemented as a replacement before
>> > removing the current version?
>> >
>> As you pointed out this is something we might want to have in GLVND.
>> Not sure if how long it will take to write/merge that so stalling on
>> it is a bit... meh.
>>
>
> I guess when removing a feature that you don't use, it is easy to say
> 'meh' about its possible future replacement. I've actually used this
> feature while debugging 2 applications in the past month.
>
Agreed on the "meh" part, it was rather selfish of me.
I think the bigger question that's missing is - what was the obstacle
of using apitrace/similar tool?

> Admittedly, this a bit more than I normally use it, but I also don't
> see a motivation to removing it. So why not stall until there is an
> alternative?
>
Until we yank this out [close to] nobody will bother with an alternative.

As you mentioned it - MESA_VERBOSE becomes useful after spending time fixing it.
Not sure why you'd want to single-handedly maintain this behemoth - oh
well, so be it.

-Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] RFC: mesa: remove MESA_VERBOSE env variable

2017-04-21 Thread Timothy Arceri 

On 21/04/17 20:51, Emil Velikov wrote:

On 21 April 2017 at 02:11, Timothy Arceri  wrote:

Thanks for doing this. Looks like you forgot to updated the docs, with that
fixed.


Not sure what you mean here - are you thinking of a note to
docs/release/17.2.0.html?


Never mind I thought this was in envvars.html



Thanks
Emil


___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] RFC: mesa: remove MESA_VERBOSE env variable

2017-04-21 Thread Brian Paul 

On 04/21/2017 05:02 AM, Emil Velikov wrote:

On 21 April 2017 at 07:18, Jordan Justen  wrote:

On 2017-04-20 18:49:57, Timothy Arceri wrote:

On 21/04/17 11:37, Jordan Justen wrote:

On 2017-04-20 12:33:45, Samuel Pitoiset wrote:

I have used it sometimes, but since VERBOSE_API is missing in a bunch of
places, that's quite useless. :)



I also use MESA_VERBOSE=api every few months or so. I've found it
pretty useful, but frustratingly I've nearly always ended up having to
add new calls for it become useful.

Something that automates wrapping the API probably is perhaps more
maintainable.


As Eric mentioned it would probably be best to add something to dispatch
generation if we want this to be reliable. It shouldn't be too difficult
to do.



Could that version could be implemented as a replacement before
removing the current version?


As you pointed out this is something we might want to have in GLVND.
Not sure if how long it will take to write/merge that so stalling on
it is a bit... meh.


FWIW, there is some old code in src/mapi/glapi/glapi_dispatch.c that'll 
print each API call with its arguments.  Though, IIRC, it requires 
disabling the x86/x64 asm dispatch code, disabling shared-glapi and 
something else.


-Brian


___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] RFC: mesa: remove MESA_VERBOSE env variable

2017-04-21 Thread Jordan Justen 
On 2017-04-21 04:02:11, Emil Velikov wrote:
> On 21 April 2017 at 07:18, Jordan Justen  wrote:
> > On 2017-04-20 18:49:57, Timothy Arceri wrote:
> >> On 21/04/17 11:37, Jordan Justen wrote:
> >> > On 2017-04-20 12:33:45, Samuel Pitoiset wrote:
> >> >> I have used it sometimes, but since VERBOSE_API is missing in a bunch of
> >> >> places, that's quite useless. :)
> >> >>
> >> >
> >> > I also use MESA_VERBOSE=api every few months or so. I've found it
> >> > pretty useful, but frustratingly I've nearly always ended up having to
> >> > add new calls for it become useful.
> >> >
> >> > Something that automates wrapping the API probably is perhaps more
> >> > maintainable.
> >>
> >> As Eric mentioned it would probably be best to add something to dispatch
> >> generation if we want this to be reliable. It shouldn't be too difficult
> >> to do.
> >>
> >
> > Could that version could be implemented as a replacement before
> > removing the current version?
> >
> As you pointed out this is something we might want to have in GLVND.
> Not sure if how long it will take to write/merge that so stalling on
> it is a bit... meh.
>

I guess when removing a feature that you don't use, it is easy to say
'meh' about its possible future replacement. I've actually used this
feature while debugging 2 applications in the past month.

Admittedly, this a bit more than I normally use it, but I also don't
see a motivation to removing it. So why not stall until there is an
alternative?

-Jordan
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] RFC: mesa: remove MESA_VERBOSE env variable

2017-04-21 Thread Emil Velikov 
On 21 April 2017 at 07:18, Jordan Justen  wrote:
> On 2017-04-20 18:49:57, Timothy Arceri wrote:
>> On 21/04/17 11:37, Jordan Justen wrote:
>> > On 2017-04-20 12:33:45, Samuel Pitoiset wrote:
>> >> I have used it sometimes, but since VERBOSE_API is missing in a bunch of
>> >> places, that's quite useless. :)
>> >>
>> >
>> > I also use MESA_VERBOSE=api every few months or so. I've found it
>> > pretty useful, but frustratingly I've nearly always ended up having to
>> > add new calls for it become useful.
>> >
>> > Something that automates wrapping the API probably is perhaps more
>> > maintainable.
>>
>> As Eric mentioned it would probably be best to add something to dispatch
>> generation if we want this to be reliable. It shouldn't be too difficult
>> to do.
>>
>
> Could that version could be implemented as a replacement before
> removing the current version?
>
As you pointed out this is something we might want to have in GLVND.
Not sure if how long it will take to write/merge that so stalling on
it is a bit... meh.

-Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] RFC: mesa: remove MESA_VERBOSE env variable

2017-04-21 Thread Emil Velikov 
On 21 April 2017 at 02:11, Timothy Arceri  wrote:
> Thanks for doing this. Looks like you forgot to updated the docs, with that
> fixed.
>
Not sure what you mean here - are you thinking of a note to
docs/release/17.2.0.html?

Thanks
Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] RFC: mesa: remove MESA_VERBOSE env variable

2017-04-21 Thread Samuel Pitoiset 



On 04/21/2017 03:37 AM, Jordan Justen wrote:

On 2017-04-20 12:33:45, Samuel Pitoiset wrote:

I have used it sometimes, but since VERBOSE_API is missing in a bunch of
places, that's quite useless. :)



I also use MESA_VERBOSE=api every few months or so. I've found it
pretty useful, but frustratingly I've nearly always ended up having to
add new calls for it become useful.

Something that automates wrapping the API probably is perhaps more
maintainable. A lightweight apitrace mode? (Or, maybe this already
exists??) Perhaps something built into a 'debug' mode for libglvnd or
libepoxy? The downside of the library shim option is that it can be
sometimes be annoying to get your app to run with the shim.


It's useful only if all calls are traced. The dispatch table idea looks 
good to me.




-Jordan


On 04/20/2017 04:12 PM, Emil Velikov wrote:

From: Emil Velikov 

The env variable was useful in the early days of mesa development, when
tools such at Apitrace were not available.

Nowadays, out of the 12 options only a third are used. With nearly all
instances printing the API state. And even then not all entry points
are annotated, thus one cannot rely upon it.

The current patch removes the variable all together alongside a few
instances (listed below) which developers may see value in keeping.

FLUSH_VERTICES/FLUSH_CURRENT macros in src/mesa/main/context.h
_mesa_GetGraphicsResetStatusARB in src/mesa/main/robustness.c
_mesa_print_state() in src/mesa/main/state.c
_mesa_notifySwapBuffers() in src/mesa/main/context.c

Do we want to keep any of the MESA_VERBOSE instances? If so documenting
the variable seems reasonable.

Cc: Brian Paul 
Signed-off-by: Emil Velikov 
---
Brian, I assume that you're the author for most of these. How do you
feel on the topic?
---
   src/mesa/main/attrib.c  |   8 ---
   src/mesa/main/blend.c   |  40 
   src/mesa/main/blit.c|  17 -
   src/mesa/main/bufferobj.c   |  38 ---
   src/mesa/main/buffers.c |   7 ---
   src/mesa/main/clear.c   |   3 -
   src/mesa/main/compute.c |  13 
   src/mesa/main/context.c |  19 +-
   src/mesa/main/context.h |   4 --
   src/mesa/main/copyimage.c   |  10 ---
   src/mesa/main/debug.c   |  42 -
   src/mesa/main/depth.c   |  12 
   src/mesa/main/dlist.c   |  17 -
   src/mesa/main/drawpix.c |  21 ---
   src/mesa/main/enable.c  |   6 --
   src/mesa/main/fbobject.c|  32 --
   src/mesa/main/feedback.c|   3 -
   src/mesa/main/getstring.c   |   6 --
   src/mesa/main/hint.c|   5 --
   src/mesa/main/light.c   |  14 -
   src/mesa/main/lines.c   |   6 --
   src/mesa/main/matrix.c  |  29 -
   src/mesa/main/mtypes.h  |  22 ---
   src/mesa/main/performance_monitor.c |   6 --
   src/mesa/main/pipelineobj.c |  33 --
   src/mesa/main/polygon.c |  23 ---
   src/mesa/main/program_resource.c|  33 --
   src/mesa/main/queryobj.c|  31 -
   src/mesa/main/readpix.c |   7 ---
   src/mesa/main/robustness.c  |  14 +
   src/mesa/main/samplerobj.c  |   3 -
   src/mesa/main/scissor.c |  11 
   src/mesa/main/shaderapi.c   |  16 -
   src/mesa/main/state.c   |   3 -
   src/mesa/main/stencil.c |  27 
   src/mesa/main/texenv.c  |   7 ---
   src/mesa/main/texgen.c  |   7 ---
   src/mesa/main/texgetimage.c |  17 -
   src/mesa/main/teximage.c|  52 ---
   src/mesa/main/texobj.c  |  29 -
   src/mesa/main/texstate.c|   8 ---
   src/mesa/main/texstorage.c  |  13 
   src/mesa/main/textureview.c |   6 --
   src/mesa/main/varray.c  |   6 --
   src/mesa/main/viewport.c|  28 -
   src/mesa/tnl/t_vb_render.c  |   5 --
   src/mesa/vbo/vbo_exec_array.c   | 122 

   47 files changed, 2 insertions(+), 879 deletions(-)

diff --git a/src/mesa/main/attrib.c b/src/mesa/main/attrib.c
index 8e738c91c8f..bb230404d29 100644
--- a/src/mesa/main/attrib.c
+++ b/src/mesa/main/attrib.c
@@ -245,9 +245,6 @@ _mesa_PushAttrib(GLbitfield mask)
   
  GET_CURRENT_CONTEXT(ctx);
   
-   if (MESA_VERBOSE & VERBOSE_API)

-  _mesa_debug(ctx, "glPushAttrib %x\n", (int) mask);
-
  if (ctx->AttribStackDepth >= MAX_ATTRIB_STACK_DEPTH) {
 _mesa_error( ctx, GL_STACK_OVERFLOW, "glPushAttrib" );
 return;
@@ -935,11 +932,6 @@ _mesa_PopAttrib(void)
   
  while (attr) {
   
-  if (MESA_VERBOSE & VERBOSE_API) {

- _mesa_debug(ctx,

Re: [Mesa-dev] [PATCH] RFC: mesa: remove MESA_VERBOSE env variable

2017-04-21 Thread Jordan Justen 
On 2017-04-20 18:49:57, Timothy Arceri wrote:
> On 21/04/17 11:37, Jordan Justen wrote:
> > On 2017-04-20 12:33:45, Samuel Pitoiset wrote:
> >> I have used it sometimes, but since VERBOSE_API is missing in a bunch of
> >> places, that's quite useless. :)
> >>
> > 
> > I also use MESA_VERBOSE=api every few months or so. I've found it
> > pretty useful, but frustratingly I've nearly always ended up having to
> > add new calls for it become useful.
> > 
> > Something that automates wrapping the API probably is perhaps more
> > maintainable. 
> 
> As Eric mentioned it would probably be best to add something to dispatch 
> generation if we want this to be reliable. It shouldn't be too difficult 
> to do.
> 

Could that version could be implemented as a replacement before
removing the current version?

-Jordan
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] RFC: mesa: remove MESA_VERBOSE env variable

2017-04-20 Thread Timothy Arceri 

On 21/04/17 11:37, Jordan Justen wrote:

On 2017-04-20 12:33:45, Samuel Pitoiset wrote:

I have used it sometimes, but since VERBOSE_API is missing in a bunch of
places, that's quite useless. :)



I also use MESA_VERBOSE=api every few months or so. I've found it
pretty useful, but frustratingly I've nearly always ended up having to
add new calls for it become useful.

Something that automates wrapping the API probably is perhaps more
maintainable. 


As Eric mentioned it would probably be best to add something to dispatch 
generation if we want this to be reliable. It shouldn't be too difficult 
to do.



A lightweight apitrace mode? (Or, maybe this already
exists??) Perhaps something built into a 'debug' mode for libglvnd or
libepoxy? The downside of the library shim option is that it can be
sometimes be annoying to get your app to run with the shim.

-Jordan


On 04/20/2017 04:12 PM, Emil Velikov wrote:

From: Emil Velikov 

The env variable was useful in the early days of mesa development, when
tools such at Apitrace were not available.

Nowadays, out of the 12 options only a third are used. With nearly all
instances printing the API state. And even then not all entry points
are annotated, thus one cannot rely upon it.

The current patch removes the variable all together alongside a few
instances (listed below) which developers may see value in keeping.

FLUSH_VERTICES/FLUSH_CURRENT macros in src/mesa/main/context.h
_mesa_GetGraphicsResetStatusARB in src/mesa/main/robustness.c
_mesa_print_state() in src/mesa/main/state.c
_mesa_notifySwapBuffers() in src/mesa/main/context.c

Do we want to keep any of the MESA_VERBOSE instances? If so documenting
the variable seems reasonable.

Cc: Brian Paul 
Signed-off-by: Emil Velikov 
---
Brian, I assume that you're the author for most of these. How do you
feel on the topic?
---
   src/mesa/main/attrib.c  |   8 ---
   src/mesa/main/blend.c   |  40 
   src/mesa/main/blit.c|  17 -
   src/mesa/main/bufferobj.c   |  38 ---
   src/mesa/main/buffers.c |   7 ---
   src/mesa/main/clear.c   |   3 -
   src/mesa/main/compute.c |  13 
   src/mesa/main/context.c |  19 +-
   src/mesa/main/context.h |   4 --
   src/mesa/main/copyimage.c   |  10 ---
   src/mesa/main/debug.c   |  42 -
   src/mesa/main/depth.c   |  12 
   src/mesa/main/dlist.c   |  17 -
   src/mesa/main/drawpix.c |  21 ---
   src/mesa/main/enable.c  |   6 --
   src/mesa/main/fbobject.c|  32 --
   src/mesa/main/feedback.c|   3 -
   src/mesa/main/getstring.c   |   6 --
   src/mesa/main/hint.c|   5 --
   src/mesa/main/light.c   |  14 -
   src/mesa/main/lines.c   |   6 --
   src/mesa/main/matrix.c  |  29 -
   src/mesa/main/mtypes.h  |  22 ---
   src/mesa/main/performance_monitor.c |   6 --
   src/mesa/main/pipelineobj.c |  33 --
   src/mesa/main/polygon.c |  23 ---
   src/mesa/main/program_resource.c|  33 --
   src/mesa/main/queryobj.c|  31 -
   src/mesa/main/readpix.c |   7 ---
   src/mesa/main/robustness.c  |  14 +
   src/mesa/main/samplerobj.c  |   3 -
   src/mesa/main/scissor.c |  11 
   src/mesa/main/shaderapi.c   |  16 -
   src/mesa/main/state.c   |   3 -
   src/mesa/main/stencil.c |  27 
   src/mesa/main/texenv.c  |   7 ---
   src/mesa/main/texgen.c  |   7 ---
   src/mesa/main/texgetimage.c |  17 -
   src/mesa/main/teximage.c|  52 ---
   src/mesa/main/texobj.c  |  29 -
   src/mesa/main/texstate.c|   8 ---
   src/mesa/main/texstorage.c  |  13 
   src/mesa/main/textureview.c |   6 --
   src/mesa/main/varray.c  |   6 --
   src/mesa/main/viewport.c|  28 -
   src/mesa/tnl/t_vb_render.c  |   5 --
   src/mesa/vbo/vbo_exec_array.c   | 122 

   47 files changed, 2 insertions(+), 879 deletions(-)

diff --git a/src/mesa/main/attrib.c b/src/mesa/main/attrib.c
index 8e738c91c8f..bb230404d29 100644
--- a/src/mesa/main/attrib.c
+++ b/src/mesa/main/attrib.c
@@ -245,9 +245,6 @@ _mesa_PushAttrib(GLbitfield mask)
   
  GET_CURRENT_CONTEXT(ctx);
   
-   if (MESA_VERBOSE & VERBOSE_API)

-  _mesa_debug(ctx, "glPushAttrib %x\n", (int) mask);
-
  if (ctx->AttribStackDepth >= MAX_ATTRIB_STACK_DEPTH) {
 _mesa_error( ctx, GL_STACK_OVERFLOW, "glPushAttrib" );
 return;
@@ -935,11 +932,6 @@ _mesa_PopAttrib(void)
   
  while (attr) {
   
-  if

Re: [Mesa-dev] [PATCH] RFC: mesa: remove MESA_VERBOSE env variable

2017-04-20 Thread Jordan Justen 
On 2017-04-20 12:33:45, Samuel Pitoiset wrote:
> I have used it sometimes, but since VERBOSE_API is missing in a bunch of
> places, that's quite useless. :)
>

I also use MESA_VERBOSE=api every few months or so. I've found it
pretty useful, but frustratingly I've nearly always ended up having to
add new calls for it become useful.

Something that automates wrapping the API probably is perhaps more
maintainable. A lightweight apitrace mode? (Or, maybe this already
exists??) Perhaps something built into a 'debug' mode for libglvnd or
libepoxy? The downside of the library shim option is that it can be
sometimes be annoying to get your app to run with the shim.

-Jordan

> On 04/20/2017 04:12 PM, Emil Velikov wrote:
> > From: Emil Velikov 
> > 
> > The env variable was useful in the early days of mesa development, when
> > tools such at Apitrace were not available.
> > 
> > Nowadays, out of the 12 options only a third are used. With nearly all
> > instances printing the API state. And even then not all entry points
> > are annotated, thus one cannot rely upon it.
> > 
> > The current patch removes the variable all together alongside a few
> > instances (listed below) which developers may see value in keeping.
> > 
> > FLUSH_VERTICES/FLUSH_CURRENT macros in src/mesa/main/context.h
> > _mesa_GetGraphicsResetStatusARB in src/mesa/main/robustness.c
> > _mesa_print_state() in src/mesa/main/state.c
> > _mesa_notifySwapBuffers() in src/mesa/main/context.c
> > 
> > Do we want to keep any of the MESA_VERBOSE instances? If so documenting
> > the variable seems reasonable.
> > 
> > Cc: Brian Paul 
> > Signed-off-by: Emil Velikov 
> > ---
> > Brian, I assume that you're the author for most of these. How do you
> > feel on the topic?
> > ---
> >   src/mesa/main/attrib.c  |   8 ---
> >   src/mesa/main/blend.c   |  40 
> >   src/mesa/main/blit.c|  17 -
> >   src/mesa/main/bufferobj.c   |  38 ---
> >   src/mesa/main/buffers.c |   7 ---
> >   src/mesa/main/clear.c   |   3 -
> >   src/mesa/main/compute.c |  13 
> >   src/mesa/main/context.c |  19 +-
> >   src/mesa/main/context.h |   4 --
> >   src/mesa/main/copyimage.c   |  10 ---
> >   src/mesa/main/debug.c   |  42 -
> >   src/mesa/main/depth.c   |  12 
> >   src/mesa/main/dlist.c   |  17 -
> >   src/mesa/main/drawpix.c |  21 ---
> >   src/mesa/main/enable.c  |   6 --
> >   src/mesa/main/fbobject.c|  32 --
> >   src/mesa/main/feedback.c|   3 -
> >   src/mesa/main/getstring.c   |   6 --
> >   src/mesa/main/hint.c|   5 --
> >   src/mesa/main/light.c   |  14 -
> >   src/mesa/main/lines.c   |   6 --
> >   src/mesa/main/matrix.c  |  29 -
> >   src/mesa/main/mtypes.h  |  22 ---
> >   src/mesa/main/performance_monitor.c |   6 --
> >   src/mesa/main/pipelineobj.c |  33 --
> >   src/mesa/main/polygon.c |  23 ---
> >   src/mesa/main/program_resource.c|  33 --
> >   src/mesa/main/queryobj.c|  31 -
> >   src/mesa/main/readpix.c |   7 ---
> >   src/mesa/main/robustness.c  |  14 +
> >   src/mesa/main/samplerobj.c  |   3 -
> >   src/mesa/main/scissor.c |  11 
> >   src/mesa/main/shaderapi.c   |  16 -
> >   src/mesa/main/state.c   |   3 -
> >   src/mesa/main/stencil.c |  27 
> >   src/mesa/main/texenv.c  |   7 ---
> >   src/mesa/main/texgen.c  |   7 ---
> >   src/mesa/main/texgetimage.c |  17 -
> >   src/mesa/main/teximage.c|  52 ---
> >   src/mesa/main/texobj.c  |  29 -
> >   src/mesa/main/texstate.c|   8 ---
> >   src/mesa/main/texstorage.c  |  13 
> >   src/mesa/main/textureview.c |   6 --
> >   src/mesa/main/varray.c  |   6 --
> >   src/mesa/main/viewport.c|  28 -
> >   src/mesa/tnl/t_vb_render.c  |   5 --
> >   src/mesa/vbo/vbo_exec_array.c   | 122 
> > 
> >   47 files changed, 2 insertions(+), 879 deletions(-)
> > 
> > diff --git a/src/mesa/main/attrib.c b/src/mesa/main/attrib.c
> > index 8e738c91c8f..bb230404d29 100644
> > --- a/src/mesa/main/attrib.c
> > +++ b/src/mesa/main/attrib.c
> > @@ -245,9 +245,6 @@ _mesa_PushAttrib(GLbitfield mask)
> >   
> >  GET_CURRENT_CONTEXT(ctx);
> >   
> > -   if (MESA_VERBOSE & VERBOSE_API)
> > -  _mesa_debug(ctx, "glPushAttrib %x\n", (int) mask);
> > -
> >  if (ctx->AttribStackDepth >= MAX_ATTRIB_STACK_DEPTH) {
> > _mesa_error( ctx, GL_STACK_OVERFLOW, "glPushAttrib" );
>

Re: [Mesa-dev] [PATCH] RFC: mesa: remove MESA_VERBOSE env variable

2017-04-20 Thread Timothy Arceri 
Thanks for doing this. Looks like you forgot to updated the docs, with 
that fixed.


Acked-by: Timothy Arceri 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] RFC: mesa: remove MESA_VERBOSE env variable

2017-04-20 Thread Samuel Pitoiset 
I have used it sometimes, but since VERBOSE_API is missing in a bunch of 
places, that's quite useless. :)


On 04/20/2017 04:12 PM, Emil Velikov wrote:

From: Emil Velikov 

The env variable was useful in the early days of mesa development, when
tools such at Apitrace were not available.

Nowadays, out of the 12 options only a third are used. With nearly all
instances printing the API state. And even then not all entry points
are annotated, thus one cannot rely upon it.

The current patch removes the variable all together alongside a few
instances (listed below) which developers may see value in keeping.

FLUSH_VERTICES/FLUSH_CURRENT macros in src/mesa/main/context.h
_mesa_GetGraphicsResetStatusARB in src/mesa/main/robustness.c
_mesa_print_state() in src/mesa/main/state.c
_mesa_notifySwapBuffers() in src/mesa/main/context.c

Do we want to keep any of the MESA_VERBOSE instances? If so documenting
the variable seems reasonable.

Cc: Brian Paul 
Signed-off-by: Emil Velikov 
---
Brian, I assume that you're the author for most of these. How do you
feel on the topic?
---
  src/mesa/main/attrib.c  |   8 ---
  src/mesa/main/blend.c   |  40 
  src/mesa/main/blit.c|  17 -
  src/mesa/main/bufferobj.c   |  38 ---
  src/mesa/main/buffers.c |   7 ---
  src/mesa/main/clear.c   |   3 -
  src/mesa/main/compute.c |  13 
  src/mesa/main/context.c |  19 +-
  src/mesa/main/context.h |   4 --
  src/mesa/main/copyimage.c   |  10 ---
  src/mesa/main/debug.c   |  42 -
  src/mesa/main/depth.c   |  12 
  src/mesa/main/dlist.c   |  17 -
  src/mesa/main/drawpix.c |  21 ---
  src/mesa/main/enable.c  |   6 --
  src/mesa/main/fbobject.c|  32 --
  src/mesa/main/feedback.c|   3 -
  src/mesa/main/getstring.c   |   6 --
  src/mesa/main/hint.c|   5 --
  src/mesa/main/light.c   |  14 -
  src/mesa/main/lines.c   |   6 --
  src/mesa/main/matrix.c  |  29 -
  src/mesa/main/mtypes.h  |  22 ---
  src/mesa/main/performance_monitor.c |   6 --
  src/mesa/main/pipelineobj.c |  33 --
  src/mesa/main/polygon.c |  23 ---
  src/mesa/main/program_resource.c|  33 --
  src/mesa/main/queryobj.c|  31 -
  src/mesa/main/readpix.c |   7 ---
  src/mesa/main/robustness.c  |  14 +
  src/mesa/main/samplerobj.c  |   3 -
  src/mesa/main/scissor.c |  11 
  src/mesa/main/shaderapi.c   |  16 -
  src/mesa/main/state.c   |   3 -
  src/mesa/main/stencil.c |  27 
  src/mesa/main/texenv.c  |   7 ---
  src/mesa/main/texgen.c  |   7 ---
  src/mesa/main/texgetimage.c |  17 -
  src/mesa/main/teximage.c|  52 ---
  src/mesa/main/texobj.c  |  29 -
  src/mesa/main/texstate.c|   8 ---
  src/mesa/main/texstorage.c  |  13 
  src/mesa/main/textureview.c |   6 --
  src/mesa/main/varray.c  |   6 --
  src/mesa/main/viewport.c|  28 -
  src/mesa/tnl/t_vb_render.c  |   5 --
  src/mesa/vbo/vbo_exec_array.c   | 122 
  47 files changed, 2 insertions(+), 879 deletions(-)

diff --git a/src/mesa/main/attrib.c b/src/mesa/main/attrib.c
index 8e738c91c8f..bb230404d29 100644
--- a/src/mesa/main/attrib.c
+++ b/src/mesa/main/attrib.c
@@ -245,9 +245,6 @@ _mesa_PushAttrib(GLbitfield mask)
  
 GET_CURRENT_CONTEXT(ctx);
  
-   if (MESA_VERBOSE & VERBOSE_API)

-  _mesa_debug(ctx, "glPushAttrib %x\n", (int) mask);
-
 if (ctx->AttribStackDepth >= MAX_ATTRIB_STACK_DEPTH) {
_mesa_error( ctx, GL_STACK_OVERFLOW, "glPushAttrib" );
return;
@@ -935,11 +932,6 @@ _mesa_PopAttrib(void)
  
 while (attr) {
  
-  if (MESA_VERBOSE & VERBOSE_API) {

- _mesa_debug(ctx, "glPopAttrib %s\n",
- _mesa_enum_to_string(attr->kind));
-  }
-
switch (attr->kind) {
   case DUMMY_BIT:
  /* do nothing */
diff --git a/src/mesa/main/blend.c b/src/mesa/main/blend.c
index 955fda1158c..52c473a9d8b 100644
--- a/src/mesa/main/blend.c
+++ b/src/mesa/main/blend.c
@@ -220,13 +220,6 @@ _mesa_BlendFuncSeparate( GLenum sfactorRGB, GLenum 
dfactorRGB,
 unsigned buf;
 bool changed = false;
  
-   if (MESA_VERBOSE & VERBOSE_API)

-  _mesa_debug(ctx, "glBlendFuncSeparate %s %s %s %s\n",
-  _mesa_enum_to_string(sfactorRGB),
-  _mesa_enum_to_string(dfactorRGB),
-  _mesa_enum_to_string(sfactorA),
-  _mesa_enum_to_string(dfactorA));
-

Re: [Mesa-dev] [PATCH] RFC: mesa: remove MESA_VERBOSE env variable

2017-04-20 Thread Eric Anholt 
Emil Velikov  writes:

> From: Emil Velikov 
>
> The env variable was useful in the early days of mesa development, when
> tools such at Apitrace were not available.
>
> Nowadays, out of the 12 options only a third are used. With nearly all
> instances printing the API state. And even then not all entry points
> are annotated, thus one cannot rely upon it.
>
> The current patch removes the variable all together alongside a few
> instances (listed below) which developers may see value in keeping.
>
> FLUSH_VERTICES/FLUSH_CURRENT macros in src/mesa/main/context.h
> _mesa_GetGraphicsResetStatusARB in src/mesa/main/robustness.c
> _mesa_print_state() in src/mesa/main/state.c
> _mesa_notifySwapBuffers() in src/mesa/main/context.c
>
> Do we want to keep any of the MESA_VERBOSE instances? If so documenting
> the variable seems reasonable.

FWIW, I'm happy to see VERBOSE_API go. I've never used it, because I
don't trust it.  If we wanted to support it, I'd rather we worked on
some sort of dispatch table stacking so that we could code-gen the
VERBOSE_API printfs.


signature.asc
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] RFC: mesa: remove MESA_VERBOSE env variable

2017-04-20 Thread Brian Paul 

On 04/20/2017 08:12 AM, Emil Velikov wrote:

From: Emil Velikov 

The env variable was useful in the early days of mesa development, when
tools such at Apitrace were not available.

Nowadays, out of the 12 options only a third are used. With nearly all
instances printing the API state. And even then not all entry points
are annotated, thus one cannot rely upon it.

The current patch removes the variable all together alongside a few
instances (listed below) which developers may see value in keeping.

FLUSH_VERTICES/FLUSH_CURRENT macros in src/mesa/main/context.h
_mesa_GetGraphicsResetStatusARB in src/mesa/main/robustness.c
_mesa_print_state() in src/mesa/main/state.c
_mesa_notifySwapBuffers() in src/mesa/main/context.c

Do we want to keep any of the MESA_VERBOSE instances? If so documenting
the variable seems reasonable.

Cc: Brian Paul 
Signed-off-by: Emil Velikov 
---
Brian, I assume that you're the author for most of these. How do you
feel on the topic?


I think Keith was the original author of MESA_VERBOSE (but that may have 
been a clean-up of some of even earlier code of mine).


I haven't used this in years so I'm OK with removing it.

-Brian


---
  src/mesa/main/attrib.c  |   8 ---
  src/mesa/main/blend.c   |  40 
  src/mesa/main/blit.c|  17 -
  src/mesa/main/bufferobj.c   |  38 ---
  src/mesa/main/buffers.c |   7 ---
  src/mesa/main/clear.c   |   3 -
  src/mesa/main/compute.c |  13 
  src/mesa/main/context.c |  19 +-
  src/mesa/main/context.h |   4 --
  src/mesa/main/copyimage.c   |  10 ---
  src/mesa/main/debug.c   |  42 -
  src/mesa/main/depth.c   |  12 
  src/mesa/main/dlist.c   |  17 -
  src/mesa/main/drawpix.c |  21 ---
  src/mesa/main/enable.c  |   6 --
  src/mesa/main/fbobject.c|  32 --
  src/mesa/main/feedback.c|   3 -
  src/mesa/main/getstring.c   |   6 --
  src/mesa/main/hint.c|   5 --
  src/mesa/main/light.c   |  14 -
  src/mesa/main/lines.c   |   6 --
  src/mesa/main/matrix.c  |  29 -
  src/mesa/main/mtypes.h  |  22 ---
  src/mesa/main/performance_monitor.c |   6 --
  src/mesa/main/pipelineobj.c |  33 --
  src/mesa/main/polygon.c |  23 ---
  src/mesa/main/program_resource.c|  33 --
  src/mesa/main/queryobj.c|  31 -
  src/mesa/main/readpix.c |   7 ---
  src/mesa/main/robustness.c  |  14 +
  src/mesa/main/samplerobj.c  |   3 -
  src/mesa/main/scissor.c |  11 
  src/mesa/main/shaderapi.c   |  16 -
  src/mesa/main/state.c   |   3 -
  src/mesa/main/stencil.c |  27 
  src/mesa/main/texenv.c  |   7 ---
  src/mesa/main/texgen.c  |   7 ---
  src/mesa/main/texgetimage.c |  17 -
  src/mesa/main/teximage.c|  52 ---
  src/mesa/main/texobj.c  |  29 -
  src/mesa/main/texstate.c|   8 ---
  src/mesa/main/texstorage.c  |  13 
  src/mesa/main/textureview.c |   6 --
  src/mesa/main/varray.c  |   6 --
  src/mesa/main/viewport.c|  28 -
  src/mesa/tnl/t_vb_render.c  |   5 --
  src/mesa/vbo/vbo_exec_array.c   | 122 
  47 files changed, 2 insertions(+), 879 deletions(-)

diff --git a/src/mesa/main/attrib.c b/src/mesa/main/attrib.c
index 8e738c91c8f..bb230404d29 100644
--- a/src/mesa/main/attrib.c
+++ b/src/mesa/main/attrib.c
@@ -245,9 +245,6 @@ _mesa_PushAttrib(GLbitfield mask)

 GET_CURRENT_CONTEXT(ctx);

-   if (MESA_VERBOSE & VERBOSE_API)
-  _mesa_debug(ctx, "glPushAttrib %x\n", (int) mask);
-
 if (ctx->AttribStackDepth >= MAX_ATTRIB_STACK_DEPTH) {
_mesa_error( ctx, GL_STACK_OVERFLOW, "glPushAttrib" );
return;
@@ -935,11 +932,6 @@ _mesa_PopAttrib(void)

 while (attr) {

-  if (MESA_VERBOSE & VERBOSE_API) {
- _mesa_debug(ctx, "glPopAttrib %s\n",
- _mesa_enum_to_string(attr->kind));
-  }
-
switch (attr->kind) {
   case DUMMY_BIT:
  /* do nothing */
diff --git a/src/mesa/main/blend.c b/src/mesa/main/blend.c
index 955fda1158c..52c473a9d8b 100644
--- a/src/mesa/main/blend.c
+++ b/src/mesa/main/blend.c
@@ -220,13 +220,6 @@ _mesa_BlendFuncSeparate( GLenum sfactorRGB, GLenum 
dfactorRGB,
 unsigned buf;
 bool changed = false;

-   if (MESA_VERBOSE & VERBOSE_API)
-  _mesa_debug(ctx, "glBlendFuncSeparate %s %s %s %s\n",
-  _mesa_enum_to_string(sfactorRGB),
-  _mesa_enum_to_string(dfactorRGB),
-

[Mesa-dev] [PATCH] RFC: mesa: remove MESA_VERBOSE env variable

2017-04-20 Thread Emil Velikov 
From: Emil Velikov 

The env variable was useful in the early days of mesa development, when
tools such at Apitrace were not available.

Nowadays, out of the 12 options only a third are used. With nearly all
instances printing the API state. And even then not all entry points
are annotated, thus one cannot rely upon it.

The current patch removes the variable all together alongside a few
instances (listed below) which developers may see value in keeping.

FLUSH_VERTICES/FLUSH_CURRENT macros in src/mesa/main/context.h
_mesa_GetGraphicsResetStatusARB in src/mesa/main/robustness.c
_mesa_print_state() in src/mesa/main/state.c
_mesa_notifySwapBuffers() in src/mesa/main/context.c

Do we want to keep any of the MESA_VERBOSE instances? If so documenting
the variable seems reasonable.

Cc: Brian Paul 
Signed-off-by: Emil Velikov 
---
Brian, I assume that you're the author for most of these. How do you
feel on the topic?
---
 src/mesa/main/attrib.c  |   8 ---
 src/mesa/main/blend.c   |  40 
 src/mesa/main/blit.c|  17 -
 src/mesa/main/bufferobj.c   |  38 ---
 src/mesa/main/buffers.c |   7 ---
 src/mesa/main/clear.c   |   3 -
 src/mesa/main/compute.c |  13 
 src/mesa/main/context.c |  19 +-
 src/mesa/main/context.h |   4 --
 src/mesa/main/copyimage.c   |  10 ---
 src/mesa/main/debug.c   |  42 -
 src/mesa/main/depth.c   |  12 
 src/mesa/main/dlist.c   |  17 -
 src/mesa/main/drawpix.c |  21 ---
 src/mesa/main/enable.c  |   6 --
 src/mesa/main/fbobject.c|  32 --
 src/mesa/main/feedback.c|   3 -
 src/mesa/main/getstring.c   |   6 --
 src/mesa/main/hint.c|   5 --
 src/mesa/main/light.c   |  14 -
 src/mesa/main/lines.c   |   6 --
 src/mesa/main/matrix.c  |  29 -
 src/mesa/main/mtypes.h  |  22 ---
 src/mesa/main/performance_monitor.c |   6 --
 src/mesa/main/pipelineobj.c |  33 --
 src/mesa/main/polygon.c |  23 ---
 src/mesa/main/program_resource.c|  33 --
 src/mesa/main/queryobj.c|  31 -
 src/mesa/main/readpix.c |   7 ---
 src/mesa/main/robustness.c  |  14 +
 src/mesa/main/samplerobj.c  |   3 -
 src/mesa/main/scissor.c |  11 
 src/mesa/main/shaderapi.c   |  16 -
 src/mesa/main/state.c   |   3 -
 src/mesa/main/stencil.c |  27 
 src/mesa/main/texenv.c  |   7 ---
 src/mesa/main/texgen.c  |   7 ---
 src/mesa/main/texgetimage.c |  17 -
 src/mesa/main/teximage.c|  52 ---
 src/mesa/main/texobj.c  |  29 -
 src/mesa/main/texstate.c|   8 ---
 src/mesa/main/texstorage.c  |  13 
 src/mesa/main/textureview.c |   6 --
 src/mesa/main/varray.c  |   6 --
 src/mesa/main/viewport.c|  28 -
 src/mesa/tnl/t_vb_render.c  |   5 --
 src/mesa/vbo/vbo_exec_array.c   | 122 
 47 files changed, 2 insertions(+), 879 deletions(-)

diff --git a/src/mesa/main/attrib.c b/src/mesa/main/attrib.c
index 8e738c91c8f..bb230404d29 100644
--- a/src/mesa/main/attrib.c
+++ b/src/mesa/main/attrib.c
@@ -245,9 +245,6 @@ _mesa_PushAttrib(GLbitfield mask)
 
GET_CURRENT_CONTEXT(ctx);
 
-   if (MESA_VERBOSE & VERBOSE_API)
-  _mesa_debug(ctx, "glPushAttrib %x\n", (int) mask);
-
if (ctx->AttribStackDepth >= MAX_ATTRIB_STACK_DEPTH) {
   _mesa_error( ctx, GL_STACK_OVERFLOW, "glPushAttrib" );
   return;
@@ -935,11 +932,6 @@ _mesa_PopAttrib(void)
 
while (attr) {
 
-  if (MESA_VERBOSE & VERBOSE_API) {
- _mesa_debug(ctx, "glPopAttrib %s\n",
- _mesa_enum_to_string(attr->kind));
-  }
-
   switch (attr->kind) {
  case DUMMY_BIT:
 /* do nothing */
diff --git a/src/mesa/main/blend.c b/src/mesa/main/blend.c
index 955fda1158c..52c473a9d8b 100644
--- a/src/mesa/main/blend.c
+++ b/src/mesa/main/blend.c
@@ -220,13 +220,6 @@ _mesa_BlendFuncSeparate( GLenum sfactorRGB, GLenum 
dfactorRGB,
unsigned buf;
bool changed = false;
 
-   if (MESA_VERBOSE & VERBOSE_API)
-  _mesa_debug(ctx, "glBlendFuncSeparate %s %s %s %s\n",
-  _mesa_enum_to_string(sfactorRGB),
-  _mesa_enum_to_string(dfactorRGB),
-  _mesa_enum_to_string(sfactorA),
-  _mesa_enum_to_string(dfactorA));
-
/* Check if we're really changing any state.  If not, return early. */
if (ctx->Color._BlendFuncPerBuffer) {
   /* Check all per-buffer states */
@@ -417,10 +410,6 @@ _mesa_BlendEquation( GLenum mode )