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

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

by Michael Mc Donnell

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.

May 29, 2011

Re: [7/8] d3d8: Use D3DRENDERSTATETYPE -> WINED3DRENDERSTATETYPE enum conversion function instead of implicit conversion (clang)

by Stefan Dösinger

On Sunday 29 May 2011 09:46:10 Frédéric Delanoy wrote: > default: > - hr = wined3d_device_set_render_state(This->wined3d_device, > State, Value); > + hr = > wined3d_device_set_render_state(This->wined3d_device, + > wined3drenderstatetype_from_d3drenderstatetype(State), Value); } > wined3d_mutex_unlock(); d3d8's SetRenderState and GetRenderState already have a switch-case statement because ZBIAS doesn't exist in wined3d, so strictly speaking a direct cast is not possible. You cannot move the switch-case from SetRenderState to the state conversion function because the meaning of the Value parameter is different between D3DRS_ZBIAS and WINED3DRS_DEPTHBIAS. So I'd suggest to put the cast into the default cases of d3ddevice8::SetRenderState and d3device8::GetRenderState In d3d9 you can cast between the d3d9 and wined3d render state types.

May 29, 2011

Re: comctl32/tests: Using unsigned constants to remove sign comparison warning.

by David Laight

On Fri, May 27, 2011 at 10:09:04PM +0400, Nikolay Sivov wrote: > On Fri, May 27, 2011 at 5:16 PM, Marko Nikolic <grkoma(a)gmail.com> wrote: > > --- > > ?dlls/comctl32/tests/treeview.c | ? ?6 +++--- > > ?1 files changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/dlls/comctl32/tests/treeview.c b/dlls/comctl32/tests/treeview.c > > index 5fd26a0..b1eb85e 100644 > > --- a/dlls/comctl32/tests/treeview.c > > +++ b/dlls/comctl32/tests/treeview.c > > @@ -634,7 +634,7 @@ static void test_get_set_bkcolor(void) > > > > ? ? /* If the value is -1, the control is using the system color for the background color. */ > > ? ? crColor = (COLORREF)SendMessage( hTree, TVM_GETBKCOLOR, 0, 0 ); > > - ? ?ok(crColor == -1, "Default background color reported as 0x%.8x\n", crColor); > > + ? ?ok(crColor == 0xffffffff, "Default background color reported as 0x%.8x\n", crColor); > > > > ~0 looks better here imo. Rather ~0u The 0xffffffff should be 0xffffffffu David -- David Laight: david(a)l8s.co.uk

May 27, 2011

Re: Support for D3DQUERY_TIMESTAMP* queries...

by Adam Martinson

On 05/27/2011 01:23 PM, Adam Martinson wrote: > On 05/25/2011 11:31 AM, Stefan Dösinger wrote: >> On Wednesday 25 May 2011 18:23:39 Adam Martinson wrote: >>> Do you think that needs to be done before I submit those or is that >>> something that can come later? >> I'd prefer to have this checked before submitting. It shouldn't take >> long. >> Just write a test app that grabs the timestamp in a loop and prints >> it to >> stdout and see what kind of values it writes. If it does wrap around >> in a >> reasonable amount of time(e.g. a few hours) write a test that waits >> for the >> wraparound to approach, starts a disjoint query, waits until after the >> wraparound, ends the disjoint query and prints its result. > Ran a test for 10 mins or so, on my card it behaves like the > GL_ARB_timer_query version, except the frequency is 10ns instead of > 1ns... so it doesn't look like it would wrap around in my lifetime. I > can include that test with an environment variable to set the number > of iterations if you like. Are these ok to submit now?

May 27, 2011

Re: comctl32/tests: Using unsigned constants to remove sign comparison warning.

by Nikolay Sivov

On Fri, May 27, 2011 at 5:16 PM, Marko Nikolic <grkoma(a)gmail.com> wrote: > --- > dlls/comctl32/tests/treeview.c | 6 +++--- > 1 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/dlls/comctl32/tests/treeview.c b/dlls/comctl32/tests/treeview.c > index 5fd26a0..b1eb85e 100644 > --- a/dlls/comctl32/tests/treeview.c > +++ b/dlls/comctl32/tests/treeview.c > @@ -634,7 +634,7 @@ static void test_get_set_bkcolor(void) > > /* If the value is -1, the control is using the system color for the background color. */ > crColor = (COLORREF)SendMessage( hTree, TVM_GETBKCOLOR, 0, 0 ); > - ok(crColor == -1, "Default background color reported as 0x%.8x\n", crColor); > + ok(crColor == 0xffffffff, "Default background color reported as 0x%.8x\n", crColor); > ~0 looks better here imo. >

May 27, 2011

Re: winealsa: Have TRACE_ON() use the same channel as TRACE (alsa).

by Alexandre Julliard

<Joerg-Cyril.Hoehle(a)t-systems.com> writes: > alsa.c:ALSA_TraceParameters writes to channel "alsa", therefore you may consider > that it makes sense that TRACE_ON dispatches on that same channel and apply this patch. > Otherwise you need to listen to 2 channels to get output, e.g. +alsa,+wave or +dsalsa. > > Back when everything was in audio.c, "wave" was the only channel used. That would probably be a better choice. -- Alexandre Julliard julliard(a)winehq.org

May 27, 2011

Re: [PATCH 1/2] mshtml: Added IHTMLScreen::get_availHeight implementation

by Alexandre Julliard

