[Pixman] [PATCH] Resolve implementation-defined behaviour for division rounded to -infinity

[Ben Avison]

On Sat, 15 Aug 2015 01:06:14 +0100, Bill Spitzak <spitzak at gmail.com> wrote:

> Can't this just assume/require b to be positive? That would make them a lot simpler:
>
>  #define DIV(a,b) ((a) / (b) - ((a) % (b) < 0 ? (b) : 0))
>  #define MOD(a,b) ((a) % (b) + ((a) % (b) < 0 ? (b) : 0))

I had contemplated Euclidean division/modulus too, reasoning that it
might come in handy somewhere if we ever get better support for reflected
images, but it just seemed even more complex to express in portable C
(and to be honest I haven't formulated a detailed argument of how it
would be used). It seemed safe to at least follow what the comments said,
and make it round to -infinity.

I've run the "make check" suite with added debugging to ensure that b is
never negative, and this does indeed seem to be the case, so we could
make such a change without any side effects (at least for current uses of
the macros). There was a slight error in your version, though - a correct
version would look like:

#define DIV(a, b) ((a) / (b) - ((a) % (b) < 0))
#define MOD(a, b) ((a) % (b) + ((a) % (b) < 0) ? (b) : 0))

Does anyone have any objections to such a change of definition?

Ben


> On Fri, Aug 14, 2015 at 7:06 AM, Ben Avison <bavison at riscosopen.org> wrote:
>
>> The previous implementations of DIV and MOD relied upon the built-in / and
>> %
>> operators performing round-to-zero. This is true for C99, but rounding is
>> implementation-defined for C89 when divisor and/or dividend is negative,
>> and
>> I believe Pixman is still supposed to support C89.
>> ---
>>  pixman/pixman-private.h |    8 ++++----
>>  1 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/pixman/pixman-private.h b/pixman/pixman-private.h
>> index 73108a0..80506be 100644
>> --- a/pixman/pixman-private.h
>> +++ b/pixman/pixman-private.h
>> @@ -889,12 +889,12 @@ pixman_list_move_to_front (pixman_list_t *list,
>> pixman_link_t *link)
>>  #endif
>>
>>  /* Integer division that rounds towards -infinity */
>> -#define DIV(a, b)                                         \
>> -    ((((a) < 0) == ((b) < 0)) ? (a) / (b) :                \
>> -     ((a) - (b) + 1 - (((b) < 0) << 1)) / (b))
>> +#define DIV(a, b) \
>> +    ((a) / (b) - ((a) % (b) != 0 && ((a) % (b) < 0) != ((b) < 0) ? 1 : 0))
>>
>>  /* Modulus that produces the remainder wrt. DIV */
>> -#define MOD(a, b) ((a) < 0 ? ((b) - ((-(a) - 1) % (b))) - 1 : (a) % (b))
>> +#define MOD(a, b) \
>> +    ((a) % (b) + ((a) % (b) != 0 && ((a) % (b) < 0) != ((b) < 0) ? (b) :
>> 0))
>>
>>  #define CLIP(v, low, high) ((v) < (low) ? (low) : ((v) > (high) ? (high)
>> : (v)))
>>
>> --
>> 1.7.5.4
>>
>> _______________________________________________
>> Pixman mailing list
>> Pixman at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/pixman