May 2011 - Wine-Devel - list.winehq.org

Re: [1/8] d3d8: Use D3DPOOL <-> WINED3DPOOL enum conversion functions instead of implicit conversions (clang)

by Alexandre Julliard

Stefan Dösinger <stefandoesinger(a)gmx.at> writes: > Am 31.05.2011 um 11:10 schrieb Alexandre Julliard: >> I still don't see the point of that kind of switch statement. To be >> honest I don't see either why we even need different enum types in the >> first place. > Because for most enums the d3d8 ones differ from the d3d9 ones but > have the same name. We cannot include d3d9types.h in d3d8 and ddraw > but we still have to talk to wined3d. Most enums are identical, or just have an extra value or two. I don't see why you can't pass either type to wined3d since they have the same name. -- Alexandre Julliard julliard(a)winehq.org

May 31, 2011

Re: [1/8] d3d8: Use D3DPOOL <-> WINED3DPOOL enum conversion functions instead of implicit conversions (clang)

by Stefan Dösinger

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Am 31.05.2011 um 11:10 schrieb Alexandre Julliard: > I still don't see the point of that kind of switch statement. To be > honest I don't see either why we even need different enum types in the > first place. Because for most enums the d3d8 ones differ from the d3d9 ones but have the same name. We cannot include d3d9types.h in d3d8 and ddraw but we still have to talk to wined3d. -----BEGIN PGP SIGNATURE----- Version: GnuPG/MacGPG2 v2.0.17 (Darwin) iQIcBAEBAgAGBQJN5LsaAAoJEN0/YqbEcdMwMBYQAJfhdtFoBi3dq0nDvK5PwdN3 V5elaAUp3KTwugfmhy9TL1VhPY9QyOQ0x0/dXbd9dqH1cNn54IvU8rl0tcRqlW5B pmeD2fh+eywSGaw2UBhQJjQTHOcRwXo8CJTb/6Z5D10Gt0YNcUAyaQOgQkThiIbI ttItwkM6N9HF+lmWSQ66ue6jNe+8lf0b9pC+r5I8HLi/fUL0L4PglgKTBP/rs8kq YgQmiSRuJ+tP4bzlr8XyuoZEynnL2QJG/60q3M6qC9iA+BR1tHHJsAXQflErICZr v/MrQwbiBjAASXMdmRqxAF9hyA6Qv9cRKbOtEIeI8GPoOg2/V3QbNRiI2GcLMz6s O8zPUxzwbGfuJDQKh04cAsMAIw32JNh1IAPz+GcS9oXzVvCuH+PlfQiLGz+Lxukf jwup8N/3UojvVHU6C8PUDgYV6+eOm0Pbr9j2BUVIj+nliycQlsnrj6P/zthI+r9m tgh152AV8RN52zBZTQlf258KqV1Thw8LnCAkA+a4mG1Bz6gGh4u/o9UDFY8rbENi tSEugEl0w6OecHJBOHTpOPenHHyE3KKCwNZ3bsC/TE4AnuXUMGLoPw88dWCfnrS9 gtvX2pyIpdbjuktAaAPoM15QNZvV16+hmstGXNsrEPgi/p+U6JHw5GvAPtgccPDD iBVo1M4T31Jph8Q3VcGj =snIO -----END PGP SIGNATURE-----

May 31, 2011

Re: kernel32: Call Fls callback (try 2)

by Alexandre Julliard

André Hentschel <nerv(a)dawncrow.de> writes: > + if (ret && callback) callback(NtCurrentTeb()->FlsSlots[index]); > + /* FIXME: add equivalent of ThreadZeroTlsCell here */ > + if (NtCurrentTeb()->FlsSlots) NtCurrentTeb()->FlsSlots[index] = 0; It's not that easy... -- Alexandre Julliard julliard(a)winehq.org

May 31, 2011

Re: [1/8] d3d8: Use D3DPOOL <-> WINED3DPOOL enum conversion functions instead of implicit conversions (clang)

by Alexandre Julliard

