Re: [Mesa-dev] [PATCH 4/4] mesa: The glArrayElement api is independent of the current program.

2018-05-20 Thread Marek Olšák
Yes, it's clear now. Thanks.

Marek

On Sun, May 20, 2018 at 3:05 AM, Mathias Fröhlich  wrote:

> Good morning,
>
> > Are you sure about this? The fixed-func vertex and fragment shaders can
> > move zero-stride vertex attribs into constants (uniforms). If a shader
> > changes, it might no longer be necessary to submit zero-stride attribs
> via
> > the vertex API, but this would be missed if _NEW_PROGRAM was ignored.
>
> If I get your question right then this is something that happens further
> down the road.
>
> So, the array element api, just looking at the code what it does and what
> it reads, Just takes a VAO, maps it and walks all the vertex at the index
> provided with glArrayElement. Then for that given index all enabled
> vertex attributes just provided the vertex attributes enabled state
> are put into the immediate mode api. Also the decision of if it should take
> the VERT_ATTRIB_POS or VERT_ATTRIB_GENERIC0 attribute to trigger the
> immediate mode vertex is completely taken from the enabled state
> in the VAO. With enabled state I really mean the application observable
> OpenGL api that you modify with glEnableVertexAttribArray and query
> using the GL_VERTEX_ATTRIB_ARRAY_ENABLED enum and that you find
> in gl_array_attributes::Enabled and condensed in the
> gl_vertex_array_object::_Enabled bitmask, both within the VAO.
> Given that no shader related value is evaluated from the code triggered
> by _ae_invalidate_state I would say I am sure about that.
>
> Putting current values into uniforms happens in the draw code path
> where we mute some may be enabled arrays from the VAO based on
> the shader mode. That again is only the decision if some of the generics
> contain material array values from within the vbo module or if these
> generics are really generics that must not be evaluated from
> ffvertex_prog.c.
> Based of these enables, not looking at zero strides, ffvertex_prog.c
> creates a
> vertex shader that exactly reads those arrays that are enabled and not
> generally muted from vertex processing due to gl_vertex_processing_mode.
> The result of ffvertex_prog.c is a shader that reads some VERT_ATTRIB_*
> values
> and put aside double inputs has exactly those VERT_BIT_* bits set in
> ...inputs_read
> that are required due to the current OpenGL fixed function state and the
> currently enabled array. Based on the ...inputs_read and the finally
> provided arrays
> from the VAO, the driver backends (well, the consumers like i965, gallium
> or TNL)
> only upload/configure those enabled arrays which are consumed by the
> shader.
>
> Putting that together, I can see current vertex attribute values being
> taken
> from uniforms if they are not enabled in the ctx->Array._DrawVAO.
>
> Back to the array element api to see that program mode is not required
> from there by an other argument:
> That VAO that is replayed by glArrayElement may never hit the draw path as
> from those immediate mode calls that the glArrayElement api generates,
> mesa assembles a new VAO/VBO in the vbo module that is finally either
> kept in a display list or used for immediate mode rendering. Depending
> on which VERT_ATTRIB_* arrays are enabled in this vbo module created
> VAO, ffvertex_prog may be triggered if we are about to draw something
> fixed function related.
> Also from that thought you can see that changes to shader program state
> do not affect the way the VAO should get replayed from the glArrayElement
> api.
> They *do* affect the way the resulting VAO produced by the vbo module is
> processed, but that is nothing that is handled from _ae_invalidate_state.
>
> Hope this helps?
>
> best
>
> Mathias
>
>
> ___
> 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


Re: [Mesa-dev] [PATCH 4/4] mesa: The glArrayElement api is independent of the current program.

2018-05-20 Thread Mathias Fröhlich
Good morning,

> Are you sure about this? The fixed-func vertex and fragment shaders can
> move zero-stride vertex attribs into constants (uniforms). If a shader
> changes, it might no longer be necessary to submit zero-stride attribs via
> the vertex API, but this would be missed if _NEW_PROGRAM was ignored.

If I get your question right then this is something that happens further
down the road.

So, the array element api, just looking at the code what it does and what
it reads, Just takes a VAO, maps it and walks all the vertex at the index
provided with glArrayElement. Then for that given index all enabled
vertex attributes just provided the vertex attributes enabled state
are put into the immediate mode api. Also the decision of if it should take
the VERT_ATTRIB_POS or VERT_ATTRIB_GENERIC0 attribute to trigger the
immediate mode vertex is completely taken from the enabled state
in the VAO. With enabled state I really mean the application observable
OpenGL api that you modify with glEnableVertexAttribArray and query
using the GL_VERTEX_ATTRIB_ARRAY_ENABLED enum and that you find
in gl_array_attributes::Enabled and condensed in the
gl_vertex_array_object::_Enabled bitmask, both within the VAO.
Given that no shader related value is evaluated from the code triggered
by _ae_invalidate_state I would say I am sure about that.

