From d62610778ac0839b8e94dc909a9010db2286f17d Mon Sep 17 00:00:00 2001 From: Emma Anholt Date: Tue, 11 Feb 2025 09:31:28 -0800 Subject: [PATCH] mesa/ffvs: Skip doing redundant stores of .xyz when doing lighting calculation. Previously, we stored the full color output before lighting, then compute lighting and store just the .xyz of the result to .xyz. We can save followup optimization work to clean up the unused .w calculations during lighting, and DCEing the first .xyz store if we just store .w when it's done, and only do lighting on .xyz. Some of that redundant store work may not have been happening on all backends. Part-of: --- src/gallium/drivers/lima/ci/lima-fails.txt | 9 ------ src/mesa/main/ffvertex_prog.c | 35 ++++++++++++++-------- 2 files changed, 23 insertions(+), 21 deletions(-) diff --git a/src/gallium/drivers/lima/ci/lima-fails.txt b/src/gallium/drivers/lima/ci/lima-fails.txt index e07775e7334..cfcb9152c63 100644 --- a/src/gallium/drivers/lima/ci/lima-fails.txt +++ b/src/gallium/drivers/lima/ci/lima-fails.txt @@ -565,15 +565,6 @@ spec@!opengl 2.1@pbo@test_polygon_stip,Fail spec@!opengl 2.1@polygon-stipple-fs,Fail spec@!opengl es 2.0@glsl-fs-pointcoord,Fail -# Regression from https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/33053, maybe others -spec@!opengl 1.1@two-sided-lighting,Fail -spec@!opengl 1.0@gl-1.0-spot-light,Fail -spec@!opengl 1.1@infinite-spot-light,Fail -spec@!opengl 1.2@draw-elements-vs-inputs,Fail -spec@!opengl 1.2@two-sided-lighting-separate-specular,Fail -spec@!opengl 1.5@normal3b3s-invariance-byte,Fail -spec@!opengl 1.5@normal3b3s-invariance-short,Fail - # see https://gitlab.freedesktop.org/mesa/piglit/-/merge_requests/730 # and https://gitlab.freedesktop.org/mesa/mesa/-/issues/7208 spec@ext_texture_compression_s3tc@compressedteximage gl_compressed_rgb_s3tc_dxt1_ext,Fail diff --git a/src/mesa/main/ffvertex_prog.c b/src/mesa/main/ffvertex_prog.c index 2bb0543b119..f08942417fc 100644 --- a/src/mesa/main/ffvertex_prog.c +++ b/src/mesa/main/ffvertex_prog.c @@ -742,6 +742,25 @@ emit_degenerate_lit(nir_builder *b, return nir_vector_insert_imm(b, tmp, nir_slt(b, zero, dots_x), 2); } +/* Emits the store to a color output before lighting is computed. If there are + * no lights, we can store the full vec4 and be done. Otherwise, we store the + * alpha to the output .w channel, and reduce the color to a vec3 for the + * following RGB computation of the lit color. + */ +static void +store_output_pre_lighting(struct tnl_program *p, int nr_lights, nir_def **value, + gl_varying_slot slot) +{ + if (*value) { + if (nr_lights == 0) { + store_output_vec4(p, slot, *value); + } else { + uint32_t a[4] = {3, 3, 3, 3}; + store_output_vec4_masked(p, slot, nir_swizzle(p->b, *value, a, 4), 0x8); + *value = nir_channels(p->b, *value, 0xf); + } + } +} /* Need to add some addtional parameters to allow lighting in object * space - STATE_SPOT_DIRECTION and STATE_HALF_VECTOR implicitly assume eye @@ -801,18 +820,10 @@ static void build_lighting( struct tnl_program *p ) _bfc1 = nir_imm_vec4(p->b, 0.0f, 0.0f, 0.0f, 1.0f); } - /* If no lights, still need to emit the scenecolor. - */ - store_output_vec4(p, VARYING_SLOT_COL0, _col0); - - if (separate) - store_output_vec4(p, VARYING_SLOT_COL1, _col1); - - if (twoside) - store_output_vec4(p, VARYING_SLOT_BFC0, _bfc0); - - if (twoside && separate) - store_output_vec4(p, VARYING_SLOT_BFC1, _bfc1); + store_output_pre_lighting(p, nr_lights, &_col0, VARYING_SLOT_COL0); + store_output_pre_lighting(p, nr_lights, &_col1, VARYING_SLOT_COL1); + store_output_pre_lighting(p, nr_lights, &_bfc0, VARYING_SLOT_BFC0); + store_output_pre_lighting(p, nr_lights, &_bfc1, VARYING_SLOT_BFC1); if (nr_lights == 0) return;