[Mesa-dev] [PATCH] llvmpipe: Honour pipe_rasterizer::point_quad_rasterization.

[Roland Scheidegger]

Am 08.01.2014 19:48, schrieb Marek Olšák:
> On Wed, Jan 8, 2014 at 7:31 PM, Roland Scheidegger <sroland at vmware.com> wrote:
>> Am 08.01.2014 19:12, schrieb Jose Fonseca:
>>>
>>>
>>> ----- Original Message -----
>>>> Am 08.01.2014 18:27, schrieb jfonseca at vmware.com:
>>>>> From: José Fonseca <jfonseca at vmware.com>
>>>>>
>>>>> Commit eda21d2a3010d9fc5a68b55a843c5e44b2abf8dd fixed the rasterization
>>>>> of points for Direct3D but ended up breaking the rasterization of OpenGL
>>>>> non-sprite points, in particular conform's pntrast.c test.
>>>>>
>>>>> The only way to get both working is to properly honour
>>>>> pipe_rasterizer::point_quad_rasterization, and follow the weird OpenGL
>>>>> rule when it is false.
>>>>> ---
>>>>>  src/gallium/drivers/llvmpipe/lp_setup_point.c | 64
>>>>>  ++++++++++++++++++++++-----
>>>>>  1 file changed, 54 insertions(+), 10 deletions(-)
>>>>>
>>>>> diff --git a/src/gallium/drivers/llvmpipe/lp_setup_point.c
>>>>> b/src/gallium/drivers/llvmpipe/lp_setup_point.c
>>>>> index 988e0c5..e5ce4ee 100644
>>>>> --- a/src/gallium/drivers/llvmpipe/lp_setup_point.c
>>>>> +++ b/src/gallium/drivers/llvmpipe/lp_setup_point.c
>>>>> @@ -338,9 +338,13 @@ try_setup_point( struct lp_setup_context *setup,
>>>>>     int fixed_width = MAX2(FIXED_ONE,
>>>>>                            (subpixel_snap(size) + FIXED_ONE/2 - 1) &
>>>>>                            ~(FIXED_ONE-1));
>>>>>
>>>>> -   const int x0 = subpixel_snap(v0[0][0] - setup->pixel_offset) -
>>>>> fixed_width/2;
>>>>> -   const int y0 = subpixel_snap(v0[0][1] - setup->pixel_offset) -
>>>>> fixed_width/2;
>>>>> -
>>>>> +   /* Yes this is necessary to accurately calculate bounding boxes
>>>>> +    * with the two fill-conventions we support.  GL (normally) ends
>>>>> +    * up needing a bottom-left fill convention, which requires
>>>>> +    * slightly different rounding.
>>>>> +    */
>>>>> +   int adj = (setup->bottom_edge_rule != 0) ? 1 : 0;
>>>>> +
>>>>>     struct lp_scene *scene = setup->scene;
>>>>>     struct lp_rast_triangle *point;
>>>>>     unsigned bytes;
>>>>> @@ -363,13 +367,14 @@ try_setup_point( struct lp_setup_context *setup,
>>>>>        print_point(setup, v0);
>>>>>
>>>>>     /* Bounding rectangle (in pixels) */
>>>>> -   {
>>>>> -      /* Yes this is necessary to accurately calculate bounding boxes
>>>>> -       * with the two fill-conventions we support.  GL (normally) ends
>>>>> -       * up needing a bottom-left fill convention, which requires
>>>>> -       * slightly different rounding.
>>>>> +   if (!lp_context->rasterizer ||
>>>>> +       lp_context->rasterizer->point_quad_rasterization) {
>>>>> +      /*
>>>>> +       * Rasterize points as quads.
>>>>>         */
>>>>> -      int adj = (setup->bottom_edge_rule != 0) ? 1 : 0;
>>>>> +
>>>>> +      const int x0 = subpixel_snap(v0[0][0] - setup->pixel_offset) -
>>>>> fixed_width/2;
>>>>> +      const int y0 = subpixel_snap(v0[0][1] - setup->pixel_offset) -
>>>>> fixed_width/2;
>>>>>
>>>>>        bbox.x0 = (x0 + (FIXED_ONE-1)) >> FIXED_ORDER;
>>>>>        bbox.x1 = (x0 + fixed_width + (FIXED_ONE-1)) >> FIXED_ORDER;
>>>>> @@ -380,8 +385,47 @@ try_setup_point( struct lp_setup_context *setup,
>>>>>         */
>>>>>        bbox.x1--;
>>>>>        bbox.y1--;
>>>>> +   } else {
>>>>> +      /*
>>>>> +       * OpenGL rasterization rules for non-sprite points.
>>>> Maybe add "legacy" here before OpenGL?
>>>
>>> Sure.
>>>
>>>>> +       *
>>>>> +       * Per OpenGL 2.1 spec, section 3.3.1, "Basic Point Rasterization".
>>>>> +       */
>>>>> +
>>>>> +      const int x0 = subpixel_snap(v0[0][0]);
>>>>> +      const int y0 = subpixel_snap(v0[0][1]) - adj;
>>>>> +
>>>>> +      int int_width = fixed_width >> FIXED_ORDER;
>>>>> +
>>>>> +      assert(setup->pixel_offset != 0);
>>>>> +
>>>>> +      if (int_width == 1) {
>>>>> +         bbox.x0 = x0 >> FIXED_ORDER;
>>>>> +         bbox.y0 = y0 >> FIXED_ORDER;
>>>>> +         bbox.x1 = bbox.x0;
>>>>> +         bbox.y1 = bbox.y0;
>>>>> +      } else {
>>>>> +         if (int_width & 1) {
>>>>> +            /* Odd width */
>>>>> +            bbox.x0 = (x0 >> FIXED_ORDER) - (int_width - 1)/2;
>>>>> +            bbox.y0 = (y0 >> FIXED_ORDER) - (int_width - 1)/2;
>>>>> +         } else {
>>>>> +            /* Even width */
>>>>> +            bbox.x0 = ((x0 + FIXED_ONE/2) >> FIXED_ORDER) - int_width/2;
>>>>> +            bbox.y0 = ((y0 + FIXED_ONE/2) >> FIXED_ORDER) - int_width/2;
>>>>> +         }
>>>>> +
>>>>> +         bbox.x1 = bbox.x0 + int_width - 1;
>>>>> +         bbox.y1 = bbox.y0 + int_width - 1;
>>>>> +      }
>>>>>     }
>>>>> -
>>>>> +
>>>>> +   if (0) {
>>>>> +      debug_printf("  bbox: (%i, %i) - (%i, %i)\n",
>>>>> +                   bbox.x0, bbox.y0,
>>>>> +                   bbox.x1, bbox.y1);
>>>>> +   }
>>>>> +
>>>>>     if (!u_rect_test_intersection(&setup->draw_regions[viewport_index],
>>>>>     &bbox)) {
>>>>>        if (0) debug_printf("offscreen\n");
>>>>>        LP_COUNT(nr_culled_tris);
>>>>>
>>>>
>>>> This looks good to me. Note though this type of point rasterization is
>>>> only available in pre 3.0 contexts (or compatibilility contexts which we
>>>> don't support) anyway.
>>>
>>> Oh really!?  Argh..  It though it was still relevant nowadays, or I wouldn't have wasted my afternoon on trying to figure out the regression and the proper fix...
>> Well it seems at least some hw (nvidia for one) still can follow these
>> rules, so maybe there are apps out there which rely on it (radeonsi otoh
>> ignores the state). At least conform relies on it apparently so it's not
>> really a waste :-).
> 
> I have never understood what point_quad_rasterization means and how we
> should use it. I think Radeons always rasterize points as axis-aligned
> rectangles (width and height can be different) and there is no support
> for round/smooth points as far as I know.

Noone can do round/smooth points afaik and they always need to be
emulated. The problem is that even square points had different
rasterization rules to point sprites, that is they render differently to
what 2 tris (or a tri-strip) with the corners placed where'd they show
up depending on the point center and the point size would - in
particular the stuff about rounding widht to whole integers then doing
things differently depending if the width is odd or even is the crazy
part. The difference may be somewhat subtle in practice I guess but
presumably you can't pass the conform test if you just render them
according to "new" (point sprite) rules.
I have no idea though if you could set some bits in your hw so it would
match legacy GL point rasterization rules.

Roland