Frédéric Delanoy <frederic.delanoy(a)gmail.com> writes: > +static inline D3DPOOL d3dpool_from_wined3dpool(WINED3DPOOL type) > +{ > + switch (type) > + { > + case WINED3DPOOL_DEFAULT: return D3DPOOL_DEFAULT; > + case WINED3DPOOL_MANAGED: return D3DPOOL_MANAGED; > + case WINED3DPOOL_SYSTEMMEM: return D3DPOOL_SYSTEMMEM; > + case WINED3DPOOL_SCRATCH: return D3DPOOL_SCRATCH; > + case WINED3DPOOL_FORCE_DWORD: return D3DPOOL_FORCE_DWORD; > + } > + return (D3DPOOL) type; > +} > + > +static inline WINED3DPOOL wined3dpool_from_d3dpool(D3DPOOL type) > +{ > + switch (type) > + { > + case D3DPOOL_DEFAULT: return WINED3DPOOL_DEFAULT; > + case D3DPOOL_MANAGED: return WINED3DPOOL_MANAGED; > + case D3DPOOL_SYSTEMMEM: return WINED3DPOOL_SYSTEMMEM; > + case D3DPOOL_SCRATCH: return WINED3DPOOL_SCRATCH; > + case D3DPOOL_FORCE_DWORD: return WINED3DPOOL_FORCE_DWORD; > + } > + return (WINED3DPOOL) type; > +} I still don't see the point of that kind of switch statement. To be honest I don't see either why we even need different enum types in the first place. -- Alexandre Julliard julliard(a)winehq.org

May 31, 2011

Re: advapi: test and fix GetUserName() in case where buffer is too small

by Hans Leidekker

