March 2011 - Wine-Devel - list.winehq.org

Re: Wine announce on the website shows weird content

by Alexandre Julliard

Divaret Mathieu <pianoedjoe(a)free.fr> writes: > Hello, > I just wanted to tell you that the page on http://www.winehq.org/announce/ > is bugged. It now shows git historic and curious non-sense links. Thanks, that's because of the gitweb upgrade, it should be fixed now. -- Alexandre Julliard julliard(a)winehq.org

March 13, 2011

Re: programs/dxdiag: beginnings of support for /t option (try 2)

by Vitaliy Margolen

On 03/12/2011 09:55 PM, Drew Goodwin wrote: > Fixed issues pointed out by Andrew Nguyen and Henri Verbeet. > There are still number of things that needs fixing: > +static BOOL PrepareFilename(WCHAR* filename, const enum OUTFORMAT format); > +static void OutputPlainFile(FILE * hFile, DxDiag_Root g_dxdiagcom_root); > +static void OutputSectionHeading(int section, FILE * hFile); > +static void OutputNewline(FILE * hFile); > +static void OutputWString(FILE * hFile, WCHAR * buffer); > +static void OutputPlainPropertyTitle(int property, FILE * hFile); > +static void OutputPlainProperty(int property, WCHAR * value, FILE * hFile); Please rearrange your functions so you don't need forward declarations. > + WideCharToMultiByte(CP_UTF8, 0, filename, -1, utf8Filename, > + MAX_STRING_LEN, NULL, NULL); Why utf8? Native creates plain text file which isn't a utf8. Haven't checked on different languages, but I think it would use some windows codepage instead of utf8. > + LoadStringW(NULL, STRING_ERROR_OPENING_FILE, messageText, > + MAX_STRING_LEN); Please don't split file on 80-character boundary, all modern widescreen displays can comfortably display 100+ characters in huge font. > + else if (format == XML) > + WINE_FIXME("/x unimplemented\n"); If it's not implemented, please remove all related code and add it back with xml implementation. > + /* trailing periods are not allowed in windows filenames */ > + /* some characters not allowed in windows filenames */ You shouldn't need to perform these checks here. Kernel/ntdll already do all of that. > +static void OutputWString(FILE * hFile, WCHAR * buffer) > +{ > + utf8BufferSize = WideCharToMultiByte(CP_UTF8, 0, buffer, -1, NULL, 0, NULL, NULL); > + utf8Buffer = HeapAlloc(GetProcessHeap(), 0, utf8BufferSize); This has really big overhead. Please use stack buffer instead and only do allocation when the buffer it too small. > +++ b/programs/dxdiag/cmdline.h > +BOOL OutputToFile(WCHAR * filename, const enum OUTFORMAT format, DxDiag_Root g_dxdiagcom_root); You don't need 2 header files for 3 external functions. Just forward declare them in the file that uses them. Or better yet put it all into the same file. And last but not the least, you might want to fix dxdiagn.dll to return a bit more for system info then this: ------------------- System Information ------------------- Time of this report: Machine name: Operating System: Language: System Manufacturer: System Model: BIOS: Processor: Memory: Page File: Windows Dir: C:\windows DirectX Version: = "DirectX 9.0c (4.09.0000.0904) DX Setup Parameters: Vitaliy

March 13, 2011

Re: advapi32/tests: Add some skips

by André Hentschel

Am 13.03.2011 15:28, schrieb Nikolay Sivov: >> to quicker see on which platforms it doesn't work >> --- >> dlls/advapi32/security.c | 2 +- >> dlls/advapi32/tests/registry.c | 11 ++++++++--- >> 2 files changed, 9 insertions(+), 4 deletions(-) >> >> diff --git a/dlls/advapi32/security.c b/dlls/advapi32/security.c >> index 1aa7990..4ee5c7b 100644 >> --- a/dlls/advapi32/security.c >> +++ b/dlls/advapi32/security.c >> @@ -2445,7 +2445,7 @@ AccessCheck( >> BOOL ret = set_ntstatus( NtAccessCheck(SecurityDescriptor, ClientToken, DesiredAccess, >> GenericMapping, PrivilegeSet, PrivilegeSetLength, >> GrantedAccess,&access_status) ); >> - if (ret) *AccessStatus = set_ntstatus( access_status ); >> + if (ret) *AccessStatus = 1;//set_ntstatus( access_status ); >> return ret; >> } > ? > > sry, bad hack for cain&abel -- Best Regards, André Hentschel

March 13, 2011

Re: advapi32/tests: Add some skips

by Nikolay Sivov

> to quicker see on which platforms it doesn't work > --- > dlls/advapi32/security.c | 2 +- > dlls/advapi32/tests/registry.c | 11 ++++++++--- > 2 files changed, 9 insertions(+), 4 deletions(-) > > diff --git a/dlls/advapi32/security.c b/dlls/advapi32/security.c > index 1aa7990..4ee5c7b 100644 > --- a/dlls/advapi32/security.c > +++ b/dlls/advapi32/security.c > @@ -2445,7 +2445,7 @@ AccessCheck( > BOOL ret = set_ntstatus( NtAccessCheck(SecurityDescriptor, ClientToken, DesiredAccess, > GenericMapping, PrivilegeSet, PrivilegeSetLength, > GrantedAccess,&access_status) ); > - if (ret) *AccessStatus = set_ntstatus( access_status ); > + if (ret) *AccessStatus = 1;//set_ntstatus( access_status ); > return ret; > } ?

March 13, 2011

re: GSOC CMD parser.

by Dan Kegel

Joel wrote: > ... > It does not seem like there are enough issues with the current > implementation to justify scrapping the current parser. The existing parser is pretty hard to understand and extend. We're probably a few man-months away from the point of diminishing returns, and I'm sure there are some bugs that can't be fixed without a rewrite. Have you read it enough to understand it yet? ( see also http://kegel.com/wine/sweng/2010/#parser ) > The way I would go about this task would be to fix all > the outstanding bugs, and add test cases for them ... Incremental improvements to the current parser would still be welcome and useful. For instance, supporting cmd /a ( see http://bugs.winehq.org/show_bug.cgi?id=21174 for someone's draft patch ). - Dan

