[Mesa-dev] [PATCH 1/2] radeonsi: simplify gl_FragCoord behavior

[Nicolai Hähnle]

On 08.01.2016 06:48, Marek Olšák wrote:
> On Fri, Jan 8, 2016 at 12:35 PM, Axel Davy <axel.davy at ens.fr> wrote:
>> On 08/01/2016 02:30, Marek Olšák wrote:
>>>
>>> From: Marek Olšák <marek.olsak at amd.com>
>>>
>>> It will become a system value, not an input.
>>> ---
>>>    src/gallium/drivers/radeonsi/si_state_shaders.c | 45
>>> ++++++++++++-------------
>>>    1 file changed, 22 insertions(+), 23 deletions(-)
>>>
>>> diff --git a/src/gallium/drivers/radeonsi/si_state_shaders.c
>>> b/src/gallium/drivers/radeonsi/si_state_shaders.c
>>> index 64adf69..460dda5 100644
>>> --- a/src/gallium/drivers/radeonsi/si_state_shaders.c
>>> +++ b/src/gallium/drivers/radeonsi/si_state_shaders.c
>>> @@ -399,30 +399,29 @@ static void si_shader_ps(struct si_shader *shader)
>>>          if (!pm4)
>>>                  return;
>>>    -     for (i = 0; i < info->num_inputs; i++) {
>>> -               switch (info->input_semantic_name[i]) {
>>> -               case TGSI_SEMANTIC_POSITION:
>>> -                       /* SPI_BARYC_CNTL.POS_FLOAT_LOCATION
>>> -                        * Possible vaules:
>>> -                        * 0 -> Position = pixel center (default)
>>> -                        * 1 -> Position = pixel centroid
>>> -                        * 2 -> Position = at sample position
>>> -                        */
>>> -                       switch (info->input_interpolate_loc[i]) {
>>> -                       case TGSI_INTERPOLATE_LOC_CENTROID:
>>> -                               spi_baryc_cntl |=
>>> S_0286E0_POS_FLOAT_LOCATION(1);
>>> -                               break;
>>> -                       case TGSI_INTERPOLATE_LOC_SAMPLE:
>>> -                               spi_baryc_cntl |=
>>> S_0286E0_POS_FLOAT_LOCATION(2);
>>> -                               break;
>>> -                       }
>>> +       /* SPI_BARYC_CNTL.POS_FLOAT_LOCATION
>>> +        * Possible vaules:
>>> +        * 0 -> Position = pixel center
>>> +        * 1 -> Position = pixel centroid
>>> +        * 2 -> Position = at sample position
>>> +        *
>>> +        * From GLSL 4.5 specification, section 7.1:
>>> +        *   "The variable gl_FragCoord is available as an input variable
>>> from
>>> +        *    within fragment shaders and it holds the window relative
>>> coordinates
>>> +        *    (x, y, z, 1/w) values for the fragment. If multi-sampling,
>>> this
>>> +        *    value can be for any location within the pixel, or one of
>>> the
>>> +        *    fragment samples. The use of centroid does not further
>>> restrict
>>> +        *    this value to be inside the current primitive."
>>> +        *
>>> +        * Meaning that centroid has no effect and we can return anything
>>> within
>>> +        * the pixel. Thus, return the value at sample position, because
>>> that's
>>> +        * the most accurate one shaders can get.
>>> +        */
>>> +       spi_baryc_cntl |= S_0286E0_POS_FLOAT_LOCATION(2);
>>
>>
>> When combined with POS_FLOAT_ULC, in which situation could we see
>> a difference in the POSITION value ?
>
> No MSAA: the value is at the pixel center
> MSAA, no sample shading: the value is at the sample position of any
> visible sample (compatible with "centroid")
> MSAA sample shading: the value is at the sample position of the computed sample
>
> As far as I know, POS_FLOAT_ULC only offsets the value by a constant (0.5).

According to the docs it forces the position into the upper-left corner, 
AFAICT overriding the choice of POS_FLOAT_LOCATION. But that's exactly 
what we want here, isn't it?

Cheers,
Nicolai

> Marek
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>