DDRAW: Surface locking patch, take 2
Matthew Mastracci
matt at aclaro.com
Sun Mar 20 14:09:28 CST 2005
Christian Costa wrote:
> Hi Matthew,
>
> This seems fine.
> There are some comments though, see below.
>
...
>> + if (src == iface) {
>> + int pitch;
>> +
>> + UnionRect(&lock_union, &lock_src, &lock_dst);
>> +
>> + /* Lock the union of the two rectangles */
>> + IDirectDrawSurface7_Lock(iface, &lock_union, &ddesc, 0, 0);
>> +
>> + pitch = This->surface_desc.u1.lPitch;
>> +
>>
> ddesc has not been properly initialized.
> you should also copy sdesc from ddesc.
It actually has. If you look higher up in the function, ddesc and sdesc
are copied from This->surface_desc:
/* Get the surface description without locking to first compute the
width / height */
ddesc = This->surface_desc;
sdesc = (ICOM_OBJECT(IDirectDrawSurfaceImpl, IDirectDrawSurface7,
src))->surface_desc;
AFAICT, ddesc and sdesc will always be valid after this initialization.
>> + if (src == iface) {
>> + IDirectDrawSurface7_Unlock(iface, &lock_union);
>> + } else {
>> + IDirectDrawSurface7_Unlock(iface, &lock_dst);
>> + IDirectDrawSurface7_Unlock(src, &lock_src);
>>
> You should check if src != NULL before trying to unlocking it.
BltFast() doesn't accept a NULL src, so this isn't necessary. The
function should probably check for NULL src before the surface lock
check. If I can figure out what the error return code should be (I
think it's DDERR_INVALIDPARAM), I can check for a NULL src at the start
of the function in a later patch.
Thanks for the review,
Matt.
More information about the wine-devel
mailing list