On Thu, Nov 4, 2010 at 2:05 AM, Soeren Sandmann <sandm...@daimi.au.dk> wrote:
> Andrea Canciani <ranm...@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
0002-Fix-opacity-check.patch
Description: Binary data
_______________________________________________ Pixman mailing list Pixman@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pixman
