September 2014 - Wine-Devel

Re: [5/17]dwrite: Shift to using localized string for family name

by Aric Stewart

Later, when I start reading the family name from the font 'name' table, is where this is more relevant. I will try rebasing and moving this patch to that area of the code so that it makes more sense. -aric On Tue Sep 2 16:51:40 2014, Nikolay Sivov wrote: >> +HRESULT clone_localizedstring(IDWriteLocalizedStrings *iface, IDWriteLocalizedStrings **strings) > > This looks unnecessary, as you can do create_localizedstrings() and a series of add_localizedstring() after. > >

Sept. 3, 2014

RE: [PATCH 2/5] reg.exe: Add path_get_key() to remove boilerplate

by Hugh McMaster

On Wed, 3 Sep 2014 13:15:43 +0200, Jonathan Vollebregt wrote: >> Single line code in if/else blocks can be compacted to use less space. E.g.: >> >> if (path_get_rootkey_name(path)) >> return HKEYs[(path_get_rootkey_name(path) - &long_HKEY_name[0][0]) / 20]; >> else >> return NULL; > > Isn't it a best practice to always use braces? You could certainly argue that it is a best practice, and, in many ways, it is. But consider the if... else if... blocks from http://source.winehq.org/source/programs/reg/reg.c#0411 and following. If braces were included for each single line, each block would be far, far longer and (in my opinion) harder to read. >> +static const WCHAR type_names[][MAX_TYPE_LENGTH] = { >> + {'R','E','G','_','N','O','N','E',0}, >> + {'R','E','G','_','S','Z',0}, >> + {'R','E','G','_','E','X','P','A','N','D','_','S','Z',0}, >> + {'R','E','G','_','B','I','N','A','R','Y',0}, >> + {'R','E','G','_','D','W','O','R','D',0}, >> + {'R','E','G','_','D','W','O','R','D','_','B','I','G','_','E','N','D','I','A','N',0}, >> + {0}, >> + {'R','E','G','_','M','U','L','T','I','_','S','Z',0}, >> +}; >> + >> + >> + >> >> Also, what is the purpose of {0}? > > REG_LINK isn't supported by anything in wine as far as I can tell, so I > thought I'd just leave it empty. Would it be better to fill it in anyway? http://source.winehq.org/ident?_i=REG_LINK&_remember=1 shows that REG_LINK appears in the source in some form. Whether it is actively used in registry operations, I'm not sure. Given that it appears, however, I'd include it anyway.

Sept. 3, 2014

Re: [PATCH 2/5] reg.exe: Add path_get_key() to remove boilerplate

by Jonathan Vollebregt

> + if(path_get_rootkey_name(path)) > + { > + return HKEYs[(path_get_rootkey_name(path) - &long_HKEY_name[0][0]) / 20]; > + } > + else > + { > + return NULL; > + } > > Single line code in if/else blocks can be compacted to use less space. E.g.: > > if (path_get_rootkey_name(path)) > return HKEYs[(path_get_rootkey_name(path) - &long_HKEY_name[0][0]) / 20]; > else > return NULL; Isn't it a best practice to always use braces? > +static const WCHAR type_names[][MAX_TYPE_LENGTH] = { > + {'R','E','G','_','N','O','N','E',0}, > + {'R','E','G','_','S','Z',0}, > + {'R','E','G','_','E','X','P','A','N','D','_','S','Z',0}, > + {'R','E','G','_','B','I','N','A','R','Y',0}, > + {'R','E','G','_','D','W','O','R','D',0}, > + {'R','E','G','_','D','W','O','R','D','_','B','I','G','_','E','N','D','I','A','N',0}, > + {0}, > + {'R','E','G','_','M','U','L','T','I','_','S','Z',0}, > +}; > + > + > + > > Also, what is the purpose of {0}? REG_LINK isn't supported by anything in wine as far as I can tell, so I thought I'd just leave it empty. Would it be better to fill it in anyway?

Sept. 3, 2014

Re: [RFC 0/11] Linux FF: Intro

by Elias Vanderstuyft

