<div dir="ltr"><div>Reviewed-by: Marek Olšák <<a href="mailto:marek.olsak@amd.com">marek.olsak@amd.com</a>></div><div> </div><div>Marek </div></div><div class="gmail_extra"> <div class="gmail_quote">On Sat, Jun 2, 2018 at 2:51 AM, <<a href="mailto:Mathias.Froehlich@gmx.net" target="_blank">Mathias.Froehlich@gmx.net</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">From: Mathias Fröhlich <<a href="mailto:mathias.froehlich@web.de">mathias.froehlich@web.de</a>> 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: <a href="https://bugs.freedesktop.org/show_bug.cgi?id=106594" rel="noreferrer" target="_blank">https://bugs.freedesktop.org/show_bug.cgi?id=106594</a> Signed-off-by: Mathias Fröhlich <<a href="mailto:Mathias.Froehlich@web.de">Mathias.Froehlich@web.de</a>> --- 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 _______________________________________________ mesa-dev mailing list <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></div> </div>