I ran all of the d3d9 tests and did not get any errors with my patch applied. I did get an error when I ran your modified visual.c test, but I get the same errors with and without my patch.
Test before patch:
fixme:d3d:IWineD3DDeviceImpl_GetAvailableTextureMem (0x167ec8) : stub, simulating 64MB for now, returning 64MB left visual.c:191: Test failed: IDirect3DDevice9_ColorFill failed with 88760096 visual.c:212: Test failed: IDirect3DDevice9_SetCursorProperties failed with 8876086c visual: 26 tests executed (0 marked as todo, 2 failures), 0 skipped.
Test after patch:
fixme:d3d:IWineD3DDeviceImpl_GetAvailableTextureMem (0x167ec8) : stub, simulating 64MB for now, returning 64MB left visual.c:191: Test failed: IDirect3DDevice9_ColorFill failed with 88760096 visual.c:212: Test failed: IDirect3DDevice9_SetCursorProperties failed with 8876086c visual: 26 tests executed (0 marked as todo, 2 failures), 0 skipped.
So, which change exactly are you concerned about? Changes are: 1) The removal of "glDeleteTextures(1, &This->cursorTexture);" in device.c This change should just keep SetCursorProperties from deleting the gl texture. By removing this call an application can set the cursor to A, change the cursor to B, then change the cursor back to A without having the texture deleted. 2) The temporary allocation of This->glDescription.textureName in surface.c This change should allow SetCursorProperties to retrieve "cursor A" the second time in the "A, B, A" sequence. Without this change, SetCursorProperties gets back a texture of "0" from the IWineD3DSurface_PreLoad call. This allocation is temporary since SetCursorProperties immediately resets that value: This->cursorTexture = pSur->glDescription.textureName; ... pSur->glDescription.textureName = 0; /* Prevent the texture from being changed or deleted */ I don't think I'm missing anything here... However, the implementation of item #2 could be changed around by using flags, as you discussed in a previous email, and this would seem more consistent with the intent of SFLAG_FORCELOAD (see the attached patch). Erich Hoover [EMAIL PROTECTED] On 3/10/07, Stefan Dösinger <[EMAIL PROTECTED]> wrote:
> The API documentation doesn't say anything about an application releasing > the d3d surface, There is a test for that somewhere in dlls/d3d9/tests/device.c (or some other test file in there). > but the documentation in the code seems to indicate that > the gl texture needs to remain when the surface is deleted - I did nothing > to change this. But IWineD3DSurfaceImpl_Release will destroy the gl texture of a surface when the surface is destroyed. Also, if the gl surface still knows the texture, the following could cause an issue: SetCursorProperties(cursorSurface, ...); <render some stuff> LockRect(cursorSurface, ...); <write some thing into the surface> UnlockRect(cursorSurface); PreLoad(cursorSurface); The PreLoad will then change the cursor without SetCursorProperties beeing called. This should not happen. I tested that with some hacky test. Unfortunately this cannot be automated in the visual tests because cursors do not show up in screenshots. This is my hacky cursor test app: http://stud4.tuwien.ac.at/~e0526822/visual.c The successor of that file without the cursor tests is now in dlls/d3d9/tests/visual.c. I think this visual.c version is not the version I used for checking later surface changes.
From b27ae021b3c52d6fc1bf918098219160dedefa5f Mon Sep 17 00:00:00 2001 From: Erich Hoover <[EMAIL PROTECTED](none)> Date: Sat, 10 Mar 2007 19:02:16 -0700 Subject: wined3d: Allow SetCursorProperties on existing cursor --- dlls/wined3d/device.c | 8 +------- dlls/wined3d/texture.c | 23 ++++++++++++++--------- 2 files changed, 15 insertions(+), 16 deletions(-) diff --git a/dlls/wined3d/device.c b/dlls/wined3d/device.c index 0998eec..0339e8d 100644 --- a/dlls/wined3d/device.c +++ b/dlls/wined3d/device.c @@ -5207,13 +5207,6 @@ static HRESULT WINAPI IWineD3DDeviceIm TRACE("(%p) : Spot Pos(%u,%u)\n", This, XHotSpot, YHotSpot); /* some basic validation checks */ - if(This->cursorTexture) { - ENTER_GL(); - glDeleteTextures(1, &This->cursorTexture); - LEAVE_GL(); - This->cursorTexture = 0; - } - if(pCursorBitmap) { /* MSDN: Cursor must be A8R8G8B8 */ if (WINED3DFMT_A8R8G8B8 != pSur->resource.format) { @@ -5243,6 +5236,7 @@ static HRESULT WINAPI IWineD3DDeviceIm This->cursorWidth = pSur->currentDesc.Width; This->cursorHeight = pSur->currentDesc.Height; pSur->glDescription.textureName = 0; /* Prevent the texture from being changed or deleted */ + /* NOTE: It is also important to keep the texture between SetCursorProperties calls */ } This->xHotSpot = XHotSpot; diff --git a/dlls/wined3d/texture.c b/dlls/wined3d/texture.c index e9a78cc..4c5cdd5 100644 --- a/dlls/wined3d/texture.c +++ b/dlls/wined3d/texture.c @@ -95,6 +95,7 @@ static void WINAPI IWineD3DTextureImpl_P /* Override the IWineD3DResource PreLoad method */ unsigned int i; + BOOL loadPerformed = FALSE; BOOL setGlTextureDesc = FALSE; IWineD3DTextureImpl *This = (IWineD3DTextureImpl *)iface; @@ -104,22 +105,26 @@ static void WINAPI IWineD3DTextureImpl_P IWineD3DTexture_BindTexture(iface); ENTER_GL(); - /* If were dirty then reload the surfaces */ - if(This->baseTexture.dirty) { - for (i = 0; i < This->baseTexture.levels; i++) { - if(setGlTextureDesc) + for (i = 0; i < This->baseTexture.levels; i++) { + IWineD3DSurfaceImpl *pSur = (IWineD3DSurfaceImpl *) This->surfaces[i]; + + /* If a texture is dirty, or a surface is forcing a load, then reload the surface */ + if (This->baseTexture.dirty || pSur->Flags & SFLAG_FORCELOAD) + { + if(setGlTextureDesc || pSur->Flags & SFLAG_FORCELOAD) IWineD3DSurface_SetGlTextureDesc(This->surfaces[i], This->baseTexture.textureName, IWineD3DTexture_GetTextureDimensions(iface)); IWineD3DSurface_LoadTexture(This->surfaces[i]); + loadPerformed = TRUE; } - - /* No longer dirty */ - This->baseTexture.dirty = FALSE; - } else { - TRACE("(%p) Texture not dirty, nothing to do\n" , iface); } + + /* No longer dirty */ + This->baseTexture.dirty = FALSE; LEAVE_GL(); + if(!loadPerformed) + TRACE("(%p) Texture not dirty, nothing to do\n" , iface); return ; } -- 1.4.1