Re: server: Android has struct user in asm/user.h
On 24 Sep 2013, at 17:57, André Hentschel n...@dawncrow.de wrote: is that for the warning about the missing casts? No, this actually introduces more :-) The cast warning is about the 4th param of ptrace. Huw.
Re: [PATCH 2/3] iphlpapi: Add interface enumeration for Android.
On 19 Sep 2013, at 15:07, Bruno Jesus wrote: Hi, Huw. I'm not sure this is an issue but in the android implementation of get_interface_indexes you are not setting +if (table) *table = NULL; Hi Bruno, Good catch! Resent. Thanks, Huw.
Re: fonts: Add some box-type glyphs to wingdings.
On Fri, Sep 13, 2013 at 11:07:19AM +0100, Huw Davies wrote: --- fonts/wingding.sfd | 86 + Please ignore this one and use 'try 2'. I had the points of one glyph selected in the outline editor which apparently gets saved to the .sfd . Huw.
Re: riched20: Use \ulnone instead of \ul0 for no underline.
On Mon, Aug 19, 2013 at 05:38:13PM -0500, Vincent Povirk wrote: Testing on Windows (by saving rtf documents with wordpad and viewing them with notepad) shows that it uses \ulnone instead of \ul0 to end underline. Wine and Windows are both able to read either one. I think these are symonyms and indeed Windows seems to use \ulnone, so this looks good to me. Huw.
Re: gdi32: Printer drivers don't use the character extra spacing if lpdx is supplied.
On Tue, Mar 26, 2013 at 04:30:14PM +0100, Alexandre Julliard wrote: Huw Davies h...@codeweavers.com writes: Ideally this would be done in wineps.drv, but by that point the conversion to device units would cause rounding errors. What about GetTextExtent functions? Do they need this too? Some test cases would be nice. Note this only applies when lpDx != NULL, when it's NULL then char_extra is still applied. So, yes the current GetTextExtent behaviour is correct. Huw.
Re: gdi32: Printer drivers don't use the character extra spacing if lpdx is supplied.
On Tue, Mar 26, 2013 at 04:50:03PM +0100, Alexandre Julliard wrote: Ah OK, I guess that makes it a PITA to test... Some kind of print-scan magic comes to mind ;-)
Re: winspool.drv: Fallback to cupsGetPPD if cupsGetPPD3 is missing or failed.
On Mon, Feb 11, 2013 at 02:03:26PM +0800, Dmitry Timoshkov wrote: -if (pcupsGetPPD3) return pcupsGetPPD3( http, name, modtime, buffer, bufsize ); +if (pcupsGetPPD3) +{ +http_status_t status; + +TRACE( Calling cupsGetPPD3\n ); +status = pcupsGetPPD3( http, name, modtime, buffer, bufsize ); +if (status == HTTP_OK) return HTTP_OK; +TRACE(cupsGetPPD3 failed (status %d)\n, status); +} Under what conditions does cupsGetPPD succeed if cupsGetPPD3 fails? Huw.
Re: winspool.drv: Fallback to cupsGetPPD if cupsGetPPD3 is missing or failed.
On Mon, Feb 11, 2013 at 06:12:57PM +0800, Dmitry Timoshkov wrote: Huw Davies h...@codeweavers.com wrote: Under what conditions does cupsGetPPD succeed if cupsGetPPD3 fails? I don't have full understanding of what is going on, but sometimes (!) cupsGetPPD3 returns 404, while cupsGetPPD works just fine here. Perhaps that's just another Ubuntu quirk. Seems odd as internally cupsGetPPD just calls cupsGetPPD3. Huw.
Re: riched20: ME_StrDup() is no longer used so remove it.
On Tue, Jan 29, 2013 at 07:05:13AM +0100, Francois Gouget wrote: --- dlls/riched20/editor.h |1 - dlls/riched20/string.c |5 - 2 files changed, 6 deletions(-) I'm about to send in a patch that uses this again, so please hold off on this one. Huw.
Re: usp10: Implement GPOS MarkToLigature Attachment Positioning Subtable
On Wed, Jan 09, 2013 at 11:36:51AM -0600, Aric Stewart wrote: +typedef struct { +WORD LigatureAnchor[1]; +} GPOS_ComponentRecord; + +typedef struct { +WORD ComponentCount; +GPOS_ComponentRecord ComponentRecord[1]; +} GPOS_LigatureAttach; + +for (i = 0; i component_count !offset; i++) +{ +int k; +for (k = 0; k class_count !offset; k++) +offset = GET_BE_WORD(lt-ComponentRecord[i].LigatureAnchor[k]); +} Hi Aric, This can't work as ComponentRecord is a variable-sized struct. Huw.
Re: [PATCH 3/3] usp10: Add support for format 2 pair adjustments.
On Thu, Dec 20, 2012 at 10:12:38PM +0800, Dmitry Timoshkov wrote: Huw Davies h...@codeweavers.com wrote: typedef struct { +WORD PosFormat; +WORD Coverage; +WORD ValueFormat1; +WORD ValueFormat2; +WORD ClassDef1; +WORD ClassDef2; +WORD Class1Count; +WORD Class2Count; +WORD Class1Record[1]; +} GPOS_PairPosFormat2; Shouldn't this structure be 2 bytes aligned? I don't see why. They're all WORDs. Huw.
Re: [PATCH 3/3] usp10: Add support for format 2 pair adjustments.
On Thu, Dec 20, 2012 at 11:05:48PM +0800, Dmitry Timoshkov wrote: Huw Davies h...@codeweavers.com wrote: typedef struct { +WORD PosFormat; +WORD Coverage; +WORD ValueFormat1; +WORD ValueFormat2; +WORD ClassDef1; +WORD ClassDef2; +WORD Class1Count; +WORD Class2Count; +WORD Class1Record[1]; +} GPOS_PairPosFormat2; Shouldn't this structure be 2 bytes aligned? I don't see why. They're all WORDs. There are 9 elements of 2 bytes in size each, 18 bytes in total, the compiler will align this structure to 4 bytes, making its size 20 bytes. The size is irrelevant - it's variable sized anyway. We just care about the offsets of the elements.
Re: [PATCH 3/3] usp10: Add support for format 2 pair adjustments.
On Thu, Dec 20, 2012 at 11:26:36PM +0800, Dmitry Timoshkov wrote: Huw Davies h...@codeweavers.com wrote: The size is irrelevant - it's variable sized anyway. We just care about the offsets of the elements. Anyway, in order to avoid any confusion or bugs in future with allocating an array of such structures or using sizeof() on them I'd suggest to explicitely specify alignment. Using an array of these structures or using sizeof() is wrong anyway as they are variable-sized...
Re: winex11: Add a few 'fall through' comments.
On Tue, Nov 27, 2012 at 06:44:35PM +0800, Dmitry Timoshkov wrote: case DirectColor: X11DRV_PALETTE_PaletteFlags |= X11DRV_PALETTE_VIRTUAL; /* fall through */ case GrayScale: would be more clear IMHO. Right now it's not obvious to which case statement 'fall through' belongs to. Resent, thanks. Huw.
Re: [PATCH 1/2] oleaut32/tests: Add test for ITypeInfo2_fnGetContainingTypeLib [try 2]
On Thu, Nov 08, 2012 at 06:13:28PM +0400, Tatyana Fokina wrote: +hr = CreateTypeLib2(SYS_WIN32, jsdeb, (ICreateTypeLib2 **)ctl2); +ok_ole_success(hr, CreateTypeLib2); + +hr = ICreateTypeLib2_CreateTypeInfo(ctl2, jsname, TKIND_DISPATCH, cti); +ok_ole_success(hr, ICreateTypeLib2_CreateTypeInfo); Please don't use obscure strings like 'jsdeb.tlb' use something like 'test.tlb'. You don't need the casts here or further down. + +hr = ICreateTypeInfo_QueryInterface(cti, IID_ITypeInfo2, (void**)ti2); +ok_ole_success(hr, ICreateTypeInfo2_QueryInterface); +if (SUCCEEDED(hr)) +{ There's no need to put this inside an if block. QI isn't going to fail. Also your indentation is first four spaces, then two - keep it at four. Huw.
Re: [PATCH 2/2] oleaut32: If pointers (ppTLib or pIndex in ITypeInfo2_fnGetContainingTypeLib) are null, we simply ignore it.[resend]
On Thu, Nov 08, 2012 at 06:13:33PM +0400, Tatyana Fokina wrote: +if(ppTLib) +{ *ppTLib = (ITypeLib *)This-typelib-ITypeLib2_iface; ICreateTypeLib_AddRef((ICreateTypeLib*)This-typelib); +} + +if(pIndex) *pIndex = This-typeinfo-typekind 16; Please indent.
Re: vbscript/tests: Skip some of the weekday tests if the first day of the week is not Sunday.
On Wed, Oct 31, 2012 at 12:34:49PM +0100, Jacek Caban wrote: Did you see those failures on Windows? test.winehq.org shows only some Linux failures, which suggests that the problem is in implementation, not tests. Hi Jacek, You're right. The current tests all pass on my Windows box with the exception of WeekDayName(1, true, 0) = Mon which is the system default. Not sure how to cleanly skip this one. I'll send in a patch for the implementation. Thanks, Huw.
Re: ole32: Add CoGetDefaultContext stub
On Wed, Sep 19, 2012 at 09:09:55AM +0200, Michael Stefaniuc wrote: +/*** + * CoGetContextToken [OLE32.@] copy and paste error ^^^ + */ +HRESULT CoGetDefaultContext(APTTYPE type, REFIID riid, LPVOID *ppv) ...and a missing WINAPI. Huw.
Re: localspl: Only store the file part of the filenames in the registry
On 16/05/2012 17:22, Detlef Riekenberg wrote: We have only filenames in the registry. The current code works fine with the adobe postscript driver and various other driver for PDF printers. No, the current code copies the entire contents of the DRIVER_INFO struct's filename entries - this isn't what Windows does. For example it clearly doesn't make sense the copy the entire path if APD_COPY_FROM_DIRECTORY is specified... I've written tests to confirm this, but they can't be added to the test suite, since we'd need a real driver file for this to succeed. Huw.
Re: [1/2] gdi32: GetGlyphOutline should fail for a bitmap font.
On Sat, Apr 28, 2012 at 05:52:13PM +0900, Dmitry Timoshkov wrote: --- dlls/gdi32/freetype.c |6 ++ dlls/gdi32/tests/font.c | 17 - 2 files changed, 18 insertions(+), 5 deletions(-) This will break the display of bitmap fonts in winex11.drv . Do you have an app that depends on this? Huw.
Re: [2/2] gdi32: Enumerated font size should not be too large.
On 23/04/2012 15:32, Dmitry Timoshkov wrote: This reverts commit f4625d1ae1109ee9a30faa8254b10779853f0ac2. --- dlls/gdi32/freetype.c |2 +- dlls/gdi32/tests/font.c |1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/dlls/gdi32/freetype.c b/dlls/gdi32/freetype.c index 3875e4f..401cfd1 100644 --- a/dlls/gdi32/freetype.c +++ b/dlls/gdi32/freetype.c @@ -5048,7 +5048,7 @@ static void GetEnumStructs(Face *face, LPENUMLOGFONTEXW pelf, font = alloc_font(); if(face-scalable) { -height = -2048; /* 2048 is the most common em size */ +height = 100; width = 0; } else { height = face-size.y_ppem 6; You'll need to fix the ntmTm.ntmCellHeight and ntmTm.ntmAvgWidth values. Huw.
Re: [2/2] gdi32: Enumerated font size should not be too large.
On 23/04/2012 16:07, Dmitry Timoshkov wrote: Huw Daviesh...@codeweavers.com wrote: You'll need to fix the ntmTm.ntmCellHeight and ntmTm.ntmAvgWidth values. As far as I can see that code always have been that way, and reverted patch didn't touch it either. But I see your point, and will add the tests (and a fix if the tests prove it's correct) in a later patch. Perhaps the original commit was added to work around this issue? Note, you'll probably have to retrieve the metrics at the design size (2048) in order to get accurate values for these members. Simply up-scaling the values obtained at 100 ppem will lead to rounding errors. As you say, tests are needed. Huw.
Re: gdi32: Add support for emulating bold faces of bitmap fonts.
On Mon, Apr 16, 2012 at 04:37:39PM +0900, Dmitry Timoshkov wrote: diff --git a/dlls/gdi32/freetype.c b/dlls/gdi32/freetype.c index 0a5d3c3..eb3f9b5 100644 --- a/dlls/gdi32/freetype.c +++ b/dlls/gdi32/freetype.c @@ -5669,12 +5669,27 @@ static inline BYTE get_max_level( UINT format ) return 255; } -static const BYTE masks[8] = {0x80, 0x40, 0x20, 0x10, 0x08, 0x04, 0x02, 0x01}; +static void emulate_bold_glyph(BYTE *buf, int pitch, int height) +{ +int x, y; +BYTE *p = buf; + +for (y = 0; y height; y++) +{ +for (x = pitch - 1; x = 0; x--) +{ +p[x] |= p[x] 1; +if (x 0) p[x] |= p[x - 1] 7; +} +p += pitch; +} +} Don't we need to add some bytes if glyph_width % 32 == 0 ? Huw.
Re: gdi32: Add support for emulating bold faces of bitmap fonts.
On Mon, Apr 16, 2012 at 05:51:20PM +0900, Dmitry Timoshkov wrote: Huw Davies h...@codeweavers.com wrote: Don't we need to add some bytes if glyph_width % 32 == 0 ? Well, I didn't see in my tests a need for that. That may be because your tests are just looking at the font metrics and not at the glyphs themselves...
Re: (try 2)quartz: add control.idl as a typelib resource
On Thu, Apr 05, 2012 at 07:51:13AM -0500, Aric Stewart wrote: revised based on comments from Jacek Caban --- dlls/quartz/Makefile.in|1 + dlls/quartz/control_v2.idl | 17 + 2 files changed, 18 insertions(+), 0 deletions(-) create mode 100644 dlls/quartz/control_v2.idl Calling this control_v2.idl seems a little odd. control_tlb.idl may be better. Huw.
Re: [PATCH 3/4] winspool/tests: Add some tests for OpenPrinter with non-NULL defaults.
On Tue, Apr 03, 2012 at 12:50:18PM +0200, Marvin wrote: === WINEBUILD (build) === Can't determine base name A winspool vs winspool.drv thing perhaps? Huw.
Re: gdi32: Add support for loading scalable font resources.
On Fri, Mar 30, 2012 at 07:31:54PM +0900, Dmitry Timoshkov wrote: Huw Davies h...@codeweavers.com wrote: +if (size.u.LowPart sizeof( *dos )) goto fail; Testing LowPart only could not work for large sizes. Of course, but we don't care if very large files fail, they won't be genuine resource files anyway. Huw.
Re: 80135: [PATCH 05/10] gdi32: Implement SetPixel.
On Wed, Oct 19, 2011 at 06:46:49AM -0700, Dan Kegel wrote: Patches 5-7 failed on WINEDEBUG=warn+heap here: ../../../tools/runtest -q -P wine -M gdi32.dll -T ../../.. -p gdi32_test.exe.so palette.c touch palette.ok ... palette.c:104: Test failed: getColor=0028 Might be flaky; patch 8 passed. patches 9 and 10 aren't done testing yet. It's because I wanted to move SetPixel and GetPixel in separate patches. I added a temporary todo_wine in patch 5 (SetPixel) to prevent this test failure, which then got removed in patch 8 (GetPixel). Huw.
Re: 80135: [PATCH 05/10] gdi32: Implement SetPixel.
On Wed, Oct 19, 2011 at 07:04:02AM -0700, Dan Kegel wrote: Sounds like your temporary todo_wine didn't quite do the trick...? Ah, I see. I was getting a failure on 111 and not 104, presumably due to different XServers. Anyway they all pass again after the entire series is applied. Huw.
Re: patch for dlls/gdi32/dib.c: fixes crash
On Thu, Jul 28, 2011 at 04:14:56PM +0200, Wolfgang Walter wrote: I think that bitmapinfo_from_user_bitmapinfo() is the real culprit. I think colors gets to big ( 256) and therefore the size for the memcpy. Hopefully http://source.winehq.org/patches/data/77076 should fix this. Huw.
Re: Statur of DIB Engine
On Sun, Jul 24, 2011 at 07:10:46PM +0300, Octavian Voicu wrote: Disclaimer: these comments are based only on what I gather from following commits and looking at the code, so can't guarantee it's 100% accurate; Huw or Alexandre would know better. This is a good summary of where we're at - nicely done Octavian. The next step is cross-device blitting, which is what Alexandre and I are working on at the moment. Huw.
Re: ole32/ole2: Don't call IDropTarget::QueryInterface() in RegisterDragDrop(). [whitespace fixed]
On Wed, May 25, 2011 at 12:45:15PM -0500, Adam Martinson wrote: On 05/25/2011 12:36 PM, Adam Martinson wrote: The AddRef is done in WrapDropTarget(), seems like the appropriate place for it. Quite right on the rest though, thanks. Oops no, the window prop should be pDropTarget, not the wrapper. The wrapper is only used for the marshalling. That window prop could probably go away, I only put it there because that's what Windows does, I doubt there's an app that relies on it though. However if it stays you do need the AddRef! Huw.
Re: ole32/ole2: Don't call IDropTarget::QueryInterface() in RegisterDragDrop(). [whitespace fixed]
On Thu, May 26, 2011 at 06:51:13AM -0500, Adam Martinson wrote: On 05/26/2011 06:47 AM, Huw Davies wrote: On Wed, May 25, 2011 at 12:45:15PM -0500, Adam Martinson wrote: On 05/25/2011 12:36 PM, Adam Martinson wrote: The AddRef is done in WrapDropTarget(), seems like the appropriate place for it. Quite right on the rest though, thanks. Oops no, the window prop should be pDropTarget, not the wrapper. The wrapper is only used for the marshalling. That window prop could probably go away, I only put it there because that's what Windows does, I doubt there's an app that relies on it though. However if it stays you do need the AddRef! Huw. Best to mirror Windows as close as possible. The AddRef() is done in WrapDropTarget(). On failure IDropTarget::Release() is done on the wrapper, which removes the ref on the original. You're storing a 2nd instance of the ptr (the first being in the wrapper), conventionally you take a 2nd ref. When you remove the window prop you release and when you destroy the wrapper you release again. Huw.
Re: ole32/ole2: Don't call IDropTarget::QueryInterface() in RegisterDragDrop(). [whitespace fixed]
+static IDropTarget* WrapDropTarget(IDropTarget* inner) +{ +DropTargetWrapper* This = HeapAlloc(GetProcessHeap(), 0, sizeof(*This)); + +if (This) +{ +This-iface = (IDropTarget){ DropTargetWrapper_VTbl }; This-iface.lpVtbl = DropTargetWrapper_VTbl; - hr = CoMarshalInterface(stream, IID_IDropTarget, unk, MSHCTX_LOCAL, NULL, MSHLFLAGS_TABLESTRONG); - IUnknown_Release(unk); + hr = CoMarshalInterface(stream, IID_IDropTarget, (IUnknown*)wrapper, MSHCTX_LOCAL, NULL, MSHLFLAGS_TABLESTRONG); if(SUCCEEDED(hr)) { hr = create_map_from_stream(stream, map); if(SUCCEEDED(hr)) { - IDropTarget_AddRef(pDropTarget); SetPropW(hwnd, prop_oledroptarget, pDropTarget); You probably want to set the window prop to the wrapper here. Either way you still need the AddRef (on the wrapper if you set that), it gets released in RevokeDragDrop. Huw.
Re: gdi32/dibdrv: The dib debug channel is unused so remove it.
On Sun, Apr 10, 2011 at 06:45:51PM +0200, Francois Gouget wrote: It might get used soon but then it's easy to add back anyway. Very soon in fact - it gets used in the PatBlt patch I just sent ;-)
Re: gdi32/tests: Start of a framework for writing dib driver tests.
On Tue, Apr 05, 2011 at 03:31:23PM +0100, Huw Davies wrote: Try2. Using the A_SHA functions to generate the hashes. Although somewhat neater than the first version it doesn't work on nt4/win2k as they don't export these functions. So we're better off using the original. Huw.
Re: [PATCH 2/5] shdocvw: Added IShellBrowser interface stub
On Mon, Mar 14, 2011 at 05:03:37PM +0100, Piotr Caban wrote: +static HRESULT WINAPI ShellBrowser_QueryInterface( +IShellBrowser* iface, +REFIID riid, +void **ppvObject) +{ +ShellBrowser *This = impl_from_IShellBrowser(iface); +*ppvObject = NULL; + +if(IsEqualGUID(IID_IShellBrowser, riid)) +*ppvObject = This-IShellBrowser_iface; + +if(*ppvObject) { +TRACE(%p %s %p\n, This, debugstr_guid(riid), ppvObject); +IUnknown_AddRef((IUnknown*)*ppvObject); +return S_OK; +} + You probably want to add support for IID_IUnknown here too. Huw.
Re: oleaut32: Implement ITypeInfo_GetNames Stub/Proxy (resend)
On Tue, Feb 22, 2011 at 09:02:38PM +1100, Alistair Leslie-Hughes wrote: From 6aa7bfa0c06b746ee64daf56f2b02e6f38542d54 Mon Sep 17 00:00:00 2001 From: Alistair Leslie-Hughes leslie_alist...@hotmail.com Date: Fri, 18 Feb 2011 12:04:56 +1100 Subject: [PATCH] Implement ITypeInfo_GetNames Stub/Proxy To: wine-patches wine-patc...@winehq.org --- dlls/oleaut32/usrmarshal.c | 10 ++ 1 files changed, 6 insertions(+), 4 deletions(-) This looks good to me. I'm not sure why this function is marked as [call_as], I expect they originally had planned to allow for a NULL pcNames if cMaxNames == 1 (rather like IEnum*_Next). However a quick test shows that this doesn't appear to be the case. Huw.
Re: widl: Added support for registering alternative progids.
On Fri, Dec 17, 2010 at 04:14:24PM +0100, Jacek Caban wrote: diff --git a/tools/widl/parser.y b/tools/widl/parser.y index c924a66..28cb1da 100644 --- a/tools/widl/parser.y +++ b/tools/widl/parser.y @@ -190,7 +190,7 @@ static statement_list_t *append_statement(statement_list_t *list, statement_t *s -%token tAGGREGATABLE tALLOCATE tANNOTATION tAPPOBJECT tASYNC tASYNCUUID +%token tAGGREGATABLE tALLOCATE tANNOTATION tAPPOBJECT tASYNC tASYNCUUID tALTPROGID Hi Jacek, This breaks alphabetical order in this line. Cheers, Huw.
Re: [PATCH] ole32: check for interface NULL which happens with e.g. Abiword
On Wed, Sep 15, 2010 at 08:04:51PM +0900, Dmitry Timoshkov wrote: Marcus Meissner mar...@jet.franken.de wrote: + if (!unk) { + FIXME(hr was %d, but unk is NULL?\n, hr); + return E_FAIL; + } IDropTarget_QueryInterface() should be fixed instead. The implementation is in abiword, so that would be tricky. Marcus, you're leaking a stream in this error path. Better to do something like: if(FAILED(hr) || !unk) Huw.
Re: [PATCH] ole32: check for interface NULL which happens with e.g. Abiword
On Wed, Sep 15, 2010 at 01:34:06PM +0200, Marcus Meissner wrote: On Wed, Sep 15, 2010 at 12:22:57PM +0100, Huw Davies wrote: On Wed, Sep 15, 2010 at 08:04:51PM +0900, Dmitry Timoshkov wrote: Marcus Meissner mar...@jet.franken.de wrote: + if (!unk) { + FIXME(hr was %d, but unk is NULL?\n, hr); + return E_FAIL; + } IDropTarget_QueryInterface() should be fixed instead. The implementation is in abiword, so that would be tricky. Marcus, you're leaking a stream in this error path. Better to do something like: if(FAILED(hr) || !unk) yeah, but what error to return :/ Yeah, sorry. Something like: if(!unk) hr = E_NOINTERFACE; right after the QI call. Huw.
Re: Use SUBLANG_NEUTRAL for the Gaelic resources?
On Tue, Aug 24, 2010 at 09:59:51AM +0200, Francois Gouget wrote: Are SUBLANG_GAELIC, SUBLANG_GAELIC_SCOTTISH and SUBLANG_GAELIC_MANX very different languages? If not it would be better to use SUBLANG_NEUTRAL instead of SUBLANG_GAELIC. For instance: I think they're different enough to warrant falling back to English rather than Irish (which is essentially what you're asking). See: http://en.wikipedia.org/wiki/Manx_language#Background Huw.
Re: ole32: Implement cross-process drag and drop.
On Mon, Jul 19, 2010 at 04:42:40PM +0100, Huw Davies wrote: Fix a typo in a comment spotted by Detlef. --- dlls/ole32/ole2.c | 199 1 files changed, 168 insertions(+), 31 deletions(-) Sorry, this is still broken, please ignore. Huw.
Re: ole32: Implement cross-process drag and drop.
On Mon, Jul 19, 2010 at 01:58:07PM +0100, Huw Davies wrote: --- dlls/ole32/ole2.c | 199 1 files changed, 168 insertions(+), 31 deletions(-) Please use the updated version which has fixed a typo in a comment (spotted by Detlef).
Re: sane.ds: Change ns to ms in resource files
On Wed, Jul 07, 2010 at 11:28:21AM +0100, Ken Sharp wrote: On 7/7/2010 10:30 AM, Alexandre Julliard wrote: Ken Sharpkennyb...@o2.co.uk writes: Apparently the English resource file should show ms (microseconds) instead of ns. This error has been copied too all the .rc files. ms doesn't mean microseconds. What does it mean? I couldn't find a definition. http://en.wikipedia.org/wiki/Millisecond For microsecond you want 'µs' or failing that, 'us'.
Re: sane.ds: Change ns to ms in resource files
On Wed, Jul 07, 2010 at 11:45:59AM +0100, Ken Sharp wrote: On 7/7/2010 11:39 AM, Huw Davies wrote: On Wed, Jul 07, 2010 at 11:28:21AM +0100, Ken Sharp wrote: On 7/7/2010 10:30 AM, Alexandre Julliard wrote: Ken Sharpkennyb...@o2.co.uk writes: Apparently the English resource file should show ms (microseconds) instead of ns. This error has been copied too all the .rc files. ms doesn't mean microseconds. What does it mean? I couldn't find a definition. http://en.wikipedia.org/wiki/Millisecond For microsecond you want 'µs' or failing that, 'us'. Well, yes, but that's what I was told. If it should be ms then the patch is ok. I don't understand what seconds are doing in a scanning DLL anyway, I don't ever recall it being used. Huh? I told you yesterday that the string should be micro seconds, that's 'µs' and not 'ms'.
Re: sane.ds: Add Welsh resource
On Mon, Jul 05, 2010 at 06:51:36PM +0100, Ken Sharp wrote: +{ + 0 + 1 px + 2 b /* What is b ? */ + 3 mm + 4 dpi /* dotiau fesul modfedd */ + 5 % + 6 ns /* What is ns ? */ +} See the SANE_UNIT_ defines in /usr/include/sane/sane.h 'b' is bits and 'ns' should be microseconds, so it looks like the English resource is incorrect too. Huw. PS Happy to see the Welsh translations coming in!
Re: IDL trouble (patch: adding IEnumShellItems interface declaration.)
On 11 Jun 2010, at 02:42, David Hedberg david.hedb...@gmail.com wrote: Hello everyone, I'm having some trouble with this simple patch. Specifically, it breaks compiling of dlls/actxprxy, producing the message: * /home/david/Projects/wine/git/dlls/actxprxy/actxprxy_shobjidl_p.c: 6507: undefined reference to `IEnumShellItems_Next_Stub' actxprxy_shobjidl_p.o:(.data.rel.ro+0xf0): undefined reference to `IEnumShellItems_Next_Proxy' * It compiles if I remove the [call_as] attribute, but I guess that would kind of break the interface. Any obvious mistakes in the patch, or grammar not supported by widl? You need to add an implementation of IEnumShellItems_Next_Proxy/Stub to usrmarshal.c Huw.
Re: [PATCH 2/5] include: Add IEnumShellItems interface declaration.
On 11 Jun 2010, at 12:17, David Hedberg david.hedb...@gmail.com wrote: --- dlls/actxprxy/usrmarshal.c | 21 + include/shobjidl.idl | 30 ++ 2 files changed, 51 insertions(+), 0 deletions(-) diff --git a/dlls/actxprxy/usrmarshal.c b/dlls/actxprxy/usrmarshal.c index f2aacf8..85e534c 100644 --- a/dlls/actxprxy/usrmarshal.c +++ b/dlls/actxprxy/usrmarshal.c + +HRESULT CALLBACK IEnumShellItems_Next_Proxy( +IEnumShellItems *This, +ULONG celt, +IShellItem **rgelt, +ULONG *pceltFetched) +{ +TRACE((%p)\n, This); +return IEnumShellItems_RemoteNext_Proxy( This, celt, rgelt, pceltFetched ); +} pceltFetched could be a NULL ptr here, so you should pass the address of a local variable in that case. See ole32/usrmarshal.c for examples on how to marshal IEnum*_Next Huw.
Re: rpcrt4: Add tests for multi-dimensional conformant arrays.
On Fri, Feb 12, 2010 at 11:53:30AM +0100, Paul Vriens wrote: On 02/11/2010 05:18 PM, Huw Davies wrote: +ptr = NdrSimpleStructMarshall(StubMsg, (unsigned char *)memsrc, +fmtstr_complex_array[32] ); Hi Huw, This particular test crashes on Win9x, WinMe and NT4. They all show: ndr_marshall.c:1973: Test failed: length 96 just before the crash so maybe that would be our means to bail out? Hi Paul, Yes, looks like it's broken on those platforms; the NdrSimpleStructMarshall call is crashing. Are you happy to send in a patch? Huw.
Re: rpcrt4: Add tests for multi-dimensional conformant arrays.
On Fri, Feb 12, 2010 at 11:27:38AM +, Huw Davies wrote: On Fri, Feb 12, 2010 at 11:53:30AM +0100, Paul Vriens wrote: On 02/11/2010 05:18 PM, Huw Davies wrote: +ptr = NdrSimpleStructMarshall(StubMsg, (unsigned char *)memsrc, +fmtstr_complex_array[32] ); Hi Huw, This particular test crashes on Win9x, WinMe and NT4. They all show: ndr_marshall.c:1973: Test failed: length 96 just before the crash so maybe that would be our means to bail out? Hi Paul, Yes, looks like it's broken on those platforms; the NdrSimpleStructMarshall call is crashing. Are you happy to send in a patch? Btw, I have a fix for the 64 bit failures. It'll conflict with your patch, so I'll send it after yours is committed. Huw.
Re: shell32: Dynamically load an ordinal only export.
On Fri, Jan 22, 2010 at 02:01:03PM +0100, Alexandre Julliard wrote: Huw Davies h...@codeweavers.com writes: --- dlls/shell32/tests/shlfileop.c | 18 +- 1 files changed, 9 insertions(+), 9 deletions(-) This shouldn't be needed, import by ordinal works just fine. I get a failure with make crosstest: ../../../tools/winegcc/winegcc -b i586-mingw32msvc -B../../../tools/winebuild --sysroot=../../.. appbar.cross.o autocomplete.cross.o generated.cross.o progman_dde.cross.o shelllink.cross.o shellpath.cross.o shfldr_special.cross.o shlexec.cross.o shlfileop.cross.o shlfolder.cross.o string.cross.o systray.cross.o rsrc.res testlist.cross.o -o shell32_crosstest.exe -lshell32 -lole32 -loleaut32 -luser32 -ladvapi32 -lkernel32 shlfileop.cross.o: In function `test_shlmenu': /home/daviesh/wine/dlls/shell32/tests/shlfileop.c:2232: undefined reference to `_shell_mergeme...@24' /home/daviesh/wine/dlls/shell32/tests/shlfileop.c:2234: undefined reference to `_shell_mergeme...@24' Huw.
Re: shell32: Dynamically load an ordinal only export.
On Fri, Jan 22, 2010 at 02:51:04PM +0100, Alexandre Julliard wrote: Huw Davies h...@codeweavers.com writes: On Fri, Jan 22, 2010 at 02:01:03PM +0100, Alexandre Julliard wrote: Huw Davies h...@codeweavers.com writes: --- dlls/shell32/tests/shlfileop.c | 18 +- 1 files changed, 9 insertions(+), 9 deletions(-) This shouldn't be needed, import by ordinal works just fine. I get a failure with make crosstest: You have to run make crosstest from the top level for the import libraries to be built. Ah! Thanks! Huw.
Re: Patch feedback requested for OleCreatePropertyFrame()
On Sun, Jan 03, 2010 at 03:00:12PM -0800, Geoffrey Hausheer wrote: I found a patch from 2001 written by TAKESHIMA Hidenori that was posted to wine-patches (http://www.winehq.org/pipermail/wine-patches/2001-October/001229.html) but never committed. Actually it was commited (2843934af5515c7f2b8370324aa98d3964a40324) but was then removed (140e7222e0d7ce76068cddc64c68105c2e569257) at his request. Huw.
Re: gdiplus: Fix a memory leak in the tests.
On Tue, Dec 15, 2009 at 02:40:26PM +0300, Nikolay Sivov wrote: On 12/15/2009 14:35, Huw Davies wrote: Found by Valgrind. --- dlls/gdiplus/tests/customlinecap.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) Hi, Huw. This leak will be fixed with corrected GdipCreateCustomLineCap(). Return code should never be Ok here. P.S. Same with previous patch for GdipCreateFontFamilyFromNam(). Yes I know. And when it's fixed the GdipDelete* calls could be removed along with the todo_wine. At the moment it's easier to get them out of the Valgrind logs so that the real leaks are more obvious. Huw.
Re: ole32: Fix invalid memory access in storage32
Nathan Gallaher wrote: In StorageImpl_construct(): For pwcsName strings shorter than DIRENTRY_NAME_BUFFER_LEN-1, an invalid read would be noted by valgrind as the memcpy wanders off the end of the string. Do the needful to calculate the required string length. By the way, in storage32.h the filename member of struct StorageBaseImpl is declared as WCHAR filename[DIRENTRY_NAME_BUFFER_LEN]; shouldn't it be WCHAR filename[DIRENTRY_NAME_MAX_LEN]; ? Huw.
Re: [PATCH 1/5] msdaps: Use 2 byte structure packing.
On Mon, Aug 24, 2009 at 01:51:24PM +0200, Alexandre Julliard wrote: Huw Davies h...@codeweavers.com writes: diff --git a/dlls/msdaps/Makefile.in b/dlls/msdaps/Makefile.in index d6f71a0..cebf659 100644 --- a/dlls/msdaps/Makefile.in +++ b/dlls/msdaps/Makefile.in @@ -5,6 +5,7 @@ VPATH = @srcdir@ MODULE= msdaps.dll IMPORTS = oleaut32 rpcrt4 kernel32 ntdll EXTRADEFS = -DREGISTER_PROXY_DLL -DPROXY_CLSID_IS={ 0x06210e88, 0x01f5, 0x11d1, { 0xb5, 0x12, 0x00, 0x80, 0xc7, 0x81, 0xc3, 0x84 } } +EXTRAIDLFLAGS = -a2 Won't this break the 64-bit build? Yes, good point. 64-bit should be left to default to 8 byte alignment. How do I do that? Huw.
Re: msctf: fully clear the Variant before setting it.
On Wed, Jul 01, 2009 at 10:29:22AM -0500, Aric Stewart wrote: diff --git a/dlls/msctf/compartmentmgr.c b/dlls/msctf/compartmentmgr.c index 3afcfda..e1c7187 100644 --- a/dlls/msctf/compartmentmgr.c +++ b/dlls/msctf/compartmentmgr.c @@ -537,6 +537,7 @@ static HRESULT WINAPI Compartment_GetValue(ITfCompartment *iface, if (!pvarValue) return E_INVALIDARG; +memset(pvarValue,0,sizeof(VARIANT)); pvarValue-n1.n2.vt = VT_EMPTY; if (!This-variant.n1.n2.vt == VT_EMPTY) hr = VariantCopy(pvarValue,This-variant); This looks wrong; there's something strange going on here. Note also there's VariantInit() which is a cleaner way to set vt to VT_EMPTY. Huw.
Re: [PATCH 4/4] gdi32: Add tests for the GetWinMetaFileBits MFCOMMENT record.
On Tue, Jun 30, 2009 at 12:40:54PM +0200, Paul Vriens wrote: Huw Davies wrote: --- dlls/gdi32/tests/metafile.c | 80 +++ 1 files changed, 80 insertions(+), 0 deletions(-) Hi Huw, Could you use CreateEnhMetaFileA here: +SetRect(rc, 1000,2000, 3000, 6000); +emf_dc = CreateEnhMetaFileW(display_dc, NULL, rc, NULL); The W-version is not implemented on Win9x so this test would crash. Hi Paul, Yup, resent. Thanks! Huw.
Re: oleaut32/tests: Add some tests for [lcid] parameters.
On Wed, Jun 17, 2009 at 03:58:33PM +0200, Paul Vriens wrote: Huw Davies wrote: --- dlls/oleaut32/tests/tmarshal.c | 53 +++- dlls/oleaut32/tests/tmarshal.idl |6 +++ dlls/oleaut32/tests/tmarshal_dispids.h |1 + 3 files changed, 59 insertions(+), 1 deletions(-) In an effort to get a green NT4, I ran your new tests on NT4: tmarshal.c:1376: Test failed: ITypeInfo_Invoke failed with error 0x80010105 tmarshal.c:1377: Test failed: got 0 tmarshal.c:1378: Test failed: got 0 On W2K3: tmarshal.c:1376: Test failed: ITypeInfo_Invoke failed with error 0x80010105 tmarshal.c:1377: Test failed: got 0 tmarshal.c:1378: Test failed: got 0 and a maybe unrelated on W2K3 (wasn't here before): tmarshal.c:605: Test failed: Struct parameter passed by pointer corrupted On W98: tmarshal.c:1376: Test failed: ITypeInfo_Invoke failed with error 0x80010105 tmarshal.c:1377: Test failed: got 0 tmarshal.c:1378: Test failed: got 0 Am I doing something wrong here? Hi Paul, Did you apply the widl patch too? Huw.
Re: ntdll/tests: Mark return value of RtlUnicodeStringToInteger(, 16) as broken for nt4.
On Tue, May 19, 2009 at 03:39:07PM +0200, Alexandre Julliard wrote: Paul Vriens paul.vriens.w...@gmail.com writes: I don't think this is correct. This test has always succeeded until March 9th. There haven't been any changes to the test in that period, but I remember an upgrade of winehq in that weekend. In other words, there is more to it. Testing a simple cross compiled version of this test doesn't yield an error on NT4. My feeling is that it's returning uninitialized data, so the results depend on the stack layout that the compiler is using. There's probably no sense in checking for a specific value, just consider this case broken no matter what it returns. Ok, I'll resend without the test on the returned value. Huw.
Re: [2/2] winex11.drv: export copied images as image/bmp
On Fri, May 01, 2009 at 01:25:33PM +0200, Alexandre Julliard wrote: Vincent Povirk vinc...@codeweavers.com writes: +if (dibinfo-biBitCount 16) +numcolors = dibinfo-biClrUsed || 1dibinfo-biBitCount; You have been doing too much Perl ;-) Hi Vincent, You may find bitmap_info_size() useful here. Huw.
Re: [PATCH 1/2] ole32: Register the clipboard formats at load time.
On Thu, Apr 23, 2009 at 03:40:11PM +0200, Alexandre Julliard wrote: Huw Davies h...@codeweavers.com writes: --- dlls/ole32/clipboard.c | 54 + dlls/ole32/compobj.c |2 + dlls/ole32/compobj_private.h | 14 +++ 3 files changed, 54 insertions(+), 16 deletions(-) It would be nicer to do this when needed, so that we don't have to initialize the graphics driver just because ole32 was imported. Good point, actually having them registered in OleInitialise is fine. Resent. Thanks, Huw.
Re: [PATCH 5/5] user32/tests: Fix tests on win9x.
On Tue, Apr 21, 2009 at 11:34:04AM +0200, Paul Vriens wrote: Huw Davies wrote: On Tue, Apr 21, 2009 at 11:22:29AM +0200, Paul Vriens wrote: Huw Davies wrote: --- dlls/user32/tests/clipboard.c |7 +-- 1 files changed, 5 insertions(+), 2 deletions(-) Was just about to sent a patch along those lines. Isn't it better to mark that win9x (and winME as well btw) one as broken()? I was debating with myself whether to mark it as broken or not. My feeling was that it's not actually broken behaviour rather missing functionality. But I don't really care. Huw. Maybe it's not broken() behavior but marking it as broken() makes sure we (Wine) will have CF_UNICODETEXT. Without that broken(), CF_UNICODETEXT can go missing without us knowing. Yeah, fair enough. I guess the point is it's broken behaviour for Wine. I'll resend, thanks. Huw.
Re: [PATCH 5/5] user32/tests: Fix tests on win9x.
On Tue, Apr 21, 2009 at 11:22:29AM +0200, Paul Vriens wrote: Huw Davies wrote: --- dlls/user32/tests/clipboard.c |7 +-- 1 files changed, 5 insertions(+), 2 deletions(-) Was just about to sent a patch along those lines. Isn't it better to mark that win9x (and winME as well btw) one as broken()? I was debating with myself whether to mark it as broken or not. My feeling was that it's not actually broken behaviour rather missing functionality. But I don't really care. Huw.
Re: Problem with compiling ole32 crosstest
On Fri, Feb 13, 2009 at 02:01:35PM +0300, Vitaly Perov wrote: Hi! Does anybody have a problem compilling the crosstest of ole32? Or maybe it's only my fault? I have: usrmarshal.cross.o: In function `func_usrmarshal': /srv/vitperov/Projects/wine-dev/dlls/ole32/tests/usrmarshal.c:445: undefined reference to `_wdtpinterfacepointer_users...@20' /srv/vitperov/Projects/wine-dev/dlls/ole32/tests/usrmarshal.c:448: undefined reference to `_wdtpinterfacepointer_usermars...@20' ... Does this help? cd dlls/ole32 make libole32.a -- h...@codeweavers.com
Re: gdi32/tests: Use the ANSI text metrics so the tests work on win9x.
On Thu, Feb 12, 2009 at 01:40:54PM +0100, Paul Vriens wrote: Huw Davies wrote: --- dlls/gdi32/tests/font.c | 20 ++-- 1 files changed, 10 insertions(+), 10 deletions(-) Looks a lot like: http://www.winehq.org/pipermail/wine-patches/2009-February/069297.html Maybe we should use yours as it has less changes to the code. Doh, I just saw that patch after I sent mine! Huw.
Re: Huw Davies : opengl/tests: Skip tests if we can't find a pixel format.
On Mon, Dec 08, 2008 at 01:44:41PM +0100, Alexandre Julliard wrote: Huw Davies [EMAIL PROTECTED] writes: Hi Alexandre, This wasn't what I intended my patch to do. The problem is that Wine's ChoosePixelFormat() rets 0 on XServers without glX. The win_skip results in a test failure. My patch was intended to mark this as a todo_wine. We could have a todo_wine around the win_skip, though I'm not quite convinced that having the opengl32 test succeed without opengl support is the right thing to do. Ok, I'll send in a patch with todo_wine and let you decide. My feeling was that without opengl support we should skip the opengl32 tests, hence the skip in the original patch. The todo_wine was to flag Wine's ChoosePixelFormat bug. Huw. -- [EMAIL PROTECTED]
Re: Huw Davies : opengl/tests: Skip tests if we can't find a pixel format.
On Fri, Dec 05, 2008 at 11:04:07AM -0600, Alexandre Julliard wrote: Module: wine Branch: master Commit: e86ff2a3128f4a0157ecfa10fee31d1416312c71 URL: http://source.winehq.org/git/wine.git/?a=commit;h=e86ff2a3128f4a0157ecfa10fee31d1416312c71 Author: Huw Davies [EMAIL PROTECTED] Date: Fri Dec 5 14:19:25 2008 + opengl/tests: Skip tests if we can't find a pixel format. --- dlls/opengl32/tests/opengl.c |9 - 1 files changed, 8 insertions(+), 1 deletions(-) diff --git a/dlls/opengl32/tests/opengl.c b/dlls/opengl32/tests/opengl.c index 35eb8ae..a3ac3af 100644 --- a/dlls/opengl32/tests/opengl.c +++ b/dlls/opengl32/tests/opengl.c @@ -555,7 +555,12 @@ START_TEST(opengl) hdc = GetDC(hwnd); iPixelFormat = ChoosePixelFormat(hdc, pfd); -ok(iPixelFormat 0, No pixelformat found!\n); /* This should never happen as ChoosePixelFormat always returns a closest match */ +if(iPixelFormat == 0) +{ +/* This should never happen as ChoosePixelFormat always returns a closest match, but currently this fails in Wine if we don't have glX */ +win_skip(Unable to find pixel format.\n); +goto cleanup; +} /* We shouldn't be able to create a context from a hdc which doesn't have a pixel format set */ hglrc = wglCreateContext(hdc); Hi Alexandre, This wasn't what I intended my patch to do. The problem is that Wine's ChoosePixelFormat() rets 0 on XServers without glX. The win_skip results in a test failure. My patch was intended to mark this as a todo_wine. Huw. -- [EMAIL PROTECTED]
Re: opengl/tests: Skip tests if we can't find a pixel format.
On Fri, Dec 05, 2008 at 03:25:56PM +0100, Paul Vriens wrote: This looks a bit strange: +if(iPixelFormat == 0) +{ +todo_wine ok(iPixelFormat 0, Is it your intention to just throw up a failed message in all cases? Hi Paul, It was my intention to throw a todo if ChoosePixelFormat rets 0 when run under Wine. Huw.
Re: [RESEND] Send page size to cups with lpr command on printing (bug 15942)
Massimo Del Fedele wrote: Steven Edwards ha scritto: On Wed, Nov 12, 2008 at 12:51 PM, Massimo Del Fedele [EMAIL PROTECTED] wrote: The best way would be, of course, replace ALL LPR code with direct cups calls. It's feasible, but it requires some refactoring of all printing code. For example, what's now exec(|lpr -P myPrinter) which I replaced with exec(|lpr -P myPrinter -o myPaperSize) should become CupsCreateJob() CupsStartDocument() CupsWriteRequestData() CupsFinishDocument() That would require some amount of work and, over all, I'm not sure it could be accepted to go into wine code. You should check if cups is present at compile time (which I guess we already do) and then dlopen the cups libraries if present (again I assume we already do this) and yes use the cups api if possible rather than directly invoking lpr. So, replace ALL existing execs to lpr with direct cups calls ? Mixing them for a single print job is not possible, AFAIK. The correct way is to fix winspool and friends to use schedule_cups() which in turn uses cupsPrintFile(). Detlef could probably comment on how far away we are from using this. Huw.
Re: inetcomm: Add an implementation of IVirtualStream.
On Fri, Nov 07, 2008 at 02:20:04PM +0100, Hans Leidekker wrote: On Friday 07 November 2008 13:48:29 Huw Davies wrote: This interface is undocumented but fortunately its name was enough of a hint to create a sensible implementation. Used by Outlook when retrieving or sending a mail. We already have MimeOleCreateVirtualStream() which is implemented on top of CreateStreamOnHGlobal()... Hmm, that may work too. I wonder if they are fully compatible though, the Read method is called with cb set to -1, which appears to mean that the stream size should be taken. Do you know off hand if that's supported by IStream? Well cb is ULONG, so it's not -1 but a big number. This should work fine. Huw. -- Huw Davies [EMAIL PROTECTED]
Re: inetcomm: Add an implementation of IVirtualStream.
Hans Leidekker wrote: This interface is undocumented but fortunately its name was enough of a hint to create a sensible implementation. Used by Outlook when retrieving or sending a mail. We already have MimeOleCreateVirtualStream() which is implemented on top of CreateStreamOnHGlobal()... Huw.
Re: gdi32: Add EMR_GDICOMMENT to the list of records producing output.
On Thu, Oct 23, 2008 at 09:59:44PM +0900, Dmitry Timoshkov wrote: Also, do you have an idea why some EMF records are not in that list (EMR_FILLPATH, EMR_STROKEANDFILLPATH, EMR_STROKEPATH and probably some others)? No idea, they probably just got left out by mistake. Clearly we need more exhaustive tests here. Huw. -- Huw Davies [EMAIL PROTECTED]
Re: ole32: implement IEnumFORMATETC_Next_Proxy (1/2) try 2
On Fri, Oct 24, 2008 at 04:45:46AM -0500, Austin English wrote: diff --git a/dlls/ole32/usrmarshal.c b/dlls/ole32/usrmarshal.c index b8487c3..dba69ca 100644 --- a/dlls/ole32/usrmarshal.c +++ b/dlls/ole32/usrmarshal.c @@ -2398,8 +2398,11 @@ HRESULT CALLBACK IEnumFORMATETC_Next_Proxy( FORMATETC *rgelt, ULONG *pceltFetched) { -FIXME(:stub\n); -return E_NOTIMPL; +ULONG fetched; +if (!pceltFetched) +pceltFetched = fetched; +return IEnumFORMATETC_RemoteNext_Proxy(This, +celt, rgelt, pceltFetched); That's still not right. It gives the impression that the 'return' is part of the 'if' block. The 'return' line should be indented by four spaces not eight. Huw. -- Huw Davies [EMAIL PROTECTED]
Re: ole32: implement IEnumFORMATETC_Next_Proxy (1/2)
On Thu, Oct 23, 2008 at 01:35:55PM -0500, Austin English wrote: diff --git a/dlls/ole32/usrmarshal.c b/dlls/ole32/usrmarshal.c index b8487c3..be9ebff 100644 --- a/dlls/ole32/usrmarshal.c +++ b/dlls/ole32/usrmarshal.c @@ -2398,8 +2398,13 @@ HRESULT CALLBACK IEnumFORMATETC_Next_Proxy( FORMATETC *rgelt, ULONG *pceltFetched) { -FIXME(:stub\n); -return E_NOTIMPL; + ULONG fetched; + if (!pceltFetched) +pceltFetched = fetched; +return IEnumFORMATETC_RemoteNext_Proxy(This, + celt, + rgelt, + pceltFetched); There's some strange indentation going on here. Could you make it all 4-space based? Thanks, Huw. -- Huw Davies [EMAIL PROTECTED]
Re: ole32: quiet a ridiculously noisy fixme (resend)
On Wed, Oct 22, 2008 at 08:34:49PM -0500, Austin English wrote: Reduces Open Office 3 application's output from infinity to 7 lines. Last patch had an unnecessary whitespace change. -- -Austin From 0dead16d2588bce8330ed862f8e5b6d3c6bb71a7 Mon Sep 17 00:00:00 2001 From: Austin English [EMAIL PROTECTED] Date: Wed, 22 Oct 2008 20:28:37 -0500 Subject: [PATCH] ole32: quiet a ridiculously noisy fixme --- dlls/ole32/usrmarshal.c |6 +- 1 files changed, 5 insertions(+), 1 deletions(-) diff --git a/dlls/ole32/usrmarshal.c b/dlls/ole32/usrmarshal.c index b8487c3..6886dfb 100644 --- a/dlls/ole32/usrmarshal.c +++ b/dlls/ole32/usrmarshal.c @@ -2398,7 +2398,11 @@ HRESULT CALLBACK IEnumFORMATETC_Next_Proxy( FORMATETC *rgelt, ULONG *pceltFetched) { -FIXME(:stub\n); +static int quietfixme = 0; +if (quietfixme == 0) { +FIXME(:stub\n); +quietfixme = 1; +} return E_NOTIMPL; } The real implementation of this would actually result in fewer lines of code! See IEnumVARIANT_Next_Proxy / Stub (in oleaut32/usrmarshal.c) for an example. Huw. -- Huw Davies [EMAIL PROTECTED]
Re: gdi32: Add EMR_GDICOMMENT to the list of records producing output.
On Thu, Oct 23, 2008 at 09:32:43PM +0900, Dmitry Timoshkov wrote: This completely fixes the print previews generated by MS Access in a Win9x mode. Access has a custom handler for an EMR_GDICOMMENT record, and GDI APIs it calls from it don't work properly due to wrong DC mapping. --- dlls/gdi32/enhmetafile.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/dlls/gdi32/enhmetafile.c b/dlls/gdi32/enhmetafile.c index e998c60..22451d2 100644 --- a/dlls/gdi32/enhmetafile.c +++ b/dlls/gdi32/enhmetafile.c @@ -692,6 +692,7 @@ static BOOL emr_produces_output(int type) case EMR_LINETO: case EMR_ARCTO: case EMR_POLYDRAW: +case EMR_GDICOMMENT: case EMR_FILLRGN: case EMR_FRAMERGN: case EMR_INVERTRGN: Maybe it's time to rename this function to something like emr_updates_transform() since EMR_GDICOMMENT clearly doesn't produce any output. Huw. -- Huw Davies [EMAIL PROTECTED]
Re: [PATCH 3/3] ole32: Call the appropriate storage function when we run the object.
On Wed, Oct 22, 2008 at 12:46:39PM +0100, Rob Shearman wrote: 2008/10/21 Huw Davies [EMAIL PROTECTED]: @@ -1728,6 +1767,10 @@ static HRESULT WINAPI DefaultHandler_IPersistStorage_HandsOffStorage( if(SUCCEEDED(hr) object_is_running(This)) hr = IPersistStorage_HandsOffStorage(This-pPSDelegate); +old_stg = InterlockedExchangePointer((void**)This-storage, NULL); IIRC, the default handler is apartment-threaded only, meaning that a default handler object cannot be used on a thread other than the one it was created on. That's true I believe. Adding constructs like this introduce confusion to future developers about whether the code is or isn't thread-safe and so it should be avoided and a non-thread-safe construct used instead. I don't buy this. Besides we're already using Interlocked{In,De}crement(). Huw. -- Huw Davies [EMAIL PROTECTED]
Re: [PATCH 3/3] ole32: Call the appropriate storage function when we run the object.
On Wed, Oct 22, 2008 at 02:03:26PM +0100, Rob Shearman wrote: 2008/10/22 Huw Davies [EMAIL PROTECTED]: On Wed, Oct 22, 2008 at 12:46:39PM +0100, Rob Shearman wrote: 2008/10/21 Huw Davies [EMAIL PROTECTED]: @@ -1728,6 +1767,10 @@ static HRESULT WINAPI DefaultHandler_IPersistStorage_HandsOffStorage( if(SUCCEEDED(hr) object_is_running(This)) hr = IPersistStorage_HandsOffStorage(This-pPSDelegate); +old_stg = InterlockedExchangePointer((void**)This-storage, NULL); IIRC, the default handler is apartment-threaded only, meaning that a default handler object cannot be used on a thread other than the one it was created on. That's true I believe. Adding constructs like this introduce confusion to future developers about whether the code is or isn't thread-safe and so it should be avoided and a non-thread-safe construct used instead. I don't buy this. Besides we're already using Interlocked{In,De}crement(). The AddRef and Release code that uses Interlocked{In,De}crement() is cookie cutter code that is pretty much the same in Wine for all COM objects and so it doesn't look out of place. This does. I'm still not convinced, but there are more interesting things to have debates about than this ;-) Huw. -- Huw Davies [EMAIL PROTECTED]
Re: oleaut32: Add support for loading typelibs from NE files.
On Wed, Sep 03, 2008 at 12:51:36PM +0100, Rob Shearman wrote: 2008/9/2 Huw Davies [EMAIL PROTECTED]: --- a/dlls/oleaut32/typelib.c +++ b/dlls/oleaut32/typelib.c @@ -64,6 +64,7 @@ #include winnls.h #include winreg.h #include winuser.h +#include wine/winbase16.h #include wine/unicode.h #include objbase.h I don't think we should be including a 16-bit header file and using 16-bit functions in a 32-bit source file if we can avoid it. In this case, we can avoid it. Hi Rob, How do you suggest we avoid it? Cheers, Huw. -- Huw Davies [EMAIL PROTECTED]
Re: [Gdiplus] Implement GdipCloneImage
On Sat, Aug 16, 2008 at 11:26:42PM -0400, Adam Petaccia wrote: This seems like the right thing to do, but OleLoadImage fails. Comments welcome. --- dlls/gdiplus/image.c | 42 -- dlls/gdiplus/tests/image.c |2 +- 2 files changed, 41 insertions(+), 3 deletions(-) diff --git a/dlls/gdiplus/image.c b/dlls/gdiplus/image.c index 197665d..a74003f 100644 --- a/dlls/gdiplus/image.c +++ b/dlls/gdiplus/image.c @@ -242,11 +242,49 @@ GpStatus WINGDIPAPI GdipBitmapUnlockBits(GpBitmap* bitmap, GpStatus WINGDIPAPI GdipCloneImage(GpImage *image, GpImage **cloneImage) { +hr = OleLoadPicture(stream, size, FALSE, IID_IPicture, +(LPVOID*)(*cloneImage)-picture); This can't be right. You need to pass the address of the interface ptr. Huw. -- Huw Davies [EMAIL PROTECTED]
Re: wininet: default to keep-alive when the server is HTTP/1.1, not when it isn't
Vincent Povirk wrote: This is a fix to a problem I introduced in fd5b97bc4d12adc69085b739061c56e9107f8d1f. The expression that checked the HTTP version was true for HTTP/1.0 servers, and I treated it as if it were true for HTTP/1.1 servers. That means we defaulted to keep-alive for HTTP/1.0 servers only, the exact opposite of what we want. This was a problem with the old code too, but what happens when we get HTTP 1.1 ? Huw.
Re: DIB engine status
On Fri, Aug 01, 2008 at 06:23:43PM +0400, Сергей Новосёлов wrote: We'd like to continue develop your version. When are you going to remain this project? Great! Please send me patches and I'll apply them. I'll probably merge with WineHQ on every WineHQ release. I'm not planning on doing any work on it for a while, but will probably return to it at some point in the future. Huw. -- Huw Davies [EMAIL PROTECTED]
Re: [Gdiplus try3 02/16] Implement GdipCreateRegion
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 = 0x1000, +RegionDataPath = 0x1001, +RegionDataEmptyRect = 0x1002, +RegionDataInfiniteRect = 0x1003, +}; + + 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 [EMAIL PROTECTED]
Re: [Gdiplus try3 02/16] Implement GdipCreateRegion
Adam Petaccia wrote: On Tue, 2008-07-22 at 11:11 +0100, Huw Davies wrote: +} 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. Will fix. Question: would it be better to declare these structures outside, or are inline declarations acceptable? inline is fine. +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. Will do. How about just num_children? Works for me. +} 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 The CombineRegion* code takes advantage over the fact that they are pointers and can be shuffled around, rather than shuffling/cloning/memcpy'ing potentially large amounts of data around. Also, because of the recursive nature of CombineMode, I found it much easier to deal with nodes separately (and recursively when necessary) than the whole Region at a time. It should only be a question of how you start your recursion. Obviously the CombineRegion stuff will have a alloc and memcpy the old root node to 'left', but then you don't need to alloc the new root node. 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. I'll move it to region.c. Great. Again, I think you'd be better off submitting a couple of patches at a time. Sorry, its kind of painful when the work is mostly done, but the problem is getting it pushed. I'm slightly behind where I want to be on my GSoC schedule, and I was trying to catch up. I'll slow down. I think you'll actually find it quicker in the long run to have smaller patch dumps, that way you'll spend less time re-doing a whole load of patches many times. Huw.
Re: [Gdiplus try2 02/16] Implement GdipCreateRegion
Adam Petaccia wrote: @@ -226,6 +277,11 @@ GpStatus WINGDIPAPI GdipSetEmpty(GpRegion*region) if(!(calls++)) FIXME(not implemented\n); +TRACE(%p\n, region); + +GdipDeleteRegion(region); +GdipCreateRegion(region); +region-node-type = RegionDataEmptyRect; return NotImplemented; } This is still wrong. You probably don't even want to implement GdipSetEmpty in this patch anyway. In general you may be better off submitting a smaller patch series rather than 16 patches in one go. Huw.
Re: [Gdiplus 02/15] Implement GdipCreateRegion
Adam Petaccia wrote: @@ -226,6 +277,11 @@ GpStatus WINGDIPAPI GdipSetEmpty(GpRegion*region) if(!(calls++)) FIXME(not implemented\n); +TRACE(%p\n, region); + +GdipDeleteRegion(region); +GdipCreateRegion(region); +region-node-type = RegionDataEmptyRect; return NotImplemented; } This can't work. You don't want to free the memory pointed to by region, simply free the nodes underneath it. Huw.
Re: [Gdiplus 2/3] Add a test for a floating-point triangle
Hi Adam, This is mainly a brain-dump, there's nothing wrong the patch. On Thu, Jul 10, 2008 at 12:44:55AM -0400, Adam Petaccia wrote: --- dlls/gdiplus/tests/region.c | 38 ++ 1 files changed, 38 insertions(+), 0 deletions(-) diff --git a/dlls/gdiplus/tests/region.c b/dlls/gdiplus/tests/region.c index 1b1b047..7b1eba5 100644 --- a/dlls/gdiplus/tests/region.c +++ b/dlls/gdiplus/tests/region.c @@ -361,6 +361,44 @@ todo_wine expect(Ok, status); status = GdipDeleteRegion(region); expect(Ok, status); +status = GdipGetRegionDataSize(region, needed); +expect(Ok, status); +ok(needed == 72, Expected 72, got %d\n, needed); +status = GdipGetRegionData(region, (BYTE*)buf, sizeof(buf), needed); +expect(Ok, status); +expect_dword(buf, 64); +trace(buf[1] = %08x\n, buf[1]); +expect_dword(buf + 2, RGNDATA_MAGIC); +expect_dword(buf + 3, 0); +expect_dword(buf + 4, RGNDATA_PATH); + +expect_dword(buf + 5, 48); +expect_dword(buf + 6, RGNDATA_MAGIC); +expect_dword(buf + 7, 4); +expect_dword(buf + 8, 0); +expect_float(buf + 9, 5.6); +expect_float(buf + 10, 6.2); +expect_float(buf + 11, 7.2); +expect_float(buf + 12, 8.9); +expect_float(buf + 13, 8.1); +expect_float(buf + 14, 1.6); +expect_float(buf + 15, 5.6); +expect_float(buf + 16, 6.2); There's one more DWORD left here (buf + 17). I realised after I sent in my last lot of tests that this is the BYTE array of PathPointType*s packed into DWORDs (hence the 0x81.. vs 0x01.. that you noted in an earlier patch). As you've also noticed there are two ways of storing paths, if all the co-ords are short ints then they get stored that way and the 0x4000 bit of buf + 8 is set. Otherwise the co-ords are stored as floats. In addition, the 0x2000 bit of that same DWORD corresponds to the FillMode of the path. Anyway, good work! Huw. -- Huw Davies [EMAIL PROTECTED]
Re: [Gdiplus 07/15] Stub GdipFillRegion
On Wed, Jul 09, 2008 at 03:33:47AM -0400, Adam Petaccia wrote: --- dlls/gdiplus/gdiplus.spec |2 +- dlls/gdiplus/region.c | 11 +++ 2 files changed, 12 insertions(+), 1 deletions(-) diff --git a/dlls/gdiplus/region.c b/dlls/gdiplus/region.c index 3d95723..717119f 100644 --- a/dlls/gdiplus/region.c +++ b/dlls/gdiplus/region.c @@ -156,6 +156,17 @@ GpStatus WINGDIPAPI GdipDeleteRegion(GpRegion *region) return NotImplemented; } +GpStatus WINGDIPAPI GdipFillRegion(GpGraphics* graphics, GpBrush* brush, +GpRegion* region) +{ +if (!(graphics brush region)) +return InvalidParameter; + +FIXME((%p, %p, %p): stub\n, graphics, brush, region); + +return NotImplemented; +} + Msdn lists this in the 'graphics' section of the gdiplus flat api, so I'd suggest it goes in graphics.c Huw. -- Huw Davies [EMAIL PROTECTED]
Re: [Gdiplus 11/15] Implement GdipCreateRegionRect
On Wed, Jul 09, 2008 at 03:34:01AM -0400, Adam Petaccia wrote: --- dlls/gdiplus/region.c | 28 dlls/gdiplus/tests/region.c |8 2 files changed, 28 insertions(+), 8 deletions(-) diff --git a/dlls/gdiplus/region.c b/dlls/gdiplus/region.c index 59e2fa6..de86ce1 100644 --- a/dlls/gdiplus/region.c +++ b/dlls/gdiplus/region.c @@ -129,12 +129,32 @@ GpStatus WINGDIPAPI GdipCreateRegionPath(GpPath *path, GpRegion **region) return Ok; } -GpStatus WINGDIPAPI GdipCreateRegionRect(GDIPCONST GpRectF *rect, GpRegion **region) +GpStatus WINGDIPAPI GdipCreateRegionRect(GDIPCONST GpRectF *rect, +GpRegion **region) { -FIXME((%p, %p): stub\n, rect, region); +GpPath *path; +GpStatus stat; -*region = NULL; -return NotImplemented; +if (!(rect region)) +return InvalidParameter; + +TRACE(%p, %p\n, rect, region); + +*region = GdipAlloc(sizeof(GpRegion*)); +if(!*region) +return OutOfMemory; + +stat = GdipCreatePath(FillModeAlternate, path); +if (stat != Ok) { GdipDeleteRegion(*region); return stat;} +stat = GdipAddPathRectangle(path, rect-X, rect-Y, +rect-Width, rect-Height); +if (stat != Ok) { GdipDeleteRegion(*region); return stat;} +stat = GdipCreateRegionPath(path, region); +if (stat != Ok) { GdipDeleteRegion(*region); return stat;} + +GdipDeletePath(path); + +return Ok; } This doesn't look right. See the GdipGetRegionData tests (and extend them to add paths), these are supposed to help understanding how regions are stored. It looks to me that a region is stored as a sequence of rects and paths that are combined with various CombineMode ops. I'll send a patch that adds paths to the test in a bit. Huw. -- Huw Davies [EMAIL PROTECTED]
Re: [Gdiplus 11/15] Implement GdipCreateRegionRect
On Wed, Jul 09, 2008 at 11:58:23AM +0100, Huw Davies wrote: This doesn't look right. See the GdipGetRegionData tests (and extend them to add paths), these are supposed to help understanding how regions are stored. It looks to me that a region is stored as a sequence of rects and paths that are combined with various CombineMode ops. Hi Adam, In fact you want to store the region elements as a binary tree, something like this: typedef enum element_type { rect_element = 0x1000, path_element, empty_element, infinite_element } element_type_t; typedef struct region_element { DWORD type_or_op; /* One of the CombineModes or element_type_t */ union { GpRectF rect; GpPath *path; struct { struct region_element *first; struct region_element *second; } operands; } elem_data; } region_element_t; struct GpRegion { region_element_t root; }; A region element is either a rect, path or a combining op (in which case it has two children, 'first' and 'second'). The data returned by GdipGetRegionData is then basically what happens if you walk the tree, favouring the 'first' node at every branch. Does that make sense? Huw. -- Huw Davies [EMAIL PROTECTED]
Re: [Gdiplus 11/15] Implement GdipCreateRegionRect
On Wed, Jul 09, 2008 at 10:56:48AM -0400, Adam Petaccia wrote: I didn't mean to imply that I ignored the region.c header, but I didn't use any of its information for the bounds tests yet, so I didn't add them to the struct (and I've learned to add things as needed). The bounds checks are really the second stage. I'd get the CreateRegion functions working correctly first of all, and check that you're happy that the GetRegionData stuff could be made to work (even if you don't actually code it). Once you have that, you know you're storing the data in a similar way to native gdiplus. Then you can go on to work on the bounds (which would appear to be non-trivial - good luck!). Huw. -- Huw Davies [EMAIL PROTECTED]
Re: user32: sizeof DDEPOKE and DDEDATA is six, which is probably not what is expected, so use offsetof instead.
James Hawkins wrote: 2008/7/8 Huw Davies [EMAIL PROTECTED]: Fixes a todo_wine and a Valgrind warning. @@ -699,10 +699,7 @@ static HDDEDATA CALLBACK server_ddeml_callback(UINT uType, UINT uFmt, HCONV hcon ptr = (LPSTR)DdeAccessData(hdata, size); ok(!lstrcmpA(ptr, poke data\r\n), Expected 'poke data\\r\\n', got %s\n, ptr); -todo_wine -{ -ok(size == 14, Expected 14, got %d\n, size); -} +ok(size == 12, Expected 12, got %d\n, size); DdeUnaccessData(hdata); size = DdeQueryStringA(server_pid, hsz2, str, MAX_PATH, CP_WINANSI); You changed the value of the test. Did you mean to do that? Yes. See 'size' in create_poke(). Huw.
Re: gdiplus: Add a test to show that bitmap fonts aren't used for fontfamilies.
Adam Petaccia wrote: On Tue, 2008-07-08 at 13:20 +0100, Huw Davies wrote: --- dlls/gdiplus/tests/font.c |7 +++ 1 files changed, 7 insertions(+), 0 deletions(-) On my Windows system I can't find MSSansSerif, only Microsoft Sans Serif. I'm not sure if that's a valid test to prove that point. It's a bitmap font. The (latin version of the) font file is called sserife.fon . Basically the FontFamily stuff relies on scalable outlines, so gdiplus seems to simply ignore bitmap fonts. It's also a hint that Wine's GdipGetGenericFontFamilySansSerif() is incorrect - in the absence of 'Microsoft Sans Serif' then 'Tahoma' may be a better choice. Huw.
Re: [Gdiplus 4/4] Implement GdipCloneImage
On Tue, Jul 01, 2008 at 09:41:22PM -0400, Adam Petaccia wrote: --- dlls/gdiplus/image.c | 19 +-- dlls/gdiplus/tests/image.c |6 -- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/dlls/gdiplus/image.c b/dlls/gdiplus/image.c index bf2ac35..59ea837 100644 --- a/dlls/gdiplus/image.c +++ b/dlls/gdiplus/image.c @@ -244,9 +244,24 @@ GpStatus WINGDIPAPI GdipCloneImage(GpImage *image, GpImage **cloneImage) +memcpy((*cloneImage)-picture, image-picture, sizeof(IPicture)); + +return Ok; } You can't simply copy an interface ptr like this. Huw. -- Huw Davies [EMAIL PROTECTED]
Re: Gdiplus [1/3] Improve accuracy in calculating height
On Fri, Jun 27, 2008 at 10:16:41AM -0400, Adam Petaccia wrote: The previous patch I sent incorrectly calculated font-height. Now its correctly calculated using points_per_inch and the user's DPI setting. diff --git a/dlls/gdiplus/font.c b/dlls/gdiplus/font.c index 86a3223..85df905 100644 --- a/dlls/gdiplus/font.c +++ b/dlls/gdiplus/font.c @@ -34,6 +34,7 @@ WINE_DEFAULT_DEBUG_CHANNEL (gdiplus); #include gdiplus_private.h static const REAL mm_per_pixel = 25.4; +static const REAL points_per_inch = 1/72; I know there are several definitions of a point, but none are this big! points_per_inch should be around 72. @@ -50,7 +51,7 @@ static inline REAL get_dpi (void) static inline REAL point_to_pixel (REAL point) { -return point * 1.5; +return point * (get_dpi() * points_per_inch); And this then becomes points * get_dpi() / points_per_inch; [Hint check the units: pixels (aka dots) = points * (dots / inch) / (points / inch) ] Huw. -- Huw Davies [EMAIL PROTECTED]
Re: Gdiplus [1/3] Improve accuracy in calculating height
On Fri, Jun 27, 2008 at 03:27:44PM +0100, Huw Davies wrote: On Fri, Jun 27, 2008 at 10:16:41AM -0400, Adam Petaccia wrote: --- a/dlls/gdiplus/font.c +++ b/dlls/gdiplus/font.c @@ -34,6 +34,7 @@ WINE_DEFAULT_DEBUG_CHANNEL (gdiplus); #include gdiplus_private.h static const REAL mm_per_pixel = 25.4; Ah and I missed this one. Please rename to mm_per_inch ! Huw. -- Huw Davies [EMAIL PROTECTED]
Re: Memory leak patch
On Thu, Jun 26, 2008 at 03:25:30AM -0700, Joris Huizer wrote: dlls/gdiplus/font.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/dlls/gdiplus/font.c b/dlls/gdiplus/font.c index 9ef8edc..86122da 100644 --- a/dlls/gdiplus/font.c +++ b/dlls/gdiplus/font.c @@ -42,6 +42,7 @@ static inline REAL get_dpi (void) HDC hdc = GetDC(0); GdipCreateFromHDC (hdc, graphics); GdipGetDpiX(graphics, dpi); +GdipFree(graphics); ReleaseDC (0, hdc); return dpi; This should be GdipDeleteGraphics. Huw. -- Huw Davies [EMAIL PROTECTED]
Re: memory leak fix
On Thu, Jun 26, 2008 at 02:05:15AM -0700, Joris Huizer wrote: Untested, but this seems trivial/obvious, unless I overlooked something From 4026f68e4404c4b1822eefe0b3419db4f896e055 Mon Sep 17 00:00:00 2001 From: Joris Huizer [EMAIL PROTECTED](none) Date: Thu, 26 Jun 2008 11:07:11 +0200 Subject: [PATCH] memory leak fix --- dlls/gdiplus/font.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/dlls/gdiplus/font.c b/dlls/gdiplus/font.c index 0bb9fc7..9ef8edc 100644 --- a/dlls/gdiplus/font.c +++ b/dlls/gdiplus/font.c @@ -378,6 +378,7 @@ GpStatus WINGDIPAPI GdipCreateFontFamilyFromName(GDIPCONST WCHAR *name, ffamily-FamilyName = GdipAlloc(LF_FACESIZE * sizeof (WCHAR)); if (!ffamily-FamilyName) { +GdipFree(ffamily-tmw); GdipFree(ffamily); return OutOfMemory; } That's a good start, but there's more. The function leaks an hfont both on success and in this failure case. Also there should be a ReleaseDC in the failure case. In addition it seems to me that the tmw element may as well be an embedded structure rather than a structure pointer. That'll save having to worry about alloc'ing/free'ing it. Huw. -- Huw Davies [EMAIL PROTECTED]