May 2016 - Wine-Devel - list.winehq.org

Re: [1/2] shlwapi/tests: Add wvnsprintfW comformance test

by Marvin

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at https://testbot.winehq.org/JobDetails.pl?Key=22849 Your paranoid android. === wxppro (32 bit string) === string: unhandled exception c0000005 at 77F6923B === w2003std (32 bit string) === string: unhandled exception c0000005 at 7D18A782

May 5, 2016

Re: [PATCH] winmm: Restrict some MCI actions to the creating thread

by Marvin

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at https://testbot.winehq.org/JobDetails.pl?Key=22848 Your paranoid android. === w864 (32 bit mci) === mci.c:1121: Test failed: got 0001 instead of MCI_NOTIFY_xyz 0004 from command play (aborted by close) === w1064 (32 bit mci) === mci.c:1121: Test failed: got 0001 instead of MCI_NOTIFY_xyz 0004 from command play (aborted by close) === w864 (64 bit mci) === mci.c:1121: Test failed: got 0001 instead of MCI_NOTIFY_xyz 0004 from command play (aborted by close) === w1064 (64 bit mci) === mci.c:1121: Test failed: got 0001 instead of MCI_NOTIFY_xyz 0004 from command play (aborted by close)

May 4, 2016

Re: [PATCH 11/11] msctf: Added ITfTransitoryExtensionSink tests.

by Marvin

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at https://testbot.winehq.org/JobDetails.pl?Key=22847 Your paranoid android. === wvistau64 (32 bit inputprocessor) === inputprocessor.c:1104: Test failed: Failed to get sink cookie === w2008s64 (32 bit inputprocessor) === inputprocessor.c:1104: Test failed: Failed to get sink cookie === w7u (32 bit inputprocessor) === inputprocessor.c:1104: Test failed: Failed to get sink cookie === w7pro64 (32 bit inputprocessor) === inputprocessor.c:1104: Test failed: Failed to get sink cookie === wvistau64 (64 bit inputprocessor) === inputprocessor.c:1104: Test failed: Failed to get sink cookie === w2008s64 (64 bit inputprocessor) === inputprocessor.c:1104: Test failed: Failed to get sink cookie === w7pro64 (64 bit inputprocessor) === inputprocessor.c:1104: Test failed: Failed to get sink cookie

May 4, 2016

Re: msctf: Improved ITfKeystrokeMgr::TestKeyUp and TestKeyDown stubs.

by Aric Stewart

Signed-off-by: Aric Stewart <aric(a)codeweavers.com>

May 4, 2016

Re: comdlg32: Use SetRect() instead of open coding it.

by Michael Stefaniuc

Frédéric, On 05/04/2016 01:08 PM, Frédéric Delanoy wrote: > On Wed, May 4, 2016 at 12:36 PM, Dmitry Timoshkov <dmitry(a)baikal.ru> wrote: >> Frédéric Delanoy <frederic.delanoy(a)gmail.com> wrote: >> >>>>> - cancel_rc.left = dialog_rc.right - cancel_width - hspacing; >>>>> - cancel_rc.top = dialog_rc.bottom - cancel_height - vspacing; >>>>> - cancel_rc.right = cancel_rc.left + cancel_width; >>>>> - cancel_rc.bottom = cancel_rc.top + cancel_height; >>>>> + SetRect(&cancel_rc, >>>>> + dialog_rc.right - cancel_width - hspacing, dialog_rc.bottom - cancel_height - vspacing, >>>>> + dialog_rc.right - hspacing , dialog_rc.bottom - vspacing); >>>> >>>> This is at least very strange and weird formatting of an API call. >>>> I don't see anything like that in the changed file. >>>> >>>> -- >>>> Dmitry. >>> >>> It's for increased readability. >> >> I hope that's a joke. > > No. Maybe I should have added IMHO? > >>> Of course, it doesn't show well in >>> proportional fonts like those used in emails. >> >> I don't use proportional fonts in e-mails and it still looks quite ugly. > > Tastes and colours I guess > >> What's the reason of inventing new fancy formatting when doing janitorial >> work? > > Rationale > 1. The whole call is too long to fit on a single line, with long parameters > 2. The too lines represent respectively the left-top and right-bottom > points of a rectangle respectively ; placing them vertically lets you > see directly the "distance" between left-right and top-bottom > respectively ; the 2nd/4th and 3rd/5th are "logically" connected. > Alignment just helps distinguish parameters quicker. > > Of course I don't do that if SetRect parameters are short enough, like > e.g. SetRect(foo, x, y, x+1, y-1) where the parameters are quickly > discernable. > > Anyway, I could put all parameters on separate lines if that's really > such a tremendous issue for you. that was actually my reason to postpone those changes for later. They didn't look better with the SetRect() call. bye michael

