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

Wed Jan 8 13:43:56 PST 2014

We have this "edge rule" state for points. It has 4 bits called Left,
Right, Top, Bottom. 1 means "in", 0 means "out". It probably
determines what happens if a primitive edge intersects a pixel or
something like that. We set Left=0, Right=1, Top=0, Bottom=1.

We also have this state:

ROUND_MODE
 Controls conversion of X,Y coordinates from IEEE to fixed-point
POSSIBLE VALUES:
00 - 0 = Truncate (OGL)
01 - 1 = Round
02 - 2 = Round to Even (D3D)
03 - 3 = Round to Odd

Not sure what the correct values are for both states.

Marek

On Wed, Jan 8, 2014 at 8:56 PM, Roland Scheidegger <sroland at vmware.com> wrote:
> 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