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
ehoover@mines.edu
On 3/10/07, Stefan Dösinger <
stefan@codeweavers.com> 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.