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

Re: [PATCH 1/3] wined3d: Add some missing state updates for buffer textures.

by Henri Verbeet

On 29 January 2017 at 03:32, Guillaume Charifi <guillaume.charifi(a)sfr.fr> wrote: > Signed-off-by: Guillaume Charifi <guillaume.charifi(a)sfr.fr> > > --- > dlls/wined3d/context.c | 12 ++++++++++++ > dlls/wined3d/state.c | 5 +++++ > dlls/wined3d/view.c | 6 ------ > 3 files changed, 17 insertions(+), 6 deletions(-) > These patches are incredibly unappealing to review. There are multiple versions of multiple interleaved series, with html and other errors mixed in. (See e.g. https://source.winehq.org/patches/data/129296.) Please only send a reasonable number of patches at once, I'd suggest sticking with 5, and please use git send-email for sending patches.

Jan. 30, 2017

Re: [PATCH resend 1/2] user32: Fix WM_NCPAINT for windows crossing screen boarders and add tests

by Alexandre Julliard

Fabian Maurer <dark.shadow4(a)web.de> writes: > @@ -662,7 +662,17 @@ static HRGN send_ncpaint( HWND hwnd, HWND *child, UINT *flags ) > if (type == SIMPLEREGION) > { > RECT window; > + LONG screenx = GetSystemMetrics(SM_CXSCREEN); > + LONG screeny = GetSystemMetrics(SM_CYSCREEN); > + > GetWindowRect( hwnd, &window ); > + > + /* Clamp the window region to screen bounds */ > + if(window.right > screenx) window.right = screenx; > + if(window.bottom > screeny) window.bottom = screeny; > + if(window.left < 0) window.left = 0; > + if(window.top < 0) window.top = 0; This won't be correct for multi-monitor setups. That sort of thing should probably be handled on the server side. -- Alexandre Julliard julliard(a)winehq.org

Jan. 30, 2017

Re: [PATCH v2] user32: Also remove WM_CHAR from queue when a menu gets VK_ESCAPE and add tests

by Alexandre Julliard

Fabian Maurer <dark.shadow4(a)web.de> writes: > @@ -3186,8 +3186,12 @@ static BOOL MENU_TrackMenu( HMENU hmenu, UINT wFlags, INT x, INT y, > break; > > case VK_ESCAPE: > + { > + MSG msg_dummy; > fEndMenu = MENU_KeyEscape(&mt, wFlags); > - break; > + PeekMessageW(&msg_dummy, 0, WM_CHAR, WM_CHAR, PM_REMOVE); > + break; > + } It would be cleaner to avoid TranslateMessage instead of removing the message (possibly getting the wrong one). -- Alexandre Julliard julliard(a)winehq.org

Jan. 30, 2017

Re: comctl32: Fix some more spec file entries

by Huw Davies

On Sun, Jan 29, 2017 at 09:58:36PM +0100, André Hentschel wrote: > Signed-off-by: André Hentschel <nerv(a)dawncrow.de> > --- > dlls/comctl32/comctl32.spec | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/dlls/comctl32/comctl32.spec b/dlls/comctl32/comctl32.spec > index c3e9167..dbfa123 100644 > --- a/dlls/comctl32/comctl32.spec > +++ b/dlls/comctl32/comctl32.spec > @@ -93,7 +93,7 @@ > 382 stdcall -noname SmoothScrollWindow(ptr) > 383 stub -noname DoReaderMode > 384 stdcall -noname SetPathWordBreakProc(ptr long) > -385 stdcall -ordinal DPA_EnumCallback(long long ptr) > +385 stdcall -ordinal DPA_EnumCallback(long ptr ptr) Should be: 385 stdcall -ordinal DPA_EnumCallback(ptr ptr ptr) Huw.

Jan. 30, 2017

Re: patches: Display multiple check marks for multiple signoffs.

by Alexandre Julliard

Bruno Jesus <00cpxxx(a)gmail.com> writes: > On Sun, Jan 29, 2017 at 6:51 PM, Bruno Jesus <00cpxxx(a)gmail.com> wrote: >>> The patch is definitly an improvement, but the problem with the patch is, that right now I can't see that my patch has been signed-off by Aric when looking at it in the patch list >>> ... >>> 129264 New Aric Stewart Re: usp10: Fix ScriptRecordDigitSubstitution spec file entry ✓ >>> ... >>> 129183 Assigned André Hentschel usp10: Fix ScriptRecordDigitSubstitution spec file entry Aric Stewart ✓ >>> ... >>> >>> I think something like that was what Bruno is looking for >> >> It is half-working then, because my patch 129061 is double checked >> because of Andrew's sign 129073. So it is missing some update after >> receiving new sign-offs, I don't know. > > Judging by the current contents of the list I infer that the other > signs are only added after the email is marked as Sign-off type. Yes, the list is not updated in real time. The patch status is pushed out together with the commits. -- Alexandre Julliard julliard(a)winehq.org

Jan. 30, 2017

Re: [PATCH] winbase.h: Add UnlockResource prototype

by Alistair Leslie-Hughes

On 30/01/17 18:55, Dmitry Timoshkov wrote: > Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com> wrote: > >> This function is obsolete and was only used in win16. However when trying >> to compile a project with winelib found this function was called. > > Then you need to add it to an appropriate win16 header, or better remove it > from your project since it won't compile with PSDK either. > This is incorrect. It is part of the current SDK, however its just a defined to nothing. Best Regards Alistair Leslie-Hughes.

Jan. 30, 2017

Re: [PATCH] winbase.h: Add UnlockResource prototype

by Dmitry Timoshkov

Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com> wrote: > This function is obsolete and was only used in win16. However when trying > to compile a project with winelib found this function was called. Then you need to add it to an appropriate win16 header, or better remove it from your project since it won't compile with PSDK either. -- Dmitry.

Jan. 30, 2017

Re: [PATCH 1/7] regedit: Use a helper for LoadString() and add debug information

by Dmitry Timoshkov

Hugh McMaster <hugh.mcmaster(a)outlook.com> wrote: > > Probably using a flavour of lstrcpy that checks the target buffer length > > would be more appropriate. Although simply returning an empty buffer and > > an error indicator to the caller could be slightly better. Under which > > conditions LoadString may fail here? > > The buffer size must be at least six WCHARs to allow for errorW, so we could > just do something like if (!buf || count < 6) return; > > In this code, LoadString() could fail if the resource ID doesn't exist > or if count is 0 or 1. But since we are controlling all of the code, none of this > is likely. > > For MessageBox captions, if we pass in NULL, the default string "Error" > is loaded by user32. So, apart from a few instances, we wouldn't have to > handle captions at all. But we would have to do something if LoadString() > failed for the body text, otherwise the user would see a blank message box. > > The original code was: > if (!LoadStringW(hInst, titleId, title, COUNT_OF(title))) lstrcpyW(title, errorW); > if (!LoadStringW(hInst, resId, errfmt, COUNT_OF(errfmt))) lstrcpyW(errfmt, unknownW); > > Do you think it is worth replacing? My main reason for adding load_string() was to avoid > duplicating the FIXME line and to avoid "Unknown error string". If LoadString should never fail under normal circumstances then perhaps simple assert(0) is enough, and there is no need to invent new code to handle errors that should never happen. -- Dmitry.

Jan. 30, 2017

Re: [PATCH 1/7] regedit: Use a helper for LoadString() and add debug information

by Hugh McMaster

On Sunday, 29 January 2017 11:23 PM, Dmitry Timoshkov wrote: >Hugh McMaster wrote: >> -static INT vmessagebox(HWND hwnd, INT buttons, INT titleId, INT resId, va_list ap) >> +static void load_string(unsigned int msg_id, WCHAR *buf, unsigned int count) >> { >> static const WCHAR errorW[] = {'E','r','r','o','r',0}; >> - static const WCHAR unknownW[] = {'U','n','k','n','o','w','n',' ','e','r','r','o','r',' ','s','t','r','i','n','g','!',0}; >> >> + if (!buf) return; >> + >> + if (!LoadStringW(hInst, msg_id, buf, count)) >> + { >> + WINE_FIXME("LoadString failed with %u\n", GetLastError()); >> + lstrcpyW(buf, errorW); >> + } >> +} > > Probably using a flavour of lstrcpy that checks the target buffer length > would be more appropriate. Although simply returning an empty buffer and > an error indicator to the caller could be slightly better. Under which > conditions LoadString may fail here? The buffer size must be at least six WCHARs to allow for errorW, so we could just do something like if (!buf || count < 6) return; In this code, LoadString() could fail if the resource ID doesn't exist or if count is 0 or 1. But since we are controlling all of the code, none of this is likely. For MessageBox captions, if we pass in NULL, the default string "Error" is loaded by user32. So, apart from a few instances, we wouldn't have to handle captions at all. But we would have to do something if LoadString() failed for the body text, otherwise the user would see a blank message box. The original code was: if (!LoadStringW(hInst, titleId, title, COUNT_OF(title))) lstrcpyW(title, errorW); if (!LoadStringW(hInst, resId, errfmt, COUNT_OF(errfmt))) lstrcpyW(errfmt, unknownW); Do you think it is worth replacing? My main reason for adding load_string() was to avoid duplicating the FIXME line and to avoid "Unknown error string". -- Hugh

Jan. 29, 2017

Re: [PATCH] MAINTAINERS: Add myself for printing

by Bruno Jesus

On Sun, Jan 29, 2017 at 8:15 PM, Detlef Riekenberg <wine.dev(a)web.de> wrote: > -- > bye bye Detlef > > Signed-off-by: Detlef Riekenberg <wine.dev(a)web.de> > --- > MAINTAINERS | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/MAINTAINERS b/MAINTAINERS > index e5224ff..d53af50 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -206,10 +206,15 @@ F: dlls/vcomp*/ > > Printing > M: Huw Davies <huw(a)codeweavers.com> > +P: Detlef Riekenberg <wine.dev(a)web.de> > F: dlls/localspl/ > -F: dlls/wineps.drv/ > +F: dlls/spoolss/ > F: dlls/winspool.drv/ > > +Postscript Driver > +M: Huw Davies <huw(a)codeweavers.com> > +F: dlls/wineps.drv/ > + I believe we are trying to keep the areas in alphabetical order.

Jan. 29, 2017