Regression (SimCity 3000 is Broke )
Tony Lambregts
tony.lambregts at gmail.com
Tue Mar 15 23:01:51 CST 2005
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);
More information about the wine-devel
mailing list