Hi, The reason this bug doesn't show up in the test suite is that it doesn't have an effect on the output. The purpose of the UN8_rb_ADD_UN8_rb macro is to add two numbers like these:
00aa00bb 00cc00dd together, but to saturate aa+cc and bb+dd to 0xff instead of allowing them to overflow. Let's pretend that bb and dd are both zero and focus on aa+cc. If that addition does in fact overflow (ie., if aa+cc > 0xff), the result looks like this: 01??0000 The macro first detects the overflow shifting this value by 8 and masking to 0x00ff00ff: 00010000 Then it subtracts this from the RB_MASK_PLUS_ONE value: with bug: 10000000 - 00010000 = 0fff0000 with bug fix: 01000000 - 00010000 = 00ff0000 The macro then ors this value with the original sum: with bug: 0fff0000 | 01??0000 = 0fff0000 with bug fix: 00ff0000 | 01??0000 = 01ff0000 But then it masks the result with 00ff00ff, and so both with and without the bug, the final result is the same: 00ff0000 which is why the bug doesn't affect the calculation. Søren On Sun, Feb 9, 2020 at 4:26 AM Shiyou Yin <yinshiyou...@loongson.cn> wrote: > > >-----Original Message----- > >From: Matt Turner [mailto:matts...@gmail.com] > >Sent: Sunday, February 9, 2020 7:01 AM > >To: Yin Shiyou > >Cc: pixman@lists.freedesktop.org > >Subject: Re: [Pixman] [PATCH] pixman-combine: Fix wrong value of > >RB_MASK_PLUS_ONE. > > > >On Mon, Feb 3, 2020 at 1:56 AM Yin Shiyou <yinshiyou...@loongson.cn> wrote: > >> > >> --- > >> pixman/pixman-combine32.h | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/pixman/pixman-combine32.h b/pixman/pixman-combine32.h > >> index cdd56a6..59bb247 100644 > >> --- a/pixman/pixman-combine32.h > >> +++ b/pixman/pixman-combine32.h > >> @@ -12,7 +12,7 @@ > >> #define RB_MASK 0xff00ff > >> #define AG_MASK 0xff00ff00 > >> #define RB_ONE_HALF 0x800080 > >> -#define RB_MASK_PLUS_ONE 0x10000100 > >> +#define RB_MASK_PLUS_ONE 0x1000100 > > > > > >Thanks. The patch looks correct, but obviously nothing in the test > >suite is failing. How did you discover this? Does this patch fix > >something for you? > > I just found this mistake while analyzing macro 'UN8_rb_ADD_UN8_rb' > and functions called this macro. I can't verify this error with existing > test suite, it all passed no matter Fix it or not. > > _______________________________________________ > Pixman mailing list > Pixman@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/pixman _______________________________________________ Pixman mailing list Pixman@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pixman