Re: wined3d: Allow SetCursorProperties on existing cursor

2007-03-13 Thread Stefan Dösinger
Am Dienstag 13 März 2007 03:44 schrieb Erich Hoover:
 Real Name:
 Erich Hoover
 Changelog:
 wined3d: Allow SetCursorProperties on existing cursor
Looks good to me, except one cosmetic issue:

 if(pCursorBitmap) {
+WINED3DLOCKED_RECT rect;
+.
 /* MSDN: Cursor must be A8R8G8B8 */

You are adding trailing whitespaces here. I think git or AJs filters filter 
them out, but they tend causing merging problems for me if my patches contain 
some of them.


pgp2OB4CuuR2P.pgp
Description: PGP signature



Re: wined3d: Allow SetCursorProperties on existing cursor

2007-03-12 Thread H. Verbeet

On 12/03/07, Erich Hoover [EMAIL PROTECTED] wrote:

Please examine the attached corrections.

Erich Hoover
[EMAIL PROTECTED]


Looks ok to me.




Re: wined3d: Allow SetCursorProperties on existing cursor

2007-03-12 Thread Stefan Dösinger
Am Sonntag 11 März 2007 23:58 schrieb Erich Hoover:
 Is the attached what you mean?
Yes, that looks better. A few minor things:

You can remove the SFLAG_FORCELOAD definition from wined3d_private.h now. 
Also, if I remember right, the cursor must be A8R8G8B8, so instead of reading 
the description from the surface you can read it from the pixel format 
table(getFormatDescEntry). I think pSur-glDescription is set on the first 
PreLoad only. Instead of hardcoding GL_RGBA in the glTexImage2D call use the 
internal format returned by getFormatDescEntry. Finally, 
pSur-resource.wineD3DDevice == This, so you don't have to read the device 
from the surface.

 Erich Hoover
 [EMAIL PROTECTED]

 On 3/11/07, Stefan Dösinger [EMAIL PROTECTED] wrote:
  Am Sonntag 11 März 2007 21:08 schrieb Erich Hoover:
   Yeah, that would make more sense wouldn't it :)  Please see attached
 
  patch.
  If you do it that way, you can remove the PreLoad, LockRect the
  surface(with
  WINED3DLOCK_READONLY), use glTexImage2D to load This-cursorTexture and
  then
  Unlock the surface. This saves uploading - downloading - uploading. If
  you
  use that way, please remove SFLAG_FORCELOAD too.
 
  You should make sure that a correct gl texture unit is activated before
  loading This-cursorTexture with GL_EXTCALL(glActiveTextureARB(X));
  Search through the code how that is done, you have to make sure that
  glActiveTextureARB is supported before using it. You don't have to enable
  GL_TEXTURE_2D to my knowledge to call glTexImage2D, but you have to
  restore
  the originally bound texture or dirtify the sampler you
  modify(IWineD3DDeviceImpl_MarkStateDirty(This, SAMPLER(X)); where X is
  the unit you selected before. (X = 0 is prefered).


pgpA6q6OXc3e3.pgp
Description: PGP signature



Re: wined3d: Allow SetCursorProperties on existing cursor

2007-03-12 Thread H. Verbeet

On 12/03/07, Stefan Dösinger [EMAIL PROTECTED] wrote:

Am Sonntag 11 März 2007 23:58 schrieb Erich Hoover:
You can remove the SFLAG_FORCELOAD definition from wined3d_private.h now.

You'll need to remove that from surface.c as well then.




Re: wined3d: Allow SetCursorProperties on existing cursor

2007-03-12 Thread Erich Hoover

Yeah, that would make more sense wouldn't it :)  Please see attached patch.

Erich Hoover
[EMAIL PROTECTED]

On 3/11/07, H. Verbeet [EMAIL PROTECTED] wrote:


On 11/03/07, Erich Hoover [EMAIL PROTECTED] wrote:
 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]

You're trying to solve the wrong problem :-) The problem is that when
we set the textureName to 0 we essentially kick the GL texture out of
the surface which is then left in a somewhat inconsistent state.
Calling SetCursorProperties again with the same surface will then fail
because the surface no longer has a GL texture associated with it. The
proper way to fix this is to make a copy of the GL texture rather than
playing tricks with the surface loading code.

From bf93490beba1129d2772c2396f293bfe7ea06ec9 Mon Sep 17 00:00:00 2001
From: Erich Hoover [EMAIL PROTECTED](none)
Date: Sun, 11 Mar 2007 14:04:51 -0600
Subject: wined3d: Allow SetCursorProperties on existing cursor
---
 dlls/wined3d/device.c |   35 +--
 1 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/dlls/wined3d/device.c b/dlls/wined3d/device.c
index 0998eec..46efb1a 100644
--- a/dlls/wined3d/device.c
+++ b/dlls/wined3d/device.c
@@ -5239,10 +5239,41 @@ static HRESULT  WINAPI  IWineD3DDeviceIm
  * it after setting the cursor image. Windows doesn't addref the set 
surface, so we can't
  * do this either without creating circular refcount dependencies. 
Copy out the gl texture instead.
  */
-This-cursorTexture = pSur-glDescription.textureName;
 This-cursorWidth = pSur-currentDesc.Width;
 This-cursorHeight = pSur-currentDesc.Height;
-pSur-glDescription.textureName = 0; /* Prevent the texture from being 
changed or deleted */
+if (pSur-glDescription.textureName  pSur-glDescription.level == 0
+   pSur-glDescription.target == GL_TEXTURE_2D) {
+BOOL texEnabled = glIsEnabled(GL_TEXTURE_2D);
+GLint format = pSur-glDescription.glFormat;
+GLint type = pSur-glDescription.glType;
+INT width = This-cursorWidth;
+INT height = This-cursorHeight;
+void *mem;
+
+mem = HeapAlloc(GetProcessHeap(), 0, width * height * 4);
+ENTER_GL();
+glEnable(GL_TEXTURE_2D);
+/* Copy the surface texture into memory */
+glGetTexImage(GL_TEXTURE_2D, 0, format, type, mem);
+checkGLcall(glGetTexImage);
+/* Create a new cursor texture */
+glGenTextures(1, This-cursorTexture);
+checkGLcall(glGenTextures);
+glBindTexture(GL_TEXTURE_2D, This-cursorTexture);
+checkGLcall(glBindTexture);
+/* Copy the memory (surface texture copy) into the cursor texture 
*/
+glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, width, height, 0, format, 
type, mem);
+checkGLcall(glTexImage2D);
+if(!texEnabled)
+glDisable(GL_TEXTURE_2D);
+LEAVE_GL();
+HeapFree(GetProcessHeap(), 0, mem);
+}
+else
+{
+FIXME(A cursor texture was not returned.\n);
+This-cursorTexture = 0;
+}
 }
 
 This-xHotSpot = XHotSpot;