March 13, 2011

Re: winex11: Quite a noisy FIXME

by André Hentschel

Am 12.03.2011 23:29, schrieb Vitaliy Margolen: > On 03/12/2011 12:10 PM, André Hentschel wrote: >> Yeah i know the fixme has good reasons, but still it shouldn't mess on console. > You shouldn't use broken Wine. Anything that indicates a broken Wine, especially due to missing dependencies during compile, should not be hidden. > > Vitaliy. to build a Wine without xrender and freetype was my intention, i wanted to try if crashes in freetype and xrender go away and they did, but the problematic app hang... need to file some bugs... -- Best Regards, André Hentschel

March 13, 2011

Re: Help: afxctl.h missing

by Stefan Dösinger

Am Samstag 12 März 2011, 06:56:53 schrieb Jaytirth S. Khairatkar: > Hello, > > I am getting the following error when I am trying to compile in Winelib > error: afxctl.h: No such file or directory > How should I resolve this? I think this file is part of MFC, and Wine does not implement MFC. A while ago it was possible to compile mfc as a winelib dll. Alternatively you can use plain PE files and bundle them with Wine, if you only want to run the application on x86 platforms. Whatever technical solution you choose also make sure that you honor all legal requirements for bundling MFC. MFC can be redistributed in binary form, but you have to meet certain conditions to do this.

March 13, 2011

GSOC CMD parser.

by Joel Teichroeb

I'm quite interested in the idea on the wiki of improving the CMD parser and fixing the bugs. I brought this up in the IRC and it was mentioned that there was some discussion of using bison/flex for parsing. I don't think that would be feasible for me to do in just three months but I'm wondering if that is the direction that is wanted for the CMD parser. It does not seem like there are enough issues with the current implementation to justify scrapping the current parser. The way I would go about this task would be to fix all the outstanding bugs, and add test cases for them, then find as many other bugs using and batch scripts from other open source projects. Thoughts and comments would be wonderful. Joel Teichroeb.

March 13, 2011

Wine announce on the website shows weird content

by Divaret Mathieu

Hello, I just wanted to tell you that the page on http://www.winehq.org/announce/ is bugged. It now shows git historic and curious non-sense links. Sorry to disturb if you already know. Continue the good job. Bye.

March 12, 2011

Re: winex11: Quite a noisy FIXME

by Vitaliy Margolen

On 03/12/2011 12:10 PM, André Hentschel wrote: > Yeah i know the fixme has good reasons, but still it shouldn't mess on console. You shouldn't use broken Wine. Anything that indicates a broken Wine, especially due to missing dependencies during compile, should not be hidden. Vitaliy.

March 12, 2011

Re: winex11: Quite a noisy FIXME

by Andrew Nguyen

On 03/12/2011 01:10 PM, André Hentschel wrote: > Yeah i know the fixme has good reasons, but still it shouldn't mess on console. > --- > dlls/winex11.drv/xrender.c | 4 +++- > 1 files changed, 3 insertions(+), 1 deletions(-) > > diff --git a/dlls/winex11.drv/xrender.c b/dlls/winex11.drv/xrender.c > index fde0b91..70fd5e2 100644 > --- a/dlls/winex11.drv/xrender.c > +++ b/dlls/winex11.drv/xrender.c > @@ -2435,7 +2435,9 @@ void X11DRV_XRender_UpdateDrawable(X11DRV_PDEVICE *physDev) > BOOL XRender_AlphaBlend( X11DRV_PDEVICE *devDst, X11DRV_PDEVICE *devSrc, > struct bitblt_coords *dst, struct bitblt_coords *src, BLENDFUNCTION blendfn ) > { > - FIXME("not supported - XRENDER headers were missing at compile time\n"); > + static int once; > + if(!once) > + FIXME("not supported - XRENDER headers were missing at compile time\n"); > return FALSE; > } > Since the static variable once is never incremented, the if condition is practically a no-op.

March 12, 2011

Re: Make sure This->bsc.doc will be initialized while opening a zero byte html file, fix bug 25999 . modified: dlls/mshtml/navigate.c

by Qian Hong

Hi Jacek, On Wed, Mar 9, 2011 at 12:41 AM, Jacek Caban <jacek(a)codeweavers.com> wrote: > Hi Zuno, > > I don't think that's the right place for the fix. What's the exact problem > you're trying to fix? AFAICS the only important difference your patch makes > is that on_start_nsrequest will be always called in read_stream_data > (possibly many times), but it's also called in on_stop_nsrequest if no data > was read. > > Jacek > > > Is there any update of this bug? Thanks! -- Regards, Qian Hong - Sent from Ubuntu http://www.ubuntu.com/

March 12, 2011

Re: correct comctl32 implementation

by Nikolay Sivov

On 3/12/2011 09:54, Andrew Green wrote: > Thank you for the quick reply. > so is it the case is that multiple dll's versions will be generated to > be used by SxS(as WinSxS contains multiple comctl32.dll)? Again, we don't want to keep multiple versions, so we need a way to work with single one. > I know uxtheme draws primitives. What I meant to ask was will any work > have to be done in uxtheme(apart from maybe the occasional bug fix, > not a new implementation of it or anything like that)? Sure, bugs are possible, especially since uxtheme is not widely used. > The last point about window class redirection I'm a bit confused > about. I really don't know anything about it. This patch exists so it > sounds like it is supported. > http://osdir.com/ml/wine-patches/2009-12/msg00871.html > So would it just be that case that comctl32(version 6) re-registers > the controls that are normally in user32? No, it's not supported. As I said here http://www.mail-archive.com/wine-devel(a)winehq.org/msg59786.html we also need some ntdll improvements for activation contexts to support this stuff better. FindActCtxSectionString() shouldn't be hard to improve but you need to figure out closed data structures format, at least some useful fields (without disassembler, wine test suite only). > If anyone could provide me with any information or reading material. I > would be very thankful. I don't think it's documented, I mean loading process itself to replace registered classes. You can't try to search for "<windowClass></windowClass>" entry description for SxS manifests. As I remember I failed to find any explanation for that, so this needs extensive tests in a first place. Also be sure to test only behaviour without looking inside any MS modules.

March 12, 2011

Re: correct comctl32 implementation

by André Hentschel

Am 12.03.2011 07:54, schrieb Andrew Green: > Thank you for the quick reply. > so is it the case is that multiple dll's versions will be generated to > be used by SxS(as WinSxS contains multiple comctl32.dll)? > > I know uxtheme draws primitives. What I meant to ask was will any work > have to be done in uxtheme(apart from maybe the occasional bug fix, > not a new implementation of it or anything like that)? No, uxtheme is in a pretty good shape. Most stuff will get into comctl32 and user32. That will enable us to have theming features like: http://www.dritimage.de/uploads/bilder/124897763294_dirty-scrollbar-hack.png http://img.dritimage.de/bilder/124922976611_dirty-captionbar-hack.png But these were only really bad hacks just for a showcase. > > The last point about window class redirection I'm a bit confused > about. I really don't know anything about it. This patch exists so it > sounds like it is supported. > http://osdir.com/ml/wine-patches/2009-12/msg00871.html That patch is not in git, mostly likely for that reason: http://www.mail-archive.com/wine-devel(a)winehq.org/msg59786.html > So would it just be that case that comctl32(version 6) re-registers > the controls that are normally in user32? Yes, there is some comctl32 function (not yet implemented) to trigger that. > > If anyone could provide me with any information or reading material. I > would be very thankful. I only can point you on a stupid test i once wrote: http://www.winehq.org/pipermail/wine-patches/2009-September/078825.html I would suggest: read over it and then delete it :) -- Best Regards, André Hentschel

March 12, 2011

Re: inetcpl.cpl: Update French translation

by Nicolas Le Cam

Hi Frédéric, Le 12 mars 2011 09:09, Frédéric Delanoy <frederic.delanoy(a)gmail.com> a écrit : > --- > dlls/inetcpl.cpl/cpl_Fr.rc | 12 ++++++------ > 1 files changed, 6 insertions(+), 6 deletions(-) > > diff --git a/dlls/inetcpl.cpl/cpl_Fr.rc b/dlls/inetcpl.cpl/cpl_Fr.rc > index 54d19e9..1244ff6 100644 > --- a/dlls/inetcpl.cpl/cpl_Fr.rc > +++ b/dlls/inetcpl.cpl/cpl_Fr.rc > @@ -35,17 +35,17 @@ CAPTION "Général" > BEGIN > > GROUPBOX " Page d'accueil ", IDC_STATIC, 4, 4, 312, 52 > - LTEXT "Vous pouvez spécifier une adresse à utiliser comme page > d'accueil :", > + LTEXT "Vous pouvez spécifier l'adresse à utiliser comme page > d'accueil :", > IDC_STATIC, 68, 10, 242, 10 > EDITTEXT IDC_HOME_EDIT, 68, 22, 242, 12, WS_VISIBLE | ES_AUTOHSCROLL > PUSHBUTTON "Page &courante", IDC_HOME_CURRENT, 68, 36, 77, 14 > PUSHBUTTON "Page par &défaut", IDC_HOME_DEFAULT, 151, 36, 77, 14 > PUSHBUTTON "Page &blanche", IDC_HOME_BLANK, 233, 36, 77, 14 > - GROUPBOX " Browsing history ", IDC_STATIC, 4, 60, 312, 46 > - LTEXT "You can delete cached pages, cookies and other data.", > - IDC_STATIC, 58, 72, 252, 10 > - PUSHBUTTON "Delete &files...", IDC_HISTORY_DELETE, 144, 86, 80, 14 > - PUSHBUTTON "&Settings...", IDC_HISTORY_SETTINGS, 230, 86, 80, 14 > + GROUPBOX " Historique de navigation ", IDC_STATIC, 4, 60, 312, 46 > + LTEXT "Vous pouvez effacer les pages en cache, les cookies et > d'autres données.", > + IDC_STATIC, 68, 72, 242, 10 > "Vous pouvez effacer les pages en cache, les cookies et autres données." should be better. > + PUSHBUTTON "&Effacer les fichiers...", IDC_HISTORY_DELETE, 151, 86, > 77, 14 > + PUSHBUTTON "&Paramètres...", IDC_HISTORY_SETTINGS, 233, 86, 77, 14 > > END > > -- > 1.7.4.1 > > -- Nicolas Le Cam

