[PATCH 0/4] Handle both ARB and NV programs in glProgramStringARB and glLoadProgramNV

[Ian Romanick-4]

This patch series enables glProgramStringARB to handle NV vertex and
fragment programs when the appropriate extensions are supported.  It
also enables glLoadProgramNV to handle ARB vertex and fragment
programs.  This behavior is described in the "Interactions with"
sections of ARB_vertex_program and NV_fragment_program.

These are the least intrusive patches for the 7.6 release branch.
Ideally the ARB parser would be extended to handle the NV programs,
and a lot of the existing NV handling code would be refactored.  This
seems too intrusive for the stable branch, especially since we don't
have a good parser test suite for the NV programs.

One quirk that bothers me about the existing code:  glLoadProgramNV
doesn't call Driver.ProgramStringNotify.  Is this intentional?

Signed-off-by: Ian Romanick <ian.d.romanick@...>

 src/mesa/shader/arbprogram.c |   44 ++++++++++++++++++++++++++++++++++++-----
 src/mesa/shader/nvprogram.c  |   29 ++++++++++++++++++++++++++-
 2 files changed, 66 insertions(+), 7 deletions(-)

------------------------------------------------------------------------------
Come build with us! The BlackBerry(R) Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay
ahead of the curve. Join us from November 9 - 12, 2009. Register now!
http://p.sf.net/sfu/devconference
_______________________________________________
Mesa3d-dev mailing list
Mesa3d-dev@...
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev

[Ian Romanick-4]

From: Ian Romanick <ian.d.romanick@...>

---
 src/mesa/shader/arbprogram.c |   20 +++++++++++++++-----
 1 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/src/mesa/shader/arbprogram.c b/src/mesa/shader/arbprogram.c
index 4d8cff0..ca71a39 100644
--- a/src/mesa/shader/arbprogram.c
+++ b/src/mesa/shader/arbprogram.c
@@ -37,6 +37,7 @@
 #include "main/mtypes.h"
 #include "arbprogram.h"
 #include "arbprogparse.h"
+#include "nvfragparse.h"
 #include "program.h"
 
 
@@ -428,6 +429,7 @@ void GLAPIENTRY
 _mesa_ProgramStringARB(GLenum target, GLenum format, GLsizei len,
                        const GLvoid *string)
 {
+   struct gl_program *base;
    GET_CURRENT_CONTEXT(ctx);
    ASSERT_OUTSIDE_BEGIN_END(ctx);
 
@@ -442,22 +444,30 @@ _mesa_ProgramStringARB(GLenum target, GLenum format, GLsizei len,
        && ctx->Extensions.ARB_vertex_program) {
       struct gl_vertex_program *prog = ctx->VertexProgram.Current;
       _mesa_parse_arb_vertex_program(ctx, target, string, len, prog);
-      
-      if (ctx->Program.ErrorPos == -1 && ctx->Driver.ProgramStringNotify)
- ctx->Driver.ProgramStringNotify( ctx, target, &prog->Base );
+
+      base = & prog->Base;
    }
    else if (target == GL_FRAGMENT_PROGRAM_ARB
             && ctx->Extensions.ARB_fragment_program) {
       struct gl_fragment_program *prog = ctx->FragmentProgram.Current;
       _mesa_parse_arb_fragment_program(ctx, target, string, len, prog);
 
-      if (ctx->Program.ErrorPos == -1 && ctx->Driver.ProgramStringNotify)
- ctx->Driver.ProgramStringNotify( ctx, target, &prog->Base );
+      base = & prog->Base;
+   }
+   else if (target == GL_FRAGMENT_PROGRAM_NV
+            && ctx->Extensions.NV_fragment_program) {
+      struct gl_fragment_program *prog = ctx->FragmentProgram.Current;
+      _mesa_parse_nv_fragment_program(ctx, target, string, len, prog);
+
+      base = & prog->Base;
    }
    else {
       _mesa_error(ctx, GL_INVALID_ENUM, "glProgramStringARB(target)");
       return;
    }
+
+   if (ctx->Program.ErrorPos == -1 && ctx->Driver.ProgramStringNotify)
+      ctx->Driver.ProgramStringNotify( ctx, target, base );
 }
 
 
--
1.6.5


------------------------------------------------------------------------------
Come build with us! The BlackBerry(R) Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay
ahead of the curve. Join us from November 9 - 12, 2009. Register now!
http://p.sf.net/sfu/devconference
_______________________________________________
Mesa3d-dev mailing list
Mesa3d-dev@...
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev

[Ian Romanick-4]

