April 2017 - Wine-Devel - list.winehq.org

Re: COM tracing ...

by Michael Meeks

Hi Michael, On 30/03/17 21:16, Michael Stefaniuc wrote: >> I'd -really- love some input on how best to trace that series of COM >> method calls on Windows itself ie. the (remote) service activation, and >> the RPC beyond that - particularly the method names, parameters etc. of >> the COM/OLE Office API. I've tried (so far): > > don't you have at least a tlb file for the culprit COM stuff? You can > look at that with oleview; Wine implements one too should you be on Linux. Sure - I have the type information (it turns out) - thanks for the pointer. >> Component models used to be all the rage in my youth ;-) surely someone >> solved the "strace for COM calls" problem elegantly some-when ! > > Well Wine has that built in for the stuff it implements. For the rest we > write tests that exercises the respective API. Makes sense. >> Crazily - might it be possible to instrument, interpose and use Wine's >> COM impl. on Windows [ which sounds a bit 'exciting' ;-], or ? > > You don't need Wine for that. You can write wrappers around the COM > classes you need, easily if you have the info from the TLB. Ok ? so I have a huge chunk of XML from tlb2xml from here - which is nice; the approach you'd suggest is manually, incrementally writing wrappers and interposing them between MS Office and the automation to work out what methods are being invoked ? an incremental approach makes the problem somewhat hard to scope I suspect. Anyhow - many thanks for the input. ATB, Michael. -- michael.meeks(a)collabora.com <><, Pseudo Engineer, itinerant idiot

April 4, 2017

Re: [PATCH] comctl32/taskdialog: Implement buttons.

by Nikolay Sivov

On 03.04.2017 20:18, Fabian Maurer wrote: >> In addition to issues I mentioned already, I don't think this works >> properly (or at all) for custom buttons. What will happen if custom >> button has common id, or several custom buttons share the same id? > Then they both will return the same id. How would you distinguish them, if > they have the same id? Of course I'd write tests to also cover this, but for a > basic implementation corner cases like this are not my priority. First we need > to get it working, then we can worry about corner cases. For some reason I thought duplicate ids are not permitted in user32 dialogs, that's not the case apparently. > > I also commented on your other points, explaining my reasoning. I did some tests, so let's go over them again. > -static const UINT DIALOG_MIN_WIDTH = 180; > +static const UINT DIALOG_MIN_WIDTH = 240; I'm not necessarily opposed, but this change does look related to button support. > -static const UINT DIALOG_BUTTON_WIDTH = 50; > -static const UINT DIALOG_BUTTON_HEIGHT = 14; > +static const UINT DIALOG_BUTTON_WIDTH_MIN = 45; > +static const UINT DIALOG_BUTTON_HEIGHT = 13; Let's keep it as is, for consistency with user32 template. > +static const UINT DIALOG_SPACING_BUTTONS_LEFT = 30; /* minimum distance from the left dialog border */ I don't see this on Win10. We could as well use DIALOG_SPACING here too, for both left and right spaces. > +static const UINT DIALOG_SPACING_BUTTON_H = 1; /* Distance between buttons */ > +static const UINT DIALOG_SPACING_BUTTON_W = 4; /* Distance between buttons */ It's tempting to use DIALOG_SPACING here as well to reduce amount of magic. > + if (IS_INTRESOURCE(text)) > + { > + if (!(length = LoadStringW(desc->taskconfig->hInstance, (UINT_PTR)text, (WCHAR *)&textW, 0))) > + { > + WARN("Failed to load text\n"); > + textW = &nulW; > + length = 0; > + } > + } > + else > + { > + textW = text; > + length = strlenW(textW); > + } > + This doesn't handle NULL text properly. At least buttons can't have text set to NULL. > - template->style = WS_VISIBLE; > + template->style = WS_VISIBLE | WS_CHILD | style; This will make no difference for buttons, you can as well skip this change. > + button.width = rect.right + 10; Why 10? > + /* Position buttons */ > + location_x = alignment; > + for (i = 0; i < count; i++) > + { > + /* When beginning new row, align the first */ > + if (location_x + buttons[i].width + DIALOG_SPACING_BUTTON_W > desc->dialog_width) > + { > + if (first_row) > + { > + int diff = desc->dialog_width - location_x; > + > + first_row = FALSE; > + for (int j = 0; j < i; j++) /* Align first row to the right */ > + buttons[j].x += diff; > + alignment = buttons[0].x; /* left-align all coming rows to the first row */ > + } > + location_x = alignment; > + desc->dialog_height += DIALOG_BUTTON_HEIGHT + DIALOG_SPACING_BUTTON_H; > + } > + > + buttons[i].x = location_x; > + buttons[i].y = desc->dialog_height; > + > + location_x += buttons[i].width + DIALOG_SPACING_BUTTON_W; > + } This does not do the right thing. Having first row right-aligned is not a goal. According to my manual testing all rows are left aligned, and wrapped as soon as next button width exceeds dialog width, which by the way is supposed to grow, if cxWidth is 0. We can ignore that for now I guess. > > Regards, > Fabian Maurer >

