From 0dcd9a09d19d25bf247118b218b4b439e8df619b Mon Sep 17 00:00:00 2001 From: Topi Pohjolainen Date: Wed, 27 Jan 2016 13:08:03 +0200 Subject: [PATCH] i965: Restore vbo after color resolve during brw_try_draw_prims() Part of brw_try_draw_prims() is a check to validate textures (brw_validate_textures()). In case of textures that currently have only level zero but are marked for mipmap generation, i965 driver will decide to replace the underlying buffer with a larger one capable of holding also the additional levels. This results into blit from the original buffer to the newly allocated (see intel_miptree_copy_teximage()). This blit is currently handled with blitter engine and hence it won't effect the ongoing draw operation. However, this blit in turn may trigger color resolve on the source buffer. In principle, this should be possible with fast cleared buffers but I only started hitting it when I enabled lossless compression (that reguires similar resolve to fast cleared buffers). Now, the color resolve is a meta operation and uses the same drawing path we are already in middle of. After quite a bit of debugging I realized that the resolve will modify the current vbo setup but it won't restore it afterwards resulting in the original draw call using wrong vertex data. When brw_try_draw_prims() gets called, the vbo logic in the Mesa core (see vbo_draw_arrays()) has just bound the vbo (see vbo_bind_arrays() and recalculate_input_bindings()). Color resolve operation will overwrite the vbo setup by calling vbo_bind_arrays() against the resolve rectangle (see brw_draw_rectlist()). Once the color resolve is done the vbo setup is left to the resolve rectangle state and the original drawing call yields bogus results. This patch aims to restore the original state after the color resolve by calling vbo_bind_arrays() yet again after the vertex array state in the core context have been restored. Now having said all this, I'd also like to state that I'm quite uncomfortable with the nested meta operations. Ths original draw call in this case is in fact a meta operation itself. It is a blit from level zero to level one when generating the additional mipmap levels (see _mesa_meta_GenerateMipmap()). Imagine the complexity if the blit in the middle from buffer to another would go to meta path also instead of blitter. I would very tempted to try to move all the resolves to happen before a meta operation is started. Additionally I still feel that work I did earlier in the spring/ summer time moving meta operations to use direct state upload bypassing the core context would make sense. v2: Force input recalculation by setting the flag explicitly v3: Do not attempt to restore vbo for opengles1 which doesn't support vertex buffer objects. Signed-off-by: Topi Pohjolainen --- src/mesa/drivers/dri/i965/brw_meta_fast_clear.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/mesa/drivers/dri/i965/brw_meta_fast_clear.c b/src/mesa/drivers/dri/i965/brw_meta_fast_clear.c index b2b07e7e58e..93f1a8524ae 100644 --- a/src/mesa/drivers/dri/i965/brw_meta_fast_clear.c +++ b/src/mesa/drivers/dri/i965/brw_meta_fast_clear.c @@ -887,6 +887,15 @@ brw_meta_resolve_color(struct brw_context *brw, _mesa_meta_end(ctx); + /* Restore in case we were called in the middle of brw_try_draw_prims(). + * But only in case the just restored context really uses vertex buffer + * objects. + */ + if (ctx->API != API_OPENGLES) { + ctx->vbo_context->exec.array.recalculate_inputs = true; + vbo_bind_arrays(ctx); + } + /* We're typically called from intel_update_state() and we're supposed to * return with the state all updated to what it was before * brw_meta_resolve_color() was called. The meta rendering will have -- 2.30.2