Jacek Caban <jacek(a)codeweavers.com> writes: > @@ -186,8 +186,15 @@ static HRESULT WINAPI HTMLScreen_get_updateInterval(IHTMLScreen *iface, LONG *p) > static HRESULT WINAPI HTMLScreen_get_availHeight(IHTMLScreen *iface, LONG *p) > { > HTMLScreen *This = impl_from_IHTMLScreen(iface); > - FIXME("(%p)->(%p)\n", This, p); > - return E_NOTIMPL; > + RECT work_area; > + > + TRACE("(%p)->(%p)\n", This, p); > + > + if(!SystemParametersInfoW(SPI_GETWORKAREA, 0, &work_area, 0)) > + return E_FAIL; > + > + *p = work_area.bottom; You probably want work_area.bottom - work_area.top. -- Alexandre Julliard julliard(a)winehq.org

May 27, 2011

Re: [PATCH 5/5] msvcp90: Added _Mutex class implementation

by Nikolay Sivov

On Fri, May 27, 2011 at 4:24 PM, Piotr Caban <piotr(a)codeweavers.com> wrote: > --- > dlls/msvcp100/msvcp100.spec | 32 ++++++++++++++++---------------- > dlls/msvcp71/msvcp71.spec | 16 ++++++++-------- > dlls/msvcp80/msvcp80.spec | 32 ++++++++++++++++---------------- > dlls/msvcp90/Makefile.in | 1 + > dlls/msvcp90/msvcp90.spec | 32 ++++++++++++++++---------------- > 5 files changed, 57 insertions(+), 56 deletions(-) > There's no misc.c in a patch.

May 27, 2011

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

by Stefan Dösinger

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Am 26.05.2011 um 16:27 schrieb Michael Mc Donnell: > Yeah I'd prefer it too to be 100% compatible. However, I think it is > highly unlikely that any programs depend on an exception being thrown > by EndScene in a drawing method. What EndScene method is this? The device's methods? I'd be surprised if this method crashed. > But you never know, some people do > crazy things :-) I think the current implementation is good enough as > long as there is no evidence of programs depending on that behavior. > > The problem is that UpdateSemantics in the d3dx9 library depends on > d3d9 and the error code is generated by vertexdeclaration_init in > d3d9. Do the d3d9 vertexdeclaration tests pass on this Windows machine? > I thought of managing an array of D3DVERTEXELEMENT9 in ID3DXMeshImpl > instead of a IDirect3DVertexDeclaration9, and then have > ID3DXMeshImpl_DrawSubset call IDirect3DDevice9_CreateVertexDeclaration > just before IDirect3DDevice9_SetVertexDeclaration. I suspect that this is what native does. This also fits the API design because UpdateSemantics accepts a D3DVERTEXELEMENT9 array and GetDeclaration returns a D3DVERTEXELEMENT9 array. Why did MS name those functions UpdateSemantics and GetDeclaration??? I don't think you'd have extra d3d9 calls by delaying the vdecl creation. Of course you have to keep the vdecl until the next UpdateSemantic call and not recreate it every time you draw. -----BEGIN PGP SIGNATURE----- Version: GnuPG/MacGPG2 v2.0.17 (Darwin) iQIbBAEBAgAGBQJN3sIsAAoJEN0/YqbEcdMwIjwP+IxUGwlGAFCtgrJBuiVc9p6u wEeG7eK412dCjQBtgjwab4SI4XEZ8+Si555ZtorcVE2LV7jhTmzpEj32VPJYuKKo asmaNmDk9X45aR9kn7/3lwX9alI0mqwFmyB4pp7C9yr9IHhPBE8+6AAspXjgBKgw QIU1yluABO2OvEsFFjEXW5jPG1PTcMoB496iOX/1bDKlm6JjcCYtlv+6jKda2fMm xBe7xTAds4whdIBQ0xbsbcWQoSTLYmRrtkyiHXolKV3IRDtKlc4FHzOkCn3CURz7 gH7z+Z4Oj5HiY42smOAgJtifCD/mwszWj7bTKtxsG4fE2rnWHu/D/8g6oi4dzufp RpWE7uMxYwGirpgcMTIESkocyfj0LHOUzcanBhHnAxdeiNYrit8G9zaF+I58YASC JBBQtWXSTOzgZ0R8G2GcVXOcrTg2bQpsZ8nd5mJVOGQAPj16cRRgBc8iJfTlUZqY S9NAzkqBQly+8cGlTxKi7pWKBcJR64T+/KoLlN3VzoannwkBvvF5nk4uB9WvIgEO +OLmZI3+5bbWBbfHhc7hWs7MYcT0pnZoePRCFjmaKeV6r+z+6XZAUWKCNDhbukXJ Sna+YSaLo47RFnNL2P1zYC8gQ3ohhHKfJbZDJ6DX+bBKsRh45OFA6H0RlYVRRkAP 8GKf5K/pnG+FBaWEIYE= =QHcz -----END PGP SIGNATURE-----

May 26, 2011

Re: [3/3]usp10: add Devanagari script

by Marvin

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at http://testbot.winehq.org/JobDetails.pl?Key=11215 Your paranoid android. === WNT4WSSP6 (32 bit usp10) === Timeout

May 26, 2011

Re: [PATCH 5/7] comdlg32: Implement support for custom control subitems.

by Marvin

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at http://testbot.winehq.org/JobDetails.pl?Key=11214 Your paranoid android. === WVISTAADM (32 bit itemdlg) === itemdlg.c:1284: Test failed: Failed to find item. itemdlg.c:1287: Test failed: Got L"" itemdlg.c:1289: Test failed: Failed to find item. itemdlg.c:1292: Test failed: Got L"" itemdlg.c:1294: Test failed: Failed to find item. itemdlg.c:1297: Test failed: Got L"" itemdlg.c:1300: Test failed: Got L"" === W2K8SE (32 bit itemdlg) === itemdlg.c:1284: Test failed: Failed to find item. itemdlg.c:1287: Test failed: Got L"" itemdlg.c:1289: Test failed: Failed to find item. itemdlg.c:1292: Test failed: Got L"" itemdlg.c:1294: Test failed: Failed to find item. itemdlg.c:1297: Test failed: Got L"" itemdlg.c:1300: Test failed: Got L""

May 26, 2011

Re: [PATCH 4/7] comdlg32: Implement some functions for modifying custom controls.

by Marvin

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at http://testbot.winehq.org/JobDetails.pl?Key=11212 Your paranoid android. === WVISTAADM (32 bit itemdlg) === itemdlg.c:1284: Test failed: Failed to find item. itemdlg.c:1287: Test failed: Got L"" itemdlg.c:1289: Test failed: Failed to find item. itemdlg.c:1292: Test failed: Got L"" === W2K8SE (32 bit itemdlg) === itemdlg.c:1284: Test failed: Failed to find item. itemdlg.c:1287: Test failed: Got L"" itemdlg.c:1289: Test failed: Failed to find item. itemdlg.c:1292: Test failed: Got L""

May 26, 2011

Re: [PATCH 1/7] comdlg32: Add a container for custom controls.

by Marvin

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at http://testbot.winehq.org/JobDetails.pl?Key=11209 Your paranoid android. === W7PROX64 (32 bit itemdlg) === itemdlg: unhandled exception c0000005 at 75B94C19

May 26, 2011

Re: winscard: Implement some functions of winscard.dll

by Alexandre Julliard

Sergey Stepanov <serg_admin(a)rambler.ru> writes: > @@ -33,6 +35,24 @@ const SCARD_IO_REQUEST g_rgSCardT0Pci = { SCARD_PROTOCOL_T0, 8 }; > const SCARD_IO_REQUEST g_rgSCardT1Pci = { SCARD_PROTOCOL_T1, 8 }; > const SCARD_IO_REQUEST g_rgSCardRawPci = { SCARD_PROTOCOL_RAW, 8 }; > > +#define MAKE_FUNCPTR(f) static typeof(f) * p##f = NULL > +MAKE_FUNCPTR(SCardEstablishContext); > +MAKE_FUNCPTR(SCardReleaseContext); > +#undef MAKE_FUNCPTR This can't work. You need to use the Unix headers (with appropriate configure checks), declare pointers to the Unix functions, and convert any types that need it before calling them. -- Alexandre Julliard julliard(a)winehq.org

May 26, 2011

Re: USB Device Support

by Juan Lang

Hi Scott, > I've been working through my Dell Inspiron 1545 with Linux/Ubuntu > 10.04 LTS experiences with ongoing reviews. I'd like to provide some help > to others considering making the transition from Windows to Linux. I'm > trying to get my Garmin GPS to talk to its Mapsource (running under WINE) > application through a USB port. This allows tracks to be read from and > written to the GPS itself. So far no luck. Can someone there please > provide some advice? If this functionality isn't available yet then so be > it. The Mapsource application runs fine it's just the USB communication > that isn't working. Many thanks. Such questions are better addressed to the Wine forum. Thanks, --Juan

May 26, 2011

kernel32/heap: Fix GlobalMemoryStatus()/GlobalMemoryStatusEx()

by Adam Martinson

There are a bunch of old apps which are known to be buggy with this, the ones I tested were: Dragon NaturallySpeaking 7 Adobe Illustrator 8 Need for Speed 2 SE (demo) Adobe Photoshop 4 Photoshop 4 has a hack in GlobalMemoryStatus() that's causing problems for some of the others. The 2nd patch removes that hack, and breaks it the same way it's broken on Windows. Normally I wouldn't advocate breaking apps, but in this case I think it's appropriate. The 3rd patch gives a workaround for Photoshop 4 that seams like the appropriate solution to me, as it mirrors the workaround for Windows. Any feedback would be appreciated.

May 26, 2011

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

by Michael Mc Donnell

On Thu, May 26, 2011 at 1:14 PM, Joris Huizer <joris_huizer(a)yahoo.com> wrote: > On 05/26/2011 10:33 AM, Michael Mc Donnell wrote: >> I've added some more tests to see if I could make it fail. Microsoft's >> UpdateSemantics is not very picky. I can't get it to return anything >> but D3D_OK except for when I pass a null pointer. My implementation >> follows this behavior except for two cases: it returns E_FAIL when >> passing an undefined type and when the offset is not 4 byte aligned. >> This is because the values are checked inside vertexdeclaration_init. >> So Wine is stricter. I don't think that it is necessarily a problem >> because applications that pass bogus declarations like those will >> likely not work anyway. My own tests with a small interactive demo >> show that in those cases the application will crash with an access >> violation when it tries to re-draw the scene. > > I don't know whether it applies here, but if I understand correctly, > there have been cases before that Wine must not be stricter than Windows. > Tthe reason is that a program may 'depend' on a function crashing (it having > an exception caught in that case). > In such a situation, Wine's version of the function not crashing would cause > a code path being executed > that normally never is (causing incorrect behavior even) > > Again I don't know whether it is relevant in this cause. Yeah I'd prefer it too to be 100% compatible. However, I think it is highly unlikely that any programs depend on an exception being thrown by EndScene in a drawing method. But you never know, some people do crazy things :-) I think the current implementation is good enough as long as there is no evidence of programs depending on that behavior. The problem is that UpdateSemantics in the d3dx9 library depends on d3d9 and the error code is generated by vertexdeclaration_init in d3d9. That means d3d9 would have to be changed to make it less picky. That might on the other hand break other things that depend on it being picky (I haven't looked at the tests in d3d9). I thought of managing an array of D3DVERTEXELEMENT9 in ID3DXMeshImpl instead of a IDirect3DVertexDeclaration9, and then have ID3DXMeshImpl_DrawSubset call IDirect3DDevice9_CreateVertexDeclaration just before IDirect3DDevice9_SetVertexDeclaration. But that would not solve the problem as it would just shift the bad call to ID3DXMeshImpl_DrawSubset and not result in an exception in EndScene. It would also result in a lot of extra calls depending on the number of subsets. So I can't see any good way around modifying d3d9, but that might cause other problems as mentioned earlier. So to recapitulate, I think the best solution is to ignore that little difference until we see programs that depend on it. Michael Mc Donnell

May 26, 2011

Re: kernel32/toolhelp: PROCESSENTRY32::szExeFile is the full path in Win9x.

by Adam Martinson

On 05/26/2011 06:38 AM, Saulius Krasuckas wrote: > WRT doesn't test win9x any longer, so what's the point of putting such > check in the test? Ye, I noticed that when I was looking through Marvin's results, the reason I sent this one was to have the testbot check it xD

May 26, 2011

Re: ole32/ole2: Don't call IDropTarget::QueryInterface() in RegisterDragDrop(). [whitespace fixed]

by Adam Martinson

On 05/26/2011 06:56 AM, Huw Davies wrote: > On Thu, May 26, 2011 at 06:51:13AM -0500, Adam Martinson wrote: >> On 05/26/2011 06:47 AM, Huw Davies wrote: >>> On Wed, May 25, 2011 at 12:45:15PM -0500, Adam Martinson wrote: >>>> On 05/25/2011 12:36 PM, Adam Martinson wrote: >>>>> The AddRef is done in WrapDropTarget(), seems like the appropriate >>>>> place for it. Quite right on the rest though, thanks. >>>> Oops no, the window prop should be pDropTarget, not the wrapper. >>>> The wrapper is only used for the marshalling. >>> That window prop could probably go away, I only put it there because >>> that's what Windows does, I doubt there's an app that relies on it >>> though. However if it stays you do need the AddRef! >>> >>> Huw. >> Best to mirror Windows as close as possible. The AddRef() is done >> in WrapDropTarget(). On failure IDropTarget::Release() is done on >> the wrapper, which removes the ref on the original. > You're storing a 2nd instance of the ptr (the first being in the > wrapper), conventionally you take a 2nd ref. When you remove the > window prop you release and when you destroy the wrapper you release > again. > > Huw. Ah, right, looks like I'm leaking the wrapper this way.

May 26, 2011

Re: ole32/ole2: Don't call IDropTarget::QueryInterface() in RegisterDragDrop(). [whitespace fixed]

by Huw Davies

On Thu, May 26, 2011 at 06:51:13AM -0500, Adam Martinson wrote: > On 05/26/2011 06:47 AM, Huw Davies wrote: > >On Wed, May 25, 2011 at 12:45:15PM -0500, Adam Martinson wrote: > >>On 05/25/2011 12:36 PM, Adam Martinson wrote: > >>>The AddRef is done in WrapDropTarget(), seems like the appropriate > >>>place for it. Quite right on the rest though, thanks. > >>Oops no, the window prop should be pDropTarget, not the wrapper. > >>The wrapper is only used for the marshalling. > >That window prop could probably go away, I only put it there because > >that's what Windows does, I doubt there's an app that relies on it > >though. However if it stays you do need the AddRef! > > > >Huw. > Best to mirror Windows as close as possible. The AddRef() is done > in WrapDropTarget(). On failure IDropTarget::Release() is done on > the wrapper, which removes the ref on the original. You're storing a 2nd instance of the ptr (the first being in the wrapper), conventionally you take a 2nd ref. When you remove the window prop you release and when you destroy the wrapper you release again. Huw.

May 26, 2011

Re: ole32/ole2: Don't call IDropTarget::QueryInterface() in RegisterDragDrop(). [whitespace fixed]

by Adam Martinson

On 05/26/2011 06:47 AM, Huw Davies wrote: > On Wed, May 25, 2011 at 12:45:15PM -0500, Adam Martinson wrote: >> On 05/25/2011 12:36 PM, Adam Martinson wrote: >>> The AddRef is done in WrapDropTarget(), seems like the appropriate >>> place for it. Quite right on the rest though, thanks. >> Oops no, the window prop should be pDropTarget, not the wrapper. >> The wrapper is only used for the marshalling. > That window prop could probably go away, I only put it there because > that's what Windows does, I doubt there's an app that relies on it > though. However if it stays you do need the AddRef! > > Huw. Best to mirror Windows as close as possible. The AddRef() is done in WrapDropTarget(). On failure IDropTarget::Release() is done on the wrapper, which removes the ref on the original.

May 26, 2011

Re: ole32/ole2: Don't call IDropTarget::QueryInterface() in RegisterDragDrop(). [whitespace fixed]

by Huw Davies

On Wed, May 25, 2011 at 12:45:15PM -0500, Adam Martinson wrote: > On 05/25/2011 12:36 PM, Adam Martinson wrote: > >The AddRef is done in WrapDropTarget(), seems like the appropriate > >place for it. Quite right on the rest though, thanks. > > Oops no, the window prop should be pDropTarget, not the wrapper. > The wrapper is only used for the marshalling. That window prop could probably go away, I only put it there because that's what Windows does, I doubt there's an app that relies on it though. However if it stays you do need the AddRef! Huw.

May 26, 2011

Re: kernel32/toolhelp: PROCESSENTRY32::szExeFile is the full path in Win9x.

by Saulius Krasuckas

* On Wed, 25 May 2011, Adam Martinson wrote: > > Dragon NaturallySpeaking 7 expects this in Win98 mode. > --- > dlls/kernel32/tests/toolhelp.c | 19 +++++++++++++++- > dlls/kernel32/toolhelp.c | 48 +++++++++++++++++++++++++++++---------- > 2 files changed, 53 insertions(+), 14 deletions(-) I am not sure you do it right (now that new testing strategy is taken): * On Tue, 30 Nov 2010, Alexandre Julliard wrote: > > The value of running tests on Win9x these days is certainly > questionable. We don't try to emulate the Win9x behavior anyway, except > in a very few cases (which most likely don't have tests...) so it only > serves to document historical behavior that nobody cares about any > longer. WRT doesn't test win9x any longer, so what's the point of putting such check in the test? > --- a/dlls/kernel32/tests/toolhelp.c > +++ b/dlls/kernel32/tests/toolhelp.c ... > @@ -106,6 +108,10 @@ static void test_process(DWORD curr_pid, DWORD sub_pcs_pid) > unsigned found = 0; > int num = 0; > int childpos = -1; > + OSVERSIONINFO osver; > + > + osver.dwOSVersionInfoSize = sizeof(osver); > + pGetVersionEx(&osver); > > hSnapshot = pCreateToolhelp32Snapshot( TH32CS_SNAPPROCESS, 0 ); > ok(hSnapshot != NULL, "Cannot create snapshot\n"); > @@ -116,10 +122,15 @@ static void test_process(DWORD curr_pid, DWORD sub_pcs_pid) > { > do > { > + TCHAR* bslash = strchr(pe.szExeFile, '\\'); > if (pe.th32ProcessID == curr_pid) found++; > if (pe.th32ProcessID == sub_pcs_pid) { childpos = num; found++; } > trace("PID=%x %s\n", pe.th32ProcessID, pe.szExeFile); > num++; > + if (osver.dwMajorVersion < 5) > + ok(bslash != NULL, "szExeFile is not a full path: %s\n", pe.szExeFile); > + else > + ok(bslash == NULL, "szExeFile is a full path: %s\n", pe.szExeFile); > } while (pProcess32Next( hSnapshot, &pe )); > } > ok(found == 2, "couldn't find self and/or sub-process in process list\n"); Now such check is pointless, IMHO. Here goes question to Alexandre: what about (testing in) such cases? S.