March 12, 2011

Re: correct comctl32 implementation

by Andrew Green

Thank you for the quick reply. so is it the case is that multiple dll's versions will be generated to be used by SxS(as WinSxS contains multiple comctl32.dll)? I know uxtheme draws primitives. What I meant to ask was will any work have to be done in uxtheme(apart from maybe the occasional bug fix, not a new implementation of it or anything like that)? The last point about window class redirection I'm a bit confused about. I really don't know anything about it. This patch exists so it sounds like it is supported. http://osdir.com/ml/wine-patches/2009-12/msg00871.html So would it just be that case that comctl32(version 6) re-registers the controls that are normally in user32? If anyone could provide me with any information or reading material. I would be very thankful. On Fri, Mar 11, 2011 at 7:28 PM, Nikolay Sivov <bunglehead(a)gmail.com> wrote: > On 3/11/2011 06:22, Andrew Green wrote: >> >> I have been researching how i could implement the gsoc Theming - >> Implement Wine theming support. >> Though I'm a bit confused what is wanted. >> So is the case that completely separate dll's should be written for >> different versions and be loaded by side-by-side assembly. >> So you would end up with >> "comctl32v5.dll","comctl32v6.dll","comctl32v7.dll"? >> Would these be 3 different code basses or should they be made to share >> most of there code? > > Multiple comctl32 should be the last thing to think about, only if there's > absolutely no way to > implement everything needed in one comctl32. >> >> > From what I understand uxtheme already loads XP theme files. So would >> all the work be isolated to creating different versions of comdlg32 >> and comctl32? > > No sure what you mean. uxtheme purpose is to load theme data, draw theme > primitives and > retrieve theme components data. > > Client dll such as comctl32 should only use that public api when drawing > controls, that's all. > Also comdlg32 was never a question, cause it doesn't provide new controls. >> >> Also does wine support side by side assembly? I did a very quick >> google search and i didn't see anything. > > Yes wine does support installation and loading of SxS assemblies, and by > assembly I mean native dlls installed in SxS > directory and referenced with embedded or external manifest file. Loader > supports manifest files, not fully but simple things should work. >> >> I hope I'm not asking too much at once and/or too silly questions. > > No, it's not silly to ask at all. But one of the most important things to be > done for that is to implement support of window class redirection. In > version 6 all user32 controls are reimplemented with theme support in > comctl32, while user32 classes are kept of course. This is done with > specific entries in comctl32 manifest, on load comctl32 all builtin classes > are re-registered to the ones from comctl32. I definitely want to see some > solution for that, having themed comctl32 controls while scrollbars and > buttons are drawn with default appearance makes theme support kind of > useless. > >

March 12, 2011

Help: afxctl.h missing

by Jaytirth S. Khairatkar

Hello, I am getting the following error when I am trying to compile in Winelib error: afxctl.h: No such file or directory How should I resolve this? thanks and regards Jaytirth Khairatkar -- Jaytirth S. Khairatkar System Architecture and Algorithms Mobile: 9223529523

March 12, 2011

Re: wineport: Add fast builtin & asm versions of ffs() + ctz() where supported.

by Adam Martinson

