[Mesa-dev] [PATCH] tgsi: lowering support for alpha test

[Rob Clark]

On Fri, Dec 19, 2014 at 4:26 PM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
> On Fri, Dec 19, 2014 at 4:04 PM, Rob Clark <robdclark at gmail.com> wrote:
>> On Fri, Dec 19, 2014 at 3:50 PM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
>>> On Fri, Dec 19, 2014 at 2:11 PM, Rob Clark <robdclark at gmail.com> wrote:
>>>> From: Rob Clark <robclark at freedesktop.org>
>>>>
>>>> This emulates alpha-test with a compare + KILL_IF.  The alpha-ref value
>>>> is passed to the shader via constant tagged with new ALPHAREF semantic.
>>>> For example:
>>>>
>>>>   FRAG
>>>>   PROPERTY FS_COLOR0_WRITES_ALL_CBUFS 1
>>>>   DCL IN[0], COLOR, COLOR
>>>>   DCL OUT[0], COLOR
>>>>     0: MOV OUT[0], IN[0]
>>>>     1: END
>>>>
>>>> with alpha-func PIPE_FUNC_LESS, becomes:
>>>>
>>>>   FRAG
>>>>   PROPERTY FS_COLOR0_WRITES_ALL_CBUFS 1
>>>>   DCL IN[0], COLOR, COLOR
>>>>   DCL OUT[0], COLOR
>>>>   IMM[0] FLT32 {    0.0000,     1.0000,   128.0000,     0.0000}
>>>>   DCL TEMP[0]
>>>>   DCL TEMP[1]
>>>>   DCL CONST[0], ALPHAREF
>>>
>>> IMO this is pretty confusing. A SV makes more sense, no? Otherwise
>>> you'd have a situation where
>>>
>>> DCL CONST[0]
>>> DCL CONST[1]
>>> DCL CONST[2], ALPHAREF
>>>
>>> MOV TEMP[0], CONST[ADDR[0].x]
>>>
>>> Could potentially be expected to move the alpharef into the temp,
>>> which is... odd. Also I don't think there's any precendent for tagging
>>> const's with semantics.
>>
>> it is a bit odd, but isn't an out of bounds access undefined anyways?
>
> With ARB_robust_buffer_access, it's defined as either 0 or some value
> in the constbuf... or some wishy-washy thing like that.

well, that won't work without bounds checking in the shader.. but
unless there are as many piglit tests as there are for
bordercolor+GL_WRAP I'd be inclined just to say: be happy you can play
your gl game on gles hw :-P

>>
>> And yes, I don't think there is any precedent for tagging const's w/
>> semantic, it was a rather logical choice..  ie. if you are lowering
>> alpha-test, that means you don't have some special register/etc to
>> store alpharef, therefore driver is going to treat it as a normal
>> const.  It just needs some way to know where to stuff the const val.
>> Treating it as something other than a const didn't seem to make much
>> sense from the perspective of the consumer of the resulting tgsi.
>
> But it does. What if CONST[0] is a UBO (although in that case it may
> actually end up as CONST[0][0] or something) -- are you going to
> modify the buffer data there? These are "system" values, not user
> values. I think they make more sense as SV's... for example all the
> sample position things are stored in a driver constbuf on nouveau, but
> are exposed as SV's -- it doesn't always have to be a register.
>

why would a CONST inserted by tgsi lower pass be in a UBO

>>
>>> The other thing to consider is whether each of these things should be
>>> semantics, or whether there should be a
>>>
>>> TGSI_SEMANTIC_DRIVER
>>>
>>> Which is then indexed in a custom way (in this case, however
>>> tgsi_lowering decides). Many drivers end up with driver-internal
>>> constbufs full of convenient values, this would be a way to represent
>>> that.
>>
>> I thought about it.. but not like we are running out of room in
>> semantic namespace, and it was a very straightforward way to hook this
>> all up (including tgsi_dump() and tgsi_text_translate())
>
> OK. I guess the main thing is that some driver author may be fooled
> into thinking that they should be implementing this somehow when they
> totally don't have to. However documentation can solve that.
>

yeah, since it is just intended as a way to lower alphatest (and I
suspect Eric may want to do a similar thing for window w/h to deal w/
RECT textures), we can just define that it will only appear as a
non-UBO CONST, etc..

at the end of the day, I can't dream up any hw that would need to
lower this sort of thing yet wants the injected constants to be
anything other than a CONST ;-)

BR,
-R

>   -ilia