July 2008 - Wine-devel - list.winehq.org

Re: [Gdiplus 1/6] Valgrind: Fix a memory leak
by James Hawkins 22 Jul '08

22 Jul '08

On Tue, Jul 22, 2008 at 10:06 PM, Adam Petaccia <adam(a)tpetaccia.com> wrote: > If this patch is rejected, the others will still apply and I will drop this from my tree. > > Technically a leak shouldn't occur here as the call before it is invalid, but because Gdiplus doesn't correctly handle this, we should call delete and expect an InvalidParamter return. > Shouldn't GdipCreateFontFamilyFromName be fixed to free the handle in this error case? Also, if you have a patch in a series that no later patch depends on, please don't add it to the series numbering. -- James Hawkins

1 0

[RFC] crypt32: fixed the base64 tests on Vista.
by Reece Dunn 22 Jul '08

22 Jul '08

This patch fixes the base64 tests on Vista (http://test.winehq.org/data/b3f4091b47e70681a9909bfccd19dee95657fabd/vista_…) however to me this feels wrong. In the call to pCryptStringToBinaryA, Vista is returning TRUE in some cases, and for those is sometimes not setting GetLastError(), and in others is setting it to ERROR_INVALID_DATA (even when it is returning TRUE!). For these cases, it is also not skipping any characters. To me, without understanding this in any more detail, it looks as if Vista is broken here (and thus the Vista return parts should be marked as broken()). However, Vista may be doing the right thing, and thus the behaviour in these cases has changed between XP and Vista. The latter seems more likely, but I do not have any experience in this area, nor understand these tests well enough to say one way or the other. Any ideas? - Reece

3 4

RE: wined3d: universal surface convertor function for unsigned integer color formats(2nd attempt)
by Stefan Dösinger 22 Jul '08

22 Jul '08

How many different conversions do the two games need? One of my concerns is that this generic code is way slower than a separate conversion function for each pair of color formats. On the other hand, one function for each pair gives a whole lot more of code, and if we run into the conversion function during rendering we can kiss the framerate goodbye anyway > -----Original Message----- > From: wine-patches-bounces(a)winehq.org [mailto:wine-patches- > bounces(a)winehq.org] On Behalf Of Victor Eremin > Sent: Thursday, July 17, 2008 11:02 AM > To: wine-patches(a)winehq.org > Subject: wined3d: universal surface convertor function for unsigned > integer color formats(2nd attempt) > > > Supports conversion between most of "unsigned color" argb/xrgb surface > formats (D3DFMT_A8R8G8B8, D3DFMT_A8R3G3B2, etc), and "luminosity" color > formats (D3DFMT_L8, etc), excluding D3DFMT_A16R16G16B16, D3DFMT_A8P8, > D3DFMT_P8 and D3DFMT_A1. > luminosity to argb/xrgb (and vice versa) conversions are not supported > > Removes "Cannot find coverter" FIXMEs from "Ancient Evil" and "Stranded > 2" games. > > Fixes water glitches in "Stranded 2" game. > --- > dlls/wined3d/surface_base.c | 203 > ++++++++++++++++++++++++++++++++++++++-- > dlls/wined3d/utils.c | 126 ++++++++++++------------ > dlls/wined3d/wined3d_private.h | 2 +- > 3 files changed, 260 insertions(+), 71 deletions(-)

2 3

Convert 565 to 0888
by Einars 22 Jul '08

22 Jul '08

I had graphical problems with an online game called Angels Online, when just a black screen was visible, and complaints about missing converter from WINED3DFMT_R5G6B5 to WINED3DFMT_X8R8G8B8, so I added a converter for this format, and the game runs flawlessly now on wine, the 1.1.1 release as well as git version (at least for me). I hope this was the right way to fix it. Kind regards, Einars Lielmanis

1 0

RE: general question..
by Stefan Dösinger 22 Jul '08

22 Jul '08

Can you give some examples of such code? Nobody here is a goto-maniac, but in all cases I know the goto is used for a good reason. Most of the time it is used for error path to avoid ugly if() nesting and/or code duplication when freeing partially allocated objects > -----Original Message----- > From: wine-patches-bounces(a)winehq.org [mailto:wine-patches- > bounces(a)winehq.org] On Behalf Of Chris Ahrendt > Sent: Monday, July 21, 2008 8:42 PM > To: wine-patches(a)winehq.org > Subject: general question.. > > As I have been going through trying to debug the everquest2 issues on > my > machine I have run into a few places where I think the code should be > changed a little ( this alot of times is where there is a GOTO in the > code). My question is in the case where I do find these goto's and I > rewrite the code should I submit the changes here for approval? > I have been doing some code reviews and found a few places where > there are goto's that don't need to be there. I guess I am old school > and really dislike code with goto's (they make debugging and > maintenance > a nightmare (I used to maintain 20 year old cobol code along time ago > =) ) > > > Chris

3 3

Re: [Gdiplus try3 02/16] Implement GdipCreateRegion
by Huw Davies 22 Jul '08

22 Jul '08

Hi Adam, Some comments inlined below: On Tue, Jul 22, 2008 at 01:22:56AM -0400, Adam Petaccia wrote: > diff --git a/dlls/gdiplus/gdiplus_private.h b/dlls/gdiplus/gdiplus_private.h > index e7ca874..9c0246b 100644 > --- a/dlls/gdiplus/gdiplus_private.h > +++ b/dlls/gdiplus/gdiplus_private.h > @@ -196,4 +198,40 @@ struct GpFontFamily{ > WCHAR FamilyName[LF_FACESIZE]; > }; > > +typedef struct RegionElement > +{ > + DWORD type; /* Rectangle, Path, SpecialRectangle, or CombineMode */ > + union > + { > + GpRectF rect; > + struct > + { > + GpPath* path; > + struct > + { > + DWORD size; > + DWORD magic; > + DWORD pointCount; > + DWORD storageFlags; > + } pathHeader; > + } pathData; > + struct > + { > + struct RegionElement *left; /* the original region */ > + struct RegionElement *right; /* what *left was combined with */ > + } combine; > + } elementData; > +} RegionElement; It would be better to avoid the mixed upper/lower case names, since this makes it look like they're win32 api structures. Something like typedef struct region_element region_element is nicer. Same goes for the element names like pathHeader etc. > +struct GpRegion{ > + struct > + { > + DWORD size; > + DWORD checksum; > + DWORD magic; > + DWORD numOps; A better name for numOps would be num_child_nodes. This is the total number of children under the root node (and is equal to twice the number of ops). I should update my comment at the beginning of region.c to reflect this. > + } header; > + RegionElement *node; How about actually embedding the structure here rather than a pointer to it? Since you'll always have a root node there's no need to allocate it explicitly. And let's call it 'root' not 'node'. Something like region_element root > diff --git a/include/gdiplusenums.h b/include/gdiplusenums.h > index 53401bd..f3110c9 100644 > --- a/include/gdiplusenums.h > +++ b/include/gdiplusenums.h > @@ -308,6 +308,15 @@ enum CombineMode > CombineModeComplement > }; > > +enum RegionType > +{ > + RegionDataRect = 0x10000000, > + RegionDataPath = 0x10000001, > + RegionDataEmptyRect = 0x10000002, > + RegionDataInfiniteRect = 0x10000003, > +}; > + > + > enum FlushIntention > { > FlushIntentionFlush = 0, > @@ -353,6 +362,7 @@ typedef enum HotkeyPrefix HotkeyPrefix; > typedef enum PenAlignment GpPenAlignment; > typedef enum ImageCodecFlags ImageCodecFlags; > typedef enum CombineMode CombineMode; > +typedef enum RegionType RegionType; > typedef enum FlushIntention FlushIntention; > typedef enum CoordinateSpace CoordinateSpace; > Are you sure RegionType is in the win32 api? I can't find this is my gdiplusenums.h. If it isn't then it should be put in gdiplus_private.h (or even just in region.c if nothing else uses it) and renamed to enum region_type. Again, I think you'd be better off submitting a couple of patches at a time. Huw. -- Huw Davies huw(a)codeweavers.com

2 2

Re: [3/9] shell32: Load Control Panel title from resources
by Alexandre Julliard 22 Jul '08

22 Jul '08

Owen Rudge <owen(a)owenrudge.net> writes: > --- a/dlls/shell32/shell32_Bg.rc > +++ b/dlls/shell32/shell32_Bg.rc > @@ -219,4 +219,6 @@ STRINGTABLE > IDS_CDBURN_AREA "Local Settings\\Application Data\\Microsoft\\CD Burning" > > IDS_NEWFOLDER "New Folder" > + > + IDS_CPANEL_TITLE "Wine Control Panel" The indentation is wrong, looks like you are using the wrong tab size. -- Alexandre Julliard julliard(a)winehq.org

2 1

Re: dinput: the DIK_ keycode is not the same as the scancode.
by Vitaliy Margolen 22 Jul '08

22 Jul '08

Aric Stewart wrote: > It is mapped with the keyboard mapping to the resulting character. so > the key 'A' is DIK_A nomatter what its scancode or vkey would be. This > is relevent to Japanese keymapping where the '@' key is in the '[' > location the scancode for both is 0x22 but dinput generates DIK_AT in > japanese and DIK_LBRACKET in us_qwerty > Looks good but have few comments: > +static int map_dik_code(DWORD scanCode) > +{ > + DWORD vk = MapVirtualKeyA(scanCode,MAPVK_VSC_TO_VK); There is a vkCode in the KBDLLHOOKSTRUCT struct. You should use that instead of doing the remapping all the time. > + CHAR c = MapVirtualKeyA(vk,MAPVK_VK_TO_CHAR); > + > + switch (c & 0xFF) > + { > + case 32: return DIK_SPACE; > + case 39: return DIK_APOSTROPHE; Magic numbers are always bad. It's better to use vk_code here instead of a char. Vitaliy.

2 1

Re: netapi32: add stub for NetShareGetInfo
by Vitaly Perov 22 Jul '08

22 Jul '08

I am sorry. It was wrong file attached. -- Best wishes, Vitaly Perov Russia, Saint-Petersburg. www.etersoft.ru

1 0

Re: [Gdiplus try3 01/16] Mark the tests todo_wine, instead of skipping over them all.
by Alexandre Julliard 22 Jul '08

22 Jul '08

Adam Petaccia <adam(a)tpetaccia.com> writes: > todo_wine > +{ > ok(status == Ok, "status %08x\n", status); > > - if(status != Ok) return; > - > status = GdipGetRegionDataSize(region, &needed); > ok(status == Ok, "status %08x\n", status); > - ok(needed == 20, "got %d\n", needed); > + expect(20, needed); This will fail randomly since needed isn't set on failure. Skipping the test was better. -- Alexandre Julliard julliard(a)winehq.org

1 0