February 2014 - Wine-Devel - list.winehq.org

Re: Assorted spelling fixes

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 https://newtestbot.winehq.org/JobDetails.pl?Key=4916 Your paranoid android. === w8 (32 bit midi) === midi.c:518: Test failed: midiOutPrepare old size rc=MMSYSERR_INVALPARAM midi.c:519: Test failed: MHDR.dwFlags when prepared 0 midi.c:523: Test failed: midiStreamOut old size rc=MIDIERR_UNPREPARED midi.c:547: Test failed: MHDR.dwFlags 0 no ISSTRM when out of queue midi.c:553: Test failed: MHDR.dwFlags 0 not DONE when out of queue midi.c:554: Test failed: Wait failed: 0102 -559038737 midi.c:554: Test failed: Expect message 3ca from midiStream callback midi.c:555: Test failed: Wait failed: 0102 -559038737 midi.c:555: Test failed: Expect message 3c9 from midiStreamOut midi.c:558: Test failed: play left MIDIHDR.dwOffset at 1234567890 midi.c:561: Test failed: midiOutUnprepare rc=MMSYSERR_INVALPARAM midi.c:563: Test failed: midiOutUnprepare #2 rc=MMSYSERR_INVALPARAM midi.c:566: Test failed: MHDR.dwFlags when done 0 midi.c:580: Test failed: stream set tempo 500000 midi.c:588: Test failed: midiOutPrepare used flags 8 rc=MMSYSERR_INVALPARAM midi.c:590: Test failed: midiOutUnprepare used flags 8 rc=MMSYSERR_INVALPARAM midi.c:518: Test failed: midiOutPrepare old size rc=MMSYSERR_INVALPARAM midi.c:519: Test failed: MHDR.dwFlags when prepared 0 midi.c:523: Test failed: midiStreamOut old size rc=MIDIERR_UNPREPARED midi.c:547: Test failed: MHDR.dwFlags 0 no ISSTRM when out of queue midi.c:553: Test failed: MHDR.dwFlags 0 not DONE when out of queue midi.c:554: Test failed: Wait failed: 0102 -559038737 midi.c:554: Test failed: Expect message 3ca from midiStream callback midi.c:555: Test failed: Wait failed: 0102 -559038737 midi.c:555: Test failed: Expect message 3c9 from midiStreamOut midi.c:558: Test failed: play left MIDIHDR.dwOffset at 1234567890 midi.c:561: Test failed: midiOutUnprepare rc=MMSYSERR_INVALPARAM midi.c:563: Test failed: midiOutUnprepare #2 rc=MMSYSERR_INVALPARAM midi.c:566: Test failed: MHDR.dwFlags when done 0 midi.c:580: Test failed: stream set tempo 500000 midi.c:588: Test failed: midiOutPrepare used flags 8 rc=MMSYSERR_INVALPARAM midi.c:590: Test failed: midiOutUnprepare used flags 8 rc=MMSYSERR_INVALPARAM

Feb. 16, 2014

Re: Advice fixing #13683 (DirectX mouse overlay?)

by Ken Thomases

On Feb 15, 2014, at 6:17 PM, Christopher Thielen wrote: > I'm attempting to fix http://bugs.winehq.org/show_bug.cgi?id=13683 but need some advice as to where to start. I'm not too familiar with hacking on Wine. > > I'd like to test Stefan Dösinger's theory outlined in the comments for #13683 that the missing second mouse cursor is perhaps being implemented using DirectX overlays and that Wine's lack of overlay compatibility flags is to blame. > > Would the proper action to test this theory be to fake a certain set of flags in dlls/d3d8/device.c's static HRESULT WINAPI d3d8_device_GetDeviceCaps(IDirect3DDevice8 *iface, D3DCAPS8 *caps) ? If that proves to be the issue, I'd then need to write proper code to report the correct set of flags presumably. Probably you should start by collecting a log with the +ddraw,+d3d,+d3d_surface channels and looking for messages that mention "overlay". >From a cursory look, it seems that wined3d supports some types of overlay surfaces and some operations on those surfaces, but not all. > Is there any easy way to see which DLLs Wine loads when running an application? You can look at a +module log. -Ken

Feb. 16, 2014

Advice fixing #13683 (DirectX mouse overlay?)

by Christopher Thielen

Hi wine-devel, I'm attempting to fix http://bugs.winehq.org/show_bug.cgi?id=13683 but need some advice as to where to start. I'm not too familiar with hacking on Wine. I'd like to test Stefan Dösinger's theory outlined in the comments for #13683 that the missing second mouse cursor is perhaps being implemented using DirectX overlays and that Wine's lack of overlay compatibility flags is to blame. Would the proper action to test this theory be to fake a certain set of flags in dlls/d3d8/device.c's static HRESULT WINAPI d3d8_device_GetDeviceCaps(IDirect3DDevice8 *iface, D3DCAPS8 *caps) ? If that proves to be the issue, I'd then need to write proper code to report the correct set of flags presumably. Is there any easy way to see which DLLs Wine loads when running an application? I appreciate any advice, I'm new but want to help, Christopher Thielen

