[Mesa-dev] [PATCH 01/12] sso: Create extensions entry points

[gregory hainaut]

On Fri, 12 Apr 2013 12:38:19 -0700
Eric Anholt <eric at anholt.net> wrote:

> Please, patches for Mesa have to actually be addressed to Mesa.

What do you mean? The github tree (was requested by Jordan)? Or because the mail header doesn't contains mesa address (mail client setup issue that it is now fixed) ? 

> I'm really excited to see progress on SSO, so hopefully we can get some
> of this landed soon.

> 
> gregory <gregory.hainaut at gmail.com> writes:
> 
> > V2: formatting improvement
> 
> patch versioning generally comes at the end of the commit message.
> 
> Also, could you fix up your git author info to include your full name?
> 
> > diff --git a/src/mapi/glapi/gen/ARB_separate_shader_objects.xml b/src/mapi/glapi/gen/ARB_separate_shader_objects.xml
> > new file mode 100644
> > index 0000000..29a37f5
> > --- /dev/null
> > +++ b/src/mapi/glapi/gen/ARB_separate_shader_objects.xml
> 
> > +      <function name="GenProgramPipelines" offset="assign">
> > +         <param name="n" type="GLsizei" />
> > +         <param name="pipelines" type="GLuint *" />
> > +      </function>
> > +      <function name="IsProgramPipeline" offset="assign">
> > +         <param name="pipeline" type="GLuint" />
> > +         <return type="GLboolean"/>
> > +      </function>
> 
> Missing ProgramParameteri here.
Unfortunately it complains about duplication. The function is already defined in ARB_get_program_binary.xml. I could put a dummy name with an alias.
I don't see any specific syntax on the python.

> 
> > +      <function name="GetProgramPipelineiv" offset="assign">
> > +         <param name="pipeline" type="GLuint" />
> > +         <param name="pname" type="GLenum" />
> > +         <param name="params" type="GLint *" />
> > +      </function>
> 
> > +      <!-- depends on GL_ARB_gpu_shader_fp64
> > +    <function name="ProgramUniform1d" offset="assign">
> > +        <param name="program" type="GLuint" />
> > +        <param name="location" type="GLint" />
> > +        <param name="x" type="GLdouble" />
> > +    </function>
> 
> Indentation suddenly changed for the double-valued uniform commands,
> could they be made consistent with the ones above?
> 
> > +    <function name="ProgramUniformMatrix2dv" offset="assign">
> > +        <param name="program" type="GLuint" />
> > +        <param name="location" type="GLint" />
> > +        <param name="count" type="GLsizei" />
> > +        <param name="transpose" type="GLboolean" />
> > +        <param name="value" type="const GLdouble *" />
> > +    </function>
> > +    <function name="ProgramUniformMatrix3dv" offset="assign">
> > +        <param name="program" type="GLuint" />
> > +        <param name="location" type="GLint" />
> > +        <param name="count" type="GLsizei" />
> > +        <param name="transpose" type="GLboolean" />
> > +        <param name="value" type="const GLdouble *" />
> > +    </function>
> > +    <function name="ProgramUniformMatrix4dv" offset="assign">
> > +        <param name="program" type="GLuint" />
> > +        <param name="location" type="GLint" />
> > +        <param name="count" type="GLsizei" />
> > +        <param name="transpose" type="GLboolean" />
> > +        <param name="value" type="const GLdouble *" />
> > +    </function>
> > +    <function name="ProgramUniform1dv" offset="assign">
> > +        <param name="program" type="GLuint" />
> > +        <param name="location" type="GLint" />
> > +        <param name="count" type="GLsizei" />
> > +        <param name="value" type="const GLdouble *" />
> > +    </function>
> > +    <function name="ProgramUniform2dv" offset="assign">
> > +        <param name="program" type="GLuint" />
> > +        <param name="location" type="GLint" />
> > +        <param name="count" type="GLsizei" />
> > +        <param name="value" type="const GLdouble *" />
> > +    </function>
> > +    <function name="ProgramUniform3dv" offset="assign">
> > +        <param name="program" type="GLuint" />
> > +        <param name="location" type="GLint" />
> > +        <param name="count" type="GLsizei" />
> > +        <param name="value" type="const GLdouble *" />
> > +    </function>
> > +    <function name="ProgramUniform4dv" offset="assign">
> > +        <param name="program" type="GLuint" />
> > +        <param name="location" type="GLint" />
> > +        <param name="count" type="GLsizei" />
> > +        <param name="value" type="const GLdouble *" />
> > +    </function>
> 
> These are sorted differently from the spec for reasons I don't follow,
> which made reviewing a little weird.
I follow that, not sure it the golden reference:
http://www.opengl.org/registry/specs/ARB/separate_shader_objects.txt