From: Ian Romanick <ian.d.romanick@...>

---
 src/mesa/shader/nvprogram.c |   15 +++++++++++++++
 1 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/src/mesa/shader/nvprogram.c b/src/mesa/shader/nvprogram.c
index 471a735..80ed410 100644
--- a/src/mesa/shader/nvprogram.c
+++ b/src/mesa/shader/nvprogram.c
@@ -47,6 +47,7 @@
 #include "prog_instruction.h"
 #include "nvfragparse.h"
 #include "nvvertparse.h"
+#include "arbprogparse.h"
 #include "nvprogram.h"
 
 
@@ -643,6 +644,20 @@ _mesa_LoadProgramNV(GLenum target, GLuint id, GLsizei len,
       }
       _mesa_parse_nv_fragment_program(ctx, target, program, len, fprog);
    }
+   else if (target == GL_FRAGMENT_PROGRAM_ARB
+            && ctx->Extensions.ARB_fragment_program) {
+      struct gl_fragment_program *fprog = (struct gl_fragment_program *) prog;
+      if (!fprog || prog == &_mesa_DummyProgram) {
+         fprog = (struct gl_fragment_program *)
+            ctx->Driver.NewProgram(ctx, target, id);
+         if (!fprog) {
+            _mesa_error(ctx, GL_OUT_OF_MEMORY, "glLoadProgramNV");
+            return;
+         }
+         _mesa_HashInsert(ctx->Shared->Programs, id, fprog);
+      }
+      _mesa_parse_arb_fragment_program(ctx, target, program, len, fprog);
+   }
    else {
       _mesa_error(ctx, GL_INVALID_ENUM, "glLoadProgramNV(target)");
    }
--
1.6.5


------------------------------------------------------------------------------
Come build with us! The BlackBerry(R) Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay
ahead of the curve. Join us from November 9 - 12, 2009. Register now!
http://p.sf.net/sfu/devconference
_______________________________________________
Mesa3d-dev mailing list
Mesa3d-dev@...
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev

[Ian Romanick-4]

From: Ian Romanick <ian.d.romanick@...>

Handle both NV vertex programs and NV vertex state programs passed to
glProgramStringARB.
---
 src/mesa/shader/arbprogram.c |   24 +++++++++++++++++++++++-
 1 files changed, 23 insertions(+), 1 deletions(-)

diff --git a/src/mesa/shader/arbprogram.c b/src/mesa/shader/arbprogram.c
index ca71a39..eb537cd 100644
--- a/src/mesa/shader/arbprogram.c
+++ b/src/mesa/shader/arbprogram.c
@@ -38,6 +38,7 @@
 #include "arbprogram.h"
 #include "arbprogparse.h"
 #include "nvfragparse.h"
+#include "nvvertparse.h"
 #include "program.h"
 
 
