Re: [Mesa-dev] [PATCH 11/25] mesa/i965/i915/r200: eliminate gl_vertex_program

2016-10-18 Thread Timothy Arceri
On Tue, 2016-10-18 at 12:07 -0700, Ian Romanick wrote:
> I'd like to see two tiny changes:
> 
> 1. A comment for the IsPositionInvariant field that it can only be
> true
> for vertex programs.

I already had that this is only used for assembly style vertex
programs. I've reworded it to be only true for :)

> 
> 2. An assertion or two like
> 
> assert(p->Target == GL_VERTEX_PROGRAM_ARB ||
>    !p->IsPositionInvariant);

I'm not sure how useful this is.
> 
>    in reasonable places.  I'm thinking:
> 
>    - Where it's assigned in src/mesa/program/arbprogparse.c

It assigned in mesa_parse_arb_vertex_program() and there is already an

assert(target == GL_VERTEX_PROGRAM_ARB);

> 
>    - Where it's used in src/mesa/state_tracker/st_program.c,

Again this is in st_translate_vertex_program()

so if its not a vp we already have problems.

>  src/mesa/drivers/dri/i965/brw_program.c, and

Its used inside case GL_VERTEX_PROGRAM_ARB:

I've added the assert to the top of the function but it seems kind of
pointless.

>  src/mesa/tnl/t_vb_program.c (both places).

In both of these the program always comes from ctx-
>VertexProgram._Current so it doesn't seems very useful here either.

