[Mesa-dev] [PATCH 17/19] mesa: Add notes about remaining NV_vertex_program code.

[Kenneth Graunke]

On 10/10/2012 07:34 AM, Roland Scheidegger wrote:
> Am 10.10.2012 03:06, schrieb Eric Anholt:
>> ---
>>   src/mesa/main/nvprogram.c   |    4 ++++
>>   src/mesa/main/vtxfmt.c      |    2 +-
>>   src/mesa/vbo/vbo_exec_api.c |    4 ++++
>>   3 files changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/mesa/main/nvprogram.c b/src/mesa/main/nvprogram.c
>> index 6ad846e3..4b24dde 100644
>> --- a/src/mesa/main/nvprogram.c
>> +++ b/src/mesa/main/nvprogram.c
>> @@ -205,6 +205,10 @@ _mesa_GetProgramStringNV(GLuint id, GLenum pname, GLubyte *program)
>>
>>   /**
>>    * Get a vertex array attribute pointer.
>> + *
>> + * While the function first appeared in GL_NV_vertex_program, it's
>> + * aliased for use by GL_ARB_vertex_program.
>> + *
>>    * \note Not compiled into display lists.
>>    * \note Called from the GL API dispatcher.
>>    */
>> diff --git a/src/mesa/main/vtxfmt.c b/src/mesa/main/vtxfmt.c
>> index aa6cf28..462cc88 100644
>> --- a/src/mesa/main/vtxfmt.c
>> +++ b/src/mesa/main/vtxfmt.c
>> @@ -135,7 +135,7 @@ install_vtxfmt(struct gl_context *ctx, struct _glapi_table *tab,
>>                                                  vfmt->DrawTransformFeedbackStreamInstanced);
>>      }
>>
>> -   /* GL_NV_vertex_program */
>> +   /* Originally for GL_NV_vertex_program, this is also used by dlist.c */
>>      if (ctx->API == API_OPENGL) {
>>         SET_VertexAttrib1fNV(tab, vfmt->VertexAttrib1fNV);
>>         SET_VertexAttrib1fvNV(tab, vfmt->VertexAttrib1fvNV);
>> diff --git a/src/mesa/vbo/vbo_exec_api.c b/src/mesa/vbo/vbo_exec_api.c
>> index 52b363d..2ddb715 100644
>> --- a/src/mesa/vbo/vbo_exec_api.c
>> +++ b/src/mesa/vbo/vbo_exec_api.c
>> @@ -958,6 +958,10 @@ static void vbo_exec_vtxfmt_init( struct vbo_exec_context *exec )
>>         vfmt->VertexAttrib4fvARB = vbo_VertexAttrib4fvARB;
>>      }
>>
>> +   /* Note that VertexAttrib4fNV is used from dlist.c and api_arrayelt.c so
>> +    * they can have a single entrypoint for updating any of the legacy
>> +    * attribs.
>> +    */
>>      vfmt->VertexAttrib1fNV = vbo_VertexAttrib1fNV;
>>      vfmt->VertexAttrib1fvNV = vbo_VertexAttrib1fvNV;
>>      vfmt->VertexAttrib2fNV = vbo_VertexAttrib2fNV;
>>
>
> I think the idea of removing NV_vertex_program is quite good, though I
> wonder if these functions shouldn't be renamed and moved instead of
> keeping them as is?
> Supporting both NV_vertex_program ARB_vertex_program always seems to
> have caused headaches (not least because of the different aliasing of
> attribs), though for a long time ARB_vertex_program in mesa actually was
> code dependent quite a bit on NV_vertex_program.
> IIRC celestia used to use NV_vertex_program but it's quite possible it
> no longer does.
>
> Roland

Wow...this simplified a lot more than I thought.  I forgot about all the 
nasty attribute aliasing.  Should make the code a lot easier to follow.

I agree with Roland here...if it's actually an internal function, we 
should probably rename it.  I'd be okay with a follow-up patch.

Either way, this series is:
Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>

Also: when Ian and I presented at XDC 2011, we discussed removing 
NV_fragment_program, and IIRC no one had any objections.  We can 
probably kill that as well (especially now that NV_vp is gone).  I'll 
take a stab at that.