Re: xbox 360 emulation with logitech cordless and dinput joy disable
On Wed, Jun 13, 2012 at 8:52 AM, Stefan Dösinger wrote: > Hi, > > The patch needs a few improvements before it can be accepted into Wine. I > can't comment on the dinput / xinput specifics of the patch, so this is a non- > exhaustive list of general suggestions. > > Concerning the big picture, this patch needs a few discussions about > xinput.dll design. There have been some discussions about that in the past, > and I hope that some dinput devs can help out. > > Am Dienstag, 12. Juni 2012, 23:48:53 schrieb Giovanni Ongaro: >> From: root > Please configure git correctly, see http://wiki.winehq.org/GitWine section > 3.3. > > Also consider using a non-root user, but that's your personal setup, so not > really my business. > >> + if (getenv("LINIXFORCENOJOY") != NULL) return FALSE; /*JoeFix disable > joystick*/ > The problem with dinput vs xinput conflicts should probably be solved in a > different manner. That aside, such configurations are usually done via > registry keys. Also comments like /* I changed this here */ are useless, the > git history stores this information anyway. > >> + <...> //JoeFix DEAD ZONE > This is a C++ comment and not allowed in Wine. See > http://wiki.winehq.org/SubmittingPatches > >> + fdJoy=open("/dev/input/js0",O_RDONLY | O_NONBLOCK); > Xinput.dll should not open joystick devices directy, and definitely not a > hardcoded one. I don't know if there's consensus about how xinput should be > implemented, but basing it on top of dinput might be a start. Xinput should not be layered on top of dinput. On Windows it is really a separate relatively low-level dll. The main problem with layering it on top of dinput is that we have to invent some Wine-specific extensions for force feedback which differs from dinput (as far as I remember). Definitely xbox support should not use the classic JS interface, but the event interface (that's also the only one which can be used for other xbox features like the rumble stuff, xbox leds, ..). > > A good start would be to write a few tests to test how Windows announces(or > doesn't announce) Xbox gamepads and other gamepads/joysticks to games. My > understanding is that xinput.dll only works for Xbox gamepads. Do those show > up in dinput on Windows? Xbox gamepads show up twice. Once the xbox360 gamepad driver is loaded, it 'injects' a fake USB Hid gamepad device which is used by dinput (and likely the legacy joystick API). There is no good way to differentiate between the two gamepads. Microsoft recommends applications which support both dinput and xinput to do enumeration through dinput and then use WMI to check if the gamepad is an xbox one. This doesn't work on Wine because we don't have proper WMI support (yet). > >> +#define DEAD_ZONE 4096 >> ... >> + if (e.number == 4) { >> + if (e.value == 1) { >> + FIXME("TRIGGER LEFT PRESSED\n"); >> + pState->Gamepad.bLeftTrigger=32767; >> + } >> + else { >> + pState->Gamepad.bLeftTrigger=0; >> + } >> ... >> +#define JOYMAX 3 >> +#define JOYMIN -3 > The hardcoded numbers look questionable. The 4 and 1 are probably specific to > the joystick, the 32767 might have a symbolic name like XINPUT_MAX or > something. The event interface provides proper limits for each axis. I think the js interface has too (forgot the specifics). Definitely axis and button names can't be hard coded to numbers. As I mention later on these numbers (especially through the js-interface) are device specific. Use BTN_A, ABS_X, and so on, but it is impossible to do this for different gamepads. > > A part of this seems to be needed due to the attempt to emulate an xbox > gamepad with a different device. Personally I'm not opposed to doing that, and > you might need some device specific tricks to pull this off. However, this > needs a more structured approach than hardcoding the values needed for one > specific Logitech device in the code. There are a lot of gamepads from different vendors which are xinput compatible. Supporting all of them is easy since they all use the same button names since they use the same driver. We can support non-xinput compatible ones, but it easily becomes tricky. What do you do if a gamepad doesn't have the same buttons? How do you map different button names (xpad devices use BTN_A/BTN_B, while others use BTN_BASE, BTN_BASE2, ..) to the layout of the gamepad? Further input ranges could in theory be different (you can scale values, but you could get still run into issues). Supporting non-xinput devices can be done, but I'm reluctant to doing it in Wine. It is impossible to get it right (every device is different) and it would be complicated for users too. Then there is the whole design part. We had this discussion a couple of times already. Alexandre used to be reluctant about adding OS-specific code to xinput. Personally I think it is the only way to properly imp
Re: [11/11] windowscodecs: Implement GetMetadataFormat. Resend.
Please ignore this patch, it doesn't really belong to this series, and correct implementation should use MetadataReaderInfo interface instead. -- Dmitry.
Re: msvcrt: Fix the name of the Portuguese locale alias.
On Wed, Jun 13, 2012 at 2:47 PM, Francois Gouget wrote: > ... > dlls/msvcrt/locale.c | 2 +- > dlls/msvcrt/tests/locale.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > ... > - "portugese", "ptb", > + "portuguese", "ptb", Just a side note. Although I'm not sure I think portuguese is "ptg", brazilian portuguese is "ptb". So the correct may be: > +"portuguese", "ptg", A Portugal user would be able to check that, or maybe it's easier to write a testcase. If I'm wrong I ask apologies in advance, Bruno
Re: [2/2] msxml3: Support xmlns[:*] attribute nodes intelligently.
I'll return to this patch -- merge, apply requested changes, test again -- a week from now. I'm unfortunately occupied elsewhere in the mean time. Thank you for your patience and quick & useful replies. - Ulrik
Re: [PATCH (try2)] winepulse.drv: Add PulseAudio driver
Andrew Eikum writes: > The configure.ac changes and parts of the driver itself were written by > Maarten Lankhorst. It doesn't work here, it's apparently using the driver even though PulseAudio is not running on this box: ../../../tools/runtest -q -P wine -M amstream.dll -T ../../.. -p amstream_test.exe.so amstream.c && touch amstream.ok err:pulse:AudioClient_Initialize PulseAudio no longer running? err:quartz:DSoundRender_create Cannot create Direct Sound object (88890010) fixme:ole:CoCreateInstance no instance created for interface {56a86895-0ad4-11ce-b03a-0020af0ba770} of class {79376820-07d0-11cf-a24d-0020afd79767}, hres is 0x88890010 amstream.c:274: Test failed: IAMMultiMediaStream_AddMediaStream returned: 88890010 -- Alexandre Julliard julli...@winehq.org
Re: [4/5] windowscodecs: Add test for IWICMetadataReaderInfo.
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=19009 Your paranoid android. === WINEBUILD (build) === Patch failed to apply
Re: [4/5] windowscodecs: Add test for IWICMetadataReaderInfo.
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=19010 Your paranoid android. === WINEBUILD (build) === Patch failed to apply
Re: [4/5] windowscodecs: Add test for IWICMetadataReaderInfo.
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=19007 Your paranoid android. === WINEBUILD (build) === Patch failed to apply
Re: [2/5] windowscodecs: Add stub IWICMetadataBlockReader to PNG decoder.
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=19006 Your paranoid android. === WINEBUILD (build) === Patch failed to apply
DllMain() seems unused in ntoskrnl.exe
The DllMain() function in ntoskrnl.exe appears to be unused: making it static causes a compiler warning to that effect and removing it entirely does not cause a compilation failure. Should it be removed or is there a build issue with ntoskrnl.exe? diff --git a/dlls/ntoskrnl.exe/ntoskrnl.c b/dlls/ntoskrnl.exe/ntoskrnl.c index 2b84297..ffeb2f8 100644 --- a/dlls/ntoskrnl.exe/ntoskrnl.c +++ b/dlls/ntoskrnl.exe/ntoskrnl.c @@ -1711,30 +1711,6 @@ VOID WINAPI IoInitializeRemoveLockEx(PIO_REMOVE_LOCK lock, ULONG tag, } /* - * DllMain - */ -BOOL WINAPI DllMain( HINSTANCE inst, DWORD reason, LPVOID reserved ) -{ -static void *handler; -LARGE_INTEGER count; - -switch(reason) -{ -case DLL_PROCESS_ATTACH: -DisableThreadLibraryCalls( inst ); -#ifdef __i386__ -handler = RtlAddVectoredExceptionHandler( TRUE, vectored_handler ); -#endif -KeQueryTickCount( &count ); /* initialize the global KeTickCount */ -break; -case DLL_PROCESS_DETACH: -RtlRemoveVectoredExceptionHandler( handler ); -break; -} -return TRUE; -} - -/* * Ke386IoSetAccessProcess (NTOSKRNL.EXE.@) */ BOOLEAN WINAPI Ke386IoSetAccessProcess(PEPROCESS *process, ULONG flag) -- Francois Gouget http://fgouget.free.fr/ Be careful of reading health books, you might die of a misprint. -- Mark Twain
Re: xbox 360 emulation with logitech cordless and dinput joy disable
Hi, The patch needs a few improvements before it can be accepted into Wine. I can't comment on the dinput / xinput specifics of the patch, so this is a non- exhaustive list of general suggestions. Concerning the big picture, this patch needs a few discussions about xinput.dll design. There have been some discussions about that in the past, and I hope that some dinput devs can help out. Am Dienstag, 12. Juni 2012, 23:48:53 schrieb Giovanni Ongaro: > From: root Please configure git correctly, see http://wiki.winehq.org/GitWine section 3.3. Also consider using a non-root user, but that's your personal setup, so not really my business. > + if (getenv("LINIXFORCENOJOY") != NULL) return FALSE; /*JoeFix disable joystick*/ The problem with dinput vs xinput conflicts should probably be solved in a different manner. That aside, such configurations are usually done via registry keys. Also comments like /* I changed this here */ are useless, the git history stores this information anyway. > + <...> //JoeFix DEAD ZONE This is a C++ comment and not allowed in Wine. See http://wiki.winehq.org/SubmittingPatches > +fdJoy=open("/dev/input/js0",O_RDONLY | O_NONBLOCK); Xinput.dll should not open joystick devices directy, and definitely not a hardcoded one. I don't know if there's consensus about how xinput should be implemented, but basing it on top of dinput might be a start. A good start would be to write a few tests to test how Windows announces(or doesn't announce) Xbox gamepads and other gamepads/joysticks to games. My understanding is that xinput.dll only works for Xbox gamepads. Do those show up in dinput on Windows? > +#define DEAD_ZONE 4096 > ... > + if (e.number == 4) { > + if (e.value == 1) { > + FIXME("TRIGGER LEFT PRESSED\n"); > + pState->Gamepad.bLeftTrigger=32767; > + } > + else { > + pState->Gamepad.bLeftTrigger=0; > + } > ... > +#define JOYMAX 3 > +#define JOYMIN -3 The hardcoded numbers look questionable. The 4 and 1 are probably specific to the joystick, the 32767 might have a symbolic name like XINPUT_MAX or something. A part of this seems to be needed due to the attempt to emulate an xbox gamepad with a different device. Personally I'm not opposed to doing that, and you might need some device specific tricks to pull this off. However, this needs a more structured approach than hardcoding the values needed for one specific Logitech device in the code. Best regards, Stefan signature.asc Description: This is a digitally signed message part.
Re: [1/2] msxml3/tests: Reduce code duplication for the namespace change tests.
On 6/13/2012 17:42, Ulrik Dickow wrote: Here's a new version of the patch that follows all of your 3 requests: Den 12-06-2012 13:11, Nikolay Sivov skrev: [...] Please use something like CLSID array with every available Document CLSID, instead of only testing 2 of them. There's a lot of examples for that in saxreader.c. [...] It's better to avoid nested test calls like that imho, you could just add another call in main test list. [...] When this is running on all CLSIDs please add free_bstrs() here. Is it ok now? Yes, that's what I meant. Thanks. Since it contains new functionality, namely test of more document versions (CLSIDs) than before, it was necessary to change the Subject of the patch. How do I make sure that the old patch changes status to Superseded on http://source.winehq.org/patches/ ? E.g. use the old subject with "(try 2)" added in the E-mail header, but the new Subject inside the attachment? Don't worry too much about that, just send it with correct subject and mention in mail body which patch is obsolete. Correct subject is more important obviously. Regards, Ulrik Dickow
Re: [1/2] msxml3/tests: Reduce code duplication for the namespace change tests.
Here's a new version of the patch that follows all of your 3 requests: Den 12-06-2012 13:11, Nikolay Sivov skrev: > [...] > Please use something like CLSID array with every available Document > CLSID, instead of only testing 2 of them. There's a lot of examples for > that in saxreader.c. > [...] > It's better to avoid nested test calls like that imho, you could just > add another call in main test list. > [...] > When this is running on all CLSIDs please add free_bstrs() here. Is it ok now? Since it contains new functionality, namely test of more document versions (CLSIDs) than before, it was necessary to change the Subject of the patch. How do I make sure that the old patch changes status to Superseded on http://source.winehq.org/patches/ ? E.g. use the old subject with "(try 2)" added in the E-mail header, but the new Subject inside the attachment? Regards, Ulrik Dickow >From 3cd892028ebf645a9caa02b55e99aded96460f07 Mon Sep 17 00:00:00 2001 From: Ulrik Dickow Date: Wed, 13 Jun 2012 16:19:02 +0200 Subject: msxml3/tests: Test namespace change for all document versions. --- dlls/msxml3/tests/domdoc.c | 159 +++- 1 files changed, 82 insertions(+), 77 deletions(-) diff --git a/dlls/msxml3/tests/domdoc.c b/dlls/msxml3/tests/domdoc.c index d4ff6a2..0c9d65a 100644 --- a/dlls/msxml3/tests/domdoc.c +++ b/dlls/msxml3/tests/domdoc.c @@ -7160,7 +7160,86 @@ static void test_testTransforms(void) free_bstrs(); } -static void test_namespaces(void) +struct namespaces_change_t { +const CLSID *clsid; +const char *name; +}; + +static const struct namespaces_change_t namespaces_change_test_data[] = { +{ &CLSID_DOMDocument, "CLSID_DOMDocument" }, +{ &CLSID_DOMDocument2, "CLSID_DOMDocument2" }, +{ &CLSID_DOMDocument26, "CLSID_DOMDocument26" }, +{ &CLSID_DOMDocument30, "CLSID_DOMDocument30" }, +{ &CLSID_DOMDocument40, "CLSID_DOMDocument40" }, +{ &CLSID_DOMDocument60, "CLSID_DOMDocument60" }, +{ 0 } +}; + +static void test_namespaces_change(void) +{ +const struct namespaces_change_t *class_ptr = namespaces_change_test_data; + +while (class_ptr->clsid) +{ +IXMLDOMDocument *doc = NULL; +IXMLDOMElement *elem = NULL; +IXMLDOMNode *node = NULL; + +VARIANT var; +HRESULT hr; +BSTR str; + +hr = CoCreateInstance(class_ptr->clsid, NULL, CLSCTX_INPROC_SERVER, + &IID_IXMLDOMDocument, (void**)&doc); +if (hr != S_OK) +{ +win_skip("failed to create class instance for %s\n", class_ptr->name); +class_ptr++; +continue; +} + +V_VT(&var) = VT_I2; +V_I2(&var) = NODE_ELEMENT; + +hr = IXMLDOMDocument_createNode(doc, var, _bstr_("ns:elem"), _bstr_("ns/uri"), &node); +EXPECT_HR(hr, S_OK); + +hr = IXMLDOMDocument_appendChild(doc, node, NULL); +EXPECT_HR(hr, S_OK); + +hr = IXMLDOMDocument_get_documentElement(doc, &elem); +EXPECT_HR(hr, S_OK); + +/* try same prefix, different uri */ +V_VT(&var) = VT_BSTR; +V_BSTR(&var) = _bstr_("ns/uri2"); + +hr = IXMLDOMElement_setAttribute(elem, _bstr_("xmlns:ns"), var); +EXPECT_HR(hr, E_INVALIDARG); + +/* try same prefix and uri */ +V_VT(&var) = VT_BSTR; +V_BSTR(&var) = _bstr_("ns/uri"); + +hr = IXMLDOMElement_setAttribute(elem, _bstr_("xmlns:ns"), var); +EXPECT_HR(hr, S_OK); + +hr = IXMLDOMElement_get_xml(elem, &str); +EXPECT_HR(hr, S_OK); +ok(!lstrcmpW(str, _bstr_("")), "got element %s for %s\n", + wine_dbgstr_w(str), class_ptr->name); +SysFreeString(str); + +IXMLDOMElement_Release(elem); +IXMLDOMDocument_Release(doc); + +free_bstrs(); + +class_ptr++; +} +} + +static void test_namespaces_basic(void) { static const CHAR namespaces_xmlA[] = "\n" @@ -7175,7 +7254,6 @@ static void test_namespaces(void) IXMLDOMNode *node; VARIANT_BOOL b; -VARIANT var; HRESULT hr; BSTR str; @@ -7252,80 +7330,6 @@ static void test_namespaces(void) IXMLDOMDocument_Release(doc); -/* create on element and try to alter namespace after that */ -doc = create_document(&IID_IXMLDOMDocument); -if (!doc) return; - -V_VT(&var) = VT_I2; -V_I2(&var) = NODE_ELEMENT; - -hr = IXMLDOMDocument_createNode(doc, var, _bstr_("ns:elem"), _bstr_("ns/uri"), &node); -EXPECT_HR(hr, S_OK); - -hr = IXMLDOMDocument_appendChild(doc, node, NULL); -EXPECT_HR(hr, S_OK); - -hr = IXMLDOMDocument_get_documentElement(doc, &elem); -EXPECT_HR(hr, S_OK); - -V_VT(&var) = VT_BSTR; -V_BSTR(&var) = _bstr_("ns/uri2"); - -hr = IXMLDOMElement_setAttribute(elem, _bstr_("xmlns:ns"), var); -EXPECT_HR(hr, E_INVALIDARG); - -V_VT(&var) = VT_BSTR; -V_BSTR(&var) = _bstr_("ns/uri"); - -hr = IXMLDOMElement_
Re: Re : d3dx9_36: D3DXQuaternionLn computes as if the norm of the input is 1
2012/6/13 Nozomi Kodama : >>>2012/6/12 Nozomi Kodama : >>> >>> + if ( (pq->w >= 1.0f) || (pq->w == -1.0f) ) >> >>I think the second comparison should be '<=', if you want to avoid getting >> NaNs. > > I checked in Vista. D3DX accepts -1.0f as input and returns what the patch > does. > > However, any value < -1.0f is a unacceptable value for D3DX ( D3DX returns > (-1#IND00,-1#IND00,-1#IND00,-1#IND00) ). > > > Best regards > Nozomi Oh, so it actually returns NaNs. Interesting... So the patch is fine as it is. Maybe you could try to add such a case to the tests, if it doesn't get too tricky. You could check for something like gotquat.x != gotquat.x, which should return true only if gotquat.x is NaN.
Re: wined3d: Avoid sizeof on structs with variable length arrays.
On 13 June 2012 13:27, Michael Stefaniuc wrote: > Uh... Any reason that field cannot be just > struct wined3d_adapter adapter; > ? > Not really, but if we actually supported multihead we'd have multiple adapters and would need to change it back again.
Re: wined3d: Avoid sizeof on structs with variable length arrays.
On 06/13/2012 12:56 PM, Henri Verbeet wrote: > On 13 June 2012 10:41, Michael Stefaniuc wrote: >> -object = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(*object)); >> +object = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, >> FIELD_OFFSET(struct wined3d, adapters[1])); > > That's not really a VLA, just a somewhat stupid array of size 1. Uh... Any reason that field cannot be just struct wined3d_adapter adapter; ? bye michael
Re: wined3d: Avoid sizeof on structs with variable length arrays.
On 13 June 2012 10:41, Michael Stefaniuc wrote: > - object = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(*object)); > + object = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, > FIELD_OFFSET(struct wined3d, adapters[1])); That's not really a VLA, just a somewhat stupid array of size 1.
Re: Misspelt Portuguese in msvcrt
On 06/13/12 05:19, Francois Gouget wrote: Is there a reason why Portuguese is misspelt in these two places? It's a typo. Thanks for spotting it.
Re: [PATCH 1/4] hhctrl.ocx: Add HTML to Unicode decoding capability (try 3).
Hi Erich, On 06/12/12 19:03, Erich E. Hoover wrote: > Real Name: > Erich Hoover > > Description: > This patch adds the ability in HTML Help to convert HTML encoded > characters (e.g. ê) into the Unicode character equivalent. This > feature is needed by the table of contents and the index for > displaying international characters in some CHM files. With this > version of the patch the decoding is done manually by parsing the HTML > characters instead of using the web browser control, the search is a > now a binary search, and some additional cleanup. It is important to > note that HTML Help only supports characters within the ANSI code > pages, so support for multi-byte characters is not necessary. Many > apologizes to Jacek Caban for missing some of his comments in his > original reply. > > Changelog: > hhctrl.ocx: Add HTML to Unicode decoding capability. The patch looks much better. There is one remaining problem: + int pos = sizeof(html_encoded_symbols)/sizeof(html_encoded_symbols[0]); + float step = pos/2; + int dir = +1; + + do + { + const char *encoded_symbol; + + pos += -dir*ceil(step); Using float here is definitely not needed, esp. that you always use ceil() on it. You've made it more complicated than it really is. See [1] for an example. Also feel free to send linear search with FIXME comment. That's fine for the first iteration patch. Jacek [1] http://source.winehq.org/git/wine.git/blob/HEAD:/dlls/mshtml/dispex.c#l709