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

gdi32: "StretchDIBits" seems to use the wrong picture offset
by Mathias Kosch July 21, 2008

July 21, 2008

Currently I'm trying to fix Bug#13344. I figured out that the problems seems to be caused by the function "StretchDIBits" inside "dlls/gdi32/dib.c". When I run "Digital Photo Professional 3.4.1.1" and open images in the editor window in "fit to window" mode all images are shifted to top so that I cannot totally see them. I provided a screenshot as attachment for you to see. The picture was colored to purple to reduce bandwidth. The dimensions of the visible image are 1276 x 850 pixels. The picture is shifted upwards by exactly 174 pixels, so a black bar appears at the bottom. I modified my local copy of "dib.c" (modifications as attachment) to output its parameters. The produced (extract of the) output is attached as "output.txt". The function "StretchDIBits" is called correctly. It shall display a portion of the bitmap at position (0, 40) of dimensions (1276, 850). This function produces two calls to "StretchBlt". (In my case only the second one is called.) The parameters it passes to "StretchBlt" appear to be wrong because (in the second call) "ySrc" is set to 174 which explains the pixel shift. If I adjust "ySrc" to 0 all images are displayed correctly. I looked at the code of "SetDIBits". It seems that it doesn't make any difference between bottom-up or top-down DIBs except that the pixel data is mirrored in y direction. I assume Windows does it the same way, but didn't verify it. However "StretchDIBits" wants to differentiate between the two cases by setting the y offset to "abs(height)-heightSrc-ySrc". I think only "ySrc" should be used as y offset. However since I'm not absolutely sure I would like to ask someone with a little more experience. By the way, I recognized another problem. The code also contains two calls to "SetDIBits" with parameter "height" for "UINT lines". However "height" may be negative. When I'm able to produce a patch for Bug#13344 I will fix this as well. -- Mathias Kosch <info(a)mkosch.de> | http://www.mkosch.de/ RSA Public Key: http://www.mkosch.de/pubkey IT-Nachrichten auf heise online: http://www.heise.de/

2 2

Re: wineesd: Make use of `esd-config --libs` for the configure check. Filter out /usr/lib and /usr/lib64. Also follow the naming convention for the 'xxx-config' variables.
by Alexandre Julliard July 21, 2008

July 21, 2008

Francois Gouget <fgouget(a)codeweavers.com> writes: > I added code to filter out -L/usr/lib and -L/usr/lib64 because we're > doing it for gphoto2 already, presumably to avoid trouble on 64bit Linux > systems. I'm not sure if that was a gphoto2-specific issue or if it > should be done more systematically. If the latter, then there are other > places that don't do that filtering: libxml-2.0, libxslt, hal and > freetype (but then maybe they don't report the -L settings). If the > filtering is not desired, then I can resubmit. We have only had problems with it with gphoto2 as far as I know. If some other libraries have the same problem then it can be added for them, but I don't see a need to do it systematically. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: kernel32/tests: Fix a failing test in vista
by Dmitry Timoshkov July 21, 2008

July 21, 2008

"Nicolas Le Cam" <niko.lecam(a)gmail.com> wrote: > Vista introduced new flags for CompareString, as stated in [1]. > The value defined for LINGUISTIC_IGNORECASE is 0x10. So the invalid flag > test don't fail as expected starting from this platform. > > [1] http://msdn.microsoft.com/en-us/library/ms647476.aspx LINGUISTIC_IGNORECASE should be added first instead of using a hex value, and CompareString implementation should handle that flag as well. -- Dmitry.

2 1

ntoskrnl tests - where to write them?
by Jeff Latimer July 21, 2008

July 21, 2008

I have been looking at writing a couple of tests for ntoskrl functions but I have had difficulty locating a suitable place. It sort of looks like ntdll/tests is the place but it is not clear. Is there a guide for locating this one? Jeff

6 5

Re: [PATCH] ntdll: Use our own implementation of atoi.
by Alexandre Julliard July 21, 2008

July 21, 2008

"Lei Zhang" <thestig(a)google.com> writes: > diff --git a/dlls/ntdll/string.c b/dlls/ntdll/string.c > index ebfa6fb..6b36364 100644 > --- a/dlls/ntdll/string.c > +++ b/dlls/ntdll/string.c > @@ -466,10 +466,31 @@ ULONG __cdecl NTDLL_strtoul( const char *nptr, char **endptr, int base ) > > /********************************************************************* > * atoi (NTDLL.@) > + * > + * Same implementation as _atoi64. If it's the same then there's no need to duplicate it, just call atoi64. Also what about atol? -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: mlang: A very basic implementation of the IMLangLineBreakConsole interface
by Alexandre Julliard July 21, 2008

July 21, 2008

> +interface IMLangLineBreakConsole : IUnknown > +{ > + HRESULT BreakLineML( > + [in] IUnknown* pSrcMLStr, /* FIXME: IMLangString */ > + [in] long lSrcPos, > + [in] long lSrcLen, > + [in] long cMinColumns, > + [in] long cMaxColumns, > + [out] long* plLineLen, > + [out] long* plSkipLen); Please add the necessary IMLangString definitions needed for the function to be declared correctly. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: [PATCH 2/2] gdiplus: Detect integer overflow in GdipCreateBitmapFromScan0.
by Alexandre Julliard July 21, 2008