May 26, 2011

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

by Joris Huizer

On 05/26/2011 10:33 AM, Michael Mc Donnell wrote: > > I've added some more tests to see if I could make it fail. Microsoft's > UpdateSemantics is not very picky. I can't get it to return anything > but D3D_OK except for when I pass a null pointer. My implementation > follows this behavior except for two cases: it returns E_FAIL when > passing an undefined type and when the offset is not 4 byte aligned. > This is because the values are checked inside vertexdeclaration_init. > So Wine is stricter. I don't think that it is necessarily a problem > because applications that pass bogus declarations like those will > likely not work anyway. My own tests with a small interactive demo > show that in those cases the application will crash with an access > violation when it tries to re-draw the scene. > > > I don't know whether it applies here, but if I understand correctly, there have been cases before that Wine must not be stricter than Windows. Tthe reason is that a program may 'depend' on a function crashing (it having an exception caught in that case). In such a situation, Wine's version of the function not crashing would cause a code path being executed that normally never is (causing incorrect behavior even) Again I don't know whether it is relevant in this cause. HTH, Joris

May 26, 2011

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

by Michael Mc Donnell

On Wed, May 25, 2011 at 9:53 AM, Michael Mc Donnell <michael(a)mcdonnell.dk> wrote: > On Tue, May 24, 2011 at 8:32 PM, Stefan Dösinger <stefandoesinger(a)gmx.at> wrote: >> On Tuesday 24 May 2011 19:56:06 Michael Mc Donnell wrote: >>> > *) In your first test you forgot to check the HeapAlloc result. >>> >>> Ok, I'll return E_OUTOFMEMORY in that case. >> I guess the callers will only abort the tests if NewTestContext fails, so I >> think the NULL / non-NULL return value was better. I've reverted those changes, so it's back to NULL / non-NULL. I've also moved it near the top of the file, so that other tests will be able to re-use the test context creation. >>> Should I try to make some more invalid >>> D3DVERTEXELEMENT9 arrays to see if I can provoke an error? >> I was thinking about something that would make CreateVertexDeclaration fail, >> e.g. declaring a usage+usage index twice or using an undefined type. >> dlls/d3d9/vertexdeclaration.c and dlls/wined3d/vertexdeclaration.c check for a >> few error conditions in their vertexdeclaration_init functions. Just pick one >> of them(no need to check all of them) >> But watch out that you don't open a can of worms here. Our >> CreateVertexDeclaration probably doesn't catch all error conditions. I >> recommend that you pick one it does catch, otherwise you'll have to fix >> d3d9.dll and wined3d.dll too for a small test. > > Ok thanks I'll do that. I've added some more tests to see if I could make it fail. Microsoft's UpdateSemantics is not very picky. I can't get it to return anything but D3D_OK except for when I pass a null pointer. My implementation follows this behavior except for two cases: it returns E_FAIL when passing an undefined type and when the offset is not 4 byte aligned. This is because the values are checked inside vertexdeclaration_init. So Wine is stricter. I don't think that it is necessarily a problem because applications that pass bogus declarations like those will likely not work anyway. My own tests with a small interactive demo show that in those cases the application will crash with an access violation when it tries to re-draw the scene.

