[Bf-committers] pixelblending.c issues

[bf-committers@blender.org]

Hello,

I'm new on this list and like to contribute to blender.

Ok, so, here I go.

I found the following function in pixelblend.c which is wrong:

(sorry for the screwed layout)

/*
  One things about key-alpha is that simply dividing by the alpha will
  sometimes cause 'overflows' in that the pixel colours will be shot
  way over full colour. This should be caught, and subsequently, the
  operation will end up modifying the alpha as well.

  Actually, when the starting colour is premul, it shouldn't overflow
  ever. Strange thing is that colours keep overflowing...

*/
void applyKeyAlphaCharCol(char* target) {

if ((!(target[3] == 0))
	|| (target[3] == 255)) {
	/* else: nothing to do */
	/* check whether div-ing is enough */
	float cf[4];
	cf[0] = target[0]/target[3];
	cf[1] = target[1]/target[3];
	cf[2] = target[2]/target[3];
	if ((cf[0] <= 1.0) && (cf[1] <= 1.0) && (cf[2] <= 1.0)) {
		/* all colours remain properly scaled? */
		/* scale to alpha */
		cf[0] = (float) target[0] * (255.0/ (float)target[3]);
		cf[1] = (float) target[1] * (255.0/ (float)target[3]);
		cf[2] = (float) target[2] * (255.0/ (float)target[3]);
		/* Clipping is important. */
		target[0] = (cf[0] > 255.0 ? 255 : (char) cf[0]);
		target[1] = (cf[1] > 255.0 ? 255 : (char) cf[1]);
		target[2] = (cf[2] > 255.0 ? 255 : (char) cf[2]);
			
	} else {
		/* shouldn't happen! we were premul, remember? */
/* should go to error handler: 			printf("Non-premul colour detected\n"); */
	}
}

} /* end of void applyKeyAlphaCharCol(char* target) */



This function/comments frighten me ;-)

Can someone (Ton?) explain to me what this function is supposed to do ?
I can read te code, but I seems someone programmed around a overflow/underflow bug. Is it just supposed to scale to alpha ?

 
The cf[x] = target[x]/ target[3] lines have a gigantic loss of precision, because the function will be calculated in chars and AFTER that (in the assignment) converted to float.
Besides the whole deviding is not necessary if you rewrite the if like:

alpha = target[3];
if ((cf[0] <= alpha) && (cf[1] <= alpha) && (cf[2] <= alpha))
{
}


So please can someone tell me what the code should do ? (not what the code does now, I can see that). I send a patch if ready.

Thanks and happy coding,

Jeroen (#RoccoD)