Feb. 16, 2014

Re: Fixing up the code style

by Henri Verbeet

On 15 February 2014 07:32, Erich E. Hoover <erich.e.hoover(a)gmail.com> wrote: > I'm not sure it's always clear what to do even the case of more > experienced contributors, I've been taking a look at fixing a double > unlock bug in RtlAcquireResourceExclusive ( > http://source.winehq.org/source/dlls/ntdll/rtl.c#L157 ). And the > style there indent-wise is so screwy that it's not clear what I should > do in my patch. This one function currently has (level wise): > 1) Four space indents > 2) A tab indent + a space indent (in some cases just the tab) > 3) A tab indent + a five space indent (alternatively, 13 spaces) > 4) Two tabs + a space indent > > So, what do I do? Do I just change line 190 to fix the bug and keep > the two tab + a space indent? Do I fix just this line to conform to > the standard since it's the only one I need to touch? Do I fix this > one function because a lot of it doesn't conform to the standard? Personally, in this specific case, I'd probably just change the single line, and replace the tabs with spaces there. In a module I'm more familiar with like wined3d I'd probably touch up slightly more. Except for reformatting the entire function, I doubt Alexandre would reject any of your approaches though, provided the patch is otherwise fine. As for the original point of this thread, if Stefan spends considerable amounts of time explaining coding style to new contributors, perhaps it's worth the effort to maintain a page on the wiki, and point people to that instead.

Feb. 15, 2014

Re: [PATCH 1/7] wined3d: Restore the pixel format of the window whose pixel format was actually changed. (try 2)

by Henri Verbeet

On 15 February 2014 03:45, Ken Thomases <ken(a)codeweavers.com> wrote: > When I implement WGL_WINE_make_current, it will be more complicated again and will need gl_info to check if the extension is supported. (I actually already have that implemented locally. I've been waiting for this pixel format stuff to clear before submitting it.) > Ok, makes sense.

Feb. 15, 2014

Re: [PATCH 2/7] d3d9/tests: Test that Direct3D9 doesn't modify the pixel format of the window it targets. (try 3)

by Henri Verbeet

On 15 February 2014 03:52, Ken Thomases <ken(a)codeweavers.com> wrote: > Hmm. I won't be able to double-check until Monday, but I'm pretty sure I got the opengl32 test_setpixelformat() to fail on a real Win7 machine just by commenting out the wglCreateContext() call (and the other tests) that is done before it. That is, running the executable from <https://newtestbot.winehq.org/JobDetails.pl?Key=4752>. > > Assuming it works without the wglCreateContext(), would you prefer the dynamic loading approach to the import I had done? > Yeah.

Feb. 15, 2014

Re: Fixing up the code style

by Nikolay Sivov

On 2/15/2014 10:32, Erich E. Hoover wrote: > On Thu, Feb 13, 2014 at 7:24 AM, Stefan Dösinger > <stefandoesinger(a)gmail.com> wrote: >> ... >> Maybe, or maybe not. The basic issue still stands. New contributors >> will look at the existing code, and if its inconsistent how are they >> supposed to write code that's consistent in itself. That's not a >> question of pickiness but approach to the problem. >> ... > I'm not sure it's always clear what to do even the case of more > experienced contributors, I've been taking a look at fixing a double > unlock bug in RtlAcquireResourceExclusive ( > http://source.winehq.org/source/dlls/ntdll/rtl.c#L157 ). And the > style there indent-wise is so screwy that it's not clear what I should > do in my patch. This one function currently has (level wise): > 1) Four space indents > 2) A tab indent + a space indent (in some cases just the tab) > 3) A tab indent + a five space indent (alternatively, 13 spaces) > 4) Two tabs + a space indent > > So, what do I do? Please send a fix that changes only what you need to change, why not? > Do I just change line 190 to fix the bug and keep > the two tab + a space indent? Do I fix just this line to conform to > the standard since it's the only one I need to touch? Do I fix this > one function because a lot of it doesn't conform to the standard? > Personally, I would like to have a separate formatting patch to fix > the function so that it's clear that I'm just fixing the one thing - > but that's expressly prohibited under the current guidelines. > >> ... >> My impression was that the answers to that were no and no, with the >> idea that the problem will be fixed by unifying the style together >> with other, functional changes. That way we end up with picky reviews >> and IMO that approach is bad. >> ... > I agree, Sebastian and I have been debating what to do with this > function (and RtlAcquireResourceShared) for some time and style is the > only thing keeping us from submitting the one-line fix for these. I don't see a problem here, if it's a one-line fix just submit it while keeping existing formatting of a changed line, what's a big deal? Yes, sometimes style is inconsistent, sometimes whitespace formatting differs from line to line but that kind of existing code "features" should never stop you from sending a fix. In a situation like that I usually change several lines before and after to use 4 spaces indentation. > Honestly, it would be really great if we could run something like > astyle on the whole codebase so that we were always working off of a > "known good" style. If we could get that implemented then we could > potentially think about verifying the patch style on submission and > auto-rejecting patches that have style problems. Personally, I think > that would be an amazing improvement to our submission process, I don't think it's ever a case that potential contributor decides not to do any work solely after looking at some code style issues and saying - oh, well, what a mess, I'm not going to touch it. What actually happens when people send patches (or attach them to a bug and then disappear) is that patch is wrong in a first place, and code style issues are a second problem that's easy to fix comparing to functional part. Of course if all code uses same style we could simply avoid step 2 but that's not a blocking issue for new contributors. Another thing is that some parts of wine have long time contributors, like wined3d, mshtml, msvcrt or audio bits. Why should we force their maintainers to some particular code style? I personally can live with any decision made regarding reformatting everything, as long as it doesn't use some ugly mixed case names, variable names with type prefixes, LPLPLP stuff and such. Anyway formatting issues never surprised me that much to stop fixing a real problem, but that's probably because I'm constantly jumping from one part to another, and from project to project on my daily job. > but I > have to use K&R style for work and I hate feeling like an idiot when I > slip into the wrong style for a project. > > Best, > Erich > > >