May 26, 2011

Re: Tahoma Font License

by Scott Ritchie

On 05/25/2011 09:27 AM, Maarten Lankhorst wrote: > Hi mark, > > 2011/5/25 Mark Page <rombust(a)hotmail.co.uk>: >> >> I am a bit confused by the Wine "tahoma" license: "Wine Tahoma Regular" ... GNU Lesser General Public License 2.1 >> >> It would be useful to add the font to the ClanLib SDK resources (ClanLib has a zlib style license) >> >> It is not clear how a font can be licensed using LGPL, since LGPL is based around software libraries. >> >> Ideally I feel it should have a creative commons license >> >> I believe Creative commons did not exist when that font was created > tahoma.sfd: Copyright: Copyright (c) 2004 Larry Snyder, Based on > Bitstream Vera Sans Copyright (c) 2003 by Bitstream, Inc. Font renamed > in accordance with former's license. Please do not contact Bitstream > Inc. for any reason regarding this font. > > So just look up the original without contacting bitstream inc. ? > The topic of relicensing wine fonts has come up before. Usually the discussion ends with "our fonts aren't as good as you think, and you probably don't want them so don't worry about licensing." I'm not so sure it's true anymore. Symbol replacement, for instance, can actually be useful to show special characters on web pages that would otherwise come up as question marks. -Scott Ritchie