-- 
1.4.1




Re: wined3d: Allow SetCursorProperties on existing cursor

2007-03-12 Thread Erich Hoover

Please examine the attached corrections.

Erich Hoover
[EMAIL PROTECTED]

On 3/11/07, H. Verbeet [EMAIL PROTECTED] wrote:


On 11/03/07, Erich Hoover [EMAIL PROTECTED] wrote:
 Is the attached what you mean?

 Erich Hoover
 [EMAIL PROTECTED]


+if (IWineD3DSurface_LockRect(pCursorBitmap, rect, NULL,
WINED3DLOCK_READONLY) == WINED3D_OK)
I know the current code doesn't do it everywhere either, but you
should probably be using SUCCEEDED() there.

+void *mem = rect.pBits;
...
+glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, width, height,
0, format, type, mem);
That may work, but it's not guaranteed to, see
http://msdn2.microsoft.com/en-us/library/bb206357.aspx

From 43eea315be3358d887f90289560f8cabbcb5d1a5 Mon Sep 17 00:00:00 2001
From: Erich Hoover [EMAIL PROTECTED](none)
Date: Sun, 11 Mar 2007 20:03:39 -0600
Subject: wined3d: Allow SetCursorProperties on existing cursor
---
 dlls/wined3d/device.c |   49 +
 1 files changed, 41 insertions(+), 8 deletions(-)

