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

Fri Apr 12 12:38:19 PDT 2013

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

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.

> +      <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.

> 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"

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?

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20130412/d871ceb3/attachment-0001.pgp>