Reviewed-by: Lionel Landwerlin <lionel.g.landwer...@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.pilia...@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@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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to