@@ -435,18 +436,39 @@ _mesa_ProgramStringARB(GLenum target, GLenum format, GLsizei len,
 
    FLUSH_VERTICES(ctx, _NEW_PROGRAM);
 
+   if (!ctx->Extensions.ARB_vertex_program
+       && !ctx->Extensions.ARB_fragment_program) {
+      _mesa_error(ctx, GL_INVALID_OPERATION, "glProgramStringARB()");
+      return;
+   }
+
    if (format != GL_PROGRAM_FORMAT_ASCII_ARB) {
       _mesa_error(ctx, GL_INVALID_ENUM, "glProgramStringARB(format)");
       return;
    }
 
+   /* The first couple cases are complicated.  The same enum value is used for
+    * ARB and NV vertex programs.  If the target is a vertex program, parse it
+    * using the ARB grammar if the string starts with "!!ARB" or if
+    * NV_vertex_program is not supported.
+    */
    if (target == GL_VERTEX_PROGRAM_ARB
-       && ctx->Extensions.ARB_vertex_program) {
+       && ctx->Extensions.ARB_vertex_program
+       && ((strncmp(string, "!!ARB", 5) == 0)
+   || !ctx->Extensions.NV_vertex_program)) {
       struct gl_vertex_program *prog = ctx->VertexProgram.Current;
       _mesa_parse_arb_vertex_program(ctx, target, string, len, prog);
 
       base = & prog->Base;
    }
+   else if ((target == GL_VERTEX_PROGRAM_ARB
+     || target == GL_VERTEX_STATE_PROGRAM_NV)
+    && ctx->Extensions.NV_vertex_program) {
+      struct gl_vertex_program *prog = ctx->VertexProgram.Current;
+      _mesa_parse_nv_vertex_program(ctx, target, string, len, prog);
+
+      base = & prog->Base;
+   }
    else if (target == GL_FRAGMENT_PROGRAM_ARB
             && ctx->Extensions.ARB_fragment_program) {
       struct gl_fragment_program *prog = ctx->FragmentProgram.Current;
--
1.6.5


------------------------------------------------------------------------------
Come build with us! The BlackBerry(R) Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay
ahead of the curve. Join us from November 9 - 12, 2009. Register now!
http://p.sf.net/sfu/devconference
_______________________________________________
Mesa3d-dev mailing list
Mesa3d-dev@...
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev

[Ian Romanick-4]

From: Ian Romanick <ian.d.romanick@...>

---
 src/mesa/shader/nvprogram.c |   14 +++++++++++++-
 1 files changed, 13 insertions(+), 1 deletions(-)

diff --git a/src/mesa/shader/nvprogram.c b/src/mesa/shader/nvprogram.c
index 80ed410..fd6cbb0 100644
--- a/src/mesa/shader/nvprogram.c
+++ b/src/mesa/shader/nvprogram.c
@@ -596,6 +596,12 @@ _mesa_LoadProgramNV(GLenum target, GLuint id, GLsizei len,
    GET_CURRENT_CONTEXT(ctx);
    ASSERT_OUTSIDE_BEGIN_END(ctx);
 
+   if (!ctx->Extensions.NV_vertex_program
+       && !ctx->Extensions.NV_fragment_program) {
+      _mesa_error(ctx, GL_INVALID_OPERATION, "glLoadProgramNV()");
+      return;
+   }
+
    if (id == 0) {
       _mesa_error(ctx, GL_INVALID_VALUE, "glLoadProgramNV(id)");
       return;
@@ -628,7 +634,13 @@ _mesa_LoadProgramNV(GLenum target, GLuint id, GLsizei len,
          }
          _mesa_HashInsert(ctx->Shared->Programs, id, vprog);
       }
-      _mesa_parse_nv_vertex_program(ctx, target, program, len, vprog);
+
+      if (ctx->Extensions.ARB_vertex_program
+  && (strncmp((char *) program, "!!ARB", 5) == 0)) {
+ _mesa_parse_arb_vertex_program(ctx, target, program, len, vprog);
+      } else {
+ _mesa_parse_nv_vertex_program(ctx, target, program, len, vprog);
+      }
    }
    else if (target == GL_FRAGMENT_PROGRAM_NV
             && ctx->Extensions.NV_fragment_program) {
--
1.6.5


------------------------------------------------------------------------------
Come build with us! The BlackBerry(R) Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay
ahead of the curve. Join us from November 9 - 12, 2009. Register now!
http://p.sf.net/sfu/devconference
_______________________________________________
Mesa3d-dev mailing list
Mesa3d-dev@...
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev

[Brian Paul-4]

On Mon, Nov 2, 2009 at 3:20 PM, Ian Romanick <idr@...> wrote:
> This patch series enables glProgramStringARB to handle NV vertex and
> fragment programs when the appropriate extensions are supported.  It
> also enables glLoadProgramNV to handle ARB vertex and fragment
> programs.  This behavior is described in the "Interactions with"
> sections of ARB_vertex_program and NV_fragment_program.

Did someone's app hit this functionality?


> These are the least intrusive patches for the 7.6 release branch.
> Ideally the ARB parser would be extended to handle the NV programs,
> and a lot of the existing NV handling code would be refactored.  This
> seems too intrusive for the stable branch, especially since we don't
> have a good parser test suite for the NV programs.
>
> One quirk that bothers me about the existing code:  glLoadProgramNV
> doesn't call Driver.ProgramStringNotify.  Is this intentional?

Probably an oversight.  Driver.ProgramStringNotify() is used to tell
the driver that a vertex/fragment program has been changed.

I didn't test your patches but they look OK to me.

-Brian

------------------------------------------------------------------------------
Come build with us! The BlackBerry(R) Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay
ahead of the curve. Join us from November 9 - 12, 2009. Register now!
http://p.sf.net/sfu/devconference
_______________________________________________
Mesa3d-dev mailing list
Mesa3d-dev@...
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev

[Keith Whitwell-3]

On Mon, 2009-11-02 at 19:32 -0800, Brian Paul wrote:
> On Mon, Nov 2, 2009 at 3:20 PM, Ian Romanick <idr@...> wrote:
> > This patch series enables glProgramStringARB to handle NV vertex and
> > fragment programs when the appropriate extensions are supported.  It
> > also enables glLoadProgramNV to handle ARB vertex and fragment
> > programs.  This behavior is described in the "Interactions with"
> > sections of ARB_vertex_program and NV_fragment_program.
>
> Did someone's app hit this functionality?

Yes, do tell...

I'm fine with these going in too.

Keith


------------------------------------------------------------------------------
Come build with us! The BlackBerry(R) Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay
ahead of the curve. Join us from November 9 - 12, 2009. Register now!
http://p.sf.net/sfu/devconference
_______________________________________________
Mesa3d-dev mailing list
Mesa3d-dev@...
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev

[Ian Romanick-4]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Brian Paul wrote:
> On Mon, Nov 2, 2009 at 3:20 PM, Ian Romanick <idr@...> wrote:
>> This patch series enables glProgramStringARB to handle NV vertex and
>> fragment programs when the appropriate extensions are supported.  It
>> also enables glLoadProgramNV to handle ARB vertex and fragment
>> programs.  This behavior is described in the "Interactions with"
>> sections of ARB_vertex_program and NV_fragment_program.
>
> Did someone's app hit this functionality?

I recently modified piglit's vpfp-generic test to use the ARB functions
for both ARB and NV vertex programs.  It worked fine on Nvidia binary
drivers, but it failed on Mesa.

>> These are the least intrusive patches for the 7.6 release branch.
>> Ideally the ARB parser would be extended to handle the NV programs,
>> and a lot of the existing NV handling code would be refactored.  This
>> seems too intrusive for the stable branch, especially since we don't
>> have a good parser test suite for the NV programs.
>>
>> One quirk that bothers me about the existing code:  glLoadProgramNV
>> doesn't call Driver.ProgramStringNotify.  Is this intentional?
>
> Probably an oversight.  Driver.ProgramStringNotify() is used to tell
> the driver that a vertex/fragment program has been changed.

Do you think I should modify LoadProgramNV to be like ProgramStringARB?
 I'll probably do some additional refactoring in the master after these
patches hit the stable branch.

> I didn't test your patches but they look OK to me.

All of the piglit asmparser tests pass.  I haven't run them through our
oglconform yet.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkrwfJEACgkQX1gOwKyEAw+wZQCeJLsGIpG4hJkhJpgmiqfI+f6K
NY8AoIvACdJuzmgC3OxE9P8Wk/zEwPsV
=gZ/F
-----END PGP SIGNATURE-----

------------------------------------------------------------------------------
Come build with us! The BlackBerry(R) Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay
ahead of the curve. Join us from November 9 - 12, 2009. Register now!
http://p.sf.net/sfu/devconference
_______________________________________________
Mesa3d-dev mailing list
Mesa3d-dev@...
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev

[Brian Paul-3]

Ian Romanick wrote:

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Brian Paul wrote:
>> On Mon, Nov 2, 2009 at 3:20 PM, Ian Romanick <idr@...> wrote:
>>> This patch series enables glProgramStringARB to handle NV vertex and
>>> fragment programs when the appropriate extensions are supported.  It
>>> also enables glLoadProgramNV to handle ARB vertex and fragment
>>> programs.  This behavior is described in the "Interactions with"
>>> sections of ARB_vertex_program and NV_fragment_program.
>> Did someone's app hit this functionality?
>
> I recently modified piglit's vpfp-generic test to use the ARB functions
> for both ARB and NV vertex programs.  It worked fine on Nvidia binary
> drivers, but it failed on Mesa.
>
>>> These are the least intrusive patches for the 7.6 release branch.
>>> Ideally the ARB parser would be extended to handle the NV programs,
>>> and a lot of the existing NV handling code would be refactored.  This
>>> seems too intrusive for the stable branch, especially since we don't
>>> have a good parser test suite for the NV programs.
>>>
>>> One quirk that bothers me about the existing code:  glLoadProgramNV
>>> doesn't call Driver.ProgramStringNotify.  Is this intentional?
>> Probably an oversight.  Driver.ProgramStringNotify() is used to tell
>> the driver that a vertex/fragment program has been changed.
>
> Do you think I should modify LoadProgramNV to be like ProgramStringARB?

If you mean call ctx->Driver.ProgramStringNotify(), yes.


>  I'll probably do some additional refactoring in the master after these
> patches hit the stable branch.

OK.

-Brian

------------------------------------------------------------------------------
Come build with us! The BlackBerry(R) Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay
ahead of the curve. Join us from November 9 - 12, 2009. Register now!
http://p.sf.net/sfu/devconference
_______________________________________________
Mesa3d-dev mailing list
Mesa3d-dev@...
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev