August 2014 - Wine-Devel - list.winehq.org

Re: [1/5]dwrite: Implement Font File Loader registration
by Nikolay Sivov Aug. 8, 2014

Aug. 8, 2014

> +static HRESULT WINAPI fontfileloader_QueryInterface(IDWriteFontFileLoader *iface, REFIID riid, void **obj) > +{ > + *obj = iface; > + return S_OK; > +} This should follow normal QI behavior, with test failure on unexpected riids. > +static HRESULT WINAPI fontfileloader_CreateStreamFromKey(IDWriteFontFileLoader *iface, const void *fontFileReferenceKey, UINT32 fontFileReferenceKeySize, IDWriteFontFileStream **fontFileStream) > +{ > + return 0x8faecafe; I'm sure this works, but it's cleaner to return E_FAIL/E_NOTIMPL imho. > + hr = IDWriteFactory_RegisterFontFileLoader(factory, &floader); > + ok(hr == S_OK, "got 0x%08x\n", hr); Probably it makes sense to test NULL argument as well, but that's certainly optional (and dwrite tends to crash in such cases).

1 0

Re: mshtml/tests: Better output when an exception is caught.
by Marvin Aug. 7, 2014

Aug. 7, 2014

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=8326 Your paranoid android. === w2000pro (32 bit) === No test summary line found === wxppro (32 bit) === No test summary line found === wvistau64 (32 bit) === No test summary line found === w2008s64 (32 bit) === No test summary line found === w7u (32 bit) === No test summary line found === w8 (32 bit) === No test summary line found === wvistau64 (64 bit) === No test summary line found === w2008s64 (64 bit) === No test summary line found

1 0

Re: [PATCH 4/4] vbscript: Implemented ScriptEngineMinorVersion(try 2)
by Jacek Caban Aug. 6, 2014

Aug. 6, 2014

Hi Shuai, On 28/07/14 04:49, Shuai Meng wrote: > static HRESULT Global_ScriptEngineMinorVersion(vbdisp_t *This, VARIANT *arg, unsigned args_cnt, VARIANT *res) > { > - FIXME("\n"); > - return E_NOTIMPL; > + int minorversion = 8; Please use something like VBSCRIPT_MINOR_VERSION in a header file instead of hardcoding it here. This will allow sharing the value with resources strings in the future. Jacek

1 0

Bad coding style of kernel32/pipe tests
by Sebastian Lackner Aug. 5, 2014

Aug. 5, 2014

Hi, I would like to submit some new tests for kernel32/pipe, but I don't really know how to deal with the existing ones. To show what I mean a few examples should be sufficient: --- snip --- [...] if (readden != sizeof(obuf)) /* Linux only returns the first message */ ok(readden == sizeof(obuf) + sizeof(obuf2), "peek3 got %d bytes\n", readden); else todo_wine ok(readden == sizeof(obuf) + sizeof(obuf2), "peek3 got %d bytes\n", readden); [...] if (avail != sizeof(obuf)) /* older Linux kernels only return the first write here */ ok(avail == sizeof(obuf) + sizeof(obuf2), "peek3 got %d bytes available\n", avail); [...] --- /snip --- There are several such tests, which basically always succeed. I can understand that all these if(...)s were added to avoid test failures ... but I don't think its really useful - especially not when the ifs depend on the actual test result, and not the platform which causes the issues. :/ Before I start submitting additional tests I would like to know how to deal with all these problems. Should my new tests also contain if(...) blocks around every test, which could fail on old/new kernels, on MacOS, on FreeBSD, ...? Hopefully you'll all agree that we have to find a better solution for that ... some possible ideas: * Write the tests in such a way, that there are no test failures on Alexandre's main Linux development machine. Accept test failures for all other Linux/MacOS/FreeBSD/... platforms. * Add additional todo_mac, todo_linux, ... macros, and define WINETEST_PLATFORM to something a bit more specific than just "wine". todo_wine would mean that it is a todo on all platforms supported by Wine. This doesn't solve the problems with old/new kernels though. * Try to detect MacOS somehow else directly in the test (what would be the recommended way?) and use if(...)s based on that condition, instead of the test result. This also provides no solution for old/new kernels ... I'm also open for additional suggestions if someone has a good idea. I would especially like to hear, that the solution is _not_ that every Wine developer always has to check all possible platforms before submitting tests, which can easily get very complicated... Regards, Sebastian BTW: For those that are interested in the details where I'm working on, just take a look at: http://bugs.winehq.org/show_bug.cgi?id=17195#c154 ;)

1 0

Re: [PATCH 2/2] vbscript: Implemented Int
by Marvin Aug. 5, 2014

Aug. 5, 2014

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=8295 Your paranoid android. === w2000pro (32 bit) === No test summary line found === wxppro (32 bit) === No test summary line found === wvistau64 (32 bit) === No test summary line found === w2008s64 (32 bit) === No test summary line found === w7u (32 bit) === No test summary line found === w8 (32 bit) === No test summary line found === wvistau64 (64 bit) === No test summary line found === w2008s64 (64 bit) === No test summary line found

1 0

Re: [PATCH 1/2] vbscript: Implemented Fix
by Marvin Aug. 5, 2014

Aug. 5, 2014

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=8294 Your paranoid android. === w2000pro (32 bit) === No test summary line found === wxppro (32 bit) === No test summary line found === wvistau64 (32 bit) === No test summary line found === w2008s64 (32 bit) === No test summary line found === w7u (32 bit) === No test summary line found === w8 (32 bit) === No test summary line found === wvistau64 (64 bit) === No test summary line found === w2008s64 (64 bit) === No test summary line found

1 0

Re: [1/2] imagehlp/tests: Add test for BindImageEx (try 12)
by Alexandre Julliard Aug. 4, 2014

Aug. 4, 2014

Bernhard Reiter <ockham(a)raz.or.at> writes: > @@ -31,6 +31,8 @@ > static HMODULE hImageHlp; > > static BOOL (WINAPI *pImageGetDigestStream)(HANDLE, DWORD, DIGEST_FUNCTION, DIGEST_HANDLE); > +static BOOL (WINAPI *pBindImageEx)(DWORD Flags, const char *ImageName, const char *DllPath, > + const char *SymbolPath, void *StatusRoutine); That's not the correct prototype. > +static BOOL WINAPI testing_status_routine(IMAGEHLP_STATUS_REASON reason, char *ImageName, > + char *DllName, ULONG_PTR Va, ULONG_PTR Parameter) > +{ > + char kernel32_path[MAX_PATH]; > + > + status_routine_called[reason]++; This will have nasty results if you get an unexpected code. > + default: > + ok(0, "expected reason to be one of BindImportModule, BindImportProcedure, or " > + "BindForwarderNOT, got %d\n", reason); Please simplify your error messages. In particular, this one would need to be changed every time the test is expanded, that's not a good idea. > +static void test_bind_image_ex(void) > +{ > + BOOL ret; > + HANDLE file; > + char temp_file[MAX_PATH] = "nonexistant.dll"; There's no reason to use a variable for this. > + /* Under 64 bit images of Vista Ultimate, 2008 Server, and 7 Pro, StatusRoutine is called with > + * reason BindForwarderNOT instead of BindImportProcedure. */ > + todo_wine ok((status_routine_called[BindImportProcedure] == 1) || > + (status_routine_called[BindForwarderNOT] == 1), > + "Expected StatusRoutine to be called once with reason BindImportProcedure or " > + "BindForwarderNOT, but it was called %d and %d times, respectively\n", > + status_routine_called[BindImportProcedure], status_routine_called[BindForwarderNOT]); You'd most likely want the test to depend on 32/64-bit instead of always accepting both. -- Alexandre Julliard julliard(a)winehq.org

2 2

Re: [1/5] wined3d: Make d3d_level_from_gl_info return an enum instead of a plain int
by Henri Verbeet Aug. 4, 2014

Aug. 4, 2014

On 1 August 2014 20:25, Andrei Slăvoiu <andrei.slavoiu(a)gmail.com> wrote: > -static UINT d3d_level_from_gl_info(const struct wined3d_gl_info *gl_info) > +static enum wined3d_d3d_level d3d_level_from_gl_info(const struct wined3d_gl_info *gl_info) > { > - UINT level = 0; > + enum wined3d_d3d_level level = WINED3D_D3D_LEVEL_5; > > if (gl_info->supported[ARB_MULTITEXTURE]) > - level = 6; > + level = WINED3D_D3D_LEVEL_6; > if (gl_info->supported[ARB_TEXTURE_COMPRESSION] > && gl_info->supported[ARB_TEXTURE_CUBE_MAP] > && gl_info->supported[ARB_TEXTURE_ENV_DOT3]) > - level = 7; > - if (level == 7 && gl_info->supported[ARB_MULTISAMPLE] > + level = WINED3D_D3D_LEVEL_7; > + if (level == WINED3D_D3D_LEVEL_7 && gl_info->supported[ARB_MULTISAMPLE] > && gl_info->supported[ARB_TEXTURE_BORDER_CLAMP]) > - level = 8; > - if (level == 8 && gl_info->supported[ARB_FRAGMENT_PROGRAM] > + level = WINED3D_D3D_LEVEL_8; > + if (level == WINED3D_D3D_LEVEL_8 && gl_info->supported[ARB_FRAGMENT_PROGRAM] > && gl_info->supported[ARB_VERTEX_SHADER]) > - level = 9; > - if (level == 9 && (gl_info->supported[EXT_GPU_SHADER4] > + level = WINED3D_D3D_LEVEL_9_SM2; > + if (level == WINED3D_D3D_LEVEL_9_SM2 && (gl_info->supported[EXT_GPU_SHADER4] > || gl_info->glsl_version >= MAKEDWORD_VERSION(1, 30))) > - level = 10; > + level = WINED3D_D3D_LEVEL_10; > > return level; > } ... > - if (d3d_level >= 9 && gl_info->supported[NV_VERTEX_PROGRAM3]) > + if (d3d_level >= WINED3D_D3D_LEVEL_9_SM3) You never return WINED3D_D3D_LEVEL_9_SM3 from d3d_level_from_gl_info(). I'd recommend to just introduce WINED3D_D3D_LEVEL_9 in this patch, and split it up in _SM2 and _SM3 in a separate patch.

2 2

Re: [PATCH 5/5] wined3d: Move the "draw_binding" field from struct wined3d_surface to struct wined3d_resource.
by Stefan Dösinger Aug. 4, 2014

Aug. 4, 2014

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Am 2014-08-04 12:17, schrieb Henri Verbeet: > @@ -2029,6 +2029,7 @@ struct wined3d_resource > DWORD usage; > enum wined3d_pool pool; > DWORD access_flags; > + DWORD draw_binding; Will this stay in wined3d_resource, or do you plan to move this to wined3d_(render_target_)view later on? -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQIcBAEBAgAGBQJT35NhAAoJEN0/YqbEcdMwKKIP/32Zs+FGhMdt06llhXpNY34v /HnQyeTpQOlyHH4Q6xZT/2gg3I5v9L75Vrtryfgh/K5TBCIhMxP1inbPs9mooumz 67+A8X6cUZ0eSONlj/KT8qrk8sloy06nRfhh1IdZSdhPV0VUUFqC3HV01+BwaDdU Nz8lr/p2DGpUQkW5SjCjNNCHUEIRorvsoioZStIHAKNaEvlVwM+V0E6+u2lPKntz qhuT+D8kLq85E36TRc/zBkqYeFI0v9mnPymczG/gdU7bXQcz/KapqRvLoINCtZgF UM8o//uMkMkcepP4Zl6nPi5eYcJ7e/gOFioI6NkHQ87E1LNmmF9PljU/YyyGNcFP Y8xRQ+toSqZlNQ2lsP68r1jIGnH2ueW9LuawP6r6HxiWEXM0HzMs45F4X1VeBRa9 uZDADSaGbgmUJ73ceLAY1tNMMpuv8cBB6SJf7tW9rIRZNcSeV0U2O7O5Qb2t9DYh gLU5BqsTtVa+WiakO4HVKejsT8J2y/qqrdAg2bS8TbsrKjJIr8K7warUtekkIPIc t8hzrwhSqrdPdV3Xozya6tsA6qL2hgYS0vTYllJJ2TlCqmG71dWXIg9DXQ3CnAp6 RiwdAmpZiplwoLqBJn/k433xQmh5T8lHr74Jiw0V56f+aOktzp2udWzR2d4X9TI8 cH5SReslhRG86g2cTPbv =F1T9 -----END PGP SIGNATURE-----

2 1

Re: [PATCH 1/2] ddraw/tests: Ignore broken behavior of D3DTRANSFORM_UNCLIPPED.
by Henri Verbeet Aug. 4, 2014

Aug. 4, 2014

On 31 July 2014 00:56, Erich E. Hoover <erich.e.hoover(a)gmail.com> wrote: > On new versions of Windows this test no longer behaves as expected > (clearly somebody at MS introduced a bug). Spotted while working on > Patch 2. > The test seems to be of somewhat questionable usefulness to begin with. Stefan, since you wrote this, is there any application that actually depends on this? Any value in keeping the test?

2 1