May 4, 2016

Re: [PATCH 3/4] winemac.drv: Really fix our handling of DIBs on the clipboard.

by Ken Thomases

Hi Chip, Thanks for working on this. I know you didn't originate this approach, but, as a general concern, is there really no better approach to converting DIBs than what we have with duplicated code for manually laying out the structure and copying data? I mean, that code has to go someplace, but shouldn't there be proper functions in GDI or something? Among other things, I worry that there might be subtle details that each duplicate gets slightly wrong or at least different from the others. For one thing, when converting from a DIBv5 to another format, you copy across the image data, but you drop the color profile. Shouldn't there be a color conversion to sRGB or whatever the default profile of a v3 DIB is? I know that most of Wine doesn't handle this stuff properly, anyway, but that's part of my point about doing these conversions in one place. When such support is added to that one place in the future, we'd want to automatically get the benefit of it here. Specific points about the code in-line below. In some cases, I may just be ignorant about the DIB format, so just let me know: On May 3, 2016, at 2:03 PM, Charles Davis <cdavis5x(a)gmail.com> wrote: > > +static HGLOBAL create_dib_from_dibv5(HGLOBAL src) … > + /* Calculate the size of the new packed DIB */ > + data_size = bv5->bV5SizeImage; MSDN says bV5SizeImage can be zero for BI_RGB DIBs. Is that only for zero-size DIBs or does it mean it's implicit in width, height, and bit count? > + packed_size = sizeof(BITMAPINFOHEADER) > + + ((bv5->bV5BitCount <= 8) ? (sizeof(RGBQUAD) * bv5->bV5ClrUsed) : 0) > + + ((bv5->bV5Compression == BI_BITFIELDS) ? (sizeof(DWORD) * 3) : 0) > + + data_size; MSDN says bV5BitCount can be 0 for BI_JPEG and BI_PNG. For that case, should a color table be included in the size? There's an existing function, bitmap_info_size(). Should that be extended for additional header types for its existing uses? Should it be used for these calculations in your new code? > +static HGLOBAL create_dibv5_from_dib(HGLOBAL src) … > + data_size = bmih->biSizeImage; > + packed_size = sizeof(BITMAPV5HEADER) > + + ((bmih->biBitCount <= 8) ? (sizeof(RGBQUAD) * bmih->biClrUsed) : 0) > + + data_size; > + offset_src = bmih->biSize > + + ((bmih->biBitCount <= 8) ? (sizeof(RGBQUAD) * bmih->biClrUsed) : 0); Shouldn't offset_src potentially include the color masks for biCompression == BI_BITFIELDS? > + if (bv5->bV5Compression == BI_BITFIELDS && bmih->biSize < sizeof(BITMAPV4HEADER)) > + { > + memcpy(&bv5->bV5RedMask, bmih+1, 3*sizeof(DWORD)); > + bv5->bV5AlphaMask = 0; > + } Is it OK to leave the masks uninitialized for other cases? > + hdc = GetDC(0); You never release this DC. > + if (GetLogColorSpaceW(GetColorSpace(hdc), &lcs, sizeof(lcs))) > + { > + if (bmih->biSize < sizeof(BITMAPV4HEADER)) > + { > + bv5->bV5CSType = lcs.lcsCSType; > + bv5->bV5Endpoints = lcs.lcsEndpoints; > + bv5->bV5GammaRed = lcs.lcsGammaRed; > + bv5->bV5GammaGreen = lcs.lcsGammaGreen; > + bv5->bV5GammaBlue = lcs.lcsGammaBlue; > + } > + bv5->bV5Intent = lcs.lcsIntent; > + } > + else > + { > + if (bmih->biSize < sizeof(BITMAPV4HEADER)) > + { > + bv5->bV5CSType = LCS_DEVICE_RGB; This type isn't listed as a valid value for bV5CSType in MSDN. > + memset(&bv5->bV5Endpoints, 0, sizeof(bv5->bV5Endpoints)); > + bv5->bV5Endpoints.ciexyzRed.ciexyzX = 0xffffffff; > + bv5->bV5Endpoints.ciexyzGreen.ciexyzY = 0xffffffff; > + bv5->bV5Endpoints.ciexyzBlue.ciexyzZ = 0xffffffff; > + bv5->bV5GammaRed = 0; > + bv5->bV5GammaGreen = 0; > + bv5->bV5GammaBlue = 0; Are gamma values of 0 right? I would expect the 16.16-fixed form of 2.2 or maybe 1.0. > @@ -724,6 +932,7 @@ static HANDLE import_bmp_to_dib(CFDataRef data) > } > > memcpy(p, bmi, len); > + Spurious whitespace change. > @@ -1254,8 +1552,35 @@ static CFDataRef export_bitmap_to_dib(HANDLE data) > dib = create_dib_from_bitmap(data); > if (dib) > { > - ret = export_clipboard_data(dib); > + HGLOBAL dib3 = create_dib_from_dibv5(dib); > + > + ret = export_clipboard_data(dib3); > + GlobalFree(dib); > + if (dib3 != dib) GlobalFree(dib3); > + } Is the above necessary? create_dib_from_bitmap() will only ever create v3 DIBs, right? It's specifically allocating and building a BITMAPINFOHEADER. -Ken

