[Mesa-dev] [PATCH] llvmpipe: abstract the code to set number of subpixel bits

[Brian Paul]

On 10/08/2013 02:59 PM, Zack Rusin wrote:
> As we're moving towards expanding the number of subpixel
> bits and the width of the variables used in the computations
> we need to make this code a bit more centralized.
>
> Signed-off-by: Zack Rusin <zackr at vmware.com>
> ---
>   src/gallium/drivers/llvmpipe/lp_rast.h      |  9 +++++++++
>   src/gallium/drivers/llvmpipe/lp_setup.c     | 14 +++++---------
>   src/gallium/drivers/llvmpipe/lp_setup_tri.c |  2 +-
>   3 files changed, 15 insertions(+), 10 deletions(-)
>
> diff --git a/src/gallium/drivers/llvmpipe/lp_rast.h b/src/gallium/drivers/llvmpipe/lp_rast.h
> index c57f2ea..43c598d 100644
> --- a/src/gallium/drivers/llvmpipe/lp_rast.h
> +++ b/src/gallium/drivers/llvmpipe/lp_rast.h
> @@ -46,9 +46,18 @@ struct lp_scene;
>   struct lp_fence;
>   struct cmd_bin;
>
> +#define FIXED_TYPE_WIDTH 32
>   /** For sub-pixel positioning */
>   #define FIXED_ORDER 4
>   #define FIXED_ONE (1<<FIXED_ORDER)
> +/** Maximum length of an edge in a primitive in pixels.
> + *  If the framebuffer is large we have to think about fixed-point
> + *  integer overflow. Coordinates need ((FIXED_TYPE_WIDTH/2) - 1) bits
> + *  to be able to fit product of two such coordinates inside
> + *  FIXED_TYPE_WIDTH, any larger and we could overflow a
> + *  FIXED_TYPE_WIDTH_-bit int.
> + */
> +#define MAX_FIXED_LENGTH (1 << (((FIXED_TYPE_WIDTH/2) - 1) - FIXED_ORDER))
>
>   /* Rasterizer output size going to jit fs, width/height */
>   #define LP_RASTER_BLOCK_SIZE 4
> diff --git a/src/gallium/drivers/llvmpipe/lp_setup.c b/src/gallium/drivers/llvmpipe/lp_setup.c
> index c8199b4..9b277d3 100644
> --- a/src/gallium/drivers/llvmpipe/lp_setup.c
> +++ b/src/gallium/drivers/llvmpipe/lp_setup.c
> @@ -1007,16 +1007,12 @@ try_update_scene_state( struct lp_setup_context *setup )
>                                            &setup->draw_regions[i]);
>            }
>         }
> -      /* If the framebuffer is large we have to think about fixed-point
> -       * integer overflow.  For 2K by 2K images, coordinates need 15 bits
> -       * (2^11 + 4 subpixel bits).  The product of two such numbers would
> -       * use 30 bits.  Any larger and we could overflow a 32-bit int.
> -       *
> -       * To cope with this problem we check if triangles are large and
> -       * subdivide them if needed.
> +      /*
> +       * Subdivide triangles if the framebuffer is larger than the
> +       * MAX_FIXED_LENGTH.
>          */
> -      setup->subdivide_large_triangles = (setup->fb.width > 2048 ||
> -                                          setup->fb.height > 2048);
> +      setup->subdivide_large_triangles = (setup->fb.width > MAX_FIXED_LENGTH ||
> +                                          setup->fb.height > MAX_FIXED_LENGTH);
>      }
>
>      setup->dirty = 0;
> diff --git a/src/gallium/drivers/llvmpipe/lp_setup_tri.c b/src/gallium/drivers/llvmpipe/lp_setup_tri.c
> index 051ffa0..9cc81e9 100644
> --- a/src/gallium/drivers/llvmpipe/lp_setup_tri.c
> +++ b/src/gallium/drivers/llvmpipe/lp_setup_tri.c
> @@ -988,7 +988,7 @@ check_subdivide_triangle(struct lp_setup_context *setup,
>                            const float (*v2)[4],
>                            triangle_func_t tri)
>   {
> -   const float maxLen = 2048.0f;  /* longest permissible edge, in pixels */
> +   const float maxLen = MAX_FIXED_LENGTH;  /* longest permissible edge, in pixels */

To avoid a lame MSVC warning, I think that line needs a cast:

    const float maxLen = (float) MAX_FIXED_LENGTH;  /* longest 
permissible edge, in pixels */


>      float dx10, dy10, len10;
>      float dx21, dy21, len21;
>      float dx02, dy02, len02;
>

Reviewed-by: Brian Paul <brianp at vmware.com>