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

[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.
>>          */
>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180619/e6eb83b9/attachment-0001.html>