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

Re: [6/7] user32: Test that QS_HOTKEY is properly set and cleared.

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=11760 Your paranoid android. === WINEBUILD (build) === Patch failed to apply

June 15, 2011

Re: [2/7] server: Post WM_HOTKEY when a hotkey is pressed.

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=11759 Your paranoid android. === WINEBUILD (build) === Patch failed to apply

June 15, 2011

Re: [1/7] server: Implement 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=11758 Your paranoid android. === WINEBUILD (build) === Patch failed to apply

June 15, 2011

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

by Michael Mc Donnell

On Tue, Jun 14, 2011 at 3:42 PM, Dylan Smith <dylan.ah.smith(a)gmail.com> wrote: > On Fri, Jun 10, 2011 at 2:08 AM, Dylan Smith <dylan.ah.smith(a)gmail.com> wrote: >> D3DXCreateMesh fails when the declaration contains a non-zero Stream >> value. I would expect UpdateSemantics to be as strict as D3DXCreateMesh, >> otherwise a test could validate the different behaviour. > > Seems like you missed this comment I made before. > > /* e.g. */ > memcpy(declaration, declaration0, sizeof(declaration0)); > declaration[1].Stream = 1; > hr = mesh->lpVtbl->UpdateSemantics(mesh, declaration); > ok(hr == D3DERR_INVALIDCALL, "Test UpdateSematics using multiple streams, " > "got %#x expected D3DERR_INVALIDCALL\n", hr); Yes you're right, it returns D3DERR_INVALIDCALL when a declaration contains a non-zero Stream value. I'll update my tests and the implementation.

June 15, 2011

Re: [PATCH 3/5] shell32: Add support of registry-registered folders to IKnownFolderManager::GetFolder

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=11756 Your paranoid android. === WINEBUILD (build) === Patch failed to apply

June 15, 2011

Re: [PATCH 2/5] shell32: Add IKnownFolderManager::UnregisterFolder implementation

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=11755 Your paranoid android. === WINEBUILD (build) === Patch failed to apply

June 15, 2011

Re: [PATCH 1/5] shell32: Add IKnownFolderManager::RegisterFolder implementation

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=11754 Your paranoid android. === WINEBUILD (build) === Patch failed to apply

June 15, 2011

Re: [PATCH 2/4] user32/tests: Added client-side raw input functions tests (try 2)

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=11753 Your paranoid android. === W7PROX64 (64 bit input) === No test summary line found

June 15, 2011

Re: [PATCH] winspool.drv: Removed a superflous NULL check (Coverity)

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=11752 Your paranoid android. === WINEBUILD (build) === Can't determine base name

June 15, 2011

Re: Add sub-message to a commit

by Frédéric Delanoy

On Wed, Jun 15, 2011 at 16:28, GOUJON Alexandre <ale.goujon(a)gmail.com> wrote: > So writing commit messages with more than one line will add them (all lines > except the first one) as a 'sub' commit message. > I'm using git for another project and being able to comment a bit more a > commit is appreciable. Simply don't forget to add a blank line after the commit message.

June 15, 2011

Re: Add sub-message to a commit

by GOUJON Alexandre

On 06/15/2011 04:17 PM, Andrew Eikum wrote: > On 06/15/2011 09:09 AM, GOUJON Alexandre wrote: >> I would like to add a 'sub'-message to a commit like this one >> http://source.winehq.org/git/wine.git/commitdiff/dbd878639424071e1d4c6cfc9b… >> >> >> I know that I can specify this text at the beginning of a patch I want >> to send to wine-patches (befor the '---'). >> But here, I'm interested in how to do that on the git server side. >> git commit -a or git commit -m let me specify a 'global' commit message. >> I googled some key words and RTFMs but didn't find anything relevant. >> >> Any hints on how to do that ? > > Just include your message in the commit message after your headline. > That will add the message before the "---", like you said, and then it > will be imported by git-am (via git-apply), which Julliard uses to > import commits from mail. > > Julliard is in the habit of removing commit messages unless they're > really informative. So, include your message as usual, and Julliard > will decide if it's worth leaving in the commit log. If you want to > make an argument for why to keep the commit message intact, you should > add that after the "---" so it will still be in the mail, but won't be > imported by git-am. > > Andrew So writing commit messages with more than one line will add them (all lines except the first one) as a 'sub' commit message. I'm using git for another project and being able to comment a bit more a commit is appreciable. Thanks !

June 15, 2011

Re: Add sub-message to a commit

by Andrew Eikum

