[Mesa-dev] [PATCH] mesa: add ff fragment shader support for geom and tess shaders

[Timothy Arceri]

On 18/06/18 18:45, Gustaw Smolarczyk wrote:
> 2018-06-18 10:39 GMT+02:00 Iago Toral <itoral at igalia.com 
> <mailto:itoral at igalia.com>>:
> 
>     On Mon, 2018-06-18 at 09:43 +0200, Gustaw Smolarczyk wrote:
>>     2018-06-18 4:39 GMT+02:00 Timothy Arceri <tarceri at itsqueeze.com
>>     <mailto:tarceri at itsqueeze.com>>:
>>>     This is required for compatibility profile support.
>>>     ---
>>>      src/mesa/main/ff_fragment_shader.cpp | 6 +++++-
>>>      1 file changed, 5 insertions(+), 1 deletion(-)
>>>
>>>     diff --git a/src/mesa/main/ff_fragment_shader.cpp
>>>     b/src/mesa/main/ff_fragment_shader.cpp
>>>     index a698931d99e..935a21624af 100644
>>>     --- a/src/mesa/main/ff_fragment_shader.cpp
>>>     +++ b/src/mesa/main/ff_fragment_shader.cpp
>>>     @@ -229,7 +229,11 @@ static GLbitfield filter_fp_input_mask(
>>>     GLbitfield fp_inputs,
>>>          * since vertex shader state validation comes after fragment
>>>     state
>>>          * validation (see additional comments in state.c).
>>>          */
>>>     -   if (vertexShader)
>>>     +   if (ctx->_Shader->CurrentProgram[MESA_SHADER_GEOMETRY] != NULL)
>>>     +      vprog = ctx->_Shader->CurrentProgram[MESA_SHADER_GEOMETRY];
>>>     +   else if (ctx->_Shader->CurrentProgram[MESA_SHADER_TESS_EVAL]
>>>     != NULL)
>>>     +      vprog = ctx->_Shader->CurrentProgram[MESA_SHADER_TESS_EVAL];
>>>     +   else if (vertexShader)
>>
>>
>>     Shouldn't you also update the if condition on line 178? Otherwise,
>>     you won't reach the if tree you change when the vertex shader is
>>     missing (unless that was intended - I am not really familiar with
>>     how fixed function shaders work alongside new features).
> 
>     You don't have Tesselation / Geometry with fixed function GL, so I
>     think this should be fine.
> 
> 
> Well, this whole file implements fixed function fragment shader, so it 
> will only be reached when the fragment shader is missing. Unless you 
> meant that tessellation/geometry shaders cannot be combined with fixed 
> function vertex shader but fixed function fragment shader is fine.

You can only use a ff vertex shader with SSO (otherwise a geom/tess 
shader must be linked with a vertex shader). The combination of SSO with 
ff vertex, geom or tess and ff fragment shaders is highly unlikely but 
we do have a task for it on our TODO list. For now I'm happy to leave 
this change as is because any such use needs further testing/fixes to 
work correctly. I've discussed this with Marek and we think its ok to 
enable GL 3.2-3.3 first and fix these theoretically possible but low 
importance use cases as we progress further with compat profile support.

As for the comment I left it as is because its talking about GLSL vertex 
shaders vs ARB asm vertex programs. In that context its still a valid 
comment, there is no need to add anything about the other stages as the 
ordering there is obvious.

> 
> Regards,
> Gustaw Smolarczyk
> 
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>