September 2016 - Wine-devel

Re: [PATCH 3/3] ddraw/tests: Port test_transform_vertices to ddraw2 and ddraw4.
by Marvin 01 Sep '16

01 Sep '16

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://testbot.winehq.org/JobDetails.pl?Key=25522 Your paranoid android. === w1064 (32 bit ddraw2) === ddraw2.c:2391: Test failed: Expected (0,0)-(512,384), got (0,0)-(1024,768). === w1064 (64 bit ddraw2) === ddraw2.c:2391: Test failed: Expected (0,0)-(512,384), got (0,0)-(1024,768). === w1064 (32 bit ddraw4) === ddraw4.c:2546: Test failed: Expected (0,0)-(512,384), got (0,0)-(1024,768). === w1064 (64 bit ddraw4) === ddraw4.c:2546: Test failed: Expected (0,0)-(512,384), got (0,0)-(1024,768).

1 0

Re: [PATCH 2/3] ddraw/tests: Test matrices in TransformVertices.
by Marvin 01 Sep '16

01 Sep '16

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://testbot.winehq.org/JobDetails.pl?Key=25521 Your paranoid android. === w1064 (32 bit ddraw1) === ddraw1.c:2240: Test failed: Expected (0,0)-(512,384), got (0,0)-(1024,768). === w1064 (64 bit ddraw1) === ddraw1.c:2240: Test failed: Expected (0,0)-(512,384), got (0,0)-(1024,768).

1 0

Re: [PATCH 1/3] ddraw/tests: Make the vec parameter of compare_vec4 const.
by Marvin 01 Sep '16

01 Sep '16

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://testbot.winehq.org/JobDetails.pl?Key=25520 Your paranoid android. === w1064 (32 bit ddraw4) === ddraw4.c:2542: Test failed: Expected (0,0)-(512,384), got (0,0)-(1024,768). === w1064 (64 bit ddraw4) === ddraw4.c:2542: Test failed: Expected (0,0)-(512,384), got (0,0)-(1024,768). === w1064 (32 bit ddraw7) === ddraw7.c:2272: Test failed: Expected (0,0)-(512,384), got (0,0)-(1024,768). === w1064 (64 bit ddraw7) === ddraw7.c:2272: Test failed: Expected (0,0)-(512,384), got (0,0)-(1024,768).

1 0

Re: Add a simple 3D APIs demo
by Matteo Bruni 01 Sep '16

01 Sep '16