May 25, 2011

Re: extern char **environ and msvc

by Stefan Dösinger

On Wednesday 25 May 2011 23:04:02 Frank Richter wrote: > The underlying issue is probably the _CRTIMP - that's most likely > _declspec(dllimport) in MSVC. I suspected something like this, but msvc didn't want to tell me what _CRTIMP was. Either way it doesn't get me much closer to a fix.

May 25, 2011

Re: [2/2] user32: Add message test for hotkeys.

by Marvin

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at http://testbot.winehq.org/JobDetails.pl?Key=11184 Your paranoid android. === W7PROX64 (64 bit msg) === Timeout

May 25, 2011

Re: extern char **environ and msvc

by Frank Richter

On 25.05.2011 22:52, Stefan Dösinger wrote: > I'm sorry for the German, I don't know how to change this. Google finds only > other clueless people. Either way it says something like "Inconsistent DLL > binding" I believe the English translation is “inconsistent DLL linkage”. > > The line in stdlib.h declares _environ as > > _CRTIMP extern char ** _environ; /* pointer to environment table */ > > The compiler just writes a warning, but afterwards linking fails: > > 1>loader.obj : error LNK2001: Nicht aufgelöstes externes Symbol "__environ". > > The linker complains about an "unresolved external symbol __environ". Removing > our declaration of environ fixes the warning, link error and libwine seems to > work OK. > > I don't know what the proper fix is, please advise. Maybe this code shouldn't > be compiled at all? I don't think I need the loader code on Windows. The underlying issue is probably the _CRTIMP - that's most likely _declspec(dllimport) in MSVC. This matters, as DLL-imported and “plain” extern variables are handled differently on Windows. IIRC DLL export/import of variables is indirect - when DLL-importing a variable you really import a _pointer_ to the variable. So accessing the variable dereferences some pointer. “extern” variables, otoh, are accessed directly (resolving the actual variable location is handled at link-time). -f.r.

May 25, 2011

Re: [1/2] user32: Add test for RegisterHotKey/UnregisterHotKey.

by Marvin

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at http://testbot.winehq.org/JobDetails.pl?Key=11183 Your paranoid android. === W7PROX64 (64 bit msg) === Timeout

May 25, 2011

re: Hans Leidekker : msi: Support rollback of failed uninstalls.

by Hans Leidekker

On Wed, 2011-05-25 at 18:49 +0000, Dan Kegel wrote: > wow, msi has really come a long way! Thanks. In hindsight we should have implemented rollbacks early on, when installers still failed regularly ;-)

May 25, 2011

extern char **environ and msvc

by Stefan Dösinger