July 21, 2008

"Lei Zhang" <thestig(a)google.com> writes: > - datalen = abs(stride * height); > + datalen = stride * height; > size = sizeof(BITMAPFILEHEADER) + sizeof(BITMAPINFOHEADER) + datalen; > + if (datalen <= 0 || size <= 0){ > + GdipFree(*bitmap); > + *bitmap = NULL; > + return InvalidParameter; > + } Testing for overflow is a good idea, but checking for a negative result is not the right way. You can get overflow with a positive result too. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: gphoto2.ds: On Solaris Blastwave's libgphoto2.so needs libintl.so.
by Alexandre Julliard July 21, 2008

July 21, 2008

Francois Gouget <fgouget(a)codeweavers.com> writes: > Blastwave's 'gphoto2-config --libs' does not say that we need to link > with libintl.so when linking with libgphoto2.so. In a way it's normal as > this dependency does not come from libgphoto2 itself, but from one of > the libraries it links with. I don't think libintl is available > everywhere so I made this patch that first tries to link without it, and > then again with. I think that should be fixed in Blastwave. The whole point of having a config --libs script is to return the necessary paths and dependencies. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: taskmgr: Add full translation [1/2]
by Detlef Riekenberg July 20, 2008

July 20, 2008

On So, 2008-07-20 at 22:58 +0500, Vladimir Pankratov wrote: First of all, your Idea is great, but your Patch is far to large. Please move only some strings (usual is from one source file) to the resources at once. Then you must keep the resources in sync. That means, when you move one string to the resources, you must add that string to all rc files (most times as english string). Check the patch-size here. It might be useful to split the string-move for one source-file to the resources in more than one patch. The strings in the Menu and Dialogs are already in the resources and translated. You must not remove them. As a hint for your first removed item: grep ID_VIEW_LARGE programs/taskmgr/*.rc > + TCHAR szLoadedString[250]; > + TCHAR szTemp[250]; ... > + LoadStringA(hInst, IDS_UNABLE_TO_ACCESS, szLoadedString, > sizeof(szLoadedString)); > + MessageBox(hMainWnd, strErrorText, szLoadedString, MB_OK| > MB_ICONSTOP); We try to avoid TCHAR and change our Apps to Unicode. When you add code to move the strings to the resources, please use WCHAR and the Unicode functions As a note, switching to unicode later (-DUNICODE -D_UNICODE) breaks with the code from your Patch. When you compare TCHAR, LoadStringA and MessageBox for ANSI and UNICODE, you see, why '#include <tchar.h>' should be forbidden. Thanks for helping Wine. -- By by ... Detlef

1 0

Re: MBCS minor bug fix
by Juan Lang July 20, 2008

July 20, 2008

Hi Hirofumi, the subject of your patch doesn't tell us very much. Most submissions are in fact bug fixes in fact, so please describe what the bug is at least. A test case showing what's wrong with the existing code, and that shows that your patch fixes it, would be much better. Furthermore, - if (!lpszPath || (iLen = strlen(lpszPath)) >= MAX_PATH) - return NULL; + while(*pch != '\0') + { + pchLast = pch; + pch = CharNext(pch); + } Here you're discarded a test of lpszPath against NULL. That can't be correct, since the existing tests show that Windows returns NULL in this case. (See http://source.winehq.org/source/dlls/shlwapi/tests/path.c#L620 ). You must run 'make test' in the tests directory of the code you're changing (at least), and the tests must pass, in order for your changes to be acceptable. Also, the call to CharNext isn't allowed in Wine's source. (Did you try to compile it? It should have failed to compile.) You must use either CharNextA or CharNextW, whichever is appropriate, instead. + if(MAX_PATH <= lstrlen(lpszPath)) + return NULL; This change seems like an unnecessary style change. The existing test of lpszPath and its length accomplished the same thing, so please leave them alone and concentrate on the fix itself. Finally, if I understand the intent of your patch correctly, the changes to PathAddBackslashW are unnecessary: the W variant doesn't have to deal with MBCS string encodings, so using strlenW to find the end of the string is sufficient. Besides, the following change is simply incorrect, and will produce incorrect results nearly all the time: + if(MAX_PATH <= lstrlen(lpszPath)) Thanks for contributing to Wine. --Juan

1 0