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

[Brian Paul]

On 12/19/2014 02:47 PM, Rob Clark wrote:
> 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 ;-)

Hi Rob,

We've implemented a few things like this in our driver (in-house).  How 
about passing the desired location of the alpha ref constant as a 
parameter in the tgsi_lowering_config?  So, you'd be specifying three 
pieces of info:
1. lower_alpha_test flag
2. the comparison function
3. the constant slot to find the reference value.

It's often the case (at least for us) that we have to construct several 
extra constants (ex: alpha ref, texture dimensions, window height) for 
the fragment shader and the driver needs to specify which constant 
locations contain which extra values.

It seems kind of odd to me to add a new semantic just for this.

-Brian