Putting current values into uniforms happens in the draw code path
where we mute some may be enabled arrays from the VAO based on
the shader mode. That again is only the decision if some of the generics
contain material array values from within the vbo module or if these
generics are really generics that must not be evaluated from ffvertex_prog.c.
Based of these enables, not looking at zero strides, ffvertex_prog.c creates a
vertex shader that exactly reads those arrays that are enabled and not
generally muted from vertex processing due to gl_vertex_processing_mode.
The result of ffvertex_prog.c is a shader that reads some VERT_ATTRIB_* values
and put aside double inputs has exactly those VERT_BIT_* bits set in 
...inputs_read
that are required due to the current OpenGL fixed function state and the
currently enabled array. Based on the ...inputs_read and the finally provided 
arrays
from the VAO, the driver backends (well, the consumers like i965, gallium or 
TNL)
only upload/configure those enabled arrays which are consumed by the shader.

Putting that together, I can see current vertex attribute values being taken
from uniforms if they are not enabled in the ctx->Array._DrawVAO.

Back to the array element api to see that program mode is not required
from there by an other argument:
That VAO that is replayed by glArrayElement may never hit the draw path as
from those immediate mode calls that the glArrayElement api generates,
mesa assembles a new VAO/VBO in the vbo module that is finally either
kept in a display list or used for immediate mode rendering. Depending
on which VERT_ATTRIB_* arrays are enabled in this vbo module created
VAO, ffvertex_prog may be triggered if we are about to draw something
fixed function related.
Also from that thought you can see that changes to shader program state
do not affect the way the VAO should get replayed from the glArrayElement api.
They *do* affect the way the resulting VAO produced by the vbo module is
processed, but that is nothing that is handled from _ae_invalidate_state.

Hope this helps?

best

Mathias


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


Re: [Mesa-dev] [PATCH 4/4] mesa: The glArrayElement api is independent of the current program.

2018-05-19 Thread Marek Olšák
Are you sure about this? The fixed-func vertex and fragment shaders can
move zero-stride vertex attribs into constants (uniforms). If a shader
changes, it might no longer be necessary to submit zero-stride attribs via
the vertex API, but this would be missed if _NEW_PROGRAM was ignored.

Marek

On Thu, May 17, 2018 at 2:37 AM,  wrote:

> From: Mathias Fröhlich 
>
> All the shader program dependent handling is done on the level
> of the gl_Context::Array._DrawVAO/_DrawVAOEnabledAttribs.
> So, skip array element invalidation on _NEW_PROGRAM.
>
> Signed-off-by: Mathias Fröhlich 
> ---
>  src/mesa/main/api_arrayelt.c | 2 +-
>  src/mesa/vbo/vbo_context.c   | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/mesa/main/api_arrayelt.c b/src/mesa/main/api_arrayelt.c
> index 2dfa74f64b..afa3012021 100644
> --- a/src/mesa/main/api_arrayelt.c
> +++ b/src/mesa/main/api_arrayelt.c
> @@ -1823,7 +1823,7 @@ _ae_invalidate_state(struct gl_context *ctx)
>  * Luckily, neither the drivers nor tnl muck with the state that
>  * concerns us here:
>  */
> -   assert(ctx->NewState & (_NEW_ARRAY | _NEW_PROGRAM));
> +   assert(ctx->NewState & _NEW_ARRAY);
>
> assert(!actx->mapped_vbos);
> actx->dirty_state = true;
> diff --git a/src/mesa/vbo/vbo_context.c b/src/mesa/vbo/vbo_context.c
> index ee2e31ab7a..cf9405df3d 100644
> --- a/src/mesa/vbo/vbo_context.c
> +++ b/src/mesa/vbo/vbo_context.c
> @@ -157,7 +157,7 @@ vbo_exec_invalidate_state(struct gl_context *ctx)
> struct vbo_context *vbo = vbo_context(ctx);
> struct vbo_exec_context *exec = &vbo->exec;
>
> -   if (ctx->NewState & (_NEW_PROGRAM | _NEW_ARRAY)) {
> +   if (ctx->NewState & _NEW_ARRAY) {
>_ae_invalidate_state(ctx);
> }
> if (ctx->NewState & _NEW_EVAL)
> --
> 2.14.3
>
> ___
> 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