From 1e0b64ced978a952bc6061cda3db2bdcfa0879d7 Mon Sep 17 00:00:00 2001 From: Brian Paul Date: Sat, 23 Dec 2017 14:16:52 -0700 Subject: [PATCH] svga: fix shadow comparison failures In some cases, We do shadow comparison cases in the fragment shader instead of with texture sampler state. But when we do so, we must disable the shadow comparison test in the sampler state. As it was, we were doing the comparison twice, which resulted in nonsense. Also, we had the texcoord and texel value swapped in the comparison instruction. Fixes about 38 Piglit tex-miplevel-selection tests. Reviewed-by: Neha Bhende Reviewed-by: Charmaine Lee --- src/gallium/drivers/svga/svga_context.h | 2 +- src/gallium/drivers/svga/svga_pipe_sampler.c | 74 ++++++++++++------- src/gallium/drivers/svga/svga_shader.h | 5 ++ src/gallium/drivers/svga/svga_state_sampler.c | 21 +++++- src/gallium/drivers/svga/svga_tgsi_vgpu10.c | 12 ++- 5 files changed, 78 insertions(+), 36 deletions(-) diff --git a/src/gallium/drivers/svga/svga_context.h b/src/gallium/drivers/svga/svga_context.h index 80c59c05e86..fd0c31222e6 100644 --- a/src/gallium/drivers/svga/svga_context.h +++ b/src/gallium/drivers/svga/svga_context.h @@ -211,7 +211,7 @@ struct svga_sampler_state { unsigned view_min_lod; unsigned view_max_lod; - SVGA3dSamplerId id; + SVGA3dSamplerId id[2]; }; diff --git a/src/gallium/drivers/svga/svga_pipe_sampler.c b/src/gallium/drivers/svga/svga_pipe_sampler.c index 2e98eb457e0..e9d1d6f4527 100644 --- a/src/gallium/drivers/svga/svga_pipe_sampler.c +++ b/src/gallium/drivers/svga/svga_pipe_sampler.c @@ -183,8 +183,6 @@ define_sampler_state_object(struct svga_context *svga, COPY_4V(bcolor.value, ps->border_color.f); - ss->id = util_bitmask_add(svga->sampler_object_id_bm); - assert(ps->min_lod <= ps->max_lod); if (ps->min_mip_filter == PIPE_TEX_MIPFILTER_NONE) { @@ -196,24 +194,41 @@ define_sampler_state_object(struct svga_context *svga, max_lod = ps->max_lod; } - /* Loop in case command buffer is full and we need to flush and retry */ - for (try = 0; try < 2; try++) { - enum pipe_error ret = - SVGA3D_vgpu10_DefineSamplerState(svga->swc, - ss->id, - filter, - ss->addressu, - ss->addressv, - ss->addressw, - ss->lod_bias, /* float */ - max_aniso, - compare_func, - bcolor, - min_lod, /* float */ - max_lod); /* float */ - if (ret == PIPE_OK) - return; - svga_context_flush(svga, NULL); + /* If shadow comparisons are enabled, create two sampler states: one + * with the given shadow compare mode, another with shadow comparison off. + * We need the later because in some cases, we have to do the shadow + * compare in the shader. So, we don't want to do it twice. + */ + STATIC_ASSERT(PIPE_TEX_COMPARE_NONE == 0); + STATIC_ASSERT(PIPE_TEX_COMPARE_R_TO_TEXTURE == 1); + ss->id[1] = SVGA3D_INVALID_ID; + + unsigned i; + for (i = 0; i <= ss->compare_mode; i++) { + ss->id[i] = util_bitmask_add(svga->sampler_object_id_bm); + + /* Loop in case command buffer is full and we need to flush and retry */ + for (try = 0; try < 2; try++) { + enum pipe_error ret = + SVGA3D_vgpu10_DefineSamplerState(svga->swc, + ss->id[i], + filter, + ss->addressu, + ss->addressv, + ss->addressw, + ss->lod_bias, /* float */ + max_aniso, + compare_func, + bcolor, + min_lod, /* float */ + max_lod); /* float */ + if (ret == PIPE_OK) + break; + svga_context_flush(svga, NULL); + } + + /* turn off the shadow compare option for second iteration */ + filter &= ~SVGA3D_FILTER_COMPARE; } } @@ -332,16 +347,21 @@ svga_delete_sampler_state(struct pipe_context *pipe, void *sampler) struct svga_context *svga = svga_context(pipe); if (svga_have_vgpu10(svga)) { - enum pipe_error ret; + unsigned i; + for (i = 0; i < 2; i++) { + enum pipe_error ret; - svga_hwtnl_flush_retry(svga); + if (ss->id[i] != SVGA3D_INVALID_ID) { + svga_hwtnl_flush_retry(svga); - ret = SVGA3D_vgpu10_DestroySamplerState(svga->swc, ss->id); - if (ret != PIPE_OK) { - svga_context_flush(svga, NULL); - ret = SVGA3D_vgpu10_DestroySamplerState(svga->swc, ss->id); + ret = SVGA3D_vgpu10_DestroySamplerState(svga->swc, ss->id[i]); + if (ret != PIPE_OK) { + svga_context_flush(svga, NULL); + ret = SVGA3D_vgpu10_DestroySamplerState(svga->swc, ss->id[i]); + } + util_bitmask_clear(svga->sampler_object_id_bm, ss->id[i]); + } } - util_bitmask_clear(svga->sampler_object_id_bm, ss->id); } FREE(sampler); diff --git a/src/gallium/drivers/svga/svga_shader.h b/src/gallium/drivers/svga/svga_shader.h index dc462c94af6..70d12469827 100644 --- a/src/gallium/drivers/svga/svga_shader.h +++ b/src/gallium/drivers/svga/svga_shader.h @@ -158,6 +158,11 @@ struct svga_shader_variant /** Is the color output just a constant value? (fragment shader only) */ boolean constant_color_output; + /** Bitmask indicating which texture units are doing the shadow + * comparison test in the shader rather than the sampler state. + */ + unsigned fs_shadow_compare_units; + /** For FS-based polygon stipple */ unsigned pstipple_sampler_unit; diff --git a/src/gallium/drivers/svga/svga_state_sampler.c b/src/gallium/drivers/svga/svga_state_sampler.c index 763437d9b87..9bd0d5303bd 100644 --- a/src/gallium/drivers/svga/svga_state_sampler.c +++ b/src/gallium/drivers/svga/svga_state_sampler.c @@ -391,8 +391,21 @@ update_samplers(struct svga_context *svga, unsigned dirty ) unsigned nsamplers; for (i = 0; i < count; i++) { + bool fs_shadow = false; + + if (shader == PIPE_SHADER_FRAGMENT) { + struct svga_shader_variant *fs = svga->state.hw_draw.fs; + /* If the fragment shader is doing the shadow comparison + * for this texture unit, don't enable shadow compare in + * the texture sampler state. + */ + if (fs->fs_shadow_compare_units & (1 << i)) { + fs_shadow = true; + } + } + if (svga->curr.sampler[shader][i]) { - ids[i] = svga->curr.sampler[shader][i]->id; + ids[i] = svga->curr.sampler[shader][i]->id[fs_shadow]; assert(ids[i] != SVGA3D_INVALID_ID); } else { @@ -435,18 +448,18 @@ update_samplers(struct svga_context *svga, unsigned dirty ) } if (svga->state.hw_draw.samplers[PIPE_SHADER_FRAGMENT][unit] - != sampler->id) { + != sampler->id[0]) { ret = SVGA3D_vgpu10_SetSamplers(svga->swc, 1, /* count */ unit, /* start */ SVGA3D_SHADERTYPE_PS, - &sampler->id); + &sampler->id[0]); if (ret != PIPE_OK) return ret; /* save the polygon stipple sampler in the hw draw state */ svga->state.hw_draw.samplers[PIPE_SHADER_FRAGMENT][unit] = - sampler->id; + sampler->id[0]; } } diff --git a/src/gallium/drivers/svga/svga_tgsi_vgpu10.c b/src/gallium/drivers/svga/svga_tgsi_vgpu10.c index c718eae8f3a..deb8e5a1eff 100644 --- a/src/gallium/drivers/svga/svga_tgsi_vgpu10.c +++ b/src/gallium/drivers/svga/svga_tgsi_vgpu10.c @@ -181,6 +181,9 @@ struct svga_shader_emitter_v10 unsigned fragcoord_input_index; /**< real fragment position input reg */ unsigned fragcoord_tmp_index; /**< 1/w modified position temp reg */ + + /** Which texture units are doing shadow comparison in the FS code */ + unsigned shadow_compare_units; } fs; /* For geometry shaders only */ @@ -4851,6 +4854,8 @@ begin_tex_swizzle(struct svga_shader_emitter_v10 *emit, } swz->inst_dst = &inst->Dst[0]; swz->coord_src = &inst->Src[0]; + + emit->fs.shadow_compare_units |= shadow_compare << unit; } @@ -4909,11 +4914,8 @@ end_tex_swizzle(struct svga_shader_emitter_v10 *emit, } /* COMPARE tmp, coord, texel */ - /* XXX it would seem that the texel and coord arguments should - * be transposed here, but piglit tests indicate otherwise. - */ emit_comparison(emit, compare_func, - &swz->tmp_dst, &texel_src, &coord_src); + &swz->tmp_dst, &coord_src, &texel_src); /* AND dest, tmp, {1.0} */ begin_emit_instruction(emit); @@ -6565,6 +6567,8 @@ svga_tgsi_vgpu10_translate(struct svga_context *svga, } } + variant->fs_shadow_compare_units = emit->fs.shadow_compare_units; + if (SVGA_DEBUG & DEBUG_TGSI) { debug_printf("#####################################\n"); debug_printf("### TGSI Shader %u\n", shader->id); -- 2.30.2