Message du 18/03/05 13:49 De : "Christian Costa" A : "Tom Wickline" , "wine-devel" Copie à : Objet : Re: ddraw correctness fixes patch
Message du 18/03/05 06:44 De : "Tom Wickline" A : "wine-devel" Copie à : Objet : ddraw correctness fixes patch
anyone know why this patch hasn't been accepted?
http://www.winehq.org/hypermail/wine-patches/2005/03/0328.html
Tom
Hi Tom,
Sorry I missed the patch on wine-patches but I do remember an old version sent to wine-devel.
So let's review it. :-)
The new locking mechanism is wrong. DDLOCK_xxx are just used for optimization.
The thing to do during the blit should be something like that :
sdesc.dwSize = sizeof(sdesc); ddesc.dwSize = sizeof(ddesc); if (src == NULL) IDirectDrawSurface7_Lock(iface, &lock_dst, &ddesc, DDLOCK_WRITEONLY, 0); else if (src == iface) { RECT lock_union = < rect that just contains lock_dst and lock_src >; IDirectDrawSurface7_Lock(iface, &lock_union, &ddesc, 0, 0); sdesc = ddesc;
I think here that sdesc and ddesc surface pointers should be remapped according to lock_src and lock_dest.
} else { IDirectDrawSurface7_Lock(src, &lock_src, &sdesc, DDLOCK_READONLY, 0); IDirectDrawSurface7_Lock(iface, &lock_dst, &ddesc, DDLOCK_WRITEONLY, 0); }
and the previous locking code should be used.
The added DD_STRUCT_INIT are useless.
Hope that helps.
Bye, Christian
Thanks for the review - comments inline.
Christian Costa wrote:
The new locking mechanism is wrong. DDLOCK_xxx are just used for optimization.
The thing to do during the blit should be something like that :
sdesc.dwSize = sizeof(sdesc); ddesc.dwSize = sizeof(ddesc); if (src == NULL) IDirectDrawSurface7_Lock(iface, &lock_dst, &ddesc, DDLOCK_WRITEONLY, 0); else if (src == iface) { RECT lock_union = < rect that just contains lock_dst and lock_src >; IDirectDrawSurface7_Lock(iface, &lock_union, &ddesc, 0, 0); sdesc = ddesc;
I think here that sdesc and ddesc surface pointers should be remapped according to lock_src and lock_dest.
The above logic makes sense - I originally tried to solve it this way, but ended up failing because of the sdesc and ddesc mapping issue. Any idea how to do this cleanly?
The added DD_STRUCT_INIT are useless.
Noted.
Matt.
Matthew Mastracci wrote:
Thanks for the review - comments inline.
Christian Costa wrote:
The new locking mechanism is wrong. DDLOCK_xxx are just used for optimization.
The thing to do during the blit should be something like that :
sdesc.dwSize = sizeof(sdesc); ddesc.dwSize = sizeof(ddesc); if (src == NULL) IDirectDrawSurface7_Lock(iface, &lock_dst, &ddesc, DDLOCK_WRITEONLY, 0); else if (src == iface) { RECT lock_union = < rect that just contains lock_dst and lock_src >; IDirectDrawSurface7_Lock(iface, &lock_union, &ddesc, 0, 0); sdesc = ddesc;
I think here that sdesc and ddesc surface pointers should be remapped according to lock_src and lock_dest.
The above logic makes sense - I originally tried to solve it this way, but ended up failing because of the sdesc and ddesc mapping issue. Any idea how to do this cleanly?
I think what will work is IntersectRect() from windows/rect.c
IntersectRect( LPRECT dest, const RECT *src1, const RECT *src2)
Something like the following perhaps?
RECT lock_union
if IntersectRect(&lockunion, &sdesc, &ddesc) IDirectDrawSurface7_Lock(iface, &lock_union, &ddesc, 0, 0); ....
--
Tony Lambregts
Tony Lambregts wrote:
Matthew Mastracci wrote:
Thanks for the review - comments inline.
Christian Costa wrote:
The new locking mechanism is wrong. DDLOCK_xxx are just used for optimization.
The thing to do during the blit should be something like that :
sdesc.dwSize = sizeof(sdesc); ddesc.dwSize = sizeof(ddesc); if (src == NULL) IDirectDrawSurface7_Lock(iface, &lock_dst, &ddesc, DDLOCK_WRITEONLY, 0); else if (src == iface) { RECT lock_union = < rect that just contains lock_dst and lock_src
; IDirectDrawSurface7_Lock(iface, &lock_union, &ddesc, 0, 0);
sdesc = ddesc;
I think here that sdesc and ddesc surface pointers should be remapped according to lock_src and lock_dest.
The above logic makes sense - I originally tried to solve it this way, but ended up failing because of the sdesc and ddesc mapping issue. Any idea how to do this cleanly?
I think what will work is IntersectRect() from windows/rect.c
I think the problem Matthew was talking is how to remap surface pointers.
I sent something to Mattew but I forgot to cc wine-devel, sorry.
Basically it was :
depth = surface_desc.u4.ddpfPixelFormat.u1.dwRGBBitCount; ddesc.lpSurface -= (lock_dst.left - lock_union.left)*(depth+7)/8 + ddesc.lPictch * (lock_dst.top - lock_union.top); sdesc.lpSurface -= (lock_src.left - lock_union.left)*(depth+7)/8) + sdesc.lPictch * (lock_src.top - lock_union.top);
IntersectRect( LPRECT dest, const RECT *src1, const RECT *src2)
Something like the following perhaps?
RECT lock_union
if IntersectRect(&lockunion, &sdesc, &ddesc) IDirectDrawSurface7_Lock(iface, &lock_union, &ddesc, 0, 0); ....
You mean UnionRect. Don't you ? :-)
Bye, Christian
Christian Costa wrote:
I think the problem Matthew was talking is how to remap surface pointers.
I sent something to Mattew but I forgot to cc wine-devel, sorry.
Basically it was :
depth = surface_desc.u4.ddpfPixelFormat.u1.dwRGBBitCount; ddesc.lpSurface -= (lock_dst.left - lock_union.left)*(depth+7)/8 + ddesc.lPictch * (lock_dst.top - lock_union.top); sdesc.lpSurface -= (lock_src.left - lock_union.left)*(depth+7)/8) + sdesc.lPictch * (lock_src.top - lock_union.top);
I implemented the surface pointer remapping in my lastest patch, but chose to adjust the surface pointers from the raw surface pointer instead of the pre-adjusted value returned from the Lock() call. It's the same logic, but easier to verify as correct to the eye.
If you could take a look at the latest patch, I'd appreciate it.
Matt.
-
Christian Costa -
Matthew Mastracci -
Tony Lambregts