Hi, This is one of the last remaining hacks needed to compile the d3d parts of Wine with Visual Studio. Our definition of **environ is conflicting with one already in the Windows SDK headers: 1>..\..\wine\libs\wine\loader.c(54): warning C4273: '_environ': Inkonsistente DLL-Bindung. 1> C:\Program Files\Microsoft Visual Studio 10.0\VC\include\stdlib.h(299): Siehe vorherige Definition von '_environ' I'm sorry for the German, I don't know how to change this. Google finds only other clueless people. Either way it says something like "Inconsistent DLL binding" The line in stdlib.h declares _environ as _CRTIMP extern char ** _environ; /* pointer to environment table */ The compiler just writes a warning, but afterwards linking fails: 1>loader.obj : error LNK2001: Nicht aufgelöstes externes Symbol "__environ". The linker complains about an "unresolved external symbol __environ". Removing our declaration of environ fixes the warning, link error and libwine seems to work OK. I don't know what the proper fix is, please advise. Maybe this code shouldn't be compiled at all? I don't think I need the loader code on Windows. Stefan

May 25, 2011

Re: USB Device Support

by Scott

Hi, I've been working through my Dell Inspiron 1545 with Linux/Ubuntu 10.04 LTS experiences with ongoing reviews. I'd like to provide some help to others considering making the transition from Windows to Linux. I'm trying to get my Garmin GPS to talk to its Mapsource (running under WINE) application through a USB port. This allows tracks to be read from and written to the GPS itself. So far no luck. Can someone there please provide some advice? If this functionality isn't available yet then so be it. The Mapsource application runs fine it's just the USB communication that isn't working. Many thanks. Best - Scott

May 25, 2011

Re: [PATCH 1/2] ddraw: COM cleanup for IDirectDrawSurface3 iface

by Michael Stefaniuc

On 05/25/2011 08:37 PM, Stefan Dösinger wrote: > On Wednesday 25 May 2011 00:39:46 Ričardas Barkauskas wrote: >> My first try for COM cleanup. Please donėt be too harsh :) >> If this is too large for one patch, please suggest how to split it. >> >> This is not automatic clean up so I don't really expect it to be good on >> first try. > I can't find anything wrong with it on first sight, but I don't really know how > to manually review a patch that large with many repetitions. I guess I could > print them on a *lot* of paper and read them at the pool or something like > that. I normally read such a patch at least 3 times, more if I find some issues and have to manually fix them. Well "read"... after hundreds of such patches it isn't really a conscious read anymore. More like optical filtering looking for odd things; that's why I *hate* inconsistent coding style. > I don't see an easy way to split them up either :-/ . I guess we'll have to > trust Puk's script verification. The patch could be split up. What I do for such big and ugly patches is to first handle the unsafe_impl_from_IFace() part, then introduce impl_from_IFace() and only afterwards do the obj->lpVtl to obj->iface changes. Of course that really depends on the sizes of the resulting patches, but most of the times the unsafe_impl_from_IFace() part is a different patch as that isn't standard search and replace cookie cutter COM cleanup. > Just one thing: in unsafe_impl_from_IDirectDrawSurface*: > >> + if (iface->lpVtbl !=&ddraw_surface7_vtbl) return NULL; > I don't think apps can pass in their own ddraw object implementations. I've > never seen an app that does that or any document that suggests this is > possible and achives something. So I'd recommend to add a FIXME() or ERR() to > that case. An ERR because the most likely cause for such a behavior is that > our own ddrawex.dll screws up. This is the only other place where we have a > IDirectDrawSurface* implementation. Well in that case an assert() is the better option. It's done that way in other unsafe_impl_from_IFace like functions. And the program is likely to crash anyway later on when doing magic with the wrong pointer. > Or we could just remove the check and see if we ever crash because trying to > read surface implementation fields returns odd values. bye michael

May 25, 2011

re: Hans Leidekker : msi: Support rollback of failed uninstalls.

by Dan Kegel

wow, msi has really come a long way!

May 25, 2011

Re: [PATCH 1/2] ddraw: COM cleanup for IDirectDrawSurface3 iface

by Stefan Dösinger

On Wednesday 25 May 2011 00:39:46 Ričardas Barkauskas wrote: > My first try for COM cleanup. Please donėt be too harsh :) > If this is too large for one patch, please suggest how to split it. > > This is not automatic clean up so I don't really expect it to be good on > first try. I can't find anything wrong with it on first sight, but I don't really know how to manually review a patch that large with many repetitions. I guess I could print them on a *lot* of paper and read them at the pool or something like that. I don't see an easy way to split them up either :-/ . I guess we'll have to trust Puk's script verification. Just one thing: in unsafe_impl_from_IDirectDrawSurface*: > + if (iface->lpVtbl != &ddraw_surface7_vtbl) return NULL; I don't think apps can pass in their own ddraw object implementations. I've never seen an app that does that or any document that suggests this is possible and achives something. So I'd recommend to add a FIXME() or ERR() to that case. An ERR because the most likely cause for such a behavior is that our own ddrawex.dll screws up. This is the only other place where we have a IDirectDrawSurface* implementation. Or we could just remove the check and see if we ever crash because trying to read surface implementation fields returns odd values.

May 25, 2011

Re: [2/5] d3dx9: Generate effect instances from materials for mesh loading.

by Dan Kegel

On Wed, May 25, 2011 at 6:23 PM, Dylan Smith <dylan.ah.smith(a)gmail.com> wrote: > The layout doesn't group all values, param names, or defaults, so it > actually seems to be best described using loops. Does the the > following comment help summarize the layout? Yes. Patch looks more readable now, thanks. Maybe an assert at the bottom as a sanity check that the final output pointer is exactly at the end of the buffer would be good. - Dan

May 25, 2011

Re: [2/5] d3dx9: Generate effect instances from materials for mesh loading.

by Dylan Smith

Hi Dan, On Wed, May 25, 2011 at 10:31 AM, Dan Kegel <dank(a)kegel.com> wrote: > A comment or two about the layout of the output buffer might > be nice. > The layout doesn't group all values, param names, or defaults, so it actually seems to be best described using loops. Does the the following comment help summarize the layout? /* effects buffer layout: * * D3DXEFFECTINSTANCE effects[num_materials]; * for (effect in effects) * { * D3DXEFFECTDEFAULT defaults[effect.NumDefaults]; * for (default in defaults) * { * *default.pParamName; * *default.pValue; * } * } */ > Can materials and material_ptr be const pointers? ID3DXBuffer_GetBufferPointer doesn't take a const, so no for materials, but material_ptr can be const. The rest of the advice I took (see the attached patch). Thanks for the patch review.

May 25, 2011

