[Mesa-dev] [PATCH] mesa: Make sure that imm draws are flushed before other draws execute.

Mathias.Froehlich at gmx.net Mathias.Froehlich at gmx.net
Sat Jun 2 06:51:11 UTC 2018


From: Mathias Fröhlich <mathias.froehlich at web.de>

Hi all,

The below patch fixes a recently introduced failure of my VAO rework.
I could finally reproduce the problem using the provided apitrace
(thanks Kai for the hint with the lower left corner).
The change is slightly different than what I put initially into the
bugreport.
The patch survived piglit quick and dEQP in radeonsi and did not
introduce regressions to the base version with intels CI system.

Please review!

best

Mathias




The recent patch

    mesa: Remove FLUSH_VERTICES from VAO state changes.

    Pending draw calls on immediate mode or display list calls do
    not depend on changes of the VAO state. So, remove calls to
    FLUSH_VERTICES and flag _NEW_ARRAY as appropriate.

uncovered a problem that non immediate mode draw calls do only
flush outstanding immediate mode draws if FLUSH_UPDATE_CURRENT
is set in ctx->Driver.NeedFlush.
In that case, due to the sequence of _mesa_set_draw_vao commands
we could end up with the VAO from the FLUSH_VERTICES call set
into gl_context::Array._DrawVAO when the array draw is executed.
So the change pulls FLUSH_CURRENT out of _mesa_validate_* calls
into the array draw calls being validated.
The change introduces a new macro FLUSH_FOR_DRAW beside FLUSH_VERTICES
and FLUSH_CURRENT that flushes on changed current attributes as well
as on outstanding immediate mode draw calls. Use FLUSH_FOR_DRAW
in the non immediate mode draw code paths.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106594
Signed-off-by: Mathias Fröhlich <Mathias.Froehlich at web.de>
---
 src/mesa/main/context.h       | 16 ++++++++
 src/mesa/main/draw_validate.c | 26 ------------
 src/mesa/vbo/vbo_exec_array.c | 77 ++++++++++++++++++-----------------
 src/mesa/vbo/vbo_save_draw.c  |  2 +-
 4 files changed, 56 insertions(+), 65 deletions(-)

diff --git a/src/mesa/main/context.h b/src/mesa/main/context.h
index 77520f678f..d50438fd7f 100644
--- a/src/mesa/main/context.h
+++ b/src/mesa/main/context.h
@@ -232,6 +232,22 @@ do {								\
    ctx->NewState |= newstate;					\
 } while (0)
 