On Mon, 2011-05-30 at 21:13 -0700, Dan Kegel wrote: > @@ -91,7 +91,7 @@ GetUserNameW( LPWSTR lpszName, LPDWORD lpSize ) > > if (len > *lpSize) > { > - SetLastError(ERROR_MORE_DATA); > + SetLastError(ERROR_INSUFFICIENT_BUFFER); GetUserNameA also needs to be fixed. It sets ERROR_MORE_DATA when WideCharToMultiByte fails, which can simply be removed because that function will set ERROR_INSUFFICIENT_BUFFER itself if the buffer is too small. And there are at least two cases, in dlls/crypt32/protectdata.c and dlls/msi/package.c, where we depend on the wrong error.

May 31, 2011

Re: bug in GetUserNameW error return values

by James McKenzie

On 5/30/11 11:52 AM, Kevin Hendricks wrote: > Hi, > > There is a small bug in the wine implementation of GetUserNameW (and the same fix is needed in GetUserNameA). > > From: > dlls/advapi32/advapi.c starting at line 92 > > > if (len> *lpSize) > { > SetLastError(ERROR_MORE_DATA); > *lpSize = len; > return FALSE; > } > > > It seems the actual GetUserNameW returns 122 not 234 which maps to ERROR_INSUFFICIENT_BUFFER and not ERROR_MORE_DATA > File a bug report with all of these details. Developers do not work bugs from the mailing list. James McKenzie

May 31, 2011

Re: GSoC-2011: Implement Missing Mesh Functions in Wine’s D3DX9

by Matteo Bruni

2011/5/30 Michael Mc Donnell <michael(a)mcdonnell.dk>: > Thanks for the comments Matteo! I have a few questions below. > > On Mon, May 30, 2011 at 2:45 PM, Matteo Bruni <matteo.mystral(a)gmail.com> wrote: >> 2011/5/29 Michael Mc Donnell <michael(a)mcdonnell.dk>: >>> On Sat, May 28, 2011 at 10:08 PM, Stefan Dösinger >>> <stefandoesinger(a)gmx.at> wrote: >>>> On Saturday 28 May 2011 19:55:55 you wrote: >>>> >>>>> It was implemented by setting the vertex declaration to null when the >>>>> declaration is invalid. That seems to match what happens on Windows, >>>>> because if a program has set an invalid declaration and then calls >>>>> GetDeclaration or DrawSubset it will fail with an access violation. >>>>> Should Wine also fail with an access violation or should it be >>>>> slightly more graceful and return E_FAIL instead (which my new patch >>>>> does)? >>>> Yes, I think returning E_FAIL is a good idea, but write a WARN if >>>> CreateVertexDeclaration fails or when you catch a NULL vdecl. A WARN will not >>>> be shown by default(unlike a FIXME), it is used when the app does something >>>> fishy but you think you handled it correctly. >>>> >>>>> + if (FAILED(hr)) >>>>> + new_vertex_declaration = NULL; >>>>> + >>>>> + /* Free old vertex declaration */ >>>>> + if (This->vertex_declaration) >>>>> + IDirect3DVertexDeclaration9_Release(This->vertex_declaration); >>>> In this situation you can just release the old decl before calling >>>> CreateVertexDeclaration and pass &This->vertex_declaration to CreateVdecl. >>>> That simplified the code a bit. Don't forget to set This->vertex_declaration to >>>> NULL in case of an error though >>> >>> Ok I've simplified the code. I've also inserted three WARNs. I've >>> skipped a WARN when freeing the mesh because that works fine on >>> Windows with an invalid declaration. >>> >>>>> + /* Set the position type to color instead of float3 (invalid >>>> declaration) */ >>>>> + hr = mesh->lpVtbl->UpdateSemantics(mesh, declaration_wrong_type_color); >>>>> + todo_wine ok(hr == D3D_OK, "Test UpdateSematics wrong type color, " >>>>> + "got %#x expected D3D_OK\n", hr); >>>> Is this really invalid? I don't know it on top of my head, but in general >>>> types and usage are independent, especially with shaders. I don't see a check >>>> in d3d9 or wined3d for this case. >>> >>> You're right it isn't invalid. It doesn't crash, it just doesn't draw >>> anything. I've change that comment and re-ordered the tests. >>> >>>> Beyond that I think the patches look OK. I'd recommend to send them to wine- >>>> devel one more time and then next week to wine-patches. Henri is on Vacation >>>> this week, and he's usually more picky than I am. I also hope some of the >>>> people who regularly contribute to d3dx9 will have a look and comment. >>> >>> Ok, I've attached the newest version. I'll let the patches sit here >>> for a week, and start working on the other functions in the mean time. >>> >> >> The patches are OK with me, just some nitpicking: > > That sounds great :-) > >> + testContext = HeapAlloc(GetProcessHeap(), 0, sizeof(struct TestContext)); >> >> We usually use something like HeapAlloc(GetProcessHeap(), 0, >> sizeof(*testContext)); instead. > > Ok, I guess that's better in case the type of the variable changes at > some point? > Yes, that's a reason. The other one is simply consistency. >> I think you can remove some obvious comments (like /* Create the test >> mesh */). Also, check your whitespaces. > > Is it too much or too little whitespace? Could you give me an example? + todo_wine ok(hr == D3D_OK, "Test UpdateSematics for bigger vertex size, " + "got %#x expected D3D_OK\n", hr); Here you put 4 spaces on the line continuation, which is neither the "align to be below the '(' + 1 space" used in most of the file, nor the 8 spaces used in wined3d. The d3dx9_36 sources aren't known for a nice and consistent coding style (I'm sure you noticed that), so that's mostly trying to not mix even more style variants... There was also something wrong with spaces in a comment that caught my eye, I think. I told you I was nitpicking... :) > > Thanks! >

May 30, 2011

Re: [1/2] README: Update Linux requirements

by Wolfram Sang

> I also thought about moving 2.4.x in one context with 2.2.x, but at > least no one else did so i didn't. Please do. Despite the question if it is "good enough", 2.4 is for sure not considered "current" anymore.

May 30, 2011

GSoC-2011: Implement Missing Mesh Functions in Wine’s D3DX9

by Michael Mc Donnell

Thanks for the comments Matteo! I have a few questions below. On Mon, May 30, 2011 at 2:45 PM, Matteo Bruni <matteo.mystral(a)gmail.com> wrote: > 2011/5/29 Michael Mc Donnell <michael(a)mcdonnell.dk>: >> On Sat, May 28, 2011 at 10:08 PM, Stefan Dösinger >> <stefandoesinger(a)gmx.at> wrote: >>> On Saturday 28 May 2011 19:55:55 you wrote: >>> >>>> It was implemented by setting the vertex declaration to null when the >>>> declaration is invalid. That seems to match what happens on Windows, >>>> because if a program has set an invalid declaration and then calls >>>> GetDeclaration or DrawSubset it will fail with an access violation. >>>> Should Wine also fail with an access violation or should it be >>>> slightly more graceful and return E_FAIL instead (which my new patch >>>> does)? >>> Yes, I think returning E_FAIL is a good idea, but write a WARN if >>> CreateVertexDeclaration fails or when you catch a NULL vdecl. A WARN will not >>> be shown by default(unlike a FIXME), it is used when the app does something >>> fishy but you think you handled it correctly. >>> >>>> + if (FAILED(hr)) >>>> + new_vertex_declaration = NULL; >>>> + >>>> + /* Free old vertex declaration */ >>>> + if (This->vertex_declaration) >>>> + IDirect3DVertexDeclaration9_Release(This->vertex_declaration); >>> In this situation you can just release the old decl before calling >>> CreateVertexDeclaration and pass &This->vertex_declaration to CreateVdecl. >>> That simplified the code a bit. Don't forget to set This->vertex_declaration to >>> NULL in case of an error though >> >> Ok I've simplified the code. I've also inserted three WARNs. I've >> skipped a WARN when freeing the mesh because that works fine on >> Windows with an invalid declaration. >> >>>> + /* Set the position type to color instead of float3 (invalid >>> declaration) */ >>>> + hr = mesh->lpVtbl->UpdateSemantics(mesh, declaration_wrong_type_color); >>>> + todo_wine ok(hr == D3D_OK, "Test UpdateSematics wrong type color, " >>>> + "got %#x expected D3D_OK\n", hr); >>> Is this really invalid? I don't know it on top of my head, but in general >>> types and usage are independent, especially with shaders. I don't see a check >>> in d3d9 or wined3d for this case. >> >> You're right it isn't invalid. It doesn't crash, it just doesn't draw >> anything. I've change that comment and re-ordered the tests. >> >>> Beyond that I think the patches look OK. I'd recommend to send them to wine- >>> devel one more time and then next week to wine-patches. Henri is on Vacation >>> this week, and he's usually more picky than I am. I also hope some of the >>> people who regularly contribute to d3dx9 will have a look and comment. >> >> Ok, I've attached the newest version. I'll let the patches sit here >> for a week, and start working on the other functions in the mean time. >> > > The patches are OK with me, just some nitpicking: That sounds great :-) > + testContext = HeapAlloc(GetProcessHeap(), 0, sizeof(struct TestContext)); > > We usually use something like HeapAlloc(GetProcessHeap(), 0, > sizeof(*testContext)); instead. Ok, I guess that's better in case the type of the variable changes at some point? > I think you can remove some obvious comments (like /* Create the test > mesh */). Also, check your whitespaces. Is it too much or too little whitespace? Could you give me an example? Thanks!

May 30, 2011

Re: [1/2] README: Update Linux requirements

by André Hentschel

Am 30.05.2011 20:51, schrieb Michael Stefaniuc: > Hello André, > > On 05/30/2011 08:09 PM, André Hentschel wrote: >> now that it's official >> --- >> README | 2 +- >> 1 files changed, 1 insertions(+), 1 deletions(-) >> >> diff --git a/README b/README >> index baa6ab3..51a080b 100644 >> --- a/README >> +++ b/README >> @@ -41,7 +41,7 @@ support kernel threads may be supported in the future. >> Linux info: >> While Linux 2.2.x should still work and Linux 2.0.x may still work >> (older 2.0.x versions had thread-related crashes), >> - it's best to have a current kernel such as 2.4.x or 2.6.x. >> + it's best to have a current kernel such as 2.4.x, 2.6.x or 3.0. > Are you sure that a 2.4 Kernel is still good enough for modern Wine? > And for the new Linux numbering: it is 3.x not 3.0 as the main change is > to have only two digits for the Linux version as released by Linus; the > third is reserved for the stable trees. > > bye > michael Hi Michael, I also thought about moving 2.4.x in one context with 2.2.x, but at least no one else did so i didn't. And yes, i intended to write 3.x -- Best Regards, André Hentschel

May 30, 2011