On 18/06/18 18:45, Gustaw Smolarczyk wrote:
2018-06-18 10:39 GMT+02:00 Iago Toral <ito...@igalia.com <mailto:ito...@igalia.com>>:

    On Mon, 2018-06-18 at 09:43 +0200, Gustaw Smolarczyk wrote:
    2018-06-18 4:39 GMT+02:00 Timothy Arceri <tarc...@itsqueeze.com
    <mailto:tarc...@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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to