Blend modes take 3
Benjamin Otte
otte at gnome.org
Tue Nov 25 13:54:17 PST 2008
On Tue, Nov 25, 2008 at 10:10 PM, Soeren Sandmann <sandmann at 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
More information about the xorg
mailing list