Reviewed-by: Marek Olšák <marek.ol...@amd.com> Marek
On Sat, Jun 2, 2018 at 2:51 AM, <mathias.froehl...@gmx.net> wrote: > From: Mathias Fröhlich <mathias.froehl...@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.froehl...@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 > > _______________________________________________ > 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
