Ian Romanick <i...@freedesktop.org> writes:
> On 03/05/2014 12:35 PM, Eric Anholt wrote:
>> Ian Romanick <i...@freedesktop.org> writes:
>>
>>> From: Gregory Hainaut <gregory.hain...@gmail.com>
>>>
>>> Now arb_separate_shader_object-GetProgramPipelineiv should pass.
>>>
>>> V3 (idr):
>>> * Change spec references to core OpenGL versions instead of issues in
>>> the extension spec.
>>> * Split out from previous uber patch.
>>>
>>> v4 (idr): Use _mesa_has_geometry_shaders in _mesa_UseProgramStages to
>>> detect availability of geometry shaders.
>>>
>>> Reviewed-by: Ian Romanick <ian.d.roman...@intel.com>
>>> ---
>>> src/mesa/main/pipelineobj.c | 115
>>> ++++++++++++++++++++++++++++++++++++++++++++
>>> 1 file changed, 115 insertions(+)
>>>
>>> diff --git a/src/mesa/main/pipelineobj.c b/src/mesa/main/pipelineobj.c
>>> index 849c781..7149578 100644
>>> --- a/src/mesa/main/pipelineobj.c
>>> +++ b/src/mesa/main/pipelineobj.c
>>
>>> + GLbitfield any_valid_stages = GL_VERTEX_SHADER_BIT |
>>> GL_FRAGMENT_SHADER_BIT;
>>> + if (_mesa_has_geometry_shaders(ctx))
>>> + any_valid_stages |= GL_GEOMETRY_SHADER_BIT;
>>> +
>>> + if (stages != GL_ALL_SHADER_BITS && (stages & ~any_valid_stages) != 0)
>>> {
>>
>> Weird double space before &.
>>
>>> + _mesa_error(ctx, GL_INVALID_VALUE, "glUseProgramStages(Stages)");
>>> + return;
>>> + }
>>
>>> + if (program) {
>>> + /* Section 2.11.1 (Shader Objects) of the OpenGL 3.1 spec (and
>>> probably
>>> + * earlier) says:
>>> + *
>>> + * "Commands that accept shader or program object names will
>>> + * generate the error INVALID_VALUE if the provided name is not
>>> the
>>> + * name of either a shader or program object and
>>> INVALID_OPERATION
>>> + * if the provided name identifies an object that is not the
>>> + * expected type."
>>> + */
>>> + struct gl_shader *sh = _mesa_lookup_shader(ctx, program);
>>> + if (sh != NULL) {
>>> + _mesa_error(ctx, GL_INVALID_OPERATION,
>>> + "glUseProgramStages(progam is a shader object)");
>>> + return;
>>> + }
>>
>> This block could get dropped into the shProg == NULL case below, right?
>> The code confused me as is.
>
> The first block is checking whether the name is a shader, and the other
> is checking that the name is a program. The neat thing is the spec says
> using a shader where a program is expected gives one error, but using a
> name that is neither give a different error.
>
> You've never seen code like this elsewhere in Mesa because it's usually
> handled by _mesa_lookup_shader_program_err. I'll change this code to
> use that function instead.
Yeah, I understood what was happening here. I was assuming, though,
that if you have a successful lookup of a shProg, then you don't need to
throw an error if there happens to be a shader with the same name (if
that's even possible). So the code doesn't need to do the extra lookup
in the success case or look confusing at all.
>>> +
>>> + shProg = _mesa_lookup_shader_program(ctx, program);
>>> + if (shProg == NULL) {
>>> + _mesa_error(ctx, GL_INVALID_VALUE,
>>> + "glUseProgramStages(progam is not a program object)");
>>> + return;
>>> + }
>>
>>
>>> + /* Section 2.11.4 (Program Pipeline Objects) of the OpenGL 4.1 spec
>>> + * says:
>>> + *
>>> + * "If UseProgramStages is called with program set to zero or with a
>>> + * program object that contains no executable code for the given
>>> + * stages, it is as if the pipeline object has no programmable stage
>>> + * configured for the indicated shader stages."
>>> + */
>>> + if ((stages & GL_VERTEX_SHADER_BIT) != 0)
>>> + _mesa_use_shader_program(ctx, GL_VERTEX_SHADER, shProg, pipe);
>>> +
>>> + if ((stages & GL_FRAGMENT_SHADER_BIT) != 0)
>>> + _mesa_use_shader_program(ctx, GL_FRAGMENT_SHADER, shProg, pipe);
>>> +
>>> + if ((stages & GL_GEOMETRY_SHADER_BIT) != 0)
>>> + _mesa_use_shader_program(ctx, GL_GEOMETRY_SHADER, shProg, pipe);
>>> }
>>
>> The spec cite here doesn't seem to be explaining the code it's next to,
>> to me.
>
> That is fair... and at least partially my fault. Greg's original
> comment above this code was:
>
> /*
> * 7. What happens if you have a program object current for a shader
> stage,
> * but the program object doesn't contain an executable for that stage?
>
> * RESOLVED: This is not an error; instead it is as though there were
> no
> * program bound to that stage. We have two different notions for
> * programs bound to shader stages. A program is "current" for a stage
> * if it bound to that stage in the active program pipeline object. A
> * program is "active" for a stage if it is current and it has an
> * executable for this stage. In this case, the program would be
> current
> * but not active.
>
> * When no program is active for a stage, the stage will be replaced
> with
> * fixed functionality logic (compatibility profile vertex and
> fragment),
> * disabled (tessellation control and evaluation, geometry), or have
> * undefined results (core profile vertex and fragment).
> */
>
> Since the issues aren't captured in the core spec, I changed it to the
> nearest thing I could find in the core spec. Comparing the code with
> the old comment and the new comment, I'm not very happy with it either.
>
> How about:
>
> /* Enable individual stages from the program as requested by the
> application.
> * If there is no shader for a requested stage in the program,
> * _mesa_use_shader_program will enable fixed-function processing as
> dictated
> * by the spec.
> *
> * Section 2.11.4 (Program Pipeline Objects) of the OpenGL 4.1 spec
> * says:
> *
> * "If UseProgramStages is called with program set to zero or with a
> * program object that contains no executable code for the given
> * stages, it is as if the pipeline object has no programmable stage
> * configured for the indicated shader stages."
> */
That seems fine.
pgpOt_QBdMA8N.pgp
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
