Will this patch be merged? >-----Original Message----- >From: [email protected] >[mailto:[email protected]] On Behalf >Of Shiyou Yin >Sent: Monday, February 10, 2020 5:50 PM >To: 'Søren Sandmann' >Cc: 'pixman' >Subject: Re: [Pixman] [PATCH] pixman-combine: Fix wrong value of >RB_MASK_PLUS_ONE. > >That's great, the analysis is very detailed. >Macro RB_MASK_PLUS_ONE is mainly used to saturate the 16-bits data to 0xff. >In function UN8_rb_ADD_UN8_rb, it can’t affect the result in the end. > > >>-----Original Message----- >>From: Søren Sandmann [mailto:[email protected]] >>Sent: Sunday, February 9, 2020 11:44 PM >>To: Shiyou Yin >>Cc: Matt Turner; pixman >>Subject: Re: [Pixman] [PATCH] pixman-combine: Fix wrong value of >>RB_MASK_PLUS_ONE. >> >>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 <[email protected]> wrote: >>> >>> >-----Original Message----- >>> >From: Matt Turner [mailto:[email protected]] >>> >Sent: Sunday, February 9, 2020 7:01 AM >>> >To: Yin Shiyou >>> >Cc: [email protected] >>> >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 <[email protected]> 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 >>> [email protected] >>> https://lists.freedesktop.org/mailman/listinfo/pixman > >_______________________________________________ >Pixman mailing list >[email protected] >https://lists.freedesktop.org/mailman/listinfo/pixman
_______________________________________________ Pixman mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/pixman
