Add placeholder entry points for GL_OES_get_program_binary.

Add placeholder entry points for GL_OES_get_program_binary.
Committed: https://code.google.com/p/angleproject/source/detail?r=1146

[apatrick1, 2012-06-04 23:22:36]

Daniel,

Another one for you. Thanks.

Al

[apatrick1, 2012-06-04 23:39:45]

Patch set #2... I realized if I implement just a little bit more, I'll be able
to commit this as a "working" implementation of the extension. At this point,
all program binaries are zero bytes and always fail to upload.

[dgkoch, 2012-06-06 19:30:27]

https://codereview.appspot.com/6279054/diff/1003/src/libGLESv2/Context.cpp
File src/libGLESv2/Context.cpp (right):

https://codereview.appspot.com/6279054/diff/1003/src/libGLESv2/Context.cpp#ne...
src/libGLESv2/Context.cpp:1659: *params = GL_PROGRAM_BINARY_ANGLE;
Presumably you'll need an accompanying glext.h header change to add this..

https://codereview.appspot.com/6279054/diff/1003/src/libGLESv2/Context.cpp#ne...
src/libGLESv2/Context.cpp:1751: case GL_RESET_NOTIFICATION_STRATEGY_EXT:
Need to add NUM_PROGRAM_BINARY_FORMATS_OES and PROGRAM_BINARY_FORMATS_OES here

https://codereview.appspot.com/6279054/diff/1003/src/libGLESv2/Context.cpp#ne...
src/libGLESv2/Context.cpp:3725: mExtensionString += "GL_OES_get_program_binary
";
While I agree this is technically a valid implementation, I'd rather not
advertise the extension string until it's more completely implemented.

https://codereview.appspot.com/6279054/diff/1003/src/libGLESv2/libGLESv2.cpp
File src/libGLESv2/libGLESv2.cpp (right):

https://codereview.appspot.com/6279054/diff/1003/src/libGLESv2/libGLESv2.cpp#...
src/libGLESv2/libGLESv2.cpp:6662: gl::Context *context =
gl::getNonLostContext();
Should validate that binaryFormat == GL_PROGRAM_BINARY_ANGLE (either now, in in
the future patch where you hook it up).

[apatrick1, 2012-06-06 20:03:35]

Done thanks.

https://codereview.appspot.com/6279054/diff/1003/src/libGLESv2/Context.cpp
File src/libGLESv2/Context.cpp (right):

https://codereview.appspot.com/6279054/diff/1003/src/libGLESv2/Context.cpp#ne...
src/libGLESv2/Context.cpp:1659: *params = GL_PROGRAM_BINARY_ANGLE;
On 2012/06/06 19:30:27, dgkoch wrote:
> Presumably you'll need an accompanying glext.h header change to add this..

Oops, ran gcl from src so it didn't pick up the change in the include directory.
Added.

https://codereview.appspot.com/6279054/diff/1003/src/libGLESv2/Context.cpp#ne...
src/libGLESv2/Context.cpp:1751: case GL_RESET_NOTIFICATION_STRATEGY_EXT:
On 2012/06/06 19:30:27, dgkoch wrote:
> Need to add NUM_PROGRAM_BINARY_FORMATS_OES and PROGRAM_BINARY_FORMATS_OES here

Done.

https://codereview.appspot.com/6279054/diff/1003/src/libGLESv2/Context.cpp#ne...
src/libGLESv2/Context.cpp:3725: mExtensionString += "GL_OES_get_program_binary
";
On 2012/06/06 19:30:27, dgkoch wrote:
> While I agree this is technically a valid implementation, I'd rather not
> advertise the extension string until it's more completely implemented.

Done.

https://codereview.appspot.com/6279054/diff/1003/src/libGLESv2/libGLESv2.cpp
File src/libGLESv2/libGLESv2.cpp (right):

https://codereview.appspot.com/6279054/diff/1003/src/libGLESv2/libGLESv2.cpp#...
src/libGLESv2/libGLESv2.cpp:6662: gl::Context *context =
gl::getNonLostContext();
On 2012/06/06 19:30:27, dgkoch wrote:
> Should validate that binaryFormat == GL_PROGRAM_BINARY_ANGLE (either now, in
in
> the future patch where you hook it up).

I think I should do it in Program::binaryProgram since if it generates an error
I think it has to unlink as well.

[dgkoch, 2012-06-06 20:08:47]

LGTM

https://codereview.appspot.com/6279054/diff/11001/include/GLES2/gl2ext.h
File include/GLES2/gl2ext.h (right):

https://codereview.appspot.com/6279054/diff/11001/include/GLES2/gl2ext.h#newc...
include/GLES2/gl2ext.h:243: #define GL_PROGRAM_BINARY_ANGLE                     
           0x6000 /* FIXME: allocate enum */
use 0x93A6

[dgkoch, 2012-06-06 20:10:30]

On 2012/06/06 20:03:35, apatrick1 wrote:
> On 2012/06/06 19:30:27, dgkoch wrote:
> > Should validate that binaryFormat == GL_PROGRAM_BINARY_ANGLE (either now, in
> in
> > the future patch where you hook it up).
> 
> I think I should do it in Program::binaryProgram since if it generates an
error
> I think it has to unlink as well.

No I don't think so.  If a GL error is returned, the operation must have no
side-effects. (unless the error is OOM).  ProgramBinaryOES reports failure to
load the program only via the LINK_STATUS program state.