April 4, 2017

Re: [PATCH 2/4] winex11: move XDestroyWindow() after free_gl_drawable() in set_win_format()

by Miklós Máté

On 04/04/17 16:06, Alexandre Julliard wrote: > Miklós Máté <mtmkls(a)gmail.com> writes: > >> On 04/04/17 12:11, Alexandre Julliard wrote: >>> Miklós Máté <mtmkls(a)gmail.com> writes: >>> >>>> @@ -1334,21 +1334,21 @@ static void free_gl_drawable( struct gl_drawable *gl ) >>>> /*********************************************************************** >>>> * create_gl_drawable >>>> */ >>>> -static BOOL create_gl_drawable( HWND hwnd, struct gl_drawable *gl ) >>>> +static BOOL create_gl_drawable( HWND hwnd, struct gl_drawable *gl, struct x11drv_win_data *data ) >>>> { >>>> gl->drawable = 0; >>>> if (GetAncestor( hwnd, GA_PARENT ) == GetDesktopWindow()) >>>> /* top-level window */ >>>> { >>>> - struct x11drv_win_data *data = get_win_data( hwnd ); >>>> - >>>> if (data) >>>> { >>>> gl->type = DC_GL_WINDOW; >>>> gl->window = create_client_window( data, gl->visual ); >>>> if (gl->window) >>>> + { >>>> gl->drawable = pglXCreateWindow( gdi_display, gl->format->fbconfig, gl->window, NULL ); >>>> - release_win_data( data ); >>>> + XSync( gdi_display, False ); >>> Do you really need all the extra XSync calls? We try to avoid server >>> round-trips as much as possible. >>> >> Yes. We must ensure that glXCreateWindow() and glXDestroyWindow() >> refer to a valid X window, so the command queue of data->display and >> gdi_display have to be in sync around those calls. > If the goal is that the X window is valid, I'd have expected an XSync on > the thread display. Why is the XSync on gdi_display needed here? What is > the scenario that fails? > > I'm not questioning that some XSyncs are necessary (and we already have > a few), but I want to make sure we don't add more than needed. > This one prevents crash in the following scenario: 1. create_gl_drawable() 2. X11DRV_ThreadDetach() calls XCloseDisplay( data->display ); 3. the Xserver processes the glXCreateWindow in the command queue of gdi_display -> X window is not valid This happens sometimes during the ddraw and d3d9 tests. MM

April 4, 2017

Re: Replacing TestLauncher... or not?

by Francois Gouget

On Mon, 3 Apr 2017, André Hentschel wrote: > Can't you just fork winetest, throw out everything unwanted, add some missing features and make that the > new testlauncher? Well, TestLauncher is 461 lines whereas WineTest is 2827 lines. I don't think there's all that much that needs to be brought over so trimming WineTest may not be much simpler. But moving the dll check / activation context to its own function (or even C file) would make it easier to copy / paste it and keep it in sync later. -- Francois Gouget <fgouget(a)codeweavers.com>

April 4, 2017

Re: test.winehq refactoring

by Francois Gouget

On Mon, 3 Apr 2017, Huw Davies wrote: [...] > Something else that would be very useful is the ability to > display just the test bot VMs. These are the ones we care > about the most, and the ones that are possible to fix without > having to get a third-party to manually run a test. I find it > difficult to just keep track of these VMs when their results get > swamped by all the other machines’ failures. [...] > Does that sound possible? It seems like it should be no more difficult than adding the extra 'flat all machines' indexes. I have not looked at exactly what that would entails but my gut feeling is that it would just require writing some more files in gather and build-index. An alternative to the static HTML file approach would be to go the JavaScript way and make the index files, which are essentially big tables, sortable, filterable on the client-side. That could be pretty cool too. Any takers? In any case I've created a bug so this does not get lost. https://bugs.winehq.org/show_bug.cgi?id=42770 -- Francois Gouget <fgouget(a)codeweavers.com>

April 4, 2017

Re: Replacing TestLauncher... or not?

by Austin English

On Tue, Apr 4, 2017 at 10:49 AM, Francois Gouget <fgouget(a)codeweavers.com> wrote: > On Mon, 3 Apr 2017, Austin English wrote: > [...] >> > One of the hoped for TestBot improvements is to get rid of >> > TestLauncher and replace it with WineTest on the theory that they >> > perform the same role. > [...] >> Or add those features to winetest, so there's one code base, not two.. > > That's what replacing TestLauncher with WineTest means. No? > > -- > Francois Gouget <fgouget(a)codeweavers.com> Yes. -- -Austin GPG: 14FB D7EA A041 937B

April 4, 2017

Re: Replacing TestLauncher... or not?

by Francois Gouget

On Mon, 3 Apr 2017, Austin English wrote: [...] > > One of the hoped for TestBot improvements is to get rid of > > TestLauncher and replace it with WineTest on the theory that they > > perform the same role. [...] > Or add those features to winetest, so there's one code base, not two.. That's what replacing TestLauncher with WineTest means. No? -- Francois Gouget <fgouget(a)codeweavers.com>

April 4, 2017

Re: [PATCH 2/4] winex11: move XDestroyWindow() after free_gl_drawable() in set_win_format()

by Alexandre Julliard

Miklós Máté <mtmkls(a)gmail.com> writes: > On 04/04/17 12:11, Alexandre Julliard wrote: >> Miklós Máté <mtmkls(a)gmail.com> writes: >> >>> @@ -1334,21 +1334,21 @@ static void free_gl_drawable( struct gl_drawable *gl ) >>> /*********************************************************************** >>> * create_gl_drawable >>> */ >>> -static BOOL create_gl_drawable( HWND hwnd, struct gl_drawable *gl ) >>> +static BOOL create_gl_drawable( HWND hwnd, struct gl_drawable *gl, struct x11drv_win_data *data ) >>> { >>> gl->drawable = 0; >>> if (GetAncestor( hwnd, GA_PARENT ) == GetDesktopWindow()) >>> /* top-level window */ >>> { >>> - struct x11drv_win_data *data = get_win_data( hwnd ); >>> - >>> if (data) >>> { >>> gl->type = DC_GL_WINDOW; >>> gl->window = create_client_window( data, gl->visual ); >>> if (gl->window) >>> + { >>> gl->drawable = pglXCreateWindow( gdi_display, gl->format->fbconfig, gl->window, NULL ); >>> - release_win_data( data ); >>> + XSync( gdi_display, False ); >> Do you really need all the extra XSync calls? We try to avoid server >> round-trips as much as possible. >> > Yes. We must ensure that glXCreateWindow() and glXDestroyWindow() > refer to a valid X window, so the command queue of data->display and > gdi_display have to be in sync around those calls. If the goal is that the X window is valid, I'd have expected an XSync on the thread display. Why is the XSync on gdi_display needed here? What is the scenario that fails? I'm not questioning that some XSyncs are necessary (and we already have a few), but I want to make sure we don't add more than needed. -- Alexandre Julliard julliard(a)winehq.org

April 4, 2017

Re: [PATCH 2/4] winex11: move XDestroyWindow() after free_gl_drawable() in set_win_format()

by Miklós Máté

On 04/04/17 12:11, Alexandre Julliard wrote: > Miklós Máté <mtmkls(a)gmail.com> writes: > >> @@ -1334,21 +1334,21 @@ static void free_gl_drawable( struct gl_drawable *gl ) >> /*********************************************************************** >> * create_gl_drawable >> */ >> -static BOOL create_gl_drawable( HWND hwnd, struct gl_drawable *gl ) >> +static BOOL create_gl_drawable( HWND hwnd, struct gl_drawable *gl, struct x11drv_win_data *data ) >> { >> gl->drawable = 0; >> >> if (GetAncestor( hwnd, GA_PARENT ) == GetDesktopWindow()) /* top-level window */ >> { >> - struct x11drv_win_data *data = get_win_data( hwnd ); >> - >> if (data) >> { >> gl->type = DC_GL_WINDOW; >> gl->window = create_client_window( data, gl->visual ); >> if (gl->window) >> + { >> gl->drawable = pglXCreateWindow( gdi_display, gl->format->fbconfig, gl->window, NULL ); >> - release_win_data( data ); >> + XSync( gdi_display, False ); > Do you really need all the extra XSync calls? We try to avoid server > round-trips as much as possible. > Yes. We must ensure that glXCreateWindow() and glXDestroyWindow() refer to a valid X window, so the command queue of data->display and gdi_display have to be in sync around those calls. MM

April 4, 2017

Re: [v2 PATCH 2/3] odbccp32: Implement SQLRemoveTranslator/W

by Huw Davies

On Tue, Apr 04, 2017 at 12:04:04PM +0000, Hugh McMaster wrote: > It's a matter of taste, but is there a reason why you're mixing > 16-bit functions (e.g. RegOpenKey) and functions defined for Vista > and up (e.g. RegGetValue)? You should use RegOpenKeyEx and > RegQueryValueEx. > > I realise this sort of mismatching occurs > throughout odbccp32.c, but to my mind, it is wrong, and should be > fixed file-wide in a separate patch.> > > I don't know how strongly Huw cares about this though. As far as I'm concerned, they're all part of the Win32 registry API and can be used together as appropriate (with the exception of the tests where we need to avoid APIs added after Win XP). Huw.

April 4, 2017