[Mesa-dev] [PATCH] mesa: Make sure that imm draws are flushed before other draws execute.
Marek Olšák
maraeo at gmail.com
Sun Jun 3 18:59:34 UTC 2018
Reviewed-by: Marek Olšák <marek.olsak at amd.com>
Marek
On Sat, Jun 2, 2018 at 2:51 AM, <Mathias.Froehlich at gmx.net> wrote:
> 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
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180603/b40c6068/attachment-0001.html>
More information about the mesa-dev
mailing list