On Tue, Mar 4, 2014 at 8:19 PM, Elias Vanderstuyft <elias.vds(a)gmail.com> wrote: > Hi, > > > This is the sequel of the mail with title: "Discussion and Improvement > of Wine's Linux Force Feedback implementation". > This mail is meant to give an introduction to the next 11 mails: > > > All tests were performed using wine-1.7.11-206-g82b3813 > (but none of the files of which I propose changes to be made in, > are affected in the current (2014-03-04) wine-version 1.7.13) > > Because I haven't created any (formal) patch yet (and instead first > request for comments), > I tried to keep the line numbers to reference the affected files > more or less based to the version I started from, where I could. > However, the orig code of a (informal) patch that has to be applied > after (and thus is dependent of) a previous patch, > may be different from the initial code of the wine-version I started from. > > After having discussed everything, I will send a proper patchset. > > In these mails, sometimes 'FEdit' is mentioned: > it's a MS application that allows to create all FF effect types > (except CustomForce and rumble/vibration effects), > and therefore the ideal test application. > Physical behavior is compared between Windows7 (using Logitech MOMO > wheel with latest drivers installed) running in Virtualbox, > and my patched Wine-version. > In the end, (using a dummy FF driver on the Linux side, thanks to Michal Malý), > the behavior of Wine is almost (limitations caused by the dummy ff > driver, such as updating an effect while playing is not implemented) > identical, > if not better (in some aspects) then the Windows equivalent. > > Sometimes I refer to linux-input mailing lists to prove a statement, > because the links may vanish (unlikely though), > I provide the titles corresponding with the links: > (http://www.mail-archive.com/linux-input(a)vger.kernel.org/msg08513.html) > "ff-core effect id handling in case of a failed effect upload" > (http://www.mail-archive.com/linux-input(a)vger.kernel.org/msg08459.html) > "Questions about the documentation/specification of Linux > ForceFeedback input.h" > > > Subjects [11] > ------------- > - Joystick control panel program [2] > - Linux effect status management [3] > - Linux effect parameter: start delay [1] > - Linux effect parameter: periodic effect's phase [1] > - Linux effect parameters: time parameters [3] > - Linux effect: conditional effect's parameters [2] > - Linux effect: axes [1] > - Linux effect direction [7] > - Linux effect parameters (global) [6] > - Linux joystick initialization [1] > - Debug message fixes/additions [6] Hi Andrew, I'm mailing you to let you know that I'll be unable to create patches and corresponding tests for these things, because I'll have no time to work on it the coming year. Excuse me for that. However, considering the amount of work I put into this, and the fact that most bugs in Wine covered by my discussion above are obviously clear bugs and some even embarrassing, someone should tackle at least part of these problems. I would be very glad if someone volunteers in doing so, I will be still available if some things are not clear, just mail me. All the best, Elias

Sept. 3, 2014

Re: winewrapper: In 64 bit trees the loader is called wine64.

by Alexandre Julliard

Francois Gouget <fgouget(a)free.fr> writes: > Now as far as WineTest is concerned I run both the 32 and 64 bit > WineTest in a WoW64 setup (it's the -wow32 and -wow64 results > respectively). Do you feel that the pure 64 bit results (-t64) are of no > value and that it's not worth running them? I don't think it's worth it, no. The results should be mostly identical, but in the cases where they differ, that's because pure 64-bit is not a supported setup so it's not something we would be interested in fixing. -- Alexandre Julliard julliard(a)winehq.org

Sept. 3, 2014

Re: winewrapper: In 64 bit trees the loader is called wine64.

by Francois Gouget

On Tue, 2 Sep 2014, Alexandre Julliard wrote: > Francois Gouget <fgouget(a)free.fr> writes: > > > This fixes running 'make test' in 64 bit trees. > > I don't mind adding this, but I would recommend running tests with a > proper WoW64 setup instead. Given that it's possible to do a 64 bit build of Wine I think 'make test' should work in that build (or more specifically cd dlls/mshtml/tests && make events.ok in my case). Now as far as WineTest is concerned I run both the 32 and 64 bit WineTest in a WoW64 setup (it's the -wow32 and -wow64 results respectively). Do you feel that the pure 64 bit results (-t64) are of no value and that it's not worth running them? -- Francois Gouget <fgouget(a)free.fr> http://fgouget.free.fr/ The greatest programming project of all took six days; on the seventh day the programmer rested. We've been trying to debug the *&^%$#@ thing ever since. Moral: design before you implement.

Sept. 3, 2014

[PATCH 3/5] reg.exe: Add wchar/type conversion functions

by Hugh McMaster

On Mon, 1 Sep 2014 15:05:32 +0200, Jonathan Vollebregt wrote: +#define MAX_TYPE_LENGTH 32 +static const WCHAR type_names[][MAX_TYPE_LENGTH] = { + {'R','E','G','_','N','O','N','E',0}, + {'R','E','G','_','S','Z',0}, + {'R','E','G','_','E','X','P','A','N','D','_','S','Z',0}, + {'R','E','G','_','B','I','N','A','R','Y',0}, + {'R','E','G','_','D','W','O','R','D',0}, + {'R','E','G','_','D','W','O','R','D','_','B','I','G','_','E','N','D','I','A','N',0}, + {0}, + {'R','E','G','_','M','U','L','T','I','_','S','Z',0}, +}; + + + Why are you defining MAX_TYPE_LENGTH as 32? The longest is 21 (including the null terminator) or, if you include REG_DWORD_LITTLE_ENDIAN, the longest is 24. Also, what is the purpose of {0}? And you have included three newlines. -- Hugh McMaster

Sept. 3, 2014

[PATCH 2/5] reg.exe: Add path_get_key() to remove boilerplate

by Hugh McMaster

Hi Jonathan, On Monday, 1 September 2014, 15:05:31 +0200, Jonathan Vollebregt wrote: + CompareStringW(CP_ACP,NORM_IGNORECASE, + path,strlenW(short_HKEY_name[i]), + short_HKEY_name[i],strlenW(short_HKEY_name[i]))==2 || + CompareStringW(CP_ACP,NORM_IGNORECASE, + path,strlenW(long_HKEY_name[i]), + long_HKEY_name[i],strlenW(long_HKEY_name[i]))==2) 1. CompareString does not accept code page identifiers. It accepts a locale.[1] You could use LOCAL_USER_DEFAULT or GetThreadLocale() instead. 2. Please use CSTR_EQUAL instead of 2. 3. You should also avoid calling strlenW four times. int len_short_HKEY_name = strlenW(short_HKEY_name[i]); or something similar. Alternatively, since wine/unicode.h is already being included, consider using strncmpiW. 4. In patches 2 to 5, please mimic the style of the surrounding/original code. That being said, if there are variations in style, just use the most common style. + if(path_get_rootkey_name(path)) + { + return HKEYs[(path_get_rootkey_name(path) - &long_HKEY_name[0][0]) / 20]; + } + else + { + return NULL; + } Single line code in if/else blocks can be compacted to use less space. E.g.: if (path_get_rootkey_name(path)) return HKEYs[(path_get_rootkey_name(path) - &long_HKEY_name[0][0]) / 20]; else return NULL; > +static const WCHAR * path_get_rootkey_name(LPWSTR path) Asterisks should be placed next to the object name or type, e.g. static const WCHAR *path_get_rootkey_name(LPWSTR path). > + if(!key) > + if (RegQueryValueW(key,value_name,NULL,NULL)==ERROR_SUCCESS) > + RegSetValueExW(key,value_name,0,reg_type,reg_data,reg_count); Also, code is easier to read if there is space after keywords, arguments and around == (equivalent to). You have been slightly inconsistent throughout. [1] http://msdn.microsoft.com/en-us/library/windows/desktop/dd317759%28v=vs.85%… -- Hugh McMaster

Sept. 3, 2014

Re: [5/17]dwrite: Shift to using localized string for family name

by Nikolay Sivov

> +HRESULT clone_localizedstring(IDWriteLocalizedStrings *iface, IDWriteLocalizedStrings **strings) This looks unnecessary, as you can do create_localizedstrings() and a series of add_localizedstring() after.

Sept. 2, 2014

Re: [17/17]dwrite: Add Custom Font Collections

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://testbot.winehq.org/JobDetails.pl?Key=8763 Your paranoid android. === build (build) === Patch failed to apply

Sept. 2, 2014