[Mesa-dev] [PATCH 1/2] draw: simplify (and correct) aaline fallback (v2)

[Roland Scheidegger]

Am 07.03.2018 um 11:34 schrieb Jose Fonseca:
> On 06/03/18 20:52, Brian Paul wrote:
>> Looks good.  That certainly does simplify things.  Two minor
>> suggestions below.
>>
>> In any case, for both, Reviewed-by: Brian Paul <brianp at vmware.com>
> 
> Looks great cleanup to me too.  Just minor issues inline. Both are
> 
> Reviewed-by: Jose Fonseca <jfonseca at vmware.com>
>>
>> On 03/06/2018 01:34 PM, sroland at vmware.com wrote:
>>> From: Roland Scheidegger <sroland at vmware.com>
>>>
>>> The motivation actually was to get rid of the additional tex
>>> instruction, since that requires the draw fallback code to intercept
>>> all sampler / view calls (even if the fallback is never hit).
>>> Basically, the idea is to use coverage of the pixel to calculate
>>> the alpha value, and coverage is simply based on the distance
>>> to the center of the line (in both line direction, which is useful
>>> for wide lines, as well as perpendicular to the line).
>>> This is much closer to what hw supporting this natively actually does.
>>> It also fixes an issue with line width not quite being correct, as
>>> well as endpoints getting stretched too far (in line direction) with
>>> wide lines, which is apparent with mesa demo line-width.
>>> (For llvmpipe, it would probably make sense to do something like this
>>> directly when drawing lines, since rendering two tris is twice as
>>> expensive as a line, but it would need some changes with state
>>> management.)
>>> Since we're no longer relying on mipmapping to get the alpha value,
>>> we also don't need to draw 3 rects (6 tris), one is sufficient.
>>>
>>> There's still issues (as before):
>>> - quite sure it's not correct without half_pixel_center, but can't test
>>> this with GL.
>>> - aaline + line stipple is incorrect (evident with line-width demo).
>>> Looking at the spec the stipple pattern should actually be based on
>>> distance (not just dx or dy for x/y major lines as without aa).
>>> - outputs (other than pos + the one used for line aa) should be
>>> reinterpolated since we actually increase line length by half a pixel
>>> (but there's no tests which would care).
>>>
>>> v2: simplify the math (should be equivalent), don't need immediate
>>> ---
>>>   src/gallium/auxiliary/draw/draw_pipe_aaline.c | 504
>>> +++++---------------------
>>>   1 file changed, 100 insertions(+), 404 deletions(-)
>>>
>>> +         aactx->tempsUsed |= (1ULL << i);
> UINT64_C(1)
Generally, this notation isn't used in mesa. Ok that's not entirely
true, but 1ULL outnumbers UINT64_C(1) by about 50 : 1. (There's exactly
5 uses of UINT64_C in mesa, among those 3 unique ones the remaining 2
are due to copy/paste.)
I guess UINT64_C() is technically better because ULL might not be 64bit,
but that assumption is so wide spread I'm not sure it's worth bothering
anymore...


>>>   -   /* XXX the ends of lines aren't quite perfect yet, but probably
>>> passable */
>>> -   dx = 0.5F * half_width;
>>> -   dy = half_width;
>>> +   half_length = sqrtf(dx * dx + dy * dy) / 2.0f + 0.5f;
> 
> 0.5f * ... ?
Ok. (Not that it would make a difference.)


>>> -   tex = v[2]->data[texPos];
>>> -   ASSIGN_4V(tex, .5, 0, 0, 1);
>>> +   tex = v[0]->data[coordPos];
>>> +   /* XXX need to adjust for pixel center? */
> 
> You could just check for pixel_center and write an warning.
Will do.

Roland