+/**
+ * Flush vertices.
+ *
+ * \param ctx GL context.
+ *
+ * Checks if dd_function_table::NeedFlush is marked to flush stored vertices
+ * or current state and calls dd_function_table::FlushVertices if so.
+ */
+#define FLUSH_FOR_DRAW(ctx)                                     \
+do {                                                            \
+   if (MESA_VERBOSE & VERBOSE_STATE)                            \
+      _mesa_debug(ctx, "FLUSH_FOR_DRAW in %s\n", __func__);     \
+   if (ctx->Driver.NeedFlush)                                   \
+      vbo_exec_FlushVertices(ctx, ctx->Driver.NeedFlush);       \
+} while (0)
+
 /**
  * Macro to assert that the API call was made outside the
  * glBegin()/glEnd() pair, with return value.
diff --git a/src/mesa/main/draw_validate.c b/src/mesa/main/draw_validate.c
index bcb2d91306..352263c5c7 100644
--- a/src/mesa/main/draw_validate.c
+++ b/src/mesa/main/draw_validate.c
@@ -696,8 +696,6 @@ _mesa_validate_DrawElements(struct gl_context *ctx,
                             GLenum mode, GLsizei count, GLenum type,
                             const GLvoid *indices)
 {
-   FLUSH_CURRENT(ctx, 0);
-
    return validate_DrawElements_common(ctx, mode, count, type, indices,
                                        "glDrawElements");
 }
@@ -716,8 +714,6 @@ _mesa_validate_MultiDrawElements(struct gl_context *ctx,
 {
    GLsizei i;
 
-   FLUSH_CURRENT(ctx, 0);
-
    /*
     * Section 2.3.1 (Errors) of the OpenGL 4.5 (Core Profile) spec says:
     *
@@ -780,8 +776,6 @@ _mesa_validate_DrawRangeElements(struct gl_context *ctx, GLenum mode,
                                  GLsizei count, GLenum type,
                                  const GLvoid *indices)
 {
-   FLUSH_CURRENT(ctx, 0);
-
    if (end < start) {
       _mesa_error(ctx, GL_INVALID_VALUE, "glDrawRangeElements(end<start)");
       return GL_FALSE;
@@ -895,8 +889,6 @@ static bool
 validate_draw_arrays(struct gl_context *ctx, const char *func,
                      GLenum mode, GLsizei count, GLsizei numInstances)
 {
-   FLUSH_CURRENT(ctx, 0);
-
    if (count < 0) {
       _mesa_error(ctx, GL_INVALID_VALUE, "%s(count)", func);
       return false;
@@ -971,8 +963,6 @@ _mesa_validate_MultiDrawArrays(struct gl_context *ctx, GLenum mode,
 {
    int i;
 
-   FLUSH_CURRENT(ctx, 0);
-
    if (!_mesa_valid_prim_mode(ctx, mode, "glMultiDrawArrays"))
       return false;
 
@@ -1018,8 +1008,6 @@ _mesa_validate_DrawElementsInstanced(struct gl_context *ctx,
                                      GLenum mode, GLsizei count, GLenum type,
                                      const GLvoid *indices, GLsizei numInstances)
 {
-   FLUSH_CURRENT(ctx, 0);
-
    if (numInstances < 0) {
       _mesa_error(ctx, GL_INVALID_VALUE,
                   "glDrawElementsInstanced(numInstances=%d)", numInstances);
@@ -1039,8 +1027,6 @@ _mesa_validate_DrawTransformFeedback(struct gl_context *ctx,
                                      GLuint stream,
                                      GLsizei numInstances)
 {
-   FLUSH_CURRENT(ctx, 0);
-
    if (!_mesa_valid_prim_mode(ctx, mode, "glDrawTransformFeedback*(mode)")) {
       return GL_FALSE;
    }
@@ -1244,8 +1230,6 @@ _mesa_validate_DrawArraysIndirect(struct gl_context *ctx,
 {
    const unsigned drawArraysNumParams = 4;
 
-   FLUSH_CURRENT(ctx, 0);
-
    return valid_draw_indirect(ctx, mode,
                               indirect, drawArraysNumParams * sizeof(GLuint),
                               "glDrawArraysIndirect");
@@ -1258,8 +1242,6 @@ _mesa_validate_DrawElementsIndirect(struct gl_context *ctx,
 {
    const unsigned drawElementsNumParams = 5;
 
-   FLUSH_CURRENT(ctx, 0);
-
    return valid_draw_indirect_elements(ctx, mode, type,
                                        indirect, drawElementsNumParams * sizeof(GLuint),
                                        "glDrawElementsIndirect");
@@ -1274,8 +1256,6 @@ _mesa_validate_MultiDrawArraysIndirect(struct gl_context *ctx,
    GLsizeiptr size = 0;
    const unsigned drawArraysNumParams = 4;
 
-   FLUSH_CURRENT(ctx, 0);
-
    /* caller has converted stride==0 to drawArraysNumParams * sizeof(GLuint) */
    assert(stride != 0);
 
