[Mesa-dev] [PATCH] i965: Advertise 8 bits subpixel precision for viewport bounds on gen6+

[Lionel Landwerlin]

Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>

Do you need me to push it?

Thanks,

-
Lionel

On 08/08/18 16:03, Danylo Piliaiev wrote:
> Hi,
>
> Since the exact value is not important for users,  Roland offered 
> compelling explanation
> and the value is also 8 on Windows - could the patch be reviewed/pushed?
>
> - Danil
>
> On 06/19/2018 03:44 PM, Roland Scheidegger wrote:
>> My guess would be 8 because that's what the rasterization subpixel
>> precision is, so precision beyond that doesn't really do much, even if
>> this actually is a float.
>> Plus, with maximum sized fb (16kx16k dimension) you don't actually
>> really get a lot more than 8 fixed points bits anyway (near those 16k).
>> So imho 8 makes most sense.
>>
>> Roland
>>
>>
>> Am 19.06.2018 um 10:35 schrieb danylo:
>>> Hi Lionel,
>>>
>>> Indeed the value 8 here is questionable. I picked it because other
>>> drivers advertise the same value e.g. in Mesa radeon returns 8 for gl
>>> and vulkan or on Windows Intel's driver returns 8. But why 8? It's some
>>> kind of mystery.
>>>
>>>> "If the implementation truely has floating point viewport bounds, it
>>>> may report a sufficiently high value to indicate this. "
>>> 8 seems to be a sufficiently high value (it seems if someone even 
>>> checks
>>> the value it's going like 'precision > 0' - it is used as a flag). But
>>> still it's probably not good enough argument...
>>>
>>> Floating point (IEEE 754) has 24 bits of significand precision, in 
>>> other
>>> way - 6 to 9 significant decimal digits. And drivers return 8, the only
>>> 8 in float-point is 8 exponent bits.
>>>
>>> Unless someone knows why 8, there two paths:
>>>
>>>    * Left it to be 8 - be the same as other drivers
>>>    * Make 24 - to reflect 24 bits of significand precision of float
>>>
>>>
>>> - Danil
>>>
>>>
>>> On 18.06.18 17:27, Lionel Landwerlin wrote:
>>>> Hey Danylo,
>>>>
>>>> Thanks for this patch.
>>>> I'm not really an expert here but my understanding is that it should
>>>> reflect the number of bits in fixed point precision.
>>>> We use 32bits floats in the packets sent to the hardware.
>>>> Quoting the spec :
>>>>
>>>> "If the implementation truely has floating point viewport bounds, it
>>>> may report a sufficiently high value to indicate this. "
>>>>
>>>> Maybe we should use something a bit bigger than 8?
>>>>
>>>> Cheers,
>>>>
>>>> -
>>>> Lionel
>>>>
>>>> On 18/06/18 13:50, Danylo Piliaiev wrote:
>>>>> We use floating-points for viewport bounds so VIEWPORT_SUBPIXEL_BITS
>>>>> should reflect this.
>>>>>
>>>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105975
>>>>>
>>>>> Signed-off-by: Danylo Piliaiev <danylo.piliaiev at globallogic.com>
>>>>> ---
>>>>>   src/mesa/drivers/dri/i965/brw_context.c | 2 +-
>>>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/src/mesa/drivers/dri/i965/brw_context.c 
>>>>> b/src/mesa/drivers/dri/i965/brw_context.c
>>>>> index 9ced230..eacf326 100644
>>>>> --- a/src/mesa/drivers/dri/i965/brw_context.c
>>>>> +++ b/src/mesa/drivers/dri/i965/brw_context.c
>>>>> @@ -688,7 +688,7 @@ brw_initialize_context_constants(struct 
>>>>> brw_context *brw)
>>>>>      /* ARB_viewport_array, OES_viewport_array */
>>>>>      if (devinfo->gen >= 6) {
>>>>>         ctx->Const.MaxViewports = GEN6_NUM_VIEWPORTS;
>>>>> -      ctx->Const.ViewportSubpixelBits = 0;
>>>>> +      ctx->Const.ViewportSubpixelBits = 8;
>>>>>           /* Cast to float before negating because 
>>>>> MaxViewportWidth is unsigned.
>>>>>          */
>>>>
>>>
>>>
>>> _______________________________________________
>>> mesa-dev mailing list
>>> mesa-dev at lists.freedesktop.org
>>> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fmesa-dev&data=02%7C01%7Csroland%40vmware.com%7Ce1ac7ef5378f4227aff508d5d5bfa182%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C1%7C0%7C636649941389721543&sdata=of%2FIoIzosaje7A3euJESjJMySE3eFjNifMg5SXzeEJU%3D&reserved=0 
>>>
>>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev