[Pixman] [PATCH 2/3] Fix opacity check

Andrea Canciani ranma42 at gmail.com
Thu Nov 4 01:03:13 PDT 2010


On Thu, Nov 4, 2010 at 2:05 AM, Soeren Sandmann <sandmann at daimi.au.dk> wrote:
> Andrea Canciani <ranma42 at gmail.com> writes:
>
>> @@ -436,6 +436,12 @@ compute_image_info (pixman_image_t *image)
>>       {
>>           int i;
>>
>> +         if (image->common.type == RADIAL &&
>> +             image->radial.a >= 0)
>> +         {
>> +             break;
>> +         }
>
> I think this code needs a comment explaining why it's necessary, and
> why checking for a >= 0 works. Also, instead of having the extra if
> statement, I think I'd prefer to refactor the switch like this:
>
>        case RADIAL:
>            /* Comment explaining why this test is necessary */
>            if (image->radial.a >= 0)
>                break;
>
>            /* Fall through */
>
>        case LINEAR:
>            ...
>
> Other than, this and the other two patches look good to me.

The patch in the attachment should provide the requested improvements.
I tried to only give a high-level explanation of the condition, leaving the
maths in pixman-radial-gradient.c (but referencing it).
Is the comment ok or should I also add the definition of a (and maybe
the equivalence A<0 <=> every point is colored)?

>
> It would of course also be very useful to extend the test suite to
> catch this problem.

Yes, I know :(. These days I'm really busy with my thesis work,
but I'll add this problem to the ones that the pixman radial test
should check.

Andrea
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Fix-opacity-check.patch
Type: application/octet-stream
Size: 1434 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/pixman/attachments/20101104/9ade77a0/attachment.obj>


More information about the Pixman mailing list