On 06/15/2011 09:09 AM, GOUJON Alexandre wrote: > I would like to add a 'sub'-message to a commit like this one > http://source.winehq.org/git/wine.git/commitdiff/dbd878639424071e1d4c6cfc9b… > > I know that I can specify this text at the beginning of a patch I want > to send to wine-patches (befor the '---'). > But here, I'm interested in how to do that on the git server side. > git commit -a or git commit -m let me specify a 'global' commit message. > I googled some key words and RTFMs but didn't find anything relevant. > > Any hints on how to do that ? Just include your message in the commit message after your headline. That will add the message before the "---", like you said, and then it will be imported by git-am (via git-apply), which Julliard uses to import commits from mail. Julliard is in the habit of removing commit messages unless they're really informative. So, include your message as usual, and Julliard will decide if it's worth leaving in the commit log. If you want to make an argument for why to keep the commit message intact, you should add that after the "---" so it will still be in the mail, but won't be imported by git-am. Andrew

June 15, 2011

Add sub-message to a commit

by GOUJON Alexandre

Hi, I would like to add a 'sub'-message to a commit like this one http://source.winehq.org/git/wine.git/commitdiff/dbd878639424071e1d4c6cfc9b… I know that I can specify this text at the beginning of a patch I want to send to wine-patches (befor the '---'). But here, I'm interested in how to do that on the git server side. git commit -a or git commit -m let me specify a 'global' commit message. I googled some key words and RTFMs but didn't find anything relevant. Any hints on how to do that ? I can ask AJ directly but 1) it may be considered as rude and selfish 2) some people may be interested by the answer Thanks in advance

June 15, 2011

Re: [6/7] dxdiagn: Add additional adapter identifier properties for DxDiag_DisplayDevices subcontainers.

by Andrew Nguyen

On 06/15/2011 07:30 AM, Marvin wrote: > 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=11745 > > Your paranoid android. > > > === WINEBUILD (build) === > Patch failed to apply The test bot doesn't appear to be applying the patches correctly.

June 15, 2011

Re: [6/7] dxdiagn: Add additional adapter identifier properties for DxDiag_DisplayDevices subcontainers.

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=11745 Your paranoid android. === WINEBUILD (build) === Patch failed to apply

June 15, 2011

Re: TaskDialog implementation

by Jeff Latimer

On 11/06/11 17:45, Hans Leidekker wrote: > On Fri, 2011-06-10 at 23:29 -0500, Vincent Povirk wrote: >> Well, I think you may be right that changing it to E_NOTIMPL at this >> point would break something and might be a bad idea. Sadly, Hans >> didn't say what the stub was originally for. Hopefully, we'll soon >> have a working implementation and it won't matter. ;) > I added it for the Internet Explorer 9 installer but it has been > broken already when someone changed it to return IDYES instead of > IDOK in pnButton. I don't know which app that was for. > I changed it to work with uTorrent so that exit would work and you could delete torrents.

June 15, 2011

Re: dxdiag: Add code for XML information output

by Andrew Nguyen

On 06/14/2011 12:18 PM, Dan Kegel wrote: > Isn't Wine was supposed to avoid non-static struct initializers? > > I think it'll get "warning: initializer element is not computable at > load time" with --std=c89 --pedantic. > > I believe you're right, and I've sent a patch that avoids the construct.

June 15, 2011

Re: [PATCH 3/5] ddraw: Add iface count for IDirectSurfaceImpl.

by Henri Verbeet