Feb. 15, 2014

Re: Fixing up the code style

by Erich E. Hoover

On Thu, Feb 13, 2014 at 7:24 AM, Stefan Dösinger <stefandoesinger(a)gmail.com> wrote: > ... > Maybe, or maybe not. The basic issue still stands. New contributors > will look at the existing code, and if its inconsistent how are they > supposed to write code that's consistent in itself. That's not a > question of pickiness but approach to the problem. > ... I'm not sure it's always clear what to do even the case of more experienced contributors, I've been taking a look at fixing a double unlock bug in RtlAcquireResourceExclusive ( http://source.winehq.org/source/dlls/ntdll/rtl.c#L157 ). And the style there indent-wise is so screwy that it's not clear what I should do in my patch. This one function currently has (level wise): 1) Four space indents 2) A tab indent + a space indent (in some cases just the tab) 3) A tab indent + a five space indent (alternatively, 13 spaces) 4) Two tabs + a space indent So, what do I do? Do I just change line 190 to fix the bug and keep the two tab + a space indent? Do I fix just this line to conform to the standard since it's the only one I need to touch? Do I fix this one function because a lot of it doesn't conform to the standard? Personally, I would like to have a separate formatting patch to fix the function so that it's clear that I'm just fixing the one thing - but that's expressly prohibited under the current guidelines. > ... > My impression was that the answers to that were no and no, with the > idea that the problem will be fixed by unifying the style together > with other, functional changes. That way we end up with picky reviews > and IMO that approach is bad. > ... I agree, Sebastian and I have been debating what to do with this function (and RtlAcquireResourceShared) for some time and style is the only thing keeping us from submitting the one-line fix for these. Honestly, it would be really great if we could run something like astyle on the whole codebase so that we were always working off of a "known good" style. If we could get that implemented then we could potentially think about verifying the patch style on submission and auto-rejecting patches that have style problems. Personally, I think that would be an amazing improvement to our submission process, but I have to use K&R style for work and I hate feeling like an idiot when I slip into the wrong style for a project. Best, Erich

Feb. 15, 2014

RE: Converting source to unicode, bug 28189

by Hugh McMaster

On Saturday, 15 February 2014 3:00 AM, Akihiro Sagawa wrote: >[My idea] >static int some_function(void) >{ > /* ... */ > output_write(STRING_FOO); > output_write(STRING_BAR); > /* ... */ >} > >static void __cdecl output_write(UINT id, ...) { > /* variable declarations */ > > if (Silent) > return; /* do nothing */ > > /* then, LoadString, FormatMessage, etc. */ } > >Does this work for you? Yes. This makes perfect sense now. Thank you.

Feb. 15, 2014

Re: [PATCH 2/7] d3d9/tests: Test that Direct3D9 doesn't modify the pixel format of the window it targets. (try 3)

by Ken Thomases

On Feb 14, 2014, at 11:53 AM, Henri Verbeet wrote: > On 14 February 2014 00:31, Ken Thomases <ken(a)codeweavers.com> wrote: >> + /* If no opengl32 function has been called, GetPixelFormat() doesn't reflect a >> + prior SetPixelFormat(). So, call wglCreateContext() even though it will fail. */ >> + hglrc = wglCreateContext(hdc); >> + ok(hglrc == NULL, "wglCreateContext succeeded when no pixel format has been set\n"); >> + if (hglrc) wglDeleteContext(hglrc); > I decided to look into this a bit, but could only reproduce it on the > testbot. It seems just loading opengl32 is enough to make > GetPixelFormat() work. (Also see > https://newtestbot.winehq.org/JobDetails.pl?Key=4908.) Hmm. I won't be able to double-check until Monday, but I'm pretty sure I got the opengl32 test_setpixelformat() to fail on a real Win7 machine just by commenting out the wglCreateContext() call (and the other tests) that is done before it. That is, running the executable from <https://newtestbot.winehq.org/JobDetails.pl?Key=4752>. Assuming it works without the wglCreateContext(), would you prefer the dynamic loading approach to the import I had done? -Ken

Feb. 15, 2014