@@ -1304,8 +1284,6 @@ _mesa_validate_MultiDrawElementsIndirect(struct gl_context *ctx,
    GLsizeiptr size = 0;
    const unsigned drawElementsNumParams = 5;
 
-   FLUSH_CURRENT(ctx, 0);
-
    /* caller has converted stride==0 to drawElementsNumParams * sizeof(GLuint) */
    assert(stride != 0);
 
@@ -1385,8 +1363,6 @@ _mesa_validate_MultiDrawArraysIndirectCount(struct gl_context *ctx,
    GLsizeiptr size = 0;
    const unsigned drawArraysNumParams = 4;
 
-   FLUSH_CURRENT(ctx, 0);
-
    /* caller has converted stride==0 to drawArraysNumParams * sizeof(GLuint) */
    assert(stride != 0);
 
@@ -1418,8 +1394,6 @@ _mesa_validate_MultiDrawElementsIndirectCount(struct gl_context *ctx,
    GLsizeiptr size = 0;
    const unsigned drawElementsNumParams = 5;
 
-   FLUSH_CURRENT(ctx, 0);
-
    /* caller has converted stride==0 to drawElementsNumParams * sizeof(GLuint) */
    assert(stride != 0);
 
diff --git a/src/mesa/vbo/vbo_exec_array.c b/src/mesa/vbo/vbo_exec_array.c
index e74e1bd458..792907ac04 100644
--- a/src/mesa/vbo/vbo_exec_array.c
+++ b/src/mesa/vbo/vbo_exec_array.c
@@ -530,9 +530,9 @@ vbo_exec_DrawArrays(GLenum mode, GLint start, GLsizei count)
       _mesa_debug(ctx, "glDrawArrays(%s, %d, %d)\n",
                   _mesa_enum_to_string(mode), start, count);
 
-   if (_mesa_is_no_error_enabled(ctx)) {
-      FLUSH_CURRENT(ctx, 0);
+   FLUSH_FOR_DRAW(ctx);
 
+   if (_mesa_is_no_error_enabled(ctx)) {
       _mesa_set_draw_vao(ctx, ctx->Array.VAO, enabled_filter(ctx));
 
       if (ctx->NewState)
@@ -568,10 +568,9 @@ vbo_exec_DrawArraysInstanced(GLenum mode, GLint start, GLsizei count,
       _mesa_debug(ctx, "glDrawArraysInstanced(%s, %d, %d, %d)\n",
                   _mesa_enum_to_string(mode), start, count, numInstances);
 
+   FLUSH_FOR_DRAW(ctx);
 
    if (_mesa_is_no_error_enabled(ctx)) {
-      FLUSH_CURRENT(ctx, 0);
-
       _mesa_set_draw_vao(ctx, ctx->Array.VAO, enabled_filter(ctx));
 
       if (ctx->NewState)
@@ -610,9 +609,9 @@ vbo_exec_DrawArraysInstancedBaseInstance(GLenum mode, GLint first,
                   _mesa_enum_to_string(mode), first, count,
                   numInstances, baseInstance);
 
-   if (_mesa_is_no_error_enabled(ctx)) {
-      FLUSH_CURRENT(ctx, 0);
+   FLUSH_FOR_DRAW(ctx);
 
+   if (_mesa_is_no_error_enabled(ctx)) {
       _mesa_set_draw_vao(ctx, ctx->Array.VAO, enabled_filter(ctx));
 
       if (ctx->NewState)
@@ -650,9 +649,9 @@ vbo_exec_MultiDrawArrays(GLenum mode, const GLint *first,
                   "glMultiDrawArrays(%s, %p, %p, %d)\n",
                   _mesa_enum_to_string(mode), first, count, primcount);
 
-   if (_mesa_is_no_error_enabled(ctx)) {
-      FLUSH_CURRENT(ctx, 0);
+   FLUSH_FOR_DRAW(ctx);
 
+   if (_mesa_is_no_error_enabled(ctx)) {
       _mesa_set_draw_vao(ctx, ctx->Array.VAO, enabled_filter(ctx));
 
       if (ctx->NewState)
@@ -873,9 +872,9 @@ vbo_exec_DrawRangeElementsBaseVertex(GLenum mode, GLuint start, GLuint end,
                   _mesa_enum_to_string(mode), start, end, count,
                   _mesa_enum_to_string(type), indices, basevertex);
 
-   if (_mesa_is_no_error_enabled(ctx)) {
-      FLUSH_CURRENT(ctx, 0);
+   FLUSH_FOR_DRAW(ctx);
 
+   if (_mesa_is_no_error_enabled(ctx)) {
       _mesa_set_draw_vao(ctx, ctx->Array.VAO, enabled_filter(ctx));
 
       if (ctx->NewState)
@@ -984,9 +983,9 @@ vbo_exec_DrawElements(GLenum mode, GLsizei count, GLenum type,
                   _mesa_enum_to_string(mode), count,
                   _mesa_enum_to_string(type), indices);
 
-   if (_mesa_is_no_error_enabled(ctx)) {
-      FLUSH_CURRENT(ctx, 0);
+   FLUSH_FOR_DRAW(ctx);
 
+   if (_mesa_is_no_error_enabled(ctx)) {
       _mesa_set_draw_vao(ctx, ctx->Array.VAO, enabled_filter(ctx));
 
       if (ctx->NewState)
@@ -1017,9 +1016,9 @@ vbo_exec_DrawElementsBaseVertex(GLenum mode, GLsizei count, GLenum type,
                   _mesa_enum_to_string(mode), count,
                   _mesa_enum_to_string(type), indices);
 
-   if (_mesa_is_no_error_enabled(ctx)) {
-      FLUSH_CURRENT(ctx, 0);
+   FLUSH_FOR_DRAW(ctx);
 
+   if (_mesa_is_no_error_enabled(ctx)) {
       _mesa_set_draw_vao(ctx, ctx->Array.VAO, enabled_filter(ctx));
 
       if (ctx->NewState)
@@ -1050,9 +1049,9 @@ vbo_exec_DrawElementsInstanced(GLenum mode, GLsizei count, GLenum type,
                   _mesa_enum_to_string(mode), count,
                   _mesa_enum_to_string(type), indices);
 
-   if (_mesa_is_no_error_enabled(ctx)) {
-      FLUSH_CURRENT(ctx, 0);
+   FLUSH_FOR_DRAW(ctx);
 
+   if (_mesa_is_no_error_enabled(ctx)) {
       _mesa_set_draw_vao(ctx, ctx->Array.VAO, enabled_filter(ctx));
 
       if (ctx->NewState)
@@ -1089,9 +1088,9 @@ vbo_exec_DrawElementsInstancedBaseVertex(GLenum mode, GLsizei count,
                   _mesa_enum_to_string(type), indices,
                   numInstances, basevertex);
 
-   if (_mesa_is_no_error_enabled(ctx)) {
-      FLUSH_CURRENT(ctx, 0);
+   FLUSH_FOR_DRAW(ctx);
 
+   if (_mesa_is_no_error_enabled(ctx)) {
       _mesa_set_draw_vao(ctx, ctx->Array.VAO, enabled_filter(ctx));
 
       if (ctx->NewState)
@@ -1130,9 +1129,9 @@ vbo_exec_DrawElementsInstancedBaseInstance(GLenum mode, GLsizei count,
                   _mesa_enum_to_string(type), indices,
                   numInstances, baseInstance);
 
-   if (_mesa_is_no_error_enabled(ctx)) {
-      FLUSH_CURRENT(ctx, 0);
+   FLUSH_FOR_DRAW(ctx);
 
+   if (_mesa_is_no_error_enabled(ctx)) {
       _mesa_set_draw_vao(ctx, ctx->Array.VAO, enabled_filter(ctx));
 
       if (ctx->NewState)
@@ -1173,9 +1172,9 @@ vbo_exec_DrawElementsInstancedBaseVertexBaseInstance(GLenum mode,
                   _mesa_enum_to_string(type), indices,
                   numInstances, basevertex, baseInstance);
 
-   if (_mesa_is_no_error_enabled(ctx)) {
-      FLUSH_CURRENT(ctx, 0);
+   FLUSH_FOR_DRAW(ctx);
 
+   if (_mesa_is_no_error_enabled(ctx)) {
       _mesa_set_draw_vao(ctx, ctx->Array.VAO, enabled_filter(ctx));
 
       if (ctx->NewState)
@@ -1337,6 +1336,8 @@ vbo_exec_MultiDrawElements(GLenum mode,
 {
    GET_CURRENT_CONTEXT(ctx);
 
+   FLUSH_FOR_DRAW(ctx);
+
    _mesa_set_draw_vao(ctx, ctx->Array.VAO, enabled_filter(ctx));
 
    if (!_mesa_validate_MultiDrawElements(ctx, mode, count, type, indices,
@@ -1360,9 +1361,9 @@ vbo_exec_MultiDrawElementsBaseVertex(GLenum mode,
 {
    GET_CURRENT_CONTEXT(ctx);
 
-   if (_mesa_is_no_error_enabled(ctx)) {
-      FLUSH_CURRENT(ctx, 0);
+   FLUSH_FOR_DRAW(ctx);
 
+   if (_mesa_is_no_error_enabled(ctx)) {
       _mesa_set_draw_vao(ctx, ctx->Array.VAO, enabled_filter(ctx));
 
       if (ctx->NewState)
@@ -1398,9 +1399,9 @@ vbo_draw_transform_feedback(struct gl_context *ctx, GLenum mode,
 {
    struct _mesa_prim prim;
 
-   if (_mesa_is_no_error_enabled(ctx)) {
-      FLUSH_CURRENT(ctx, 0);
+   FLUSH_FOR_DRAW(ctx);
 
+   if (_mesa_is_no_error_enabled(ctx)) {
       _mesa_set_draw_vao(ctx, ctx->Array.VAO, enabled_filter(ctx));
 
       if (ctx->NewState)
@@ -1615,9 +1616,9 @@ vbo_exec_DrawArraysIndirect(GLenum mode, const GLvoid *indirect)
       _mesa_debug(ctx, "glDrawArraysIndirect(%s, %p)\n",
                   _mesa_enum_to_string(mode), indirect);
 
-   if (_mesa_is_no_error_enabled(ctx)) {
-      FLUSH_CURRENT(ctx, 0);
+   FLUSH_FOR_DRAW(ctx);
 
+   if (_mesa_is_no_error_enabled(ctx)) {
       _mesa_set_draw_vao(ctx, ctx->Array.VAO, enabled_filter(ctx));
 
       if (ctx->NewState)
@@ -1646,9 +1647,9 @@ vbo_exec_DrawElementsIndirect(GLenum mode, GLenum type, const GLvoid *indirect)
                   _mesa_enum_to_string(mode),
                   _mesa_enum_to_string(type), indirect);
 
-   if (_mesa_is_no_error_enabled(ctx)) {
-      FLUSH_CURRENT(ctx, 0);
+   FLUSH_FOR_DRAW(ctx);
 
+   if (_mesa_is_no_error_enabled(ctx)) {
       _mesa_set_draw_vao(ctx, ctx->Array.VAO, enabled_filter(ctx));
 
       if (ctx->NewState)
@@ -1681,9 +1682,9 @@ vbo_exec_MultiDrawArraysIndirect(GLenum mode, const GLvoid *indirect,
    if (stride == 0)
       stride = 4 * sizeof(GLuint);      /* sizeof(DrawArraysIndirectCommand) */
 
-   if (_mesa_is_no_error_enabled(ctx)) {
-      FLUSH_CURRENT(ctx, 0);
+   FLUSH_FOR_DRAW(ctx);
 
+   if (_mesa_is_no_error_enabled(ctx)) {
       _mesa_set_draw_vao(ctx, ctx->Array.VAO, enabled_filter(ctx));
 
       if (ctx->NewState)
@@ -1720,9 +1721,9 @@ vbo_exec_MultiDrawElementsIndirect(GLenum mode, GLenum type,
    if (stride == 0)
       stride = 5 * sizeof(GLuint);      /* sizeof(DrawElementsIndirectCommand) */
 
-   if (_mesa_is_no_error_enabled(ctx)) {
-      FLUSH_CURRENT(ctx, 0);
+   FLUSH_FOR_DRAW(ctx);
 
+   if (_mesa_is_no_error_enabled(ctx)) {
       _mesa_set_draw_vao(ctx, ctx->Array.VAO, enabled_filter(ctx));
 
       if (ctx->NewState)
@@ -1815,9 +1816,9 @@ vbo_exec_MultiDrawArraysIndirectCount(GLenum mode, GLintptr indirect,
    if (stride == 0)
       stride = 4 * sizeof(GLuint);      /* sizeof(DrawArraysIndirectCommand) */
 
-   if (_mesa_is_no_error_enabled(ctx)) {
-      FLUSH_CURRENT(ctx, 0);
+   FLUSH_FOR_DRAW(ctx);
 
+   if (_mesa_is_no_error_enabled(ctx)) {
       _mesa_set_draw_vao(ctx, ctx->Array.VAO, enabled_filter(ctx));
 
       if (ctx->NewState)
@@ -1860,9 +1861,9 @@ vbo_exec_MultiDrawElementsIndirectCount(GLenum mode, GLenum type,
    if (stride == 0)
       stride = 5 * sizeof(GLuint);      /* sizeof(DrawElementsIndirectCommand) */
 
-   if (_mesa_is_no_error_enabled(ctx)) {
-      FLUSH_CURRENT(ctx, 0);
+   FLUSH_FOR_DRAW(ctx);
 
+   if (_mesa_is_no_error_enabled(ctx)) {
       _mesa_set_draw_vao(ctx, ctx->Array.VAO, enabled_filter(ctx));
 
       if (ctx->NewState)
diff --git a/src/mesa/vbo/vbo_save_draw.c b/src/mesa/vbo/vbo_save_draw.c
index f4b2c80748..71620e9a3c 100644
--- a/src/mesa/vbo/vbo_save_draw.c
+++ b/src/mesa/vbo/vbo_save_draw.c
@@ -168,7 +168,7 @@ vbo_save_playback_vertex_list(struct gl_context *ctx, void *data)
       remap_vertex_store = GL_TRUE;
    }
 
-   FLUSH_CURRENT(ctx, 0);
+   FLUSH_FOR_DRAW(ctx);
 
    if (node->prim_count > 0) {
 
-- 
2.17.1



More information about the mesa-dev mailing list