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



Reply via email to