> 
> I'd also support a follow-up patch that converts IsPositionInvariant
> from GLboolean to bool. :)
> 
> On 10/17/2016 11:12 PM, Timothy Arceri wrote:
> > 
> > Here we move the only field in gl_vertex_program to the
> > ARB program fields in gl_program.
> > ---
> >  src/mesa/drivers/common/meta.c   | 10 +--
> >  src/mesa/drivers/common/meta.h   |  2 +-
> >  src/mesa/drivers/dri/i915/i915_fragprog.c|  4 +-
> >  src/mesa/drivers/dri/i965/brw_context.h  |  8 +--
> >  src/mesa/drivers/dri/i965/brw_curbe.c|  2 +-
> >  src/mesa/drivers/dri/i965/brw_draw.c |  4 +-
> >  src/mesa/drivers/dri/i965/brw_program.c  |  5 +-
> >  src/mesa/drivers/dri/i965/brw_vs.c   | 41 ++--
> >  src/mesa/drivers/dri/i965/brw_vs_surface_state.c |  2 +-
> >  src/mesa/drivers/dri/i965/gen6_vs_state.c|  4 +-
> >  src/mesa/drivers/dri/r200/r200_context.h |  2 +-
> >  src/mesa/drivers/dri/r200/r200_state_init.c  |  4 +-
> >  src/mesa/drivers/dri/r200/r200_tcl.c |  2 +-
> >  src/mesa/drivers/dri/r200/r200_vertprog.c| 82
> > 
> >  src/mesa/main/arbprogram.c   | 19 +++---
> >  src/mesa/main/context.c  |  8 +--
> >  src/mesa/main/ff_fragment_shader.cpp |  2 +-
> >  src/mesa/main/ffvertex_prog.c| 72 ++
> > ---
> >  src/mesa/main/ffvertex_prog.h|  2 +-
> >  src/mesa/main/mtypes.h   | 17 ++---
> >  src/mesa/main/shared.c   |  5 +-
> >  src/mesa/main/state.c| 26 
> >  src/mesa/main/state.h|  2 +-
> >  src/mesa/program/arbprogparse.c  | 46 ++
> > ---
> >  src/mesa/program/arbprogparse.h  |  2 +-
> >  src/mesa/program/prog_statevars.c|  8 +--
> >  src/mesa/program/program.c   | 15 ++---
> >  src/mesa/program/program.h   | 26 
> >  src/mesa/program/programopt.c| 42 ++--
> >  src/mesa/program/programopt.h|  2 +-
> >  src/mesa/state_tracker/st_atom.c |  4 +-
> >  src/mesa/state_tracker/st_atom_constbuf.c|  2 +-
> >  src/mesa/state_tracker/st_atom_rasterizer.c  |  8 +--
> >  src/mesa/state_tracker/st_atom_sampler.c |  2 +-
> >  src/mesa/state_tracker/st_atom_shader.c  |  4 +-
> >  src/mesa/state_tracker/st_atom_texture.c |  2 +-
> >  src/mesa/state_tracker/st_cb_feedback.c  |  2 +-
> >  src/mesa/state_tracker/st_cb_program.c   |  2 +-
> >  src/mesa/state_tracker/st_debug.c|  4 +-
> >  src/mesa/state_tracker/st_program.c  | 35 +-
> >  src/mesa/state_tracker/st_program.h  |  4 +-
> >  src/mesa/tnl/t_context.c |  4 +-
> >  src/mesa/tnl/t_vb_program.c  | 24 +++
> >  src/mesa/tnl/t_vp_build.c|  4 +-
> >  src/mesa/vbo/vbo_exec_draw.c |  4 +-
> >  src/mesa/vbo/vbo_save_draw.c |  4 +-
> >  46 files changed, 264 insertions(+), 311 deletions(-)
> > 
> > diff --git a/src/mesa/drivers/common/meta.c
> > b/src/mesa/drivers/common/meta.c
> > index 890e98a..ab81eed 100644
> > --- a/src/mesa/drivers/common/meta.c
> > +++ b/src/mesa/drivers/common/meta.c
> > @@ -566,8 +566,8 @@ _mesa_meta_begin(struct gl_context *ctx,
> > GLbitfield state)
> >  
> >    if (ctx->Extensions.ARB_vertex_program) {
> >   save->VertexProgramEnabled = ctx->VertexProgram.Enabled;
> > - _mesa_reference_vertprog(ctx, >VertexProgram,
> > -  

Re: [Mesa-dev] [PATCH 11/25] mesa/i965/i915/r200: eliminate gl_vertex_program

2016-10-18 Thread Ian Romanick
I'd like to see two tiny changes:

1. A comment for the IsPositionInvariant field that it can only be true
for vertex programs.

2. An assertion or two like

assert(p->Target == GL_VERTEX_PROGRAM_ARB ||
   !p->IsPositionInvariant);

   in reasonable places.  I'm thinking:

   - Where it's assigned in src/mesa/program/arbprogparse.c

   - Where it's used in src/mesa/state_tracker/st_program.c,
 src/mesa/drivers/dri/i965/brw_program.c, and
 src/mesa/tnl/t_vb_program.c (both places).

I'd also support a follow-up patch that converts IsPositionInvariant
from GLboolean to bool. :)

On 10/17/2016 11:12 PM, Timothy Arceri wrote:
> Here we move the only field in gl_vertex_program to the
> ARB program fields in gl_program.
> ---
>  src/mesa/drivers/common/meta.c   | 10 +--
>  src/mesa/drivers/common/meta.h   |  2 +-
>  src/mesa/drivers/dri/i915/i915_fragprog.c|  4 +-
>  src/mesa/drivers/dri/i965/brw_context.h  |  8 +--
>  src/mesa/drivers/dri/i965/brw_curbe.c|  2 +-
>  src/mesa/drivers/dri/i965/brw_draw.c |  4 +-
>  src/mesa/drivers/dri/i965/brw_program.c  |  5 +-
>  src/mesa/drivers/dri/i965/brw_vs.c   | 41 ++--
>  src/mesa/drivers/dri/i965/brw_vs_surface_state.c |  2 +-
>  src/mesa/drivers/dri/i965/gen6_vs_state.c|  4 +-
>  src/mesa/drivers/dri/r200/r200_context.h |  2 +-
>  src/mesa/drivers/dri/r200/r200_state_init.c  |  4 +-
>  src/mesa/drivers/dri/r200/r200_tcl.c |  2 +-
>  src/mesa/drivers/dri/r200/r200_vertprog.c| 82 
> 
>  src/mesa/main/arbprogram.c   | 19 +++---
>  src/mesa/main/context.c  |  8 +--
>  src/mesa/main/ff_fragment_shader.cpp |  2 +-
>  src/mesa/main/ffvertex_prog.c| 72 ++---
>  src/mesa/main/ffvertex_prog.h|  2 +-
>  src/mesa/main/mtypes.h   | 17 ++---
>  src/mesa/main/shared.c   |  5 +-
>  src/mesa/main/state.c| 26 
>  src/mesa/main/state.h|  2 +-
>  src/mesa/program/arbprogparse.c  | 46 ++---
>  src/mesa/program/arbprogparse.h  |  2 +-
>  src/mesa/program/prog_statevars.c|  8 +--
>  src/mesa/program/program.c   | 15 ++---
>  src/mesa/program/program.h   | 26 
>  src/mesa/program/programopt.c| 42 ++--
>  src/mesa/program/programopt.h|  2 +-
>  src/mesa/state_tracker/st_atom.c |  4 +-
>  src/mesa/state_tracker/st_atom_constbuf.c|  2 +-
>  src/mesa/state_tracker/st_atom_rasterizer.c  |  8 +--
>  src/mesa/state_tracker/st_atom_sampler.c |  2 +-
>  src/mesa/state_tracker/st_atom_shader.c  |  4 +-
>  src/mesa/state_tracker/st_atom_texture.c |  2 +-
>  src/mesa/state_tracker/st_cb_feedback.c  |  2 +-
>  src/mesa/state_tracker/st_cb_program.c   |  2 +-
>  src/mesa/state_tracker/st_debug.c|  4 +-
>  src/mesa/state_tracker/st_program.c  | 35 +-
>  src/mesa/state_tracker/st_program.h  |  4 +-
>  src/mesa/tnl/t_context.c |  4 +-
>  src/mesa/tnl/t_vb_program.c  | 24 +++
>  src/mesa/tnl/t_vp_build.c|  4 +-
>  src/mesa/vbo/vbo_exec_draw.c |  4 +-
>  src/mesa/vbo/vbo_save_draw.c |  4 +-
>  46 files changed, 264 insertions(+), 311 deletions(-)
> 
> diff --git a/src/mesa/drivers/common/meta.c b/src/mesa/drivers/common/meta.c
> index 890e98a..ab81eed 100644
> --- a/src/mesa/drivers/common/meta.c
> +++ b/src/mesa/drivers/common/meta.c
> @@ -566,8 +566,8 @@ _mesa_meta_begin(struct gl_context *ctx, GLbitfield state)
>  
>if (ctx->Extensions.ARB_vertex_program) {
>   save->VertexProgramEnabled = ctx->VertexProgram.Enabled;
> - _mesa_reference_vertprog(ctx, >VertexProgram,
> -   ctx->VertexProgram.Current);
> + _mesa_reference_program(ctx, >VertexProgram,
> +  ctx->VertexProgram.Current);
>   _mesa_set_enable(ctx, GL_VERTEX_PROGRAM_ARB, GL_FALSE);
>}
>  
> @@ -945,9 +945,9 @@ _mesa_meta_end(struct gl_context *ctx)
>if (ctx->Extensions.ARB_vertex_program) {
>   _mesa_set_enable(ctx, GL_VERTEX_PROGRAM_ARB,
>save->VertexProgramEnabled);
> - _mesa_reference_vertprog(ctx, >VertexProgram.Current, 
> -  save->VertexProgram);
> -  _mesa_reference_vertprog(ctx, >VertexProgram, NULL);
> + _mesa_reference_program(ctx, >VertexProgram.Current,
> + save->VertexProgram);
> +  _mesa_reference_program(ctx, >VertexProgram, 

[Mesa-dev] [PATCH 11/25] mesa/i965/i915/r200: eliminate gl_vertex_program

2016-10-18 Thread Timothy Arceri
Here we move the only field in gl_vertex_program to the
ARB program fields in gl_program.
---
 src/mesa/drivers/common/meta.c   | 10 +--
 src/mesa/drivers/common/meta.h   |  2 +-
 src/mesa/drivers/dri/i915/i915_fragprog.c|  4 +-
 src/mesa/drivers/dri/i965/brw_context.h  |  8 +--
 src/mesa/drivers/dri/i965/brw_curbe.c|  2 +-
 src/mesa/drivers/dri/i965/brw_draw.c |  4 +-
 src/mesa/drivers/dri/i965/brw_program.c  |  5 +-
 src/mesa/drivers/dri/i965/brw_vs.c   | 41 ++--
 src/mesa/drivers/dri/i965/brw_vs_surface_state.c |  2 +-
 src/mesa/drivers/dri/i965/gen6_vs_state.c|  4 +-
 src/mesa/drivers/dri/r200/r200_context.h |  2 +-
 src/mesa/drivers/dri/r200/r200_state_init.c  |  4 +-
 src/mesa/drivers/dri/r200/r200_tcl.c |  2 +-
 src/mesa/drivers/dri/r200/r200_vertprog.c| 82 
 src/mesa/main/arbprogram.c   | 19 +++---
 src/mesa/main/context.c  |  8 +--
 src/mesa/main/ff_fragment_shader.cpp |  2 +-
 src/mesa/main/ffvertex_prog.c| 72 ++---
 src/mesa/main/ffvertex_prog.h|  2 +-
 src/mesa/main/mtypes.h   | 17 ++---
 src/mesa/main/shared.c   |  5 +-
 src/mesa/main/state.c| 26 
 src/mesa/main/state.h|  2 +-
 src/mesa/program/arbprogparse.c  | 46 ++---
 src/mesa/program/arbprogparse.h  |  2 +-
 src/mesa/program/prog_statevars.c|  8 +--
 src/mesa/program/program.c   | 15 ++---
 src/mesa/program/program.h   | 26 
 src/mesa/program/programopt.c| 42 ++--
 src/mesa/program/programopt.h|  2 +-
 src/mesa/state_tracker/st_atom.c |  4 +-
 src/mesa/state_tracker/st_atom_constbuf.c|  2 +-
 src/mesa/state_tracker/st_atom_rasterizer.c  |  8 +--
 src/mesa/state_tracker/st_atom_sampler.c |  2 +-
 src/mesa/state_tracker/st_atom_shader.c  |  4 +-
 src/mesa/state_tracker/st_atom_texture.c |  2 +-
 src/mesa/state_tracker/st_cb_feedback.c  |  2 +-
 src/mesa/state_tracker/st_cb_program.c   |  2 +-
 src/mesa/state_tracker/st_debug.c|  4 +-
 src/mesa/state_tracker/st_program.c  | 35 +-
 src/mesa/state_tracker/st_program.h  |  4 +-
 src/mesa/tnl/t_context.c |  4 +-
 src/mesa/tnl/t_vb_program.c  | 24 +++
 src/mesa/tnl/t_vp_build.c|  4 +-
 src/mesa/vbo/vbo_exec_draw.c |  4 +-
 src/mesa/vbo/vbo_save_draw.c |  4 +-
 46 files changed, 264 insertions(+), 311 deletions(-)

diff --git a/src/mesa/drivers/common/meta.c b/src/mesa/drivers/common/meta.c
index 890e98a..ab81eed 100644
--- a/src/mesa/drivers/common/meta.c
+++ b/src/mesa/drivers/common/meta.c
@@ -566,8 +566,8 @@ _mesa_meta_begin(struct gl_context *ctx, GLbitfield state)
 
   if (ctx->Extensions.ARB_vertex_program) {
  save->VertexProgramEnabled = ctx->VertexProgram.Enabled;
- _mesa_reference_vertprog(ctx, >VertexProgram,
- ctx->VertexProgram.Current);
+ _mesa_reference_program(ctx, >VertexProgram,
+ctx->VertexProgram.Current);
  _mesa_set_enable(ctx, GL_VERTEX_PROGRAM_ARB, GL_FALSE);
   }
 
@@ -945,9 +945,9 @@ _mesa_meta_end(struct gl_context *ctx)
   if (ctx->Extensions.ARB_vertex_program) {
  _mesa_set_enable(ctx, GL_VERTEX_PROGRAM_ARB,
   save->VertexProgramEnabled);
- _mesa_reference_vertprog(ctx, >VertexProgram.Current, 
-  save->VertexProgram);
-_mesa_reference_vertprog(ctx, >VertexProgram, NULL);
+ _mesa_reference_program(ctx, >VertexProgram.Current,
+ save->VertexProgram);
+_mesa_reference_program(ctx, >VertexProgram, NULL);
   }
 
   if (ctx->Extensions.ARB_fragment_program) {
diff --git a/src/mesa/drivers/common/meta.h b/src/mesa/drivers/common/meta.h
index ba83a6d..4d3b8ec 100644
--- a/src/mesa/drivers/common/meta.h
+++ b/src/mesa/drivers/common/meta.h
@@ -121,7 +121,7 @@ struct save_state
 
/** MESA_META_SHADER */
GLboolean VertexProgramEnabled;
-   struct gl_vertex_program *VertexProgram;
+   struct gl_program *VertexProgram;
GLboolean FragmentProgramEnabled;
struct gl_fragment_program *FragmentProgram;
GLboolean ATIFragmentShaderEnabled;
diff --git a/src/mesa/drivers/dri/i915/i915_fragprog.c 
b/src/mesa/drivers/dri/i915/i915_fragprog.c
index 1944b3d..4e1df73 100644
--- a/src/mesa/drivers/dri/i915/i915_fragprog.c
+++ b/src/mesa/drivers/dri/i915/i915_fragprog.c
@@ -1148,8