Regression (SimCity 3000 is Broke )

Matthew Mastracci matt at aclaro.com
Tue Mar 15 21:39:51 CST 2005


> 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.
-------------- next part --------------
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);


More information about the wine-devel mailing list