[Pixman] [PATCH] Improve handling of tangent circles

Andrea Canciani ranma42 at gmail.com
Tue Jan 4 06:33:18 PST 2011

```On Tue, Jan 4, 2011 at 11:12 AM, Andrea Canciani <ranma42 at gmail.com> wrote:
> On Tue, Jan 4, 2011 at 10:59 AM, Siarhei Siamashka
> <siarhei.siamashka at gmail.com> wrote:
>> On Monday 03 January 2011 10:15:09 Andrea Canciani wrote:
>>> When b is 0, avoid the division by zero and just return transparent
>>> black.
>>>
>>> When the solution t would have an invalid radius (negative or outside
>>> [0,1] for none-extended gradients), return transparent black.
>>
>>
>> I just wonder if it would be difficult to add a test to pixman for this
>> particular division by zero case? Or is it somehow triggered by cairo
>> tests?
>
>
> The refimages have been created with a patched pixman, so the tests
> currently fail on cairo-image, but would pass with this change.
>
>>
>> One of the problems with the pixman radial code is that it is slow.
>> And this path further slows it down a bit by adding a new branch in the
>> inner loop. This is perfectly fine for a reference implementation, but if
>> somebody decides to add some performance optimizations, then we need to have
>> at least some basic tests which can catch possible errors and regressions.

The proposed code is equivalent to:

if (a == 0)
{
double t;

if (b == 0)
return 0;

t = pixman_fixed_1 / 2 * c / b;
if (repeat == PIXMAN_REPEAT_NONE || t * dr > mindr)

return 0;
}

or even to

if (a == 0)
{
double t;

if (b == 0)
return 0;

t = pixman_fixed_1 / 2 * c / b;
if (t * dr > mindr)

return 0;
}

because the drawn part (the range [0,1]) of a REPEAT_NONE gradient
return 0 for anything outside [0,1] (and we cannot have multiple solutions
for the same point if a==0).
This would remove a branch, but I'd rather do it only if the code is still
clear. The proposed patch looks more "obviously  correct" to me because
it validates the solutions just as in the a!=0 case.

In http://cgit.freedesktop.org/~ranma42/cairo/log/?h=wip/gl3 I used this
property to have just one shader for the a==0 case, ignoring the extend
mode.

Andrea
```