Regression (SimCity 3000 is Broke )
After working prefectly for over 2 years SimCity 3000 is broken. The patch that broke it is http://www.winehq.org/hypermail/wine-cvs/2005/03/0098.html basicly the problem is simple. The source and the destination are both the same surface in DIB_DirectDrawSurface_Blt() and with the new locking in place the destination never gets updated. So althought it might fix Picasa2 it is definatly not correct. -- Tony Lambregts
On Tue, 2005-03-15 at 07:35, Tony Lambregts wrote:
After working prefectly for over 2 years SimCity 3000 is broken.
The patch that broke it is http://www.winehq.org/hypermail/wine-cvs/2005/03/0098.html
basicly the problem is simple. The source and the destination are both the same surface in DIB_DirectDrawSurface_Blt() and with the new locking in place the destination never gets updated. So althought it might fix Picasa2 it is definatly not correct.
--
Tony Lambregts
Hi Tony, I don't know why I'm cc-ed. I only fixed another regression ;-). I don't known anything about locking, so Matt? Cheers, Paul.
(removing Paul from CC) I think the problem is that we lock both the src and dest surfaces, without checking to see if they are the same. I think we then try to lock both surfaces and one of the locks fails. Perhaps this might be a better solution: if (src == iface) { DD_STRUCT_INIT(&ddesc); DD_STRUCT_INIT(&sdesc); IDirectDrawSurface7_Lock(iface,NULL,&ddesc,0,0); DD_STRUCT_COPY_BYSIZE(&ddesc,&sdesc); } else { DD_STRUCT_INIT(&ddesc); DD_STRUCT_INIT(&sdesc); sdesc.dwSize = sizeof(sdesc); if (src) IDirectDrawSurface7_Lock(src, NULL, &sdesc, DDLOCK_READONLY, 0); ddesc.dwSize = sizeof(ddesc); IDirectDrawSurface7_Lock(iface,NULL,&ddesc,DDLOCK_WRITEONLY,0); } I don't have access to my box with my Wine tree right now so I don't know if this will compile. Note that the fix will need to be done for both _Blt() and _BltFast(). Paul Vriens wrote:
On Tue, 2005-03-15 at 07:35, Tony Lambregts wrote:
After working prefectly for over 2 years SimCity 3000 is broken.
The patch that broke it is http://www.winehq.org/hypermail/wine-cvs/2005/03/0098.html
basicly the problem is simple. The source and the destination are both the same surface in DIB_DirectDrawSurface_Blt() and with the new locking in place the destination never gets updated. So althought it might fix Picasa2 it is definatly not correct.
--
Tony Lambregts
Hi Tony,
I don't know why I'm cc-ed. I only fixed another regression ;-). I don't known anything about locking, so Matt?
Cheers,
Paul.
Matthew Mastracci wrote:
(removing Paul from CC)
I think the problem is that we lock both the src and dest surfaces, without checking to see if they are the same. I think we then try to lock both surfaces and one of the locks fails.
Perhaps this might be a better solution:
if (src == iface) { DD_STRUCT_INIT(&ddesc); DD_STRUCT_INIT(&sdesc);
IDirectDrawSurface7_Lock(iface,NULL,&ddesc,0,0);
DD_STRUCT_COPY_BYSIZE(&ddesc,&sdesc); } else { DD_STRUCT_INIT(&ddesc); DD_STRUCT_INIT(&sdesc);
sdesc.dwSize = sizeof(sdesc); if (src) IDirectDrawSurface7_Lock(src, NULL, &sdesc, DDLOCK_READONLY, 0); ddesc.dwSize = sizeof(ddesc); IDirectDrawSurface7_Lock(iface,NULL,&ddesc,DDLOCK_WRITEONLY,0); }
I don't have access to my box with my Wine tree right now so I don't know if this will compile. Note that the fix will need to be done for both _Blt() and _BltFast(). Paul Vriens wrote:
I have fixed it on my machine with the following patch. The reasoning for it is that I get the following waring in my trace's warn:ddraw:Main_DirectDrawSurface_Lock - unsupported locking flag : DDLOCK_NOSYSLOCK That would indicate that we do not want to actually "Lock" it to prevent others from using it. I think that it can be improved on but I would like you to try this and see if it works for you. If it does then can work on getting it acceptable for cvs. Index: dlls/ddraw/dsurface/main.c =================================================================== RCS file: /home/wine/wine/dlls/ddraw/dsurface/main.c,v retrieving revision 1.64 diff -u -r1.64 main.c --- dlls/ddraw/dsurface/main.c 7 Mar 2005 12:23:34 -0000 1.64 +++ dlls/ddraw/dsurface/main.c 15 Mar 2005 19:15:04 -0000 @@ -1147,9 +1147,9 @@ } else { This->lock_update(This, NULL, flags); } - - This->locked = TRUE; - + if (flags & ~(DDLOCK_WAIT|DDLOCK_READONLY|DDLOCK_WRITEONLY)) { + This->locked = TRUE; + } TRACE("locked surface returning description : \n"); if (TRACE_ON(ddraw)) DDRAW_dump_surface_desc(pDDSD); @@ -1421,13 +1421,13 @@ HRESULT WINAPI Main_DirectDrawSurface_Unlock(LPDIRECTDRAWSURFACE7 iface, LPRECT pRect) { + IDirectDrawSurfaceImpl *This = (IDirectDrawSurfaceImpl *)iface; TRACE("(%p)->Unlock(%p)\n",This,pRect); if (!This->locked) { - WARN("Surface not locked - returing DDERR_NOTLOCKED\n"); - return DDERR_NOTLOCKED; + WARN("Surface not locked\n"); } This->locked = FALSE;
DDLOCK_NOSYSLOCK prevents DirectDraw from locking "system lock", which is a global 16-bit lock on Windows 9x systems. I don't think this is the right way to fix this. Check out this page here: http://msdn.microsoft.com/library/default.asp?url=/library/en-us/dndxgen/htm... This flag has no effect on Windows 2000 or Wine, since they don't have global Win16 locks (AFAIK). Matt. Tony Lambregts wrote:
Matthew Mastracci wrote:
(removing Paul from CC)
I think the problem is that we lock both the src and dest surfaces, without checking to see if they are the same. I think we then try to lock both surfaces and one of the locks fails.
Perhaps this might be a better solution:
if (src == iface) { DD_STRUCT_INIT(&ddesc); DD_STRUCT_INIT(&sdesc);
IDirectDrawSurface7_Lock(iface,NULL,&ddesc,0,0);
DD_STRUCT_COPY_BYSIZE(&ddesc,&sdesc); } else { DD_STRUCT_INIT(&ddesc); DD_STRUCT_INIT(&sdesc);
sdesc.dwSize = sizeof(sdesc); if (src) IDirectDrawSurface7_Lock(src, NULL, &sdesc, DDLOCK_READONLY, 0); ddesc.dwSize = sizeof(ddesc); IDirectDrawSurface7_Lock(iface,NULL,&ddesc,DDLOCK_WRITEONLY,0); }
I don't have access to my box with my Wine tree right now so I don't know if this will compile. Note that the fix will need to be done for both _Blt() and _BltFast(). Paul Vriens wrote:
I have fixed it on my machine with the following patch. The reasoning for it is that I get the following waring in my trace's
warn:ddraw:Main_DirectDrawSurface_Lock - unsupported locking flag : DDLOCK_NOSYSLOCK
That would indicate that we do not want to actually "Lock" it to prevent others from using it. I think that it can be improved on but I would like you to try this and see if it works for you. If it does then can work on getting it acceptable for cvs.
------------------------------------------------------------------------
Index: dlls/ddraw/dsurface/main.c =================================================================== RCS file: /home/wine/wine/dlls/ddraw/dsurface/main.c,v retrieving revision 1.64 diff -u -r1.64 main.c --- dlls/ddraw/dsurface/main.c 7 Mar 2005 12:23:34 -0000 1.64 +++ dlls/ddraw/dsurface/main.c 15 Mar 2005 19:15:04 -0000 @@ -1147,9 +1147,9 @@ } else { This->lock_update(This, NULL, flags); } - - This->locked = TRUE; - + if (flags & ~(DDLOCK_WAIT|DDLOCK_READONLY|DDLOCK_WRITEONLY)) { + This->locked = TRUE; + } TRACE("locked surface returning description : \n"); if (TRACE_ON(ddraw)) DDRAW_dump_surface_desc(pDDSD);
@@ -1421,13 +1421,13 @@ HRESULT WINAPI Main_DirectDrawSurface_Unlock(LPDIRECTDRAWSURFACE7 iface, LPRECT pRect) { + IDirectDrawSurfaceImpl *This = (IDirectDrawSurfaceImpl *)iface;
TRACE("(%p)->Unlock(%p)\n",This,pRect);
if (!This->locked) { - WARN("Surface not locked - returing DDERR_NOTLOCKED\n"); - return DDERR_NOTLOCKED; + WARN("Surface not locked\n"); }
This->locked = FALSE;
Mike Hearn wrote:
On Tue, 15 Mar 2005 12:28:48 -0700, Matthew Mastracci wrote:
This flag has no effect on Windows 2000 or Wine, since they don't have global Win16 locks (AFAIK).
Well we have the Win16 syslevel, is that what you mean?
Whoops - I guess Wine does have the same system-level mutex. :) I just read through the syslevel documentation on WineHQ. Since the DirectDraw stuff doesn't seem use the Wine syslevel (from a cursory examination), I suppose the flag is still a noop. Matt.
Matthew Mastracci wrote:
DDLOCK_NOSYSLOCK prevents DirectDraw from locking "system lock", which is a global 16-bit lock on Windows 9x systems. I don't think this is the right way to fix this.
Check out this page here:
http://msdn.microsoft.com/library/default.asp?url=/library/en-us/dndxgen/htm...
This flag has no effect on Windows 2000 or Wine, since they don't have global Win16 locks (AFAIK).
AFAICS The locking system that you have put in place is exactly that: a "system lock". Not "global 16-bit" perhaps but it definatly is a system lock. I am fairly familiar with how this program operates. It is the program itself that is calling the locks with this flag and this is the only way I can see it can work with the system that you have put into place. The bottom line here I have a problem with the locking system you have put into place since it breaks my favorite program. Please try the patch and see if it works for your program. If it does then we/I can work up a patch that works with your program and mine. If you are not willing to look at it am perfectly willing to submit a patch that removes your patch since it is clearly incorrect as it stands. Please let's work together on this. -- Tony Lambregts
Tony Lambregts wrote:
Matthew Mastracci wrote:
DDLOCK_NOSYSLOCK prevents DirectDraw from locking "system lock", which is a global 16-bit lock on Windows 9x systems. I don't think this is the right way to fix this.
Check out this page here:
http://msdn.microsoft.com/library/default.asp?url=/library/en-us/dndxgen/htm...
This flag has no effect on Windows 2000 or Wine, since they don't have global Win16 locks (AFAIK).
AFAICS The locking system that you have put in place is exactly that: a "system lock". Not "global 16-bit" perhaps but it definatly is a system lock.
The locking system is actually a per-surface lock. Basically, DirectDraw under Windows requires you to lock a surface in memory before blitting to it. Wine doesn't strictly enforce this locking requirement, but some programs rely on the correct state behaviour.
I am fairly familiar with how this program operates. It is the program itself that is calling the locks with this flag and this is the only way I can see it can work with the system that you have put into place.
The bottom line here I have a problem with the locking system you have put into place since it breaks my favorite program. Please try the patch and see if it works for your program. If it does then we/I can work up a patch that works with your program and mine. If you are not willing to look at it am perfectly willing to submit a patch that removes your patch since it is clearly incorrect as it stands.
Please let's work together on this.
I'm not trying to be difficult, don't worry. :) I can take a look at it, but, as I mentioned earlier, I don't currently have access to the machine to compile/test. Could you try out the change I posted earlier that changes the locking behaviour when src == dest? I haven't seen any followup mentioning whether that worked or not, but from what you've mentioned it sounds like it should solve the problem. It's a pseudo-patch, but it should be pretty obvious which part of the Blt() function to apply it to. I'll cook up a patch to implement my locking behaviour fix in Blt and BltFast, but it'll have to wait a couple of hours. I can also see if I can find a demo version of SimCity 3000 to try out. Matt.
Matthew Mastracci wrote:
Tony Lambregts wrote:
Matthew Mastracci wrote:
DDLOCK_NOSYSLOCK prevents DirectDraw from locking "system lock", which is a global 16-bit lock on Windows 9x systems. I don't think this is the right way to fix this.
Check out this page here:
http://msdn.microsoft.com/library/default.asp?url=/library/en-us/dndxgen/htm...
This flag has no effect on Windows 2000 or Wine, since they don't have global Win16 locks (AFAIK).
AFAICS The locking system that you have put in place is exactly that: a "system lock". Not "global 16-bit" perhaps but it definatly is a system lock.
The locking system is actually a per-surface lock. Basically, DirectDraw under Windows requires you to lock a surface in memory before blitting to it. Wine doesn't strictly enforce this locking requirement, but some programs rely on the correct state behaviour.
I think that it really doesn't matter, we can argue symantics forever. What I am really concerned with is that these programs run.
I am fairly familiar with how this program operates. It is the program itself that is calling the locks with this flag and this is the only way I can see it can work with the system that you have put into place.
The bottom line here I have a problem with the locking system you have put into place since it breaks my favorite program. Please try the patch and see if it works for your program. If it does then we/I can work up a patch that works with your program and mine. If you are not willing to look at it am perfectly willing to submit a patch that removes your patch since it is clearly incorrect as it stands.
Please let's work together on this.
I'm not trying to be difficult, don't worry. :)
I can take a look at it, but, as I mentioned earlier, I don't currently have access to the machine to compile/test. Could you try out the change I posted earlier that changes the locking behaviour when src == dest? I haven't seen any followup mentioning whether that worked or not, but from what you've mentioned it sounds like it should solve the problem. It's a pseudo-patch, but it should be pretty obvious which part of the Blt() function to apply it to.
I tried putting in the patch as you demonstrated and I could not get it to work (get the proper behavior from SimCity 3000). That is why I came back with the patch that I did. It is possible I did not understand your intent so I would be happy to try a real patch from you.
I'll cook up a patch to implement my locking behaviour fix in Blt and BltFast, but it'll have to wait a couple of hours. I can also see if I can find a demo version of SimCity 3000 to try out.
http://www.fileshack.com/login.x?url=http%3A%2F%2Fwww.fileshack.com%2Ffile.x... has a free "sign up" demo.(118.2 MB) It took about 15 minutes to download. It displays the same difficulty as the real version. (you are not able to move around by clicking on the playing area). -- Tony Lambregts
I'll cook up a patch to implement my locking behaviour fix in Blt and BltFast, but it'll have to wait a couple of hours. I can also see if I can find a demo version of SimCity 3000 to try out.
http://www.fileshack.com/login.x?url=http%3A%2F%2Fwww.fileshack.com%2Ffile.x...
has a free "sign up" demo.(118.2 MB) It took about 15 minutes to download. It displays the same difficulty as the real version. (you are not able to move around by clicking on the playing area).
The demo is also here: http://www.3dgamers.com/dlselect/games/simcity3/sc3000.zip.html No sign up required..... Tom
--
Tony Lambregts
I tried putting in the patch as you demonstrated and I could not get it to work (get the proper behavior from SimCity 3000). That is why I came back with the patch that I did. It is possible I did not understand your intent so I would be happy to try a real patch from you.
I ran the Sim City 3000 demo and traced the problem down to _Blt() failing to release a lock if an invalid rectangle is passed to it. The attached patch should fix a number of correctness problems, including this one. Give it a shot and let me know if it fixes things for you. I checked both SC3000 and Picasa 2 and both continue to work after this patch is applied. Matt. Index: ddraw_private.h =================================================================== RCS file: /home/wine/wine/dlls/ddraw/ddraw_private.h,v retrieving revision 1.46 diff -u -r1.46 ddraw_private.h --- ddraw_private.h 7 Mar 2005 12:23:34 -0000 1.46 +++ ddraw_private.h 16 Mar 2005 03:29:34 -0000 @@ -277,7 +277,8 @@ RECT lastlockrect; DWORD lastlocktype; BOOL dc_in_use; - BOOL locked; + BOOL locked_read; + BOOL locked_write; HRESULT (*duplicate_surface)(IDirectDrawSurfaceImpl* src, LPDIRECTDRAWSURFACE7* dst); Index: dsurface/dib.c =================================================================== RCS file: /home/wine/wine/dlls/ddraw/dsurface/dib.c,v retrieving revision 1.46 diff -u -r1.46 dib.c --- dsurface/dib.c 10 Mar 2005 11:13:11 -0000 1.46 +++ dsurface/dib.c 16 Mar 2005 03:29:37 -0000 @@ -255,7 +255,8 @@ This->surface_desc.dwFlags |= DDSD_LPSURFACE; - if (This->surface_desc.u4.ddpfPixelFormat.dwFlags & DDPF_FOURCC) { + /* Ensure that DDSD_PITCH is respected for DDPF_FOURCC surfaces too */ + if (This->surface_desc.u4.ddpfPixelFormat.dwFlags & DDPF_FOURCC && !(This->surface_desc.dwFlags & DDSD_PITCH)) { This->surface_desc.lpSurface = VirtualAlloc(NULL, This->surface_desc.u1.dwLinearSize, MEM_COMMIT, PAGE_READWRITE); This->surface_desc.dwFlags |= DDSD_LINEARSIZE; @@ -383,7 +383,7 @@ } /* This is used to factorize the decompression between the Blt and BltFast code */ -static void DoDXTCDecompression(const DDSURFACEDESC2 *sdesc, const DDSURFACEDESC2 *ddesc) +static BOOL DoDXTCDecompression(const DDSURFACEDESC2 *sdesc, const DDSURFACEDESC2 *ddesc) { DWORD rs,rb,rm; DWORD gs,gb,gm; @@ -394,7 +394,7 @@ /* FIXME: We may fake this by rendering the texture into the framebuffer using OpenGL functions and reading back * the framebuffer. This will be slow and somewhat ugly. */ FIXME("Manual S3TC decompression is not supported in native mode\n"); - return; + return FALSE; } rm = ddesc->u4.ddpfPixelFormat.u2.dwRBitMask; @@ -474,6 +474,9 @@ else *((DWORD*)(dst+y*pitch+x*(is16?2:4))) = pixel; } + } else { + /* not handled */ + return FALSE; } #if 0 /* Usefull for debugging */ { @@ -486,6 +489,8 @@ fclose(f); } #endif + + return TRUE; } HRESULT WINAPI @@ -514,7 +519,7 @@ } } - if ((This->locked) || ((src != NULL) && (((IDirectDrawSurfaceImpl *)src)->locked))) { + if ((This->locked_write) || ((src != NULL) && (((IDirectDrawSurfaceImpl *)src)->locked_read))) { WARN(" Surface is busy, returning DDERR_SURFACEBUSY\n"); return DDERR_SURFACEBUSY; } @@ -545,10 +550,11 @@ if ((sdesc.u4.ddpfPixelFormat.dwFlags & DDPF_FOURCC) && (!(ddesc.u4.ddpfPixelFormat.dwFlags & DDPF_FOURCC))) { - DoDXTCDecompression(&sdesc, &ddesc); - goto release; + /* If DoDXTCDecompression handled this blit, we're done */ + if (DoDXTCDecompression(&sdesc, &ddesc)) + goto release; } - + if (rdst) { memcpy(&xdst,rdst,sizeof(xdst)); } else { @@ -581,7 +587,8 @@ (xsrc.right > sdesc.dwWidth) || (xsrc.right < 0) || (xsrc.right < xsrc.left) || (xsrc.bottom < xsrc.top))) { WARN("Application gave us bad source rectangle for Blt.\n"); - return DDERR_INVALIDRECT; + ret = DDERR_INVALIDRECT; + goto release; } /* For the Destination rect, it can be out of bounds on the condition that a clipper is set for the given surface. @@ -593,7 +600,8 @@ (xdst.right > ddesc.dwWidth) || (xdst.right < 0) || (xdst.right < xdst.left) || (xdst.bottom < xdst.top))) { WARN("Application gave us bad destination rectangle for Blt without a clipper set.\n"); - return DDERR_INVALIDRECT; + ret = DDERR_INVALIDRECT; + goto release; } /* Now handle negative values in the rectangles. Warning: only supported for now @@ -990,7 +998,7 @@ TRACE(" srcrect: NULL\n"); } - if ((This->locked) || ((src != NULL) && (((IDirectDrawSurfaceImpl *)src)->locked))) { + if ((This->locked_write) || ((src != NULL) && (((IDirectDrawSurfaceImpl *)src)->locked_read))) { WARN(" Surface is busy, returning DDERR_SURFACEBUSY\n"); return DDERR_SURFACEBUSY; } @@ -1044,8 +1052,11 @@ lock_dst.bottom = dsty + h; /* We need to lock the surfaces, or we won't get refreshes when done. */ + DD_STRUCT_INIT(&ddesc); + DD_STRUCT_INIT(&sdesc); + sdesc.dwSize = sizeof(sdesc); - IDirectDrawSurface7_Lock(src, &lock_src, &sdesc, DDLOCK_READONLY, 0); + if (src) IDirectDrawSurface7_Lock(src, &lock_src, &sdesc, DDLOCK_READONLY, 0); ddesc.dwSize = sizeof(ddesc); IDirectDrawSurface7_Lock(iface, &lock_dst, &ddesc, DDLOCK_WRITEONLY, 0); Index: dsurface/main.c =================================================================== RCS file: /home/wine/wine/dlls/ddraw/dsurface/main.c,v retrieving revision 1.64 diff -u -r1.64 main.c --- dsurface/main.c 7 Mar 2005 12:23:34 -0000 1.64 +++ dsurface/main.c 16 Mar 2005 03:29:39 -0000 @@ -1095,7 +1095,9 @@ } /* If the surface is already locked, return busy */ - if (This->locked) { + if ( ((flags & DDLOCK_READONLY) && This->locked_read) + || ((flags & DDLOCK_WRITEONLY) && This->locked_write) + || ((flags & (DDLOCK_READONLY|DDLOCK_WRITEONLY)) == 0 && (This->locked_read || This->locked_write)) ) { WARN(" Surface is busy, returning DDERR_SURFACEBUSY\n"); return DDERR_SURFACEBUSY; } @@ -1148,7 +1150,8 @@ This->lock_update(This, NULL, flags); } - This->locked = TRUE; + This->locked_read = (flags & DDLOCK_WRITEONLY) == 0; + This->locked_write = (flags & DDLOCK_READONLY) == 0; TRACE("locked surface returning description : \n"); if (TRACE_ON(ddraw)) DDRAW_dump_surface_desc(pDDSD); @@ -1425,12 +1428,12 @@ TRACE("(%p)->Unlock(%p)\n",This,pRect); - if (!This->locked) { + if (!This->locked_read && !This->locked_write) { WARN("Surface not locked - returing DDERR_NOTLOCKED\n"); return DDERR_NOTLOCKED; } - This->locked = FALSE; + This->locked_read = This->locked_write = FALSE; This->unlock_update(This, pRect); if (This->aux_unlock) This->aux_unlock(This->aux_ctx, This->aux_data, pRect);
Matthew Mastracci wrote:
I tried putting in the patch as you demonstrated and I could not get it to work (get the proper behavior from SimCity 3000). That is why I came back with the patch that I did. It is possible I did not understand your intent so I would be happy to try a real patch from you.
I ran the Sim City 3000 demo and traced the problem down to _Blt() failing to release a lock if an invalid rectangle is passed to it. The attached patch should fix a number of correctness problems, including this one. Give it a shot and let me know if it fixes things for you.
I checked both SC3000 and Picasa 2 and both continue to work after this patch is applied.
Matt.
works much better for me (insted of the single BOOL locked). After looking at the code more I was wondering why you could not have used lastlock type for locking. If that is not possible I would suggest that instead of locked_read and locked_write you could use DWORD locktype.
------------------------------------------------------------------------
Index: ddraw_private.h =================================================================== RCS file: /home/wine/wine/dlls/ddraw/ddraw_private.h,v retrieving revision 1.46 diff -u -r1.46 ddraw_private.h --- ddraw_private.h 7 Mar 2005 12:23:34 -0000 1.46 +++ ddraw_private.h 16 Mar 2005 03:29:34 -0000 @@ -277,7 +277,8 @@ RECT lastlockrect; DWORD lastlocktype; BOOL dc_in_use; - BOOL locked; + BOOL locked_read; + BOOL locked_write;
HRESULT (*duplicate_surface)(IDirectDrawSurfaceImpl* src, LPDIRECTDRAWSURFACE7* dst); Index: dsurface/dib.c =================================================================== RCS file: /home/wine/wine/dlls/ddraw/dsurface/dib.c,v retrieving revision 1.46 diff -u -r1.46 dib.c --- dsurface/dib.c 10 Mar 2005 11:13:11 -0000 1.46 +++ dsurface/dib.c 16 Mar 2005 03:29:37 -0000 @@ -255,7 +255,8 @@
This->surface_desc.dwFlags |= DDSD_LPSURFACE;
- if (This->surface_desc.u4.ddpfPixelFormat.dwFlags & DDPF_FOURCC) { + /* Ensure that DDSD_PITCH is respected for DDPF_FOURCC surfaces too */ + if (This->surface_desc.u4.ddpfPixelFormat.dwFlags & DDPF_FOURCC && !(This->surface_desc.dwFlags & DDSD_PITCH)) { This->surface_desc.lpSurface = VirtualAlloc(NULL, This->surface_desc.u1.dwLinearSize, MEM_COMMIT, PAGE_READWRITE); This->surface_desc.dwFlags |= DDSD_LINEARSIZE; @@ -383,7 +383,7 @@ }
/* This is used to factorize the decompression between the Blt and BltFast code */ -static void DoDXTCDecompression(const DDSURFACEDESC2 *sdesc, const DDSURFACEDESC2 *ddesc) +static BOOL DoDXTCDecompression(const DDSURFACEDESC2 *sdesc, const DDSURFACEDESC2 *ddesc) { DWORD rs,rb,rm; DWORD gs,gb,gm; @@ -394,7 +394,7 @@ /* FIXME: We may fake this by rendering the texture into the framebuffer using OpenGL functions and reading back * the framebuffer. This will be slow and somewhat ugly. */ FIXME("Manual S3TC decompression is not supported in native mode\n"); - return; + return FALSE; }
rm = ddesc->u4.ddpfPixelFormat.u2.dwRBitMask; @@ -474,6 +474,9 @@ else *((DWORD*)(dst+y*pitch+x*(is16?2:4))) = pixel; } + } else { + /* not handled */ + return FALSE; } #if 0 /* Usefull for debugging */ { @@ -486,6 +489,8 @@ fclose(f); } #endif + + return TRUE; }
HRESULT WINAPI @@ -514,7 +519,7 @@ } }
- if ((This->locked) || ((src != NULL) && (((IDirectDrawSurfaceImpl *)src)->locked))) { + if ((This->locked_write) || ((src != NULL) && (((IDirectDrawSurfaceImpl *)src)->locked_read))) { WARN(" Surface is busy, returning DDERR_SURFACEBUSY\n"); return DDERR_SURFACEBUSY; } @@ -545,10 +550,11 @@
if ((sdesc.u4.ddpfPixelFormat.dwFlags & DDPF_FOURCC) && (!(ddesc.u4.ddpfPixelFormat.dwFlags & DDPF_FOURCC))) { - DoDXTCDecompression(&sdesc, &ddesc); - goto release; + /* If DoDXTCDecompression handled this blit, we're done */ + if (DoDXTCDecompression(&sdesc, &ddesc)) + goto release; } - + if (rdst) { memcpy(&xdst,rdst,sizeof(xdst)); } else { @@ -581,7 +587,8 @@ (xsrc.right > sdesc.dwWidth) || (xsrc.right < 0) || (xsrc.right < xsrc.left) || (xsrc.bottom < xsrc.top))) { WARN("Application gave us bad source rectangle for Blt.\n"); - return DDERR_INVALIDRECT; + ret = DDERR_INVALIDRECT; + goto release; } /* For the Destination rect, it can be out of bounds on the condition that a clipper is set for the given surface. @@ -593,7 +600,8 @@ (xdst.right > ddesc.dwWidth) || (xdst.right < 0) || (xdst.right < xdst.left) || (xdst.bottom < xdst.top))) { WARN("Application gave us bad destination rectangle for Blt without a clipper set.\n"); - return DDERR_INVALIDRECT; + ret = DDERR_INVALIDRECT; + goto release; }
/* Now handle negative values in the rectangles. Warning: only supported for now @@ -990,7 +998,7 @@ TRACE(" srcrect: NULL\n"); }
- if ((This->locked) || ((src != NULL) && (((IDirectDrawSurfaceImpl *)src)->locked))) { + if ((This->locked_write) || ((src != NULL) && (((IDirectDrawSurfaceImpl *)src)->locked_read))) { WARN(" Surface is busy, returning DDERR_SURFACEBUSY\n"); return DDERR_SURFACEBUSY; } @@ -1044,8 +1052,11 @@ lock_dst.bottom = dsty + h;
/* We need to lock the surfaces, or we won't get refreshes when done. */ + DD_STRUCT_INIT(&ddesc); + DD_STRUCT_INIT(&sdesc); + sdesc.dwSize = sizeof(sdesc); - IDirectDrawSurface7_Lock(src, &lock_src, &sdesc, DDLOCK_READONLY, 0); + if (src) IDirectDrawSurface7_Lock(src, &lock_src, &sdesc, DDLOCK_READONLY, 0); ddesc.dwSize = sizeof(ddesc); IDirectDrawSurface7_Lock(iface, &lock_dst, &ddesc, DDLOCK_WRITEONLY, 0);
Index: dsurface/main.c =================================================================== RCS file: /home/wine/wine/dlls/ddraw/dsurface/main.c,v retrieving revision 1.64 diff -u -r1.64 main.c --- dsurface/main.c 7 Mar 2005 12:23:34 -0000 1.64 +++ dsurface/main.c 16 Mar 2005 03:29:39 -0000 @@ -1095,7 +1095,9 @@ }
/* If the surface is already locked, return busy */ - if (This->locked) { + if ( ((flags & DDLOCK_READONLY) && This->locked_read) + || ((flags & DDLOCK_WRITEONLY) && This->locked_write) + || ((flags & (DDLOCK_READONLY|DDLOCK_WRITEONLY)) == 0 && (This->locked_read || This->locked_write)) ) { WARN(" Surface is busy, returning DDERR_SURFACEBUSY\n"); return DDERR_SURFACEBUSY; } @@ -1148,7 +1150,8 @@ This->lock_update(This, NULL, flags); }
- This->locked = TRUE; + This->locked_read = (flags & DDLOCK_WRITEONLY) == 0; + This->locked_write = (flags & DDLOCK_READONLY) == 0;
TRACE("locked surface returning description : \n"); if (TRACE_ON(ddraw)) DDRAW_dump_surface_desc(pDDSD); @@ -1425,12 +1428,12 @@
TRACE("(%p)->Unlock(%p)\n",This,pRect);
- if (!This->locked) { + if (!This->locked_read && !This->locked_write) { WARN("Surface not locked - returing DDERR_NOTLOCKED\n"); return DDERR_NOTLOCKED; }
- This->locked = FALSE; + This->locked_read = This->locked_write = FALSE; This->unlock_update(This, pRect); if (This->aux_unlock) This->aux_unlock(This->aux_ctx, This->aux_data, pRect);
participants (5)
-
Matthew Mastracci -
Mike Hearn -
Paul Vriens -
Tom Wickline -
Tony Lambregts