Andrea Canciani <[email protected]> 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.

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


Thanks,
Soren
_______________________________________________
Pixman mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/pixman

Reply via email to