Looks good to me. Thanks for catching this! Reviewed-by: Jason Ekstrand <ja...@jlekstrand.net> Fixes: 1e5b09f42f694687ac "spirv: Tidy some repeated if checks..."
On Wed, May 2, 2018 at 10:49 AM, Neil Roberts <nrobe...@igalia.com> wrote: > This behaviour was changed in 1e5b09f42f694687ac. The commit message > for that says it is just a “tidy up” so my assumption is that the > behaviour change was a mistake. It’s a little hard to decipher looking > at the diff, but the previous code before that patch was: > > if (builtin == SpvBuiltInFragCoord || builtin == > SpvBuiltInSamplePosition) > nir_var->data.origin_upper_left = b->origin_upper_left; > > if (builtin == SpvBuiltInFragCoord) > nir_var->data.pixel_center_integer = b->pixel_center_integer; > > After the patch the code was: > > case SpvBuiltInSamplePosition: > nir_var->data.origin_upper_left = b->origin_upper_left; > /* fallthrough */ > case SpvBuiltInFragCoord: > nir_var->data.pixel_center_integer = b->pixel_center_integer; > break; > > Before the patch origin_upper_left affected both builtins and > pixel_center_integer only affected FragCoord. After the patch > origin_upper_left only affects SamplePosition and pixel_center_integer > affects both variables. > > This patch tries to restore the previous behaviour by changing the > code to: > > case SpvBuiltInFragCoord: > nir_var->data.pixel_center_integer = b->pixel_center_integer; > /* fallthrough */ > case SpvBuiltInSamplePosition: > nir_var->data.origin_upper_left = b->origin_upper_left; > break; > > This change will be important for ARB_gl_spirv which is meant to > support OriginLowerLeft. > --- > src/compiler/spirv/vtn_variables.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/src/compiler/spirv/vtn_variables.c b/src/compiler/spirv/vtn_ > variables.c > index 9679ff6526c..fd8ab7f247a 100644 > --- a/src/compiler/spirv/vtn_variables.c > +++ b/src/compiler/spirv/vtn_variables.c > @@ -1419,11 +1419,11 @@ apply_var_decoration(struct vtn_builder *b, > nir_variable *nir_var, > case SpvBuiltInTessLevelInner: > nir_var->data.compact = true; > break; > - case SpvBuiltInSamplePosition: > - nir_var->data.origin_upper_left = b->origin_upper_left; > - /* fallthrough */ > case SpvBuiltInFragCoord: > nir_var->data.pixel_center_integer = b->pixel_center_integer; > + /* fallthrough */ > + case SpvBuiltInSamplePosition: > + nir_var->data.origin_upper_left = b->origin_upper_left; > break; > default: > break; > -- > 2.14.3 > > _______________________________________________ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev >
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
