From 80d8da9d7d3c7a1981a084b39bf55e92544b90da Mon Sep 17 00:00:00 2001 From: Gert Wollny Date: Sun, 31 Mar 2024 22:39:58 +0200 Subject: [PATCH] mesa/st: don't use base shader serialization when uniforms are not packed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When loading the base shader serialization there is a discrepancy between the state parameters that may already have been optimized, because after storing the serialization the shader went through st_finalize_nir, and _mesa_optimize_state_parameters was run, so that original state parameters may have been optimized and replaced by new parameters. After get_nir_shader is called, the original state parameters are re-added - in addition to the optimized parameters. This lead to an bug with the uniform offsets when lowering uniforms to UBOs. Therefore, as a hotfix for drivers that don't support packed uniforms, ignore the base serialization and use the serialization obtained after st_finalize_nir was run. With that the problem can be avoided. Fixes: 5eb0136a3c561e25d3f274e33a86812cfb2af589 mesa/st: when creating draw shader variants, use the base nir and skip driver opts Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/10881 v2: reorder conditional evaluation for better readability (zmike) v3: revert c72bb8de7 ("r300: mark new fails") (Pavel Ondračka) Signed-off-by: Gert Wollny Part-of: (cherry picked from commit 7de8a010876b6e1fdf7fc8cf15f3f0e10ba5c569) --- .pick_status.json | 2 +- .../drivers/r300/ci/r300-rv530-nohiz-fails.txt | 11 ----------- src/mesa/state_tracker/st_program.c | 5 +++-- 3 files changed, 4 insertions(+), 14 deletions(-) diff --git a/.pick_status.json b/.pick_status.json index 0487bce28d7..23dafdf9a92 100644 --- a/.pick_status.json +++ b/.pick_status.json @@ -104,7 +104,7 @@ "description": "mesa/st: don't use base shader serialization when uniforms are not packed", "nominated": true, "nomination_type": 1, - "resolution": 0, + "resolution": 1, "main_sha": null, "because_sha": "5eb0136a3c561e25d3f274e33a86812cfb2af589", "notes": null diff --git a/src/gallium/drivers/r300/ci/r300-rv530-nohiz-fails.txt b/src/gallium/drivers/r300/ci/r300-rv530-nohiz-fails.txt index a981dd6fe95..47540f540c7 100644 --- a/src/gallium/drivers/r300/ci/r300-rv530-nohiz-fails.txt +++ b/src/gallium/drivers/r300/ci/r300-rv530-nohiz-fails.txt @@ -376,12 +376,6 @@ spec@!opengl 1.0@gl-1.0-blend-func,Fail spec@!opengl 1.0@gl-1.0-edgeflag,Fail spec@!opengl 1.0@gl-1.0-edgeflag-quads,Fail spec@!opengl 1.0@gl-1.0-no-op-paths,Fail -spec@!opengl 1.0@gl-1.0-rendermode-feedback,Fail -spec@!opengl 1.0@gl-1.0-rendermode-feedback@GL_2D,Fail -spec@!opengl 1.0@gl-1.0-rendermode-feedback@GL_3D,Fail -spec@!opengl 1.0@gl-1.0-rendermode-feedback@GL_3D_COLOR,Fail -spec@!opengl 1.0@gl-1.0-rendermode-feedback@GL_3D_COLOR_TEXTURE,Fail -spec@!opengl 1.0@gl-1.0-rendermode-feedback@GL_4D_COLOR_TEXTURE,Fail spec@!opengl 1.0@gl-1.0-scissor-offscreen,Fail spec@!opengl 1.1@depthstencil-default_fb-blit samples=2,Fail @@ -407,11 +401,6 @@ spec@!opengl 1.1@getteximage-formats,Fail spec@!opengl 1.1@getteximage-simple,Fail spec@!opengl 1.1@gl-1.1-read-pixels-after-display-list,Fail spec@!opengl 1.1@gl-1.2-texture-base-level,Fail -spec@!opengl 1.1@gl_select - alpha-test enabled,Fail -spec@!opengl 1.1@gl_select - depth-test enabled,Fail -spec@!opengl 1.1@gl_select - no test function,Fail -spec@!opengl 1.1@gl_select - scissor-test enabled,Fail -spec@!opengl 1.1@gl_select - stencil-test enabled,Fail spec@!opengl 1.1@line-aa-width,Fail spec@!opengl 1.1@line-smooth-stipple,Fail spec@!opengl 1.1@linestipple,Fail diff --git a/src/mesa/state_tracker/st_program.c b/src/mesa/state_tracker/st_program.c index 32ed4a4702f..bebd3703a0c 100644 --- a/src/mesa/state_tracker/st_program.c +++ b/src/mesa/state_tracker/st_program.c @@ -630,7 +630,7 @@ static const struct nir_shader_compiler_options draw_nir_options = { static struct nir_shader * get_nir_shader(struct st_context *st, struct gl_program *prog, bool is_draw) { - if (!is_draw && prog->nir) { + if ((!is_draw || !st->ctx->Const.PackedDriverUniformStorage) && prog->nir) { nir_shader *nir = prog->nir; /* The first shader variant takes ownership of NIR, so that there is @@ -646,7 +646,8 @@ get_nir_shader(struct st_context *st, struct gl_program *prog, bool is_draw) const struct nir_shader_compiler_options *options = is_draw ? &draw_nir_options : st_get_nir_compiler_options(st, prog->info.stage); - if (is_draw && (!prog->shader_program || prog->shader_program->data->LinkStatus != LINKING_SKIPPED)) { + if (is_draw && st->ctx->Const.PackedDriverUniformStorage && + (!prog->shader_program || prog->shader_program->data->LinkStatus != LINKING_SKIPPED)) { assert(prog->base_serialized_nir); blob_reader_init(&blob_reader, prog->base_serialized_nir, prog->base_serialized_nir_size); } else {