Re: Support for 24bpp when DDBLT_KEYSRC etc are set

[Lionel Ulmer]

> Change log: added support for 24bpp when  the flags 
> DDBLT_KEYSRC/DDBLT_KEYDEST and company are set.

Well, I have the following comment :
> -
>    case 3: {
> -
> int last_sy = -1;
> -
> for (y = sy = 0; y < dstheight; y++, sy += yinc) {
> -
>     sbuf = sbase + (sy >> 16) * sdesc.u1.lPitch;
> -
>     if ((sy >> 16) == (last_sy >> 16)) {
> -
> 	/* this sourcerow is the same as last sourcerow -
> -
> 	 * copy already stretched row
> -
> 	 */
> -
> 	memcpy(dbuf, dbuf - ddesc.u1.lPitch, width);

Are you sure that this works ? If you have pixels in the destination surface
that are NOT overwritten by the source surface (due to color-keying) you
should not copy these from one line to the other.
> -
>     } else {
> -
> 	LPBYTE s,d = dbuf;
> -
> 	for (x = sx = 0; x < dstwidth; x++, sx+= xinc) {
> -
> 	    DWORD pixel;
> -
> -
> 	    s = sbuf+3*(sx>>16);
> -
> 	    pixel = (s[0]<<16)|(s[1]<<8)|s[2];
> -
>                        if (pixel < keylow || pixel > keyhigh){
> -
> 	       d[0] = (pixel>>16)&0xff;
> -
> 	       d[1] = (pixel>> 8)&0xff;
> -
> 	       d[2] = (pixel    )&0xff;
> -
> 	       d+=3;

Why only increase the destination pixel when the color is not keyed off ?
> -
>                        }
> -
> 	}
> -
>     }
> -
>     dbuf += ddesc.u1.lPitch;
> -
>     last_sy = sy;
> -
>            }
> -
>            break;}

The rest of the patch looks OK :)

By the way, which game uses 24 bpp ? It's *EVIL* :-)

Lionel

Support for 24bpp when DDBLT_KEYSRC etc are set

[Tony Lambregts]

Lionel Ulmer wrote:
> > Change log: added support for 24bpp when  the flags 
> > DDBLT_KEYSRC/DDBLT_KEYDEST and company are set.
>
> Well, I have the following comment :
> > -
> >    case 3: {
> > -
> > int last_sy = -1;
> > -
> > for (y = sy = 0; y < dstheight; y++, sy += yinc) {
> > -
> >     sbuf = sbase + (sy >> 16) * sdesc.u1.lPitch;
> > -
> >     if ((sy >> 16) == (last_sy >> 16)) {
> > -
> > 	/* this sourcerow is the same as last sourcerow -
> > -
> > 	 * copy already stretched row
> > -
> > 	 */
> > -
> > 	memcpy(dbuf, dbuf - ddesc.u1.lPitch, width);
>
> Are you sure that this works ? If you have pixels in the destination surface
> that are NOT overwritten by the source surface (due to color-keying) you
> should not copy these from one line to the other.

Doh!
> > -
> >     } else {
> > -
> > 	LPBYTE s,d = dbuf;
> > -
> > 	for (x = sx = 0; x < dstwidth; x++, sx+= xinc) {
> > -
> > 	    DWORD pixel;
> > -
> > -
> > 	    s = sbuf+3*(sx>>16);
> > -
> > 	    pixel = (s[0]<<16)|(s[1]<<8)|s[2];
> > -
> >                        if (pixel < keylow || pixel > keyhigh){
> > -
> > 	       d[0] = (pixel>>16)&0xff;
> > -
> > 	       d[1] = (pixel>> 8)&0xff;
> > -
> > 	       d[2] = (pixel    )&0xff;
> > -
> > 	       d+=3;
>
> Why only increase the destination pixel when the color is not keyed off ?

Doh!!
> > -
> >                        }
> > -
> > 	}
> > -
> >     }
> > -
> >     dbuf += ddesc.u1.lPitch;
> > -
> >     last_sy = sy;
> > -
> >            }
> > -
> >            break;}
>
> The rest of the patch looks OK :)
>
> By the way, which game uses 24 bpp ? It's *EVIL* :-)

I do not have any game that uses 24 bits (to the best of my knowledge) 
 This is more of an exersize in getting to know the code better.  Of 
course if we don't want to support 24 bpp...

Thankyou for looking at it, I will have to do some changes before I 
resubmit.

Tony Lambregts

Support for 24bpp when DDBLT_KEYSRC etc are set

[Lionel Ulmer]

> I do not have any game that uses 24 bits (to the best of my knowledge) 
>  This is more of an exersize in getting to know the code better.  Of 
> course if we don't want to support 24 bpp...

No, it's fine to support it. I was just curious that's all :-)
> Thankyou for looking at it, I will have to do some changes before I 
> resubmit.

No problem... Peer review is always a good way to find bugs before
committing to CVS.

Lionel