[apatrick1, 2012-06-06 20:39:51]

On 2012/06/06 20:10:30, dgkoch wrote:
> On 2012/06/06 20:03:35, apatrick1 wrote:
> > On 2012/06/06 19:30:27, dgkoch wrote:
> > > Should validate that binaryFormat == GL_PROGRAM_BINARY_ANGLE (either now,
in
> > in
> > > the future patch where you hook it up).
> > 
> > I think I should do it in Program::binaryProgram since if it generates an
> error
> > I think it has to unlink as well.
> 
> No I don't think so.  If a GL error is returned, the operation must have no
> side-effects. (unless the error is OOM).  ProgramBinaryOES reports failure to
> load the program only via the LINK_STATUS program state.

Sorry, I rebased and that'll mess up the deltas. Only change is putting in the
constant for GL_PROGRAM_BINARY_ANGLE.

Regarding unlinking on failure, I was going by the resolution of this issue in
the OES_get_program_binary spec:

5. Where are the binary formats defined and described?

    RESOLVED: This extension provides a common infrastructure for retrieving and
    loading program binaries.  A vendor extension must also be present in order
    to define one or more binary formats, thereby populating the list of
    PROGRAM_BINARY_FORMATS_OES.  The <binaryFormat> returned by
    GetProgramBinaryOES is always one of the binary formats in this list.  If
    ProgramBinaryOES is called with a <binaryFormat> not in this list, the
    implementation will throw an INVALID_ENUM error.
    
And this from the description of ProgramBinaryOES:
    
    If ProgramBinaryOES failed, any information about a previous link or load of
    that program object is lost.  Thus, a failed load does not restore the old
    state of <program>.

[dgkoch, 2012-06-07 00:04:32]

On 2012/06/06 20:39:51, apatrick1 wrote:
> On 2012/06/06 20:10:30, dgkoch wrote:
> > On 2012/06/06 20:03:35, apatrick1 wrote:
> > > On 2012/06/06 19:30:27, dgkoch wrote:
> > > > Should validate that binaryFormat == GL_PROGRAM_BINARY_ANGLE (either
now,
> in
> > > in
> > > > the future patch where you hook it up).
> > > 
> > > I think I should do it in Program::binaryProgram since if it generates an
> > error
> > > I think it has to unlink as well.
> > 
> > No I don't think so.  If a GL error is returned, the operation must have no
> > side-effects. (unless the error is OOM).  ProgramBinaryOES reports failure
to
> > load the program only via the LINK_STATUS program state.
> 
> Sorry, I rebased and that'll mess up the deltas. Only change is putting in the
> constant for GL_PROGRAM_BINARY_ANGLE.
That's fine. I wasn't necessarily expecting it in this patch.

> Regarding unlinking on failure, I was going by the resolution of this issue in
> the OES_get_program_binary spec:
> 
> 5. Where are the binary formats defined and described?
> 
>     RESOLVED: This extension provides a common infrastructure for retrieving
and
>     loading program binaries.  A vendor extension must also be present in
order
>     to define one or more binary formats, thereby populating the list of
>     PROGRAM_BINARY_FORMATS_OES.  The <binaryFormat> returned by
>     GetProgramBinaryOES is always one of the binary formats in this list.  If
>     ProgramBinaryOES is called with a <binaryFormat> not in this list, the
>     implementation will throw an INVALID_ENUM error.

Right -- this would be handled by the entry point in libGLESv2. However, the GL
spec has blanket language in Section 2.5 that says "the command generating the
error is ignored so that it has no effect on GL state or framebuffer contents."

> And this from the description of ProgramBinaryOES:
>     
>     If ProgramBinaryOES failed, any information about a previous link or load
of
>     that program object is lost.  Thus, a failed load does not restore the old
>     state of <program>.

ProgramBinaryOES failing is NOT the same as it returning an error.  It would
fail because the binary could not be loaded for some reason (e.g. the compiler
or driver changed invalidating the cached copy).  The OES_get_program_binary
extension says:
    A program object's program binary is replaced by calls to LinkProgram or
    ProgramBinaryOES.  Either command sets the program object's LINK_STATUS to
    TRUE or FALSE, as queried with GetProgramiv, to reflect success or failure.
This is the case that the previous contents are destroyed.

[dgkoch, 2012-06-07 00:13:40]

LGTM, although I might prefer if we finish off the refactoring in Program first.

https://codereview.appspot.com/6279054/diff/15002/src/libGLESv2/libGLESv2.cpp
File src/libGLESv2/libGLESv2.cpp (right):

https://codereview.appspot.com/6279054/diff/15002/src/libGLESv2/libGLESv2.cpp...
src/libGLESv2/libGLESv2.cpp:6792: return error(GL_INVALID_OPERATION);
As per my previous comment - INVALID_OPERATION isn't technically correct here. 
We should mark this as unlinked.  I'm ok with leaving this as-is though until
you actually hook this up, esp. since we aren't advertising the extension yet.

[apatrick1, 2012-06-08 00:53:10]

Add placeholder entry points for GL_OES_get_program_binary.

[apatrick1, 2012-06-08 00:55:55]

On 2012/06/08 00:53:10, apatrick1 wrote:
> Add placeholder entry points for GL_OES_get_program_binary.

gcl does this when you change clients. ProgramBinary now unlinks the program.

[dgkoch, 2012-06-08 01:18:52]

LGTM