May 4, 2016

Re: comdlg32: Use SetRect() instead of open coding it.

by Dmitry Timoshkov

Frédéric Delanoy <frederic.delanoy(a)gmail.com> wrote: > Anyway, I could put all parameters on separate lines if that's really > such a tremendous issue for you. Please resend with formatting following the style of surrounding code. -- Dmitry.

May 4, 2016

Re: comdlg32: Use SetRect() instead of open coding it.

by Frédéric Delanoy

On Wed, May 4, 2016 at 12:36 PM, Dmitry Timoshkov <dmitry(a)baikal.ru> wrote: > Frédéric Delanoy <frederic.delanoy(a)gmail.com> wrote: > >> >> - cancel_rc.left = dialog_rc.right - cancel_width - hspacing; >> >> - cancel_rc.top = dialog_rc.bottom - cancel_height - vspacing; >> >> - cancel_rc.right = cancel_rc.left + cancel_width; >> >> - cancel_rc.bottom = cancel_rc.top + cancel_height; >> >> + SetRect(&cancel_rc, >> >> + dialog_rc.right - cancel_width - hspacing, dialog_rc.bottom - cancel_height - vspacing, >> >> + dialog_rc.right - hspacing , dialog_rc.bottom - vspacing); >> > >> > This is at least very strange and weird formatting of an API call. >> > I don't see anything like that in the changed file. >> > >> > -- >> > Dmitry. >> >> It's for increased readability. > > I hope that's a joke. No. Maybe I should have added IMHO? >> Of course, it doesn't show well in >> proportional fonts like those used in emails. > > I don't use proportional fonts in e-mails and it still looks quite ugly. Tastes and colours I guess > What's the reason of inventing new fancy formatting when doing janitorial > work? Rationale 1. The whole call is too long to fit on a single line, with long parameters 2. The too lines represent respectively the left-top and right-bottom points of a rectangle respectively ; placing them vertically lets you see directly the "distance" between left-right and top-bottom respectively ; the 2nd/4th and 3rd/5th are "logically" connected. Alignment just helps distinguish parameters quicker. Of course I don't do that if SetRect parameters are short enough, like e.g. SetRect(foo, x, y, x+1, y-1) where the parameters are quickly discernable. Anyway, I could put all parameters on separate lines if that's really such a tremendous issue for you. -- Frédéric

