Re: [Mesa-dev] [PATCH 11/25] mesa/i965/i915/r200: eliminate gl_vertex_program
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, &save->VertexProgram, >
Re: [Mesa-dev] [PATCH 11/25] mesa/i965/i915/r200: eliminate gl_vertex_program
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, &save->VertexProgram, > - ctx->VertexProgram.Current); > + _mesa_reference_program(ctx, &save->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, &ctx->VertexProgram.Current, > - save->VertexProgram); > - _mesa_reference_vertprog(ctx, &save->VertexProgram, NULL); > + _mesa_reference_program(ctx, &ctx->VertexProgram.Current, > + save->VertexProgram); > + _mesa_reference_progra
[Mesa-dev] [PATCH 11/25] mesa/i965/i915/r200: eliminate gl_vertex_program
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, &save->VertexProgram, - ctx->VertexProgram.Current); + _mesa_reference_program(ctx, &save->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, &ctx->VertexProgram.Current, - save->VertexProgram); -_mesa_reference_vertprog(ctx, &save->VertexProgram, NULL); + _mesa_reference_program(ctx, &ctx->VertexProgram.Current, + save->VertexProgram); +_mesa_reference_program(ctx, &save->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