2011/6/14 <rbarkauskas(a)codeweavers.com>: > +static void ddraw_surface_add_iface(IDirectDrawSurfaceImpl *This) > +{ > + ULONG refCount = InterlockedIncrement(&This->numIfaces); Please use wined3d code style for new code. I.e., This -> surface, refCount -> refcount, numIface -> iface_count, space after control statements, etc.

June 15, 2011

Re: [PATCH 2/5] ddraw: Adjust reference counts of created surfaces in ddraw_set_surface_version.

by Henri Verbeet

2011/6/14 <rbarkauskas(a)codeweavers.com>: > + switch(version) > + { > + case 4: > + IDirectDrawSurface4_AddRef(&surface->IDirectDrawSurface4_iface); > + break; > + case 3: > + case 2: > + case 1: > + IDirectDrawSurface_AddRef(&surface->IDirectDrawSurface_iface); > + break; > + } > + IDirectDrawSurface7_Release(&surface->IDirectDrawSurface7_iface); Why do you need this here instead of in the relevant surface creation functions? (I suppose because of the mipmaps, but I think it may be better to do all this when the surface is created instead of afterwards.)

June 15, 2011

Re: [PATCH 5/5] ddraw: Separate IDirectDrawSurface2 reference count.

by Alexandre Julliard

rbarkauskas(a)codeweavers.com writes: > @@ -766,6 +766,8 @@ DestroyCallback(IDirectDrawSurface7 *surf, > > /* Destroy the surface */ > while(ref) ref = IDirectDrawSurface7_Release(surf); > + ref = Impl->ref2; > + while(ref) ref = IDirectDrawSurface2_Release(&Impl->IDirectDrawSurface2_iface); Ref counts should only be accessed through the interlocked functions. -- Alexandre Julliard julliard(a)winehq.org

June 15, 2011

Re: deadlock in mmdevapi AudioClient_Stop (winealsa.drv/mmdevdrv.c)

by Maarten Lankhorst

Hi Joerg, Op 15-06-11 10:00, Joerg-Cyril.Hoehle(a)t-systems.com schreef: > Hi, > > please try and convince me that a deadlock cannot happen. > > http://source.winehq.org/source//dlls/winealsa.drv/mmdevdrv.c > > AudioClient_Start calls > CreateTimerQueueTimer > which creates a new thread periodically invoking the timer callback. > > The callback invokes > alsa_push_buffer_data() > which ceases the audio client object lock (This->lock). > > AudioClient_Stop performs > EnterCriticalSection(&This->lock); then > DeleteTimerQueueTimer(g_timer_q, This->timer, INVALID_HANDLE_VALUE); > > Now imagine Stop is invoked, EnterCriticalSection executed then the thread is preempted. > Suppose the timer callback kicks in: alsa_push_buffer_data will block on This->lock. > When the thread is resumed, DeleteTimerQueue will hang forever waiting for all timer callbacks to come to an end. > > My opinion is that critical sections are great to prevent concurrent execution while > things are running, but they are hell to tear down properly. > > IMHO, LeaveCriticalSection + Re-EnterCriticalSection around DeleteTimerQueue is not a solution. > I consider using Leave+EnterCS around anything that can wait/block an anti-pattern > (there are a few places like that in Wine). I believe a "restart op" is needed in general > (like a "restart transaction if it failed" in databases) as anything could have happened while waiting. You're right, This->timer should be set to NULL inside the lock, and DeleteTimerQueueTimer should be run afterwards. ~Maarten

June 15, 2011

deadlock in mmdevapi AudioClient_Stop (winealsa.drv/mmdevdrv.c)

by Joerg-Cyril.Hoehle＠t-systems.com

Hi, please try and convince me that a deadlock cannot happen. http://source.winehq.org/source//dlls/winealsa.drv/mmdevdrv.c AudioClient_Start calls CreateTimerQueueTimer which creates a new thread periodically invoking the timer callback. The callback invokes alsa_push_buffer_data() which ceases the audio client object lock (This->lock). AudioClient_Stop performs EnterCriticalSection(&This->lock); then DeleteTimerQueueTimer(g_timer_q, This->timer, INVALID_HANDLE_VALUE); Now imagine Stop is invoked, EnterCriticalSection executed then the thread is preempted. Suppose the timer callback kicks in: alsa_push_buffer_data will block on This->lock. When the thread is resumed, DeleteTimerQueue will hang forever waiting for all timer callbacks to come to an end. My opinion is that critical sections are great to prevent concurrent execution while things are running, but they are hell to tear down properly. IMHO, LeaveCriticalSection + Re-EnterCriticalSection around DeleteTimerQueue is not a solution. I consider using Leave+EnterCS around anything that can wait/block an anti-pattern (there are a few places like that in Wine). I believe a "restart op" is needed in general (like a "restart transaction if it failed" in databases) as anything could have happened while waiting. Regards, Jörg Höhle

June 15, 2011

Re: [4/6] winex11.drv: Grab registered hotkeys in the X server.

by Ken Thomases

On Jun 14, 2011, at 12:40 AM, Vincent Povirk wrote: >> Is it necessary to test QS_HOTKEY in dlls/winex11.drv/event.c:filter_event, where QS_KEY is currently tested? If somebody calls MsgWaitForMultipleObjectsEx with QS_HOTKEY but not QS_KEY, shouldn't KeyPress events be accepted? >> > > I guess? Wouldn't we have to actually set QS_HOTKEY before that does any good? You mean your patch 5? Well, it's a bit of a chicken-and-egg situation. I don't think it matters much which order the two things are added. They'll both ultimately be necessary. In some sense, doing winex11.drv first makes sense, since it's the first "gate" for the event to make it into the system. (Admittedly, it's probably very unlikely that MsgWaitForMultipleObjectsEx will be called with QS_HOTKEY and not QS_KEY.) >> You may be able to use the non-left-right-sensitive vkeys (e.g. VK_MENU) here, and thus eliminate the alt_vkey parameter. And if you can't, then update_modifier_state doesn't do the right thing when alt_vkey is the one that's pressed in the keystate but it's no longer pressed in event->state. In that case, you send a key-up for vkey, when you should send it for alt_vkey. If both were down and now neither is, you'll need to send a key-up for both. Note, to send keyboard input for a right-hand modifier, you'll need to include KEYEVENTF_EXTENDEDKEY in the flags and different scan codes. > > There's no such combined vkey for the windows key. Even if there were, > releasing VK_MENU when VK_LMENU was pressed doesn't seem like a good > idea. I was thinking that maybe the send-input machinery would take a side-generic key and do the right thing. A little more research reveals that it doesn't. However, that still leaves you doing the wrong thing with your side-sensitive code. If event->state doesn't include MetaMask, then you send a key-up for VK_LMENU when it may have been VK_RMENU that was previously down. You need to send a key-up for whichever was previously down, and that might even be both. Note that the important thing is not that you have sent a key event for the wrong key, it's that you haven't properly transitioned to a state which matches event->state. That is, you sent the key-up but the overall key state will still have a menu key down. > Now you've got me worried about what happens when we press VK_LWIN and > it later turns out VK_RWIN was really pressed (and has been released). > :/ For when you send a key-down, it's not so important. In that case, you're properly transitioning from neither being down to at least one of them being down. If it later turns out that you synthesized a VK_LWIN-down when the user had actually pressed VK_RWIN, then that will get corrected on the next KeymapNotify. -Ken

June 14, 2011

Re: [PATCH 1/6] server: Add server-side raw input functions definitions

by Austin English

2011/6/14 Vincas Miliūnas <vincas.miliunas(a)gmail.com>: > This is a partial raw input implementation for mouse and keyboard. > > I've read in the IRC chat, that products of tools/make_request should not be included in a patch, so I left them out. > > My first attempts were by leeching input events from queue_mouse_message and queue_keyboard_message functions, but I couldn't get mouse cursor movement right. Instead, I copied the way dinput receives input - by installing low level hooks and forwarding input to the server. This is just to get "try 1" out of the door. Of course, it would be better if the input feed would be server-side only. > > I hit a very weird bug, where in peek_message function, info.msg.lParam was impossible to set with the proper lParam (it was correct in reply->lparam) value, instead is was overwritten by some constant. That was happening only for the messages sent by the patch using post_message. I guess the tales of server-side dragons hold some truth... If anyone wants to try to reproduce it, just recompile without the unused #include "user.h" in raw_input.c and run user32 tests. > > This patch game-tested and works for Quake Live (in_mouse 2 for raw input), World of Tanks and Risen. > > Feedback is very much appreciated. Thanks. > > --- > server/protocol.def | 79 +++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 files changed, 79 insertions(+), 0 deletions(-) Howdy Vincas, This patch breaks the compile: $ patch -p1 < ~/75274 $ ./tools/make_requests $ ./configure --disable-tests $ make make[1]: Entering directory `/home/austin/wine-git/server' gcc -o wineserver async.o atom.o change.o class.o clipboard.o completion.o console.o debugger.o device.o directory.o event.o fd.o file.o handle.o hook.o mach.o mailslot.o main.o mapping.o mutex.o named_pipe.o object.o process.o procfs.o ptrace.o queue.o region.o registry.o request.o semaphore.o serial.o signal.o snapshot.o sock.o symlink.o thread.o timer.o token.o trace.o unicode.o user.o window.o winstation.o -L../libs/wine -lwine ../libs/port/libwine_port.a -Wl,--rpath,\$ORIGIN/../libs/wine request.o:(.rodata+0x494): undefined reference to `req_get_raw_input_device_list' request.o:(.rodata+0x498): undefined reference to `req_register_raw_input_device' request.o:(.rodata+0x49c): undefined reference to `req_unregister_raw_input_device' request.o:(.rodata+0x4a0): undefined reference to `req_is_raw_input_data' request.o:(.rodata+0x4a4): undefined reference to `req_get_raw_input_data' request.o:(.rodata+0x4a8): undefined reference to `req_get_registered_raw_input_devices' request.o:(.rodata+0x4ac): undefined reference to `req_add_raw_input_data' request.o:(.rodata+0x4b0): undefined reference to `req_get_raw_input_device_info' collect2: ld returned 1 exit status make[1]: *** [wineserver] Error 1 make[1]: Leaving directory `/home/austin/wine-git/server' make: *** [server] Error 2 which you implement in patch 5. Every patch needs to stand on its own, without breaking the tests/compile. For such a complicated changeset, it's probably best to send the tests in on their own first, with the failing tests wrapped in todo_wine(). See also http://wiki.winehq.org/SubmittingPatches BTW, thanks for working on this, I used your earlier patches to play a bit of Brink ;-). -- -Austin

June 14, 2011

Re: [PATCH 6/6] user32/tests: Add client-side raw input functions tests

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=11737 Your paranoid android. === W7PROX64 (64 bit input) === No test summary line found

June 14, 2011