May 4, 2016

Re: [PATCH 3/3] wininet: Added IRD_NO_WAIT flag tests.

by Marvin

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at https://testbot.winehq.org/JobDetails.pl?Key=22837 Your paranoid android. === w1064 (32 bit http) === http.c:523: Test failed: HTTP_QUERY_RAW_HEADERS: expected length 0, but got 1 http.c:523: Test failed: HTTP_QUERY_RAW_HEADERS: expected length 0, but got 1 http.c:523: Test failed: HTTP_QUERY_RAW_HEADERS: expected length 0, but got 1 http.c:523: Test failed: HTTP_QUERY_RAW_HEADERS: expected length 0, but got 1 http.c:523: Test failed: HTTP_QUERY_RAW_HEADERS: expected length 0, but got 1 http.c:1762: Test failed: Expected 2, got 4 http.c:1763: Test failed: Expected CRLF, got ' http.c:2413: Test failed: count was wrong http.c:2414: Test failed: http data wrong, got: Garbage: Header http.c:3077: Test failed: HttpAddRequestHeaders succeeded http.c:3078: Test failed: Expected error ERROR_HTTP_HEADER_NOT_FOUND, got 0 http.c:3083: Test failed: code = 200, expected 400 http.c:3083: Test failed: unexpected status code 200, expected 400 http.c:3083: Test failed: unexpected status code 200, expected 400 http.c:523: Test failed: HTTP_QUERY_RAW_HEADERS: expected length 0, but got 1 === w1064 (64 bit http) === http.c:523: Test failed: HTTP_QUERY_RAW_HEADERS: expected length 0, but got 1 http.c:523: Test failed: HTTP_QUERY_RAW_HEADERS: expected length 0, but got 1 http.c:523: Test failed: HTTP_QUERY_RAW_HEADERS: expected length 0, but got 1 http.c:523: Test failed: HTTP_QUERY_RAW_HEADERS: expected length 0, but got 1 http.c:523: Test failed: HTTP_QUERY_RAW_HEADERS: expected length 0, but got 1 http.c:1762: Test failed: Expected 2, got 4 http.c:1763: Test failed: Expected CRLF, got ' http.c:2413: Test failed: count was wrong http.c:2414: Test failed: http data wrong, got: Garbage: Header http.c:3077: Test failed: HttpAddRequestHeaders succeeded http.c:3078: Test failed: Expected error ERROR_HTTP_HEADER_NOT_FOUND, got 0 http.c:3083: Test failed: code = 200, expected 400 http.c:3083: Test failed: unexpected status code 200, expected 400 http.c:3083: Test failed: unexpected status code 200, expected 400 http.c:523: Test failed: HTTP_QUERY_RAW_HEADERS: expected length 0, but got 1

May 4, 2016

Re: comdlg32: Use SetRect() instead of open coding it.

by Dmitry Timoshkov

Frédéric Delanoy <frederic.delanoy(a)gmail.com> wrote: > >> - cancel_rc.left = dialog_rc.right - cancel_width - hspacing; > >> - cancel_rc.top = dialog_rc.bottom - cancel_height - vspacing; > >> - cancel_rc.right = cancel_rc.left + cancel_width; > >> - cancel_rc.bottom = cancel_rc.top + cancel_height; > >> + SetRect(&cancel_rc, > >> + dialog_rc.right - cancel_width - hspacing, dialog_rc.bottom - cancel_height - vspacing, > >> + dialog_rc.right - hspacing , dialog_rc.bottom - vspacing); > > > > This is at least very strange and weird formatting of an API call. > > I don't see anything like that in the changed file. > > > > -- > > Dmitry. > > It's for increased readability. I hope that's a joke. > Of course, it doesn't show well in > proportional fonts like those used in emails. I don't use proportional fonts in e-mails and it still looks quite ugly. What's the reason of inventing new fancy formatting when doing janitorial work? -- Dmitry.

May 4, 2016