diff --git a/dlls/wined3d/device.c b/dlls/wined3d/device.c
index 0998eec..919d06d 100644
--- a/dlls/wined3d/device.c
+++ b/dlls/wined3d/device.c
@@ -5215,6 +5215,8 @@ static HRESULT  WINAPI  IWineD3DDeviceIm
 }
 
 if(pCursorBitmap) {
+WINED3DLOCKED_RECT rect;
+
 /* MSDN: Cursor must be A8R8G8B8 */
 if (WINED3DFMT_A8R8G8B8 != pSur-resource.format) {
 ERR((%p) : surface(%p) has an invalid format\n, This, 
pCursorBitmap);
@@ -5229,20 +5231,51 @@ static HRESULT  WINAPI  IWineD3DDeviceIm
 }
 
 /* TODO: MSDN: Cursor sizes must be a power of 2 */
-/* This is to tell our texture code to load a SCRATCH surface. This 
allows us to use out
- * Texture and Blitting code to draw the cursor
- */
-pSur-Flags |= SFLAG_FORCELOAD;
-IWineD3DSurface_PreLoad(pCursorBitmap);
-pSur-Flags = ~SFLAG_FORCELOAD;
+
 /* Do not store the surface's pointer because the application may 
release
  * it after setting the cursor image. Windows doesn't addref the set 
surface, so we can't
  * do this either without creating circular refcount dependencies. 
Copy out the gl texture instead.
  */
-This-cursorTexture = pSur-glDescription.textureName;
 This-cursorWidth = pSur-currentDesc.Width;
 This-cursorHeight = pSur-currentDesc.Height;
-pSur-glDescription.textureName = 0; /* Prevent the texture from being 
changed or deleted */
+if (SUCCEEDED(IWineD3DSurface_LockRect(pCursorBitmap, rect, NULL, 
WINED3DLOCK_READONLY)))
+{
+GLint format = pSur-glDescription.glFormat;
+GLint type = pSur-glDescription.glType;
+char *mem, *bits = (char *)rect.pBits;
+INT height = This-cursorHeight;
+INT width = This-cursorWidth;
+INT bpp = pSur-bytesPerPixel;
+INT i;
+
+/* Reformat the texture memory (pitch and width can be different) 
*/
+mem = HeapAlloc(GetProcessHeap(), 0, width * height * bpp);
+for(i = 0; i  height; i++)
+memcpy(mem[width * bpp * i], bits[rect.Pitch * i], width * 
bpp);
+IWineD3DSurface_UnlockRect(pCursorBitmap);
+ENTER_GL();
+/* Make sure that a proper texture unit is selected */
+if (GL_SUPPORT(ARB_MULTITEXTURE)) {
+GL_EXTCALL(glActiveTextureARB(GL_TEXTURE0_ARB));
+checkGLcall(glActiveTextureARB);
+}
+IWineD3DDeviceImpl_MarkStateDirty(pSur-resource.wineD3DDevice, 
STATE_SAMPLER(0));
+/* Create a new cursor texture */
+glGenTextures(1, This-cursorTexture);
+checkGLcall(glGenTextures);
+glBindTexture(GL_TEXTURE_2D, This-cursorTexture);
+checkGLcall(glBindTexture);
+/* Copy the bitmap memory into the cursor texture */
+glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, width, height, 0, format, 
type, mem);
+HeapFree(GetProcessHeap(), 0, mem);
+checkGLcall(glTexImage2D);
+LEAVE_GL();
+}
+else
+{
+FIXME(A cursor texture was not returned.\n);
+This-cursorTexture = 0;
+}
 }
 
 This-xHotSpot = XHotSpot;
-- 
1.4.1




Re: wined3d: Allow SetCursorProperties on existing cursor

2007-03-12 Thread Erich Hoover

Is the attached what you mean?

Erich Hoover
[EMAIL PROTECTED]

On 3/11/07, Stefan Dösinger [EMAIL PROTECTED] wrote:


Am Sonntag 11 März 2007 21:08 schrieb Erich Hoover:
 Yeah, that would make more sense wouldn't it :)  Please see attached
patch.
If you do it that way, you can remove the PreLoad, LockRect the
surface(with
WINED3DLOCK_READONLY), use glTexImage2D to load This-cursorTexture and
then
Unlock the surface. This saves uploading - downloading - uploading. If
you
use that way, please remove SFLAG_FORCELOAD too.