Re: ole32/ole2: Don't call IDropTarget::QueryInterface() in RegisterDragDrop(). [whitespace fixed]

by Adam Martinson

On 05/25/2011 12:36 PM, Adam Martinson wrote: > On 05/24/2011 03:37 AM, Huw Davies wrote: >>> +static IDropTarget* WrapDropTarget(IDropTarget* inner) >>> +{ >>> + DropTargetWrapper* This = HeapAlloc(GetProcessHeap(), 0, >>> sizeof(*This)); >>> + >>> + if (This) >>> + { >>> + This->iface = (IDropTarget){&DropTargetWrapper_VTbl }; >> This->iface.lpVtbl =&DropTargetWrapper_VTbl; >> >>> - hr = CoMarshalInterface(stream,&IID_IDropTarget, unk, >>> MSHCTX_LOCAL, NULL, MSHLFLAGS_TABLESTRONG); >>> - IUnknown_Release(unk); >>> + hr = CoMarshalInterface(stream,&IID_IDropTarget, >>> (IUnknown*)wrapper, MSHCTX_LOCAL, NULL, MSHLFLAGS_TABLESTRONG); >>> >>> if(SUCCEEDED(hr)) >>> { >>> hr = create_map_from_stream(stream,&map); >>> if(SUCCEEDED(hr)) >>> { >>> - IDropTarget_AddRef(pDropTarget); >>> SetPropW(hwnd, prop_oledroptarget, pDropTarget); >> You probably want to set the window prop to the wrapper here. Either >> way you still need the AddRef (on the wrapper if you set that), it >> gets released in RevokeDragDrop. >> >> Huw. > > The AddRef is done in WrapDropTarget(), seems like the appropriate > place for it. Quite right on the rest though, thanks. Oops no, the window prop should be pDropTarget, not the wrapper. The wrapper is only used for the marshalling.

May 25, 2011

Re: ole32/ole2: Don't call IDropTarget::QueryInterface() in RegisterDragDrop(). [whitespace fixed]

by Adam Martinson

On 05/24/2011 03:37 AM, Huw Davies wrote: >> +static IDropTarget* WrapDropTarget(IDropTarget* inner) >> +{ >> + DropTargetWrapper* This = HeapAlloc(GetProcessHeap(), 0, sizeof(*This)); >> + >> + if (This) >> + { >> + This->iface = (IDropTarget){&DropTargetWrapper_VTbl }; > This->iface.lpVtbl =&DropTargetWrapper_VTbl; > >> - hr = CoMarshalInterface(stream,&IID_IDropTarget, unk, MSHCTX_LOCAL, NULL, MSHLFLAGS_TABLESTRONG); >> - IUnknown_Release(unk); >> + hr = CoMarshalInterface(stream,&IID_IDropTarget, (IUnknown*)wrapper, MSHCTX_LOCAL, NULL, MSHLFLAGS_TABLESTRONG); >> >> if(SUCCEEDED(hr)) >> { >> hr = create_map_from_stream(stream,&map); >> if(SUCCEEDED(hr)) >> { >> - IDropTarget_AddRef(pDropTarget); >> SetPropW(hwnd, prop_oledroptarget, pDropTarget); > You probably want to set the window prop to the wrapper here. Either > way you still need the AddRef (on the wrapper if you set that), it > gets released in RevokeDragDrop. > > Huw. The AddRef is done in WrapDropTarget(), seems like the appropriate place for it. Quite right on the rest though, thanks.

May 25, 2011

Re: Tahoma Font License

by Maarten Lankhorst

Hi mark, 2011/5/25 Mark Page <rombust(a)hotmail.co.uk>: > > I am a bit confused by the Wine "tahoma" license: "Wine Tahoma Regular" ... GNU Lesser General Public License 2.1 > > It would be useful to add the font to the ClanLib SDK resources (ClanLib has a zlib style license) > > It is not clear how a font can be licensed using LGPL, since LGPL is based around software libraries. > > Ideally I feel it should have a creative commons license > > I believe Creative commons did not exist when that font was created tahoma.sfd: Copyright: Copyright (c) 2004 Larry Snyder, Based on Bitstream Vera Sans Copyright (c) 2003 by Bitstream, Inc. Font renamed in accordance with former's license. Please do not contact Bitstream Inc. for any reason regarding this font. So just look up the original without contacting bitstream inc. ? Cheers, Maarten

May 25, 2011

Tahoma Font License

by Mark Page

I am a bit confused by the Wine "tahoma" license: "Wine Tahoma Regular" ... GNU Lesser General Public License 2.1 It would be useful to add the font to the ClanLib SDK resources (ClanLib has a zlib style license) It is not clear how a font can be licensed using LGPL, since LGPL is based around software libraries. Ideally I feel it should have a creative commons license I believe Creative commons did not exist when that font was created (I initially posted this message to forum.winehq.org, and it was suggested that I should post here instead) Mark.

May 25, 2011

re: [2/5] d3dx9: Generate effect instances from materials for mesh loading.

by Dan Kegel

+ static const char diffuse_paramname[] = "Diffuse"; + static const char power_paramname[] = "Power"; + static const char specular_paramname[] = "Specular"; + static const char emissive_paramname[] = "Emissive"; + static const char ambient_paramname[] = "Ambient"; + static const DWORD basic_paramname_sizes = sizeof(diffuse_paramname) + + sizeof(power_paramname) + + sizeof(specular_paramname) + + sizeof(emissive_paramname) + + sizeof(ambient_paramname); ... It might be more readable to define material_effects[] up next to where those paramnames are defined, since it kind of clutters the loop, and it's nicer to keep the whole table definition together. (Also, at least conceptually, basic_paramname_sizes could be computed by a loop over the table. Then you wouldn't need those five _paramname variables, you could just use string constants. But if the strlen's would be expensive, I guess that's ok.) A comment or two about the layout of the output buffer might be nice. Can materials and material_ptr be const pointers? - Dan

May 25, 2011

Re: [PATCH] version: mark internal function with hidden visibility

by Alexandre Julliard

Marcus Meissner <meissner(a)suse.de> writes: > @@ -348,7 +348,7 @@ static BOOL find_pe_resource( HFILE lzfd, DWORD *resLen, DWORD *resOff ) > /*********************************************************************** > * find_version_resource [internal] > */ > -DWORD find_version_resource( HFILE lzfd, DWORD *reslen, DWORD *offset ) > +DECLSPEC_HIDDEN DWORD find_version_resource( HFILE lzfd, DWORD *reslen, DWORD *offset ) > { This should be in a header. It's not very useful to make it hidden if callers are not aware of the fact. -- Alexandre Julliard julliard(a)winehq.org

May 25, 2011

Re: [1/5] wined3d: Don't check the FBO status if FIXMEs are off

by Stefan Dösinger

On Tuesday 24 May 2011 22:57:34 Henri Verbeet wrote: > I'm not sure that's really worth it. If the GL lock is really such a > concern, you should probably change the calls in context.c to already > be under the GL lock anyway. You can easily change > context_apply_draw_buffers() for example to require locking by the > caller. I planned to do this on top of this patch, but it turned out to be way easier than expected. Please ignore this patch for now. The others should still apply though.

May 25, 2011

Re: usp10/tests: Make enumFontProc() static.

by Marvin

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at http://testbot.winehq.org/JobDetails.pl?Key=11170 Your paranoid android. === WNT4WSSP6 (32 bit usp10) === Timeout

May 25, 2011

Re: [PATCH 1/2] ddraw: COM cleanup for IDirectDrawSurface3 iface

by Michael Stefaniuc

Damn ... I have attached the wrong diff. Michael Stefaniuc wrote: > Ričardas Barkauskas wrote: >> My first try for COM cleanup. Please donėt be too harsh :) >> If this is too large for one patch, please suggest how to split it. >> >> This is not automatic clean up so I don't really expect it to be good on >> first try. > Heh! I have attached the raw diff generated by my coccinelle script. > Looking at the interdiff between the two patches it looks like you > caught everything. And the BADBADBAD() (*) from the generated patch are > replaced with unsafe_impl_from_IDirectDrawSurface3(), very good. > > If you want to do more COM cleanup I can give you the raw generated > patches to save you some time. > > bye > michael > > P.S.: (*) My script validates that impl_from_IFace() is called only from > methods of that IFace. All other impl_from_IFace() calls are replaced > with BADBADBAD().

May 25, 2011

Re: [PATCH 1/2] ddraw: COM cleanup for IDirectDrawSurface3 iface

by Michael Stefaniuc

Hello Ričardas! Ričardas Barkauskas wrote: > My first try for COM cleanup. Please donėt be too harsh :) > If this is too large for one patch, please suggest how to split it. > > This is not automatic clean up so I don't really expect it to be good on > first try. Heh! I have attached the raw diff generated by my coccinelle script. Looking at the interdiff between the two patches it looks like you caught everything. And the BADBADBAD() (*) from the generated patch are replaced with unsafe_impl_from_IDirectDrawSurface3(), very good. If you want to do more COM cleanup I can give you the raw generated patches to save you some time. bye michael P.S.: (*) My script validates that impl_from_IFace() is called only from methods of that IFace. All other impl_from_IFace() calls are replaced with BADBADBAD().

May 25, 2011

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

by Michael Mc Donnell

On Tue, May 24, 2011 at 8:32 PM, Stefan Dösinger <stefandoesinger(a)gmx.at> wrote: > On Tuesday 24 May 2011 19:56:06 Michael Mc Donnell wrote: > >> Sorry I phrased that in a wrong way. In the UpdateSemantics function I >> call IDirect3DDevice9_CreateVertexDeclaration which allocates a new >> Vertex Declaration on the heap. > Is there an alternative? I guess you could cache existing declarations. The > d3d9 API doesn't give you an option to reuse an existing declaration. > > I doubt apps will regularly change the declaration, usually this doesn't make > sense without also changing the data. Of course that doesn't mean there isn't > a broken app out there that does this. Yeah it might not be a problem at all. I'll give it a quick second look to see if it's possible to cache things. I anyway need to read some more code too to see exactly how the APIs interact. >> I also noticed that I didn't call >> IDirect3DDevice9_SetVertexDeclaration. I've added it to stick with the >> defined locking scheme. > Locking scheme? I've just started reading into into the Mesh API, but I don't > think you're supposed to apply the declaration until the mesh is used for > drawing. Ok good, I started to doubt this last night. I'll remove it and try to get a better sense of how the API works. >> > *) In your first test you forgot to check the HeapAlloc result. >> >> Ok, I'll return E_OUTOFMEMORY in that case. > I guess the callers will only abort the tests if NewTestContext fails, so I > think the NULL / non-NULL return value was better. > >> Should I try to make some more invalid >> D3DVERTEXELEMENT9 arrays to see if I can provoke an error? > I was thinking about something that would make CreateVertexDeclaration fail, > e.g. declaring a usage+usage index twice or using an undefined type. > dlls/d3d9/vertexdeclaration.c and dlls/wined3d/vertexdeclaration.c check for a > few error conditions in their vertexdeclaration_init functions. Just pick one > of them(no need to check all of them) > But watch out that you don't open a can of worms here. Our > CreateVertexDeclaration probably doesn't catch all error conditions. I > recommend that you pick one it does catch, otherwise you'll have to fix > d3d9.dll and wined3d.dll too for a small test. Ok thanks I'll do that. >> > *) In the wined3d code(and its client libs) Henri and I avoid structure >> > typedefs. The existing d3dx9 code has a few of them. I'm ok with either >> > way, but maybe you and the other d3dx9 devs want to go the wined3d way. >> >> Sure I can change them. So just normal structs? Is it to keep the >> namespace clean? > Henri started with this change, I think keeping the namespace clean was a > consideration. What you do is up to you and the other devs working on d3dx9. > I'm only suggesting that you discuss the code style and agree on a format. > > Either way it is a fairly minor point IMO, but it tends to end up in holy war. Ok :-) I don't have any strong feelings about it, so I'll just follow your convention until somebody complains.

May 25, 2011

Re: cmd: Don't read past end of thisVar in WCMD_expand_envvar (valgrind)

by Dan Kegel

On Tue, May 24, 2011 at 10:26 PM, Marvin <testbot(a)testbot.winehq.org> wrote: > === WNT4WSSP6 (32 bit) === > batch.c:223: Test failed: unexpected char 0x54 position 0 in line 86 (got 'This Line Is Never Reached. If this runs, it is a bug.', wanted '65536') Well, foo. Guess NT is different. I'll rework the tests sometime and resubmit. > === WXPPROSP3 (32 bit) === > batch.c:223: Test failed: unexpected char 0x30 position 0 in line 92 (got '0 should be 3 now', wanted '3 should be 3 now') This one surprised me, doesn't seem to match local testing on XP.

May 25, 2011