Gradients are broken with glamor when RepeatReflect is set

[Chris Wilson]

Quoting Chris Wilson (2018-01-23 15:26:50)
> Quoting Jeffrey Smith (2018-01-23 15:15:10)
> > On Mon, Jan 22, 2018 at 3:01 PM, Chris Wilson <chris at chris-wilson.co.uk> wrote:
> > > Quoting Adam Jackson (2018-01-22 20:09:52)
> > >> On Sat, 2017-12-23 at 19:26 +0100, Clemens Eisserer wrote:
> > >> > Hi there,
> > >> >
> > >> > Glamor's gradient acceleration code is broken in case RepeatReflect is
> > >> > used, please see: https://bugs.freedesktop.org/show_bug.cgi?id=98508
> > >> > I've filed the bug report over a year ago, but except for a
> > >> > confirmation from Michel Dänzer nothing happend.
> > >> >
> > >> > Unfourntunatly I lack the expertise to fix it myself - however instead
> > >> > of leaving it broken forever, could we fall back to software for
> > >> > RepeatReflect.
> > >> > I guess slow is better than completly broken?
> > >>
> > >> Just want to note that this isn't forgotten. I got as far as testing
> > >> the reproducer with Xephyr and verifying glamor was wrong and fb was
> > >> right, but don't yet get what the RepeatReflect math is getting wrong.
> > >> I'll definitely have a fix for 1.20 one way or another, but that may
> > >> just be forcing a fallback.
> > >>
> > >> If anyone wanted to investigate this, I think this is the guilty
> > >> conditional:
> > >>
> > >> https://cgit.freedesktop.org/xorg/xserver/tree/glamor/glamor_gradient.c#n296
> > >
> > > -t = abs(fract(t * 0.5 + 0.5) * 2.0 - 1.0);
> > > +t = abs(fract(abs(t) * 0.5 + 0.5) * 2.0 - 1.0);
> > 
> > Chris, where did this definition for fract come from?
> 
> Naivety.
> 
> > For negative numbers, it does not match the OpenGL definition for
> > fract, which would be
> > #define fract(t) ((t) - floor(t))
> > With fract defined thus, the original transformation of t appears to work fine.
> 
> nir also uses the same definition:
> operation("fract", 1, source_types=real_types, c_expression={'f':
> "{src0} - floorf({src0})", 'd': "{src0} - floor({src0})"}),
> 
> So maybe it's purely an amdgpu compiler issue? Michel?

static LLVMValueRef emit_ffract(struct ac_llvm_context *ctx,
                                LLVMValueRef src0)
{
        const char *intr = "llvm.floor.f32";
        LLVMValueRef fsrc0 = ac_to_float(ctx, src0);
        LLVMValueRef params[] = {
                fsrc0,
        };
        LLVMValueRef floor = ac_build_intrinsic(ctx, intr,
                                                ctx->f32, params, 1,
                                                AC_FUNC_ATTR_READNONE);
        return LLVMBuildFSub(ctx->builder, fsrc0, floor, "");
}

which looks like it should be correct?
-Chris