On 03/11/2011 07:28 AM, Alexandre Julliard wrote: > Adam Martinson<amartinson(a)codeweavers.com> writes: > >> @@ -236,7 +241,40 @@ extern int getopt_long_only (int ___argc, char *const *___argv, >> #endif /* HAVE_GETOPT_LONG */ >> >> #ifndef HAVE_FFS >> -int ffs( int x ); >> + #if defined(__i386__) || defined(__x86_64__) >> + #define HAVE_FFS >> + static inline int ffs( int x ) >> + { >> + if (!x) >> + { >> + return 0; >> + } >> + else >> + { >> + int ret; >> + __asm__("bsfl %1, %0; incl %0" : "=r" (ret) : "r" (x)); >> + return ret; >> + } >> + } >> + #elif defined(__GNUC__)&& GCC_VERSION>= 29503 >> + #define HAVE_FFS >> + #define ffs(x) __builtin_ffs(x) >> + #else >> + int ffs( int x ); >> + #endif >> +#endif > You'd have to show benchmarks to prove that this complexity is > necessary. Given that ffs() should already be inlined on all decent > platforms, I doubt you'd be able to demonstrate a difference (if > anything, your version would be slower because of the extra increment). > I did this because it was easy and I was doing ctz() anyhow; I don't actually need these versions of ffs() for anything. On any system with HAVE_FFS the system version takes precedence.

March 11, 2011

Re: wineport: Use a lookup table to make our slow ffs() a bit faster.

by Adam Martinson

On 03/11/2011 07:19 AM, Alexandre Julliard wrote: > Henri Verbeet<hverbeet(a)gmail.com> writes: > >> Something like the following may be faster: >> >> static const int bit_pos[] = >> { >> 0x00, 0x01, 0x0e, 0x02, 0x0f, 0x1b, 0x12, 0x03, >> 0x1e, 0x10, 0x1c, 0x16, 0x18, 0x13, 0x0a, 0x04, >> 0x1f, 0x0d, 0x1a, 0x11, 0x1d, 0x15, 0x17, 0x09, >> 0x0c, 0x19, 0x14, 0x08, 0x0b, 0x07, 0x06, 0x05, >> }; >> const unsigned int c = 0x7dc4d65; >> return bit_pos[((x& -x) * c)>> 27]; >> >> I'd expect that mostly due to eliminating the branches and the >> implicit shift& mask operations in something like "y.c[1]". > Maybe, but there's no point. This is just a fallback implementation for > broken platforms, any platform that we care about should never use this. > For systems with fast multiplication that way is probably faster. This was just something I noticed that took me 5 mins and was clearly better than what we had.

March 11, 2011

Re: wined3d/context: Remove a trivial call to context_validate_onscreen_formats() in context_apply_blit_state().

by Adam Martinson

On 03/11/2011 06:50 AM, Henri Verbeet wrote: > Sigh. I've asked you a very simple question about the performance > impact of these changes *twice* now, and you ignored the question both > times. > > Unless you have evidence that the compiler fails to eliminate the call > by itself, and this actually makes any kind of difference, consistency > is more important. Obviously for a change this small the difference is not measurable. I don't think trivial function calls are ever correct, here or anywhere else. I don't know whether that call compiles out, but I know for sure that a comment does.

March 11, 2011

Re: include: Add rmxftmpl.h header.

by Dylan Smith

On Fri, Mar 11, 2011 at 9:44 AM, Alexandre Julliard <julliard(a)winehq.org> wrote: > > That stuff needs to be generated from source, you can't just copy the > native header contents. > Is the following acceptable? 1. Implement IDirectXFileSaveObject::SaveTemplates 2. Create programs/dxfconv as a wine program that uses d3dxof to convert x files to different encodings (e.g. text -> binary) 3. Write the text encoding for D3DRM_XTEMPLATES in include/rmxftmpl.x 4. Use dxfconv and xxd -i to generate the binary contents of D3DRM_XTEMPLATES, and add a comment to rmxftmpl.h saying it was generated from rmxftmpl.x 5. Test to make sure the contents is the same for compatibility.

March 11, 2011

Re: wineport: Use a lookup table to make our slow ffs() a bit faster.

by David Laight

On Fri, Mar 11, 2011 at 05:51:27AM -0800, Dan Kegel wrote: > > Large lookup tables can dirty the L1 cache and slow other things down. And even small tables can also be a slow data cache miss in the critical path. Of course, when benchmarking, the table is all in the cache! In real life it is likely to be absent (unless you are doing a lot of lookups together). David -- David Laight: david(a)l8s.co.uk

March 11, 2011

Re: wineport: Add fast builtin & asm versions of ffs() + ctz() where supported.

by David Laight

On Fri, Mar 11, 2011 at 02:28:27PM +0100, Alexandre Julliard wrote: > Adam Martinson <amartinson(a)codeweavers.com> writes: > > > @@ -236,7 +241,40 @@ extern int getopt_long_only (int ___argc, char *const *___argv, > > #endif /* HAVE_GETOPT_LONG */ > > > > #ifndef HAVE_FFS > > -int ffs( int x ); > > + #if defined(__i386__) || defined(__x86_64__) ... > > + __asm__("bsfl %1, %0; incl %0" : "=r" (ret) : "r" (x)); > > + return ret; ... > > + #elif defined(__GNUC__) && GCC_VERSION >= 29503 > > + #define HAVE_FFS > > + #define ffs(x) __builtin_ffs(x) > > + #else > > + int ffs( int x ); > > + #endif > > +#endif > > You'd have to show benchmarks to prove that this complexity is > necessary. Given that ffs() should already be inlined on all decent > platforms, I doubt you'd be able to demonstrate a difference (if > anything, your version would be slower because of the extra increment). Never mind the fact that it is possible to write code that is (probably) faster than the bsfl instruction on most x86 processors! You'd also get a gain from getting the 'if (x == 0)' statically predicted correctly (probably for not zero) - that is also likely to be large. David -- David Laight: david(a)l8s.co.uk

March 11, 2011

Re: [PATCH 1/6] user32: Add get/release routines for accessing cursor frames.

by Alexandre Julliard

Erich Hoover <ehoover(a)mines.edu> writes: > The idea of the get_icon_data() is to return the frame data for a > particular icon step, so for split animated cursors (patch 3) the > routine can appropriately call get_icon_ptr() on the "sub-icon" (and > release_icon_data can call release_icon_ptr). I considered calling > this get_icon_frame() and having it return the existing > "cursoricon_frame" structure, but DrawIconEx needs to access the > frame-specific width and height - since many animated cursors do not > have a global width and height. Would you prefer that I call these > get/release_icon_frame() and move the width and height into > "cursoricon_frame" ? I'd have to see what the resulting code look like, but it does sound like it would be cleaner. -- Alexandre Julliard julliard(a)winehq.org

March 11, 2011

Re: [PATCH 1/6] user32: Add get/release routines for accessing cursor frames.

by Erich Hoover

On Fri, Mar 11, 2011 at 8:01 AM, Alexandre Julliard <julliard(a)winehq.org> wrote: > Erich Hoover <ehoover(a)mines.edu> writes: >> @@ -135,6 +139,16 @@ static void release_icon_ptr( HICON handle, struct cursoricon_object *ptr ) >> release_user_handle_ptr( ptr ); >> } >> >> +struct static_cursoricon_object *get_icon_data( struct cursoricon_object *obj, int istep ) >> +{ >> + return &obj->frames[istep]; >> +} >> + >> +void release_icon_data( struct cursoricon_object *obj, int istep, struct static_cursoricon_object *icon_data ) >> +{ >> + /* placeholder */ >> +} > > I find your data structures confusing. You add a concept of icon_data > separate from icon_ptr, but it doesn't correspond to the data > structures, there's no such thing as an icon_data. There should be a way > of making this clearer. The idea of the get_icon_data() is to return the frame data for a particular icon step, so for split animated cursors (patch 3) the routine can appropriately call get_icon_ptr() on the "sub-icon" (and release_icon_data can call release_icon_ptr). I considered calling this get_icon_frame() and having it return the existing "cursoricon_frame" structure, but DrawIconEx needs to access the frame-specific width and height - since many animated cursors do not have a global width and height. Would you prefer that I call these get/release_icon_frame() and move the width and height into "cursoricon_frame" ? Erich Hoover ehoover(a)mines.edu

March 11, 2011