You should make sure that a correct gl texture unit is activated before
loading This-cursorTexture with GL_EXTCALL(glActiveTextureARB(X)); Search
through the code how that is done, you have to make sure that
glActiveTextureARB is supported before using it. You don't have to enable
GL_TEXTURE_2D to my knowledge to call glTexImage2D, but you have to
restore
the originally bound texture or dirtify the sampler you
modify(IWineD3DDeviceImpl_MarkStateDirty(This, SAMPLER(X)); where X is the
unit you selected before. (X = 0 is prefered).


From 8bbe5304a263338ac6ccada3bdb3612bf968120d Mon Sep 17 00:00:00 2001
From: Erich Hoover [EMAIL PROTECTED](none)
Date: Sun, 11 Mar 2007 16:55:06 -0600
Subject: wined3d: Allow SetCursorProperties on existing cursor
---
 dlls/wined3d/device.c |   42 ++
 1 files changed, 34 insertions(+), 8 deletions(-)

diff --git a/dlls/wined3d/device.c b/dlls/wined3d/device.c
index 0998eec..561b7e3 100644
--- a/dlls/wined3d/device.c
+++ b/dlls/wined3d/device.c
@@ -5215,6 +5215,8 @@ static HRESULT  WINAPI  IWineD3DDeviceIm
 }
 
 if(pCursorBitmap) {
+WINED3DLOCKED_RECT rect;
+
 /* MSDN: Cursor must be A8R8G8B8 */
 if (WINED3DFMT_A8R8G8B8 != pSur-resource.format) {
 ERR((%p) : surface(%p) has an invalid format\n, This, 
pCursorBitmap);
@@ -5229,20 +5231,44 @@ static HRESULT  WINAPI  IWineD3DDeviceIm
 }
 
 /* TODO: MSDN: Cursor sizes must be a power of 2 */
-/* This is to tell our texture code to load a SCRATCH surface. This 
allows us to use out
- * Texture and Blitting code to draw the cursor
- */
-pSur-Flags |= SFLAG_FORCELOAD;
-IWineD3DSurface_PreLoad(pCursorBitmap);
-pSur-Flags = ~SFLAG_FORCELOAD;
+
 /* Do not store the surface's pointer because the application may 
release
  * it after setting the cursor image. Windows doesn't addref the set 
surface, so we can't
  * do this either without creating circular refcount dependencies. 
Copy out the gl texture instead.
  */
-This-cursorTexture = pSur-glDescription.textureName;
 This-cursorWidth = pSur-currentDesc.Width;
 This-cursorHeight = pSur-currentDesc.Height;
-pSur-glDescription.textureName = 0; /* Prevent the texture from being 
changed or deleted */
+if (IWineD3DSurface_LockRect(pCursorBitmap, rect, NULL, 
WINED3DLOCK_READONLY) == WINED3D_OK)
+{
+GLint format = pSur-glDescription.glFormat;
+GLint type = pSur-glDescription.glType;
+INT width = This-cursorWidth;
+INT height = This-cursorHeight;
+void *mem = rect.pBits;
+
+ENTER_GL();
+/* Make sure that a proper texture unit is selected */
+if (GL_SUPPORT(ARB_MULTITEXTURE)) {
+GL_EXTCALL(glActiveTextureARB(GL_TEXTURE0_ARB));
+checkGLcall(glActiveTextureARB);
+}
+IWineD3DDeviceImpl_MarkStateDirty(pSur-resource.wineD3DDevice, 
STATE_SAMPLER(0));
+/* Create a new cursor texture */
+glGenTextures(1, This-cursorTexture);
+checkGLcall(glGenTextures);
+glBindTexture(GL_TEXTURE_2D, This-cursorTexture);
+checkGLcall(glBindTexture);
+/* Copy the memory (surface texture copy) into the cursor texture 
*/
+glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, width, height, 0, format, 
type, mem);
+checkGLcall(glTexImage2D);
+LEAVE_GL();
+IWineD3DSurface_UnlockRect(pCursorBitmap);
+}
+else
+{
+FIXME(A cursor texture was not returned.\n);
+This-cursorTexture = 0;
+}
 }
 
 This-xHotSpot = XHotSpot;
-- 
1.4.1




Re: wined3d: Allow SetCursorProperties on existing cursor

2007-03-11 Thread Stefan Dösinger
Am Sonntag 11 März 2007 03:08 schrieb Erich Hoover:
 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.
Yes; The existing d3d9 tests do not check what happens when the surface is 
actually modified, and they do not draw an actual cursor, because the device 
test just checks return values and not the actual drawing. But the test shows 
that the surface can be destroyed without modifying the cursor.

I can try to extend the test cases for modifying the surface if you want to?


pgp8kx8TXmMWN.pgp
Description: PGP signature



Re: wined3d: Allow SetCursorProperties on existing cursor

2007-03-11 Thread Erich Hoover

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

Re: wined3d: Allow SetCursorProperties on existing cursor

2007-03-11 Thread H. Verbeet

On 11/03/07, Erich Hoover [EMAIL PROTECTED] wrote:

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]


You're trying to solve the wrong problem :-) The problem is that when
we set the textureName to 0 we essentially kick the GL texture out of
the surface which is then left in a somewhat inconsistent state.
Calling SetCursorProperties again with the same surface will then fail
because the surface no longer has a GL texture associated with it. The
proper way to fix this is to make a copy of the GL texture rather than
playing tricks with the surface loading code.




Re: wined3d: Allow SetCursorProperties on existing cursor

2007-03-11 Thread Stefan Dösinger
Am Sonntag 11 März 2007 21:08 schrieb Erich Hoover:
 Yeah, that would make more sense wouldn't it :)  Please see attached patch.
If you do it that way, you can remove the PreLoad, LockRect the surface(with 
WINED3DLOCK_READONLY), use glTexImage2D to load This-cursorTexture and then 
Unlock the surface. This saves uploading - downloading - uploading. If you 
use that way, please remove SFLAG_FORCELOAD too.

You should make sure that a correct gl texture unit is activated before 
loading This-cursorTexture with GL_EXTCALL(glActiveTextureARB(X)); Search 
through the code how that is done, you have to make sure that 
glActiveTextureARB is supported before using it. You don't have to enable 
GL_TEXTURE_2D to my knowledge to call glTexImage2D, but you have to restore 
the originally bound texture or dirtify the sampler you 
modify(IWineD3DDeviceImpl_MarkStateDirty(This, SAMPLER(X)); where X is the 
unit you selected before. (X = 0 is prefered).


pgpySCtA8vWNO.pgp
Description: PGP signature



Re: wined3d: Allow SetCursorProperties on existing cursor

2007-03-11 Thread H. Verbeet

On 11/03/07, Erich Hoover [EMAIL PROTECTED] wrote:

Is the attached what you mean?

Erich Hoover
[EMAIL PROTECTED]




+if (IWineD3DSurface_LockRect(pCursorBitmap, rect, NULL,

WINED3DLOCK_READONLY) == WINED3D_OK)
I know the current code doesn't do it everywhere either, but you
should probably be using SUCCEEDED() there.


+void *mem = rect.pBits;

...

+glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, width, height,

0, format, type, mem);
That may work, but it's not guaranteed to, see
http://msdn2.microsoft.com/en-us/library/bb206357.aspx




Re: wined3d: Allow SetCursorProperties on existing cursor

2007-03-10 Thread Stefan Dösinger
 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.


pgp8QvedqIKI7.pgp
Description: PGP signature



Re: wined3d: Allow SetCursorProperties on existing cursor

2007-03-10 Thread Erich Hoover


Am Samstag 10 März 2007 06:10 schrieb Erich Hoover:
* Real Name:
** Erich Hoover
** Description:
** The SetCursorProperties call needs to be capable of being performed on
** existing cursors.  The current behavior removes the cursor handle at the
** beginning of any SetCursorProperties call (and on Uninit3D).  The removal
** of the cursor on SetCursorProperties is counter to the documentation and
** causes some applications to lose the cursor when SetCursorProperties is
** re-issued (See Bug #7619 http://bugs.winehq.org/show_bug.cgi?id=7619).
** This patch corrects this issue and permits SetCursorProperties to retrieve
** the gl texture of an old cursor.
** Changelog:
** wined3d: Allow SetCursorProperties on existing cursor
*This is not completely correct:

The application can Release the d3d surface after setting the cursor. This
would also delete the gl texture used to draw the cursor. Also, the apps can
potentially change the surface and call PreLoad. This would change the cursor
without SetCursorProperties(The cursor texture does not have to be in
D3DPOOL_SCRATCH, so the app can preload it manually).

But you are right, a non-dirty surface causes issues with the current code. I
think you can fix that by modifying LoadTexture not to release the surface
memory is SFLAG_FORCELOAD is passed. (Or better, fix the SFLAG_DONOTFREE
macro in wined3d_private.h). Then, when SetCursorProperties sets the surface
texture to 0 remove SFLAG_INTEXTURE from the surface flags.

The API documentation doesn't say anything about an application releasing

the d3d surface, 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.  What I did see as requiring a change is the call to
glDeleteTextures(1, This-cursorTexture); at the beginning of
SetCursorProperties.  Maybe I'm not understanding you correctly, but in my
mind it is necessary to remove this call so that applications that re-use
the same cursor can do so.  This change (in combination with the change for
non-dirty surfaces) was also experimentally verified to fix the problem I
was experiencing (See Bug #7619http://bugs.winehq.org/show_bug.cgi?id=7619).
I will look into changing/implementing the flags to eliminate the non-dirty
surface problem.

Erich Hoover
[EMAIL PROTECTED]