Re: [Mesa-dev] [PATCH 1/3] vbo: Reestablish VAO limit checking on imm VAOs.
Hi Christian, On Sunday, 3 June 2018 20:08:14 CEST Christian Gmeiner wrote: > Am Sa., 2. Juni 2018 um 08:53 Uhr schrieb : > > > > From: Mathias Fröhlich > > > > Bail out with out of memory, when the required stride exceeds the > > maximum possible stride as stored in Const.MaxVertexAttribStride. > > Etnaviv can only handle less than the OpenGL standard mandated > > minimum of 2048. For this one, at least give some 'resource > > exhaustion error' to the application. > > The virgl driver provides 0 for Const.MaxVertexAttribStride > > as it may not be able to query the relevant informatition from the > > underlying OpenGL stack. In this case we just make the assumption > > that the underlying OpenGL stack is able to cope with then commonly > > seen small strides. Note that this is the same than what applications > > had to do before the queries to GL_MAX_VERTEX_ATTRIB_STRIDE, they had to > > hope for the best that the driver and hardware can safely handle the > > used stride values. > > > > Signed-off-by: Mathias Fröhlich > > --- > > src/mesa/vbo/vbo_exec_draw.c | 24 +++- > > 1 file changed, 23 insertions(+), 1 deletion(-) > > > > diff --git a/src/mesa/vbo/vbo_exec_draw.c b/src/mesa/vbo/vbo_exec_draw.c > > index 8d74725db3..0ab9a0d3ea 100644 > > --- a/src/mesa/vbo/vbo_exec_draw.c > > +++ b/src/mesa/vbo/vbo_exec_draw.c > > @@ -177,6 +177,29 @@ vbo_exec_bind_arrays(struct gl_context *ctx) > > struct gl_vertex_array_object *vao = vbo->VAO; > > struct vbo_exec_context *exec = >exec; > > > > + const GLuint stride = exec->vtx.vertex_size*sizeof(GLfloat); > > + > > + /* Bail out with out of memory, when the required stride exceeds the > > +* maximum possible stride. > > +* Etnaviv can only handle less than the OpenGL standard mandated > > +* minimum of 2048. For this one, at least give some 'resource > > +* exhaustion error' to the application. > > +* The virgl driver provides 0 for Const.MaxVertexAttribStride > > +* as it may not be able to query the relevant informatition from the > > +* underlying OpenGL stack. In this case we just make the assumption > > +* that the underlying OpenGL stack is able to cope with then commonly > > +* seen small strides. Note that this is the same than what applications > > +* had to do before the queries to GL_MAX_VERTEX_ATTRIB_STRIDE, they > > had to > > +* hope for the best that the driver and hardware can safely handle the > > +* used stride values. > > +*/ > > Do we really need to duplicate the commit message text here? If the driver > fetch > up e.g. by supporting other/newer GPUs with a more sane limit the patch author > needs to update this comment too. No, we do not need to duplicate that. What I wanted to do here initially is to describe which cases the code below is supposed to handle. Then, to back up why these cases are needed name the driver backends that currently need that behavior. Still, I would consider virgil something special as it may not be able to know the limit. So, how about: /* Bail out with out of memory, when the required stride exceeds the * maximum possible stride. * Note that mesa currently has drivers that can only handle less than * the OpenGL standard mandated minimum of 2048. For these, at least give * some 'resource exhaustion error' to the application. * The virgl driver provides 0 for Const.MaxVertexAttribStride */ best Mathias ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/3] vbo: Reestablish VAO limit checking on imm VAOs.
Am Sa., 2. Juni 2018 um 08:53 Uhr schrieb :
>
> From: Mathias Fröhlich
>
> Bail out with out of memory, when the required stride exceeds the
> maximum possible stride as stored in Const.MaxVertexAttribStride.
> Etnaviv can only handle less than the OpenGL standard mandated
> minimum of 2048. For this one, at least give some 'resource
> exhaustion error' to the application.
> The virgl driver provides 0 for Const.MaxVertexAttribStride
> as it may not be able to query the relevant informatition from the
> underlying OpenGL stack. In this case we just make the assumption
> that the underlying OpenGL stack is able to cope with then commonly
> seen small strides. Note that this is the same than what applications
> had to do before the queries to GL_MAX_VERTEX_ATTRIB_STRIDE, they had to
> hope for the best that the driver and hardware can safely handle the
> used stride values.
>
> Signed-off-by: Mathias Fröhlich
> ---
> src/mesa/vbo/vbo_exec_draw.c | 24 +++-
> 1 file changed, 23 insertions(+), 1 deletion(-)
>
> diff --git a/src/mesa/vbo/vbo_exec_draw.c b/src/mesa/vbo/vbo_exec_draw.c
> index 8d74725db3..0ab9a0d3ea 100644
> --- a/src/mesa/vbo/vbo_exec_draw.c
> +++ b/src/mesa/vbo/vbo_exec_draw.c
> @@ -177,6 +177,29 @@ vbo_exec_bind_arrays(struct gl_context *ctx)
> struct gl_vertex_array_object *vao = vbo->VAO;
> struct vbo_exec_context *exec = >exec;
>
> + const GLuint stride = exec->vtx.vertex_size*sizeof(GLfloat);
> +
> + /* Bail out with out of memory, when the required stride exceeds the
> +* maximum possible stride.
> +* Etnaviv can only handle less than the OpenGL standard mandated
> +* minimum of 2048. For this one, at least give some 'resource
> +* exhaustion error' to the application.
> +* The virgl driver provides 0 for Const.MaxVertexAttribStride
> +* as it may not be able to query the relevant informatition from the
> +* underlying OpenGL stack. In this case we just make the assumption
> +* that the underlying OpenGL stack is able to cope with then commonly
> +* seen small strides. Note that this is the same than what applications
> +* had to do before the queries to GL_MAX_VERTEX_ATTRIB_STRIDE, they had
> to
> +* hope for the best that the driver and hardware can safely handle the
> +* used stride values.
> +*/
Do we really need to duplicate the commit message text here? If the driver fetch
up e.g. by supporting other/newer GPUs with a more sane limit the patch author
needs to update this comment too.
> + if (ctx->Const.MaxVertexAttribStride &&
> + stride > ctx->Const.MaxVertexAttribStride) {
> + _mesa_set_draw_vao(ctx, ctx->Array._EmptyVAO, 0);
> + _mesa_error(ctx, GL_OUT_OF_MEMORY, "Max VAO binding stride exceeded");
> + return;
> + }
> +
> GLintptr buffer_offset;
> if (_mesa_is_bufferobj(exec->vtx.bufferobj)) {
>assert(exec->vtx.bufferobj->Mappings[MAP_INTERNAL].Pointer);
> @@ -200,7 +223,6 @@ vbo_exec_bind_arrays(struct gl_context *ctx)
> assert((~vao_enabled & vao->_Enabled) == 0);
>
> /* Bind the buffer object */
> - const GLuint stride = exec->vtx.vertex_size*sizeof(GLfloat);
> _mesa_bind_vertex_buffer(ctx, vao, 0, exec->vtx.bufferobj, buffer_offset,
> stride);
>
> --
> 2.17.1
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
--
greets
--
Christian Gmeiner, MSc
https://christian-gmeiner.info
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/3] vbo: Reestablish VAO limit checking on imm VAOs.
From: Mathias Fröhlich
Bail out with out of memory, when the required stride exceeds the
maximum possible stride as stored in Const.MaxVertexAttribStride.
Etnaviv can only handle less than the OpenGL standard mandated
minimum of 2048. For this one, at least give some 'resource
exhaustion error' to the application.
The virgl driver provides 0 for Const.MaxVertexAttribStride
as it may not be able to query the relevant informatition from the
underlying OpenGL stack. In this case we just make the assumption
that the underlying OpenGL stack is able to cope with then commonly
seen small strides. Note that this is the same than what applications
had to do before the queries to GL_MAX_VERTEX_ATTRIB_STRIDE, they had to
hope for the best that the driver and hardware can safely handle the
used stride values.
Signed-off-by: Mathias Fröhlich
---
src/mesa/vbo/vbo_exec_draw.c | 24 +++-
1 file changed, 23 insertions(+), 1 deletion(-)
diff --git a/src/mesa/vbo/vbo_exec_draw.c b/src/mesa/vbo/vbo_exec_draw.c
index 8d74725db3..0ab9a0d3ea 100644
--- a/src/mesa/vbo/vbo_exec_draw.c
+++ b/src/mesa/vbo/vbo_exec_draw.c
@@ -177,6 +177,29 @@ vbo_exec_bind_arrays(struct gl_context *ctx)
struct gl_vertex_array_object *vao = vbo->VAO;
struct vbo_exec_context *exec = >exec;
+ const GLuint stride = exec->vtx.vertex_size*sizeof(GLfloat);
+
+ /* Bail out with out of memory, when the required stride exceeds the
+* maximum possible stride.
+* Etnaviv can only handle less than the OpenGL standard mandated
+* minimum of 2048. For this one, at least give some 'resource
+* exhaustion error' to the application.
+* The virgl driver provides 0 for Const.MaxVertexAttribStride
+* as it may not be able to query the relevant informatition from the
+* underlying OpenGL stack. In this case we just make the assumption
+* that the underlying OpenGL stack is able to cope with then commonly
+* seen small strides. Note that this is the same than what applications
+* had to do before the queries to GL_MAX_VERTEX_ATTRIB_STRIDE, they had to
+* hope for the best that the driver and hardware can safely handle the
+* used stride values.
+*/
+ if (ctx->Const.MaxVertexAttribStride &&
+ stride > ctx->Const.MaxVertexAttribStride) {
+ _mesa_set_draw_vao(ctx, ctx->Array._EmptyVAO, 0);
+ _mesa_error(ctx, GL_OUT_OF_MEMORY, "Max VAO binding stride exceeded");
+ return;
+ }
+
GLintptr buffer_offset;
if (_mesa_is_bufferobj(exec->vtx.bufferobj)) {
assert(exec->vtx.bufferobj->Mappings[MAP_INTERNAL].Pointer);
@@ -200,7 +223,6 @@ vbo_exec_bind_arrays(struct gl_context *ctx)
assert((~vao_enabled & vao->_Enabled) == 0);
/* Bind the buffer object */
- const GLuint stride = exec->vtx.vertex_size*sizeof(GLfloat);
_mesa_bind_vertex_buffer(ctx, vao, 0, exec->vtx.bufferobj, buffer_offset,
stride);
--
2.17.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