2016-08-19 21:41 GMT+02:00 Ruslan Kabatsayev <b7.10110111(a)gmail.com>: > This adds a 3d-demo wine application. It will allow users to check that their > system can handle at least basic WGL, D3D8 and D3D9 before they need to look > deeper into their problem. > > Signed-off-by: Ruslan Kabatsayev <b7.10110111(a)gmail.com> > --- > configure | 26 +- > configure.ac | 1 + > programs/3d-demo/Makefile.in | 14 + > programs/3d-demo/common.c | 107 ++++++ > programs/3d-demo/common.h | 36 ++ > programs/3d-demo/d3d.c | 605 ++++++++++++++++++++++++++++++++ > programs/3d-demo/d3d8.c | 2 + > programs/3d-demo/d3d9.c | 2 + > programs/3d-demo/demo.rc | 61 ++++ > programs/3d-demo/main.c | 175 +++++++++ > programs/3d-demo/resource.h | 33 ++ > programs/3d-demo/wgl.c | 473 +++++++++++++++++++++++++ > programs/3d-demo/winehq_logo_glass.png | Bin 0 -> 31512 bytes > 13 files changed, 1517 insertions(+), 18 deletions(-) > create mode 100644 programs/3d-demo/Makefile.in > create mode 100644 programs/3d-demo/common.c > create mode 100644 programs/3d-demo/common.h > create mode 100644 programs/3d-demo/d3d.c > create mode 100644 programs/3d-demo/d3d8.c > create mode 100644 programs/3d-demo/d3d9.c > create mode 100644 programs/3d-demo/demo.rc > create mode 100644 programs/3d-demo/main.c > create mode 100644 programs/3d-demo/resource.h > create mode 100644 programs/3d-demo/wgl.c > create mode 100644 programs/3d-demo/winehq_logo_glass.png I have a few pretty generic suggestions. You don't need to include configure changes in the patch, they will be regenerated by Alexandre anyway since those depend on the version of the autotools. Did you look into adding those tests to dxdiag instead of as a separate program? I don't think a separate program is necessarily bad but if integrating into dxdiag is reasonable it's probably better to do that from the start. Avoid camelCase or similar mixed-case identifier styles, just use lowercase_with_underscores. For pointer declarations you want to stick the '*' to the variable, not the type. Also please avoid LPTYPES, just use explicit pointer types. C++ comments (// comment) aren't allowed in Wine sources. Please replace them with normal C comments (/* comment */), or drop them entirely if they don't add anything that isn't clear from just looking at the code. It would be better to use explicit float constants when assigning to float variables (e.g. D3DMATERIAL). There aren't many other general style rules in Wine (it's mostly about using the style already used in the component so for new components you're mostly free to pick your own) as long as there is consistency. I see you mixing space after ',' with no space, pick one (preferably the space variant IMO) and stick to it. We usually use parentheses with the "sizeof" operator in Wine sources, so while it's not strictly required it would probably be nice to follow the general trend. More specific comments: I'd split this over multiple patches. You can probably make an initial patch adding the program with no real functionality, then multiple patches adding the various graphical API demos. In that regard, the d3d8 / d3d9 "merged" handling via macros seems a bit obfuscating to me. Just replicating the code twice seems cleaner. Maybe you can create a few helper functions for the common code. BTW, you can probably make things a bit more structured by moving the main loop to common code and have it call API backend functions. You'd have for each backend a structure like: static const struct render_backend d3d9_backend { d3d9_list_modes, d3d9_init, d3d9_draw, d3d9_wndproc, d3d9_destroy, }; (just an idea WRT the actual functions, I haven't really looked in detail) and you'd pass the structure from the selected API to the "main loop" function. I don't think you're using GLU anymore (good) so you should drop the glu32 import. I haven't looked in detail at the actual demos / renderers but I think there is enough stuff to work on already :)

4 6

Re: [v2] Add a GUI for dxdiag program
by Matteo Bruni 01 Sep '16

01 Sep '16

2016-08-27 10:26 GMT+02:00 Ruslan Kabatsayev <b7.10110111(a)gmail.com>: > This adds a dialog with only System tab, reflecting what save-to-file > functionality currently provides. It can also save the report as file > from the GUI in a way similar to its Windows counterpart. > This version now also shows DMI information which current dxdiagn.dll doesn't > provide, but may do so in the future. Also some spacing issues are fixed. > > Signed-off-by: Ruslan Kabatsayev <b7.10110111(a)gmail.com> > --- > programs/dxdiag/Makefile.in | 3 +- > programs/dxdiag/dxdiag.rc | 64 +++++++++++ > programs/dxdiag/dxdiag_private.h | 9 +- > programs/dxdiag/main.c | 20 ++-- > programs/dxdiag/main_dialog.c | 220 ++++++++++++++++++++++++++++++++++++++ > programs/dxdiag/resource.h | 56 ++++++++++ > 6 files changed, 356 insertions(+), 16 deletions(-) > create mode 100644 programs/dxdiag/main_dialog.c > create mode 100644 programs/dxdiag/resource.h Hi Ruslan, first of all, I think this patch should be splitted. For example, you could split the "save all information" button functionality, together with the common dialog include and dll linkage, to a separate patch. Aside from that, I have some additional comments. I'm not exactly a Win32 GUI expert so I might certainly have missed something significant. Anyway... > diff --git a/programs/dxdiag/Makefile.in b/programs/dxdiag/Makefile.in > index 437cc5e..d971a55 100644 > --- a/programs/dxdiag/Makefile.in > +++ b/programs/dxdiag/Makefile.in > @@ -1,10 +1,11 @@ > MODULE = dxdiag.exe > APPMODE = -mwindows -municode > -IMPORTS = dxguid ole32 oleaut32 user32 > +IMPORTS = dxguid ole32 oleaut32 user32 comdlg32 Keep it sorted alphabetically. > > C_SRCS = \ > information.c \ > main.c \ > + main_dialog.c \ > output.c Formatting. > +IDD_SYSTEM_TAB DIALOG 0, 0, 387, 176 > +STYLE DS_SETFONT | WS_CHILDWINDOW > +FONT 8, "Ms Shell Dlg" > +{ > + GROUPBOX "System Information", IDC_SYSINFO_GROUP, 3, 65, 381, 136, 0, WS_EX_LEFT > + LTEXT "If you know what area is causing the problem, click the appropriate tab above. Otherwise, you can use the ""Next Page"" button below to visit each page in sequence.", IDC_SYSTEMTAB_LABEL1, 9, 37, 344, 17, SS_LEFT, WS_EX_LEFT > + LTEXT "This tool reports information about the DirectX components and drivers installed on your system. It lets you test functionality and diagnose problems.", IDC_SYSTEMTAB_LABEL2, 9, 11, 370, 17, SS_LEFT, WS_EX_LEFT This text seems way too similar to the text in the original application. I'm not sure it adds much, maybe just dropping it entirely is fine? > + RTEXT "Current Date/Time:", IDC_SYSTEMTAB_DATETIME_LABEL, 10, 74, 140, 9, SS_RIGHT, WS_EX_LEFT > + LTEXT "", IDC_SYSTEMTAB_DATETIME_VALUE, 153, 74, 227, 9, SS_LEFT, WS_EX_LEFT > + > + RTEXT "Computer name:", IDC_SYSTEMTAB_PCNAME_LABEL, 10, 84, 140, 9, SS_RIGHT, WS_EX_LEFT > + LTEXT "", IDC_SYSTEMTAB_PCNAME_VALUE, 153, 84, 227, 9, SS_LEFT, WS_EX_LEFT > + > + RTEXT "Operating System:", IDC_SYSTEMTAB_OS_LABEL, 10, 94, 140, 9, SS_RIGHT, WS_EX_LEFT > + LTEXT "", IDC_SYSTEMTAB_OS_VALUE, 153, 94, 227, 9, SS_LEFT, WS_EX_LEFT > + > + RTEXT "Language:", IDC_SYSTEMTAB_LANG_LABEL, 10, 104, 140, 9, SS_RIGHT, WS_EX_LEFT > + LTEXT "", IDC_SYSTEMTAB_LANG_VALUE, 153, 104, 227, 9, SS_LEFT, WS_EX_LEFT > + > + RTEXT "System Manufacturer:", IDC_SYSTEMTAB_VENDOR_LABEL, 10, 114, 140, 9, SS_RIGHT, WS_EX_LEFT > + LTEXT "", IDC_SYSTEMTAB_VENDOR_VALUE, 153, 114, 227, 9, SS_LEFT, WS_EX_LEFT > + > + RTEXT "System Model:", IDC_SYSTEMTAB_MODEL_LABEL, 10, 124, 140, 9, SS_RIGHT, WS_EX_LEFT > + LTEXT "", IDC_SYSTEMTAB_MODEL_VALUE, 153, 124, 227, 9, SS_LEFT, WS_EX_LEFT > + > + RTEXT "BIOS:", IDC_SYSTEMTAB_BIOS_LABEL, 10, 134, 140, 9, SS_RIGHT, WS_EX_LEFT > + LTEXT "", IDC_SYSTEMTAB_BIOS_VALUE, 153, 134, 227, 9, SS_LEFT, WS_EX_LEFT > + > + RTEXT "Processor:", IDC_SYSTEMTAB_CPU_LABEL, 10, 144, 140, 9, SS_RIGHT, WS_EX_LEFT > + LTEXT "", IDC_SYSTEMTAB_CPU_VALUE, 153, 144, 227, 9, SS_LEFT, WS_EX_LEFT > + > + RTEXT "Memory:", IDC_SYSTEMTAB_RAM_LABEL, 10, 154, 140, 9, SS_RIGHT, WS_EX_LEFT > + LTEXT "", IDC_SYSTEMTAB_RAM_VALUE, 153, 154, 227, 9, SS_LEFT, WS_EX_LEFT > + > + RTEXT "Page File:", IDC_SYSTEMTAB_PAGEFILE_LABEL, 10, 164, 140, 9, SS_RIGHT, WS_EX_LEFT > + LTEXT "", IDC_SYSTEMTAB_PAGEFILE_VALUE, 153, 164, 227, 9, SS_LEFT, WS_EX_LEFT > + > + RTEXT "DirectX Version:", IDC_SYSTEMTAB_DXVERSION_LABEL, 10, 174, 140, 9, SS_RIGHT, WS_EX_LEFT > + LTEXT "", IDC_SYSTEMTAB_DXVERSION_VALUE, 153, 174, 227, 9, SS_LEFT, WS_EX_LEFT > + > + AUTOCHECKBOX "Check for WHQL digital signatures", IDC_WHQL_CHECKBOX, 10, 186, 369, 9, 0, WS_EX_LEFT > } Similarly here. Not sure about these, I'd drop the unused entries though. > - WINE_FIXME("Information dialog is not implemented\n"); > + DialogBoxW(hInst,(WCHAR*)MAKEINTRESOURCE(IDD_MAIN),NULL,main_dlg_proc); Space after the commas. Also you don't need to cast, just use MAKEINTRESOURCEW() (see for example notepad/dialog.c). > +extern HINSTANCE hInstance; Please avoid camelcase or hungarian notation. > +extern struct dxdiag_information * dxdiag_info; The '*' should be next to the variable name, as in the preexisting dxdiag sources. > +void set_main_dialog_tab(int new_tab_index) > +{ > + RECT tab_rect; > + POINT top_left, bottom_right; > + const HWND new_tab = main_dialog_tabs[new_tab_index]; > + int tab = 0; > + for(;tab<MAIN_DIALOG_TAB_COUNT;++tab) > + ShowWindow(main_dialog_tabs[tab], SW_HIDE); Please leave a blank line between variable declarations and code. Stick to consistently having a whitespace between the '<' and its operands and after the ';'. Also, but this is probably just my preference, I'd initialize tab to 0 in the "for" instead. > +void main_dialog_insert_tab(int index, int caption_id, int template_id, DLGPROC dlg_proc) > +{ > + WCHAR tab_caption[MAX_STRING_LEN]; > + const TCITEMW tab_item = {.pszText = tab_caption, .mask = TCIF_TEXT}; Designated initializers aren't allowed in C89. > + LoadStringW(hInstance, caption_id, tab_caption, sizeof tab_caption/sizeof tab_caption[0]); Parentheses around the sizeof operand. > +void populate_system_tab(void) > +{ > + const HWND page = main_dialog_tabs[SYSTEM_TAB_INDEX]; > + const struct system_information zeros = {0}; > + const struct system_information*const si = dxdiag_info ? &dxdiag_info->system_info : &zeros; Spaces. > +void save_info_dialog(HWND parent) > +{ > + static const WCHAR filter[] = {'T','e','x','t',' ','f','i','l','e',' ','(','*','.','t','x','t',')',0, '*','.','t','x','t',0, > + 'X','M','L',' ','f','i','l','e',' ','(','*','.','x','m','l',')',0, '*','.','x','m','l',0,0}; > + OPENFILENAMEW ofn = {0}; > + WCHAR filename[MAX_PATH]; > + filename[0] = 0; You can initialize filename right in the definition, like: WCHAR filename[MAX_PATH] = {0}; Actually, do you need to initialize it at all? > + ofn.lStructSize = sizeof ofn; > + ofn.hwndOwner = parent; > + ofn.lpstrFilter = filter; > + ofn.lpstrFile = filename; > + ofn.nMaxFile = MAX_PATH; > + ofn.Flags = OFN_EXPLORER|OFN_HIDEREADONLY|OFN_OVERWRITEPROMPT; Spaces. > +void update_dxdiag_info(void) > +{ > + const HWND checkbox = GetDlgItem(main_dialog, IDC_WHQL_CHECKBOX); > + const HWND save_all_button = GetDlgItem(main_dialog, IDC_SAVE_INFO); > + const BOOL check_whql = SendMessageW(checkbox, BM_GETCHECK, 0L, 0L) == BST_CHECKED; > + free_dxdiag_information(dxdiag_info); > + dxdiag_info = collect_dxdiag_information(check_whql); > + /* update the dialog before showing error message */ > + populate_tabs(); > + if(!dxdiag_info) Nitpick, space after "if". > +INT_PTR CALLBACK system_tab_proc(HWND hsystab, UINT msg, WPARAM wparam, LPARAM lparam) > +{ > + switch(msg) > + { > + case WM_COMMAND: > + switch(HIWORD(wparam)) > + { > + case BN_CLICKED: > + switch(wparam) > + { > + case IDC_WHQL_CHECKBOX: > + { > + update_dxdiag_info(); > + return TRUE; > + } > + } This looks pretty ugly. You don't need a separate block inside the case so you can just drop the inner braces. Actually I don't think you're ever going to have other buttons in the system tab so the switch seems unnecessary.

2 3

Re: Bruno Jesus : xinput1_3: Stub XInputGetStateEx.
by Huw Davies 01 Sep '16

01 Sep '16

On 1 Sep 2016, at 15:55, Alexandre Julliard <julliard(a)winehq.org> wrote: > Module: wine > Branch: master > Commit: 0fe758046ddc0f37944b8a3e075b8a5fdc5d81a4 > URL: http://source.winehq.org/git/wine.git/?a=commit;h=0fe758046ddc0f37944b8a3e0… > > Author: Bruno Jesus <00cpxxx(a)gmail.com> > Date: Wed Aug 31 15:17:44 2016 -0300 > > xinput1_3: Stub XInputGetStateEx. > > Signed-off-by: Bruno Jesus <00cpxxx(a)gmail.com> > Signed-off-by: Alexandre Julliard <julliard(a)winehq.org> > + if (pXInputGetStateEx_Ordinal) > + ok (pXInputGetStateEx_Ordinal == pXInputGetStateEx, "XInputGetStateEx in the wrong ordinal\n"); > + else > + ok (broken(1), "XInputGetStateEx not found in this dll version\n”) Hi Bruno, This is causing test failures: https://testbot.winehq.org/JobDetails.pl?Key=25476 https://test.winehq.org/data/tests/xinput1_3:xinput.html Could you take a look? Thanks, Huw.

2 1

Experimental gcc optimization for 64-bit wine
by Daniel Santos 01 Sep '16

01 Sep '16

Over the last two weeks I've been working on an experimental gcc optimization for wine that out-of-lines the expensive prologues & epilogues required for an ms x64 abi function to call a 64-bit sysv function. The theory is that the added expense of each function executing a few extra instructions to facilitate this will be offset by the reduction in instruction cache misses. As far as I can tell, gcc's ix86 back end has never supported any type of out-of-line pro/epilogue stubs, so adding this support has been somewhat tedious as it tends to get broken by subsequent optimization passes. It currently works in many cases, but in other cases produces bad code so this is a work in progress. None the less, I thought it was interesting enough to post the current results in terms of .text size reduction. On gcc 5.4.0 (with the 8-byte aligned stack assumption flaw corrected), the total .text size of all dlls on my build was 45.2MiB while the out-of-lined build was 35.9MiB, a reduction of 20.5%, so it looks promising so far. I presume it will be a little while before all of the flaws are worked out so that it always produces good code, but once there I can start to produce some real performance numbers. It's probably also worth mentioning that the ideal long-term solution is to find ways eliminate the register save/restores through various means-- static analysis, better evaluation of branch costs, and other optimizations in Wine its self. For example, there are many WINAPI functions who's only sysv call is to wine_dbg_log() via a TRACE(). Just marking this function as __attribute__((__cold__)) should reduce the number of unneeded register save/restores in the function's main path by a great amount. (I haven't actually tested it yet.) Daniel

2 3

Re: wintrust/tests: Fix test for pfnAddSgnr2Chain.
by Marvin 01 Sep '16

01 Sep '16

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://testbot.winehq.org/JobDetails.pl?Key=25506 Your paranoid android. === w864 (32 bit softpub) === softpub: unhandled exception c0000005 at 77723ED2 === w1064 (32 bit softpub) === softpub: unhandled exception c0000005 at 768AC81B === w864 (64 bit softpub) === softpub: unhandled exception c0000005 at 00007FFB7AFF266C === w1064 (64 bit softpub) === softpub.c:254: Test failed: Expected S_OK, got 00000001 softpub.c:257: Test failed: Expected S_OK, got 00000001 softpub: unhandled exception c0000005 at 00007FFC77A79A5D

1 0

Re: rpcrt4/tests: Remove an unnecessary and confusing message.
by Marvin 01 Sep '16

01 Sep '16

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://testbot.winehq.org/JobDetails.pl?Key=25500 Your paranoid android. === w864 (32 bit rpc) === rpc.c:812: Test failed: GUID does not appear to contain a MAC address: {f03f0f60-7046-11e6-be78-e71666c2e04a} === w864 (64 bit rpc) === rpc.c:812: Test failed: GUID does not appear to contain a MAC address: {ab19995e-7047-11e6-be78-e71666c2e04a}

1 0

Re: [PATCH] msctf/tests: Return interface pointers instead of pointers to impl structure
by Marvin 01 Sep '16

01 Sep '16

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://testbot.winehq.org/JobDetails.pl?Key=25492 Your paranoid android. === w864 (64 bit inputprocessor) === inputprocessor.c:831: Test failed: Deactivation came unexpectedly

2 1