On Thu, 2017-06-08 at 18:25 +0200, Lucas Stach wrote:
> It seems that newer cores don't use the PA_ATTRIBUTES to decide if the
> varying should bypass the flat shading, but derive this from the component
> use. This fixes flat shading on GC880+.
>
> VARYING_COMPONENT_USE_POINTCOORD is a bit of a misnomer now, as it isn't
> only used for pointcoords, but missing a better name I left it as-is.
>
> Signed-off-by: Lucas Stach <l.st...@pengutronix.de>
>
> ---
> src/gallium/drivers/etnaviv/etnaviv_compiler.c | 24 ++++++++++--------------
> 1 file changed, 10 insertions(+), 14 deletions(-)
>
> diff --git a/src/gallium/drivers/etnaviv/etnaviv_compiler.c
> b/src/gallium/drivers/etnaviv/etnaviv_compiler.c
> index eafb511bb813..2605924613c7 100644
> --- a/src/gallium/drivers/etnaviv/etnaviv_compiler.c
> +++ b/src/gallium/drivers/etnaviv/etnaviv_compiler.c
> @@ -2552,6 +2552,7 @@ etna_link_shader(struct etna_shader_link_info *info,
> const struct etna_shader_inout *fsio = &fs->infile.reg[idx];
> const struct etna_shader_inout *vsio = etna_shader_vs_lookup(vs, fsio);
> struct etna_varying *varying;
> + bool interpolate = fsio->semantic.Name != TGSI_SEMANTIC_COLOR;
>
> assert(fsio->reg > 0 && fsio->reg <= ARRAY_SIZE(info->varyings));
>
> @@ -2561,28 +2562,23 @@ etna_link_shader(struct etna_shader_link_info *info,
> varying = &info->varyings[fsio->reg - 1];
> varying->num_components = fsio->num_components;
>
> - if (fsio->semantic.Name == TGSI_SEMANTIC_COLOR) /* colors affected by
> flat shading */
> + if (!interpolate) /* colors affected by flat shading */
> varying->pa_attributes = 0x200;
> else /* texture coord or other bypasses flat shading */
> varying->pa_attributes = 0x2f1;
>
> - if (fsio->semantic.Name == TGSI_SEMANTIC_PCOORD) {
> - varying->use[0] = VARYING_COMPONENT_USE_POINTCOORD_X;
> - varying->use[1] = VARYING_COMPONENT_USE_POINTCOORD_Y;
> - varying->use[2] = VARYING_COMPONENT_USE_USED;
> - varying->use[3] = VARYING_COMPONENT_USE_USED;
> - varying->reg = 0; /* replaced by point coord -- doesn't matter */
> + varying->use[0] = interpolate ? VARYING_COMPONENT_USE_POINTCOORD_X :
> VARYING_COMPONENT_USE_USED;
> + varying->use[1] = interpolate ? VARYING_COMPONENT_USE_POINTCOORD_Y :
> VARYING_COMPONENT_USE_USED;
> + varying->use[2] = VARYING_COMPONENT_USE_USED;
> + varying->use[3] = VARYING_COMPONENT_USE_USED;
This only changins varying->use[0,1] to POINTCOORD in the
non-SEMANTIC_COLOR, non-SEMANTIC_PCOORD case, which is what the patch is
about.
> + varying->reg = vsio->reg; /* replaced by point coord -- doesn't matter
> */
This changes varying->vreg in the SEMANTIC_PCOORD case. As the comment
says, it shouldn't matter. But this also dereferences the vsio pointer,
which according to the check below could be NULL.
> +
> +
> + if (fsio->semantic.Name == TGSI_SEMANTIC_PCOORD)
> continue;
> - }
>
> if (vsio == NULL)
> return true; /* not found -- link error */
regards
Philipp
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