The only different is that I pack all "double stuff" at the end to avoid multiple comment.

> 
> > diff --git a/src/mesa/main/pipelineobj.c b/src/mesa/main/pipelineobj.c
> > new file mode 100644
> > index 0000000..651c1a0
> > --- /dev/null
> > +++ b/src/mesa/main/pipelineobj.c
> > @@ -0,0 +1,135 @@
> > +/*
> > + * Mesa 3-D graphics library
> > + *
> > + * Copyright (C) 2013-2013  Gregory Hainaut <gregory.hainaut at gmail.com>
> > + *
> > + * Permission is hereby granted, free of charge, to any person obtaining a
> > + * copy of this software and associated documentation files (the "Software"),
> > + * to deal in the Software without restriction, including without limitation
> > + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> > + * and/or sell copies of the Software, and to permit persons to whom the
> > + * Software is furnished to do so, subject to the following conditions:
> > + *
> > + * The above copyright notice and this permission notice shall be included
> > + * in all copies or substantial portions of the Software.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> > + * GREGORY HAINAUT BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER
> > + * IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
> > + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
> > + */
> 
> Please use the updated MIT license, which doesn't explicitly name the
> author in the license text.  The problem with this older variant of the
> license is that later, when someone else makes enough change to put
> their own copyright on the file, too, then they have to have a
> completely separate license block.
> 
> A sample:
> 
> /*
>  * Copyright © 2013 My Name
>  *
>  * Permission is hereby granted, free of charge, to any person obtaining a
>  * copy of this software and associated documentation files (the "Software"),
>  * to deal in the Software without restriction, including without limitation
>  * the rights to use, copy, modify, merge, publish, distribute, sublicense,
>  * and/or sell copies of the Software, and to permit persons to whom the
>  * Software is furnished to do so, subject to the following conditions:
>  *
>  * The above copyright notice and this permission notice (including the next
>  * paragraph) shall be included in all copies or substantial portions of the
>  * Software.
>  *
>  * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>  * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>  * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
>  * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
>  * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
>  * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
>  * IN THE SOFTWARE.
>  */
> 
> > +/**
> > + * \file pipelineobj.c
> > + * \author Hainaut Gregory <gregory.hainaut at gmail.com>
> > + *
> > + * Implementation of pipeline object related API functions. Based on GL_ARB_separate_shader_objects
> > + * extension.
> > + *
> > + *
> > + * XXX things to do:
> > + * 1. Check that the right error code is generated for all _mesa_error() calls.
> > + * 2. Insert FLUSH_VERTICES calls in various places
> > + */
> 
> Weird todo comment here, given that a correct TODO comment at this point
> would be "everything but the uniform update commands"
Well that because I copy/paste header of shaderapi at startup. TODO remains true but it would be better to move them at the end of my patches. 
> 
> 
> I'm surprised this patch builds -- I thought we had requirements that
> Mesa functions be present for all the new stuff in the API XML right
> away (unless explicitly marked somehow), but the uniform double
> functions aren't present.  Have you run make check on this patch?
> 
Normally it must build fine except if I screw up on a rebase. I comment the uniform double on the xml. If you prefer I can reenable them on the xml and
put dummy uniform function.

I will fix everythings. Thanks.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20130412/a6a640d3/attachment.pgp>