[prev in list] [next in list] [prev in thread] [next in thread] 

List:       cairo
Subject:    Re: [cairo] Blend modes take 3
From:       "Benjamin Otte" <otte () gnome ! org>
Date:       2008-11-25 21:54:17
Message-ID: a4ed255b0811251354l4cf97e55q8a3c06b1909fa56c () mail ! gmail ! com
[Download RAW message or body]

On Tue, Nov 25, 2008 at 10:10 PM, Soeren Sandmann <sandmann@daimi.au.dk> wrote:
>    - When mask is 0, there is no reason to read the source.
>
>    - There is no reason to read the destination if the inverse
>      combined src/mask is 0
>
Don't compilers take care of that?
The code looks better to me if the variable assignments all occur in one place.

>    - The continue is incorrect; it needs to write out 0 in that case,
>      since the intermediate buffer is not zero-filled.
>
Oh, the code is supposed to modify the _source_ parameter, too?
Someone really needs to document what component alpha does properly so
I really grok it.
Fixed.

> - In FlashSubtractC, the m = ~m is not used. And fbCombineMaskC()
>  seems like it could just be fbCombineMaskValueC.
>
Fixed.

>>   if (Max (tmp) > sa) while (1);
>>   if (Max (dest) > 255 * 255) while (1);
>
> Are these supposed to never happen?
>
Yeah, It's my poor man's choice at an assertion statement while
debugging. Unfortunately I always forget removing them after I'm done
debugging.
Fixed.

>>  if (min < 0) {
>>    tmp[0] = l + (tmp[0] - l) / 4 * l / (l - min) * 4;
>>    tmp[1] = l + (tmp[1] - l) / 4 * l / (l - min) * 4;
>>    tmp[2] = l + (tmp[2] - l) / 4 * l / (l - min) * 4;
>>  }
>>  if (max > a) {
>>    tmp[0] = l + (tmp[0] - l) / 4 * (a - l) / (max - l) * 4;
>>    tmp[1] = l + (tmp[1] - l) / 4 * (a - l) / (max - l) * 4;
>>    tmp[2] = l + (tmp[2] - l) / 4 * (a - l) / (max - l) * 4;
>>  }
>
> Where do those 4s come from?
>
Overflow protection. The tmp value can range from -COMP2_T_MAX .. 2 *
COMP2_T_MAX (I think), so I took the shortcut to just divide by a high
enough number to avoid it. Also, this code will break on 64bit cases
as I'm using ints there for lack of a signed comp4_t type.
It's one of the cases I asked about previously on IRC I think as I was
unsure if this is a case for doubles or how it should best be handled.


Benjamin
_______________________________________________
cairo mailing list
cairo@cairographics.org
http://lists.cairographics.org/mailman/listinfo/cairo
[prev in list] [next in list] [prev in thread] [next in thread]