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

Re: [PATCH 2/5] d3d10core: Implement d3d10_device_CopyResource().
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: > + if (dst_resource->type != WINED3D_RTYPE_TEXTURE) > + { > + FIXME("Not implemented for %s resources.\n", debug_d3dresourcetype(dst_resource->type)); > + return; > + } What's the point of implementing this based on textures and blits? It seems to me that a major restructuring will be needed to make this work on buffers. -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQIcBAEBAgAGBQJT35GOAAoJEN0/YqbEcdMwljwQAI2wEkyQHnUbwVIc6wV1bkm6 25ySqo8ZC27L72eHHPpA6nBNDObcbN19MpnTGh57cK1xzt6aC/KBWwCotXkbgKzK almnHatukw7Nkms1jMGn7C0ROQRcdBC4VHKpeINKcCdTxfosm81NOpj9eofy0hB1 BGqWzAhS9oOKGmQZznIHeWJJNuRe5skvsOVvSg4bbaHGi1528BgKLLxRtlFfbXbO LNT3SnGvAI9jJC2lJEJ1SH62NkPXJmh8gIm5ptGS2JNzkT7EjNQyxqBJ1uTjlsAH jyIX1+XZx+H950ucH1RktOyccowPzi1TY2qe6nBDDr4G5u/mgfzXcGbHcUd1G51K DPP09g4NdaPExjfi40c0CgD0xRNgu7JInhzqLiBeEU2LBiXiSNC+wR8oXSlOroMj rC93/J2CqJ+7emZ1SQSq+zQECdaqscQ11iPPQGfKkwxlm8XwF+knMh99hnyknaQs wTbRh9Fp0OzM7cs81E3GL/KxJ+pmqctLBZSVAn6VHAXs06d3BQ+moX5D7sZaopQ/ y2U83wECb3DwSHp1ihbhoM6en9CWxdkLn+JqA8VRLiS803el7e7QvpmRzR7t8+VA O4jvo6v7TWszpsL2HbuyCJeOnWsVXAjIxlB2Eok8CI+FfYc0S20QCBMaFw1ixVIr BiyVPU9VwG/qicUSK53R =5T4j -----END PGP SIGNATURE-----

2 2

Re: [PATCH 2/2] ddraw: Return empty D3D hardware flags for HEL device enumeration.
by Henri Verbeet Aug. 4, 2014

Aug. 4, 2014

On 31 July 2014 00:57, Erich E. Hoover <erich.e.hoover(a)gmail.com> wrote: > @@ -537,12 +537,16 @@ static HRESULT WINAPI enumDevicesCallback(GUID *Guid, char *DeviceDescription, > > ok(hal->dcmColorModel == 0, "RGB Device %u hal caps has colormodel %u\n", ver, hal->dcmColorModel); > ok(hel->dcmColorModel == D3DCOLOR_RGB, "RGB Device %u hel caps has colormodel %u\n", ver, hel->dcmColorModel); > + > + ok(hal->dwFlags == 0, "RGB Device %u has hardware flags %x\n", ver, hal->dwFlags); > } > else if(IsEqualGUID(&IID_IDirect3DHALDevice, Guid)) > { > trace("HAL Device %d\n", ver); > ok(hal->dcmColorModel == D3DCOLOR_RGB, "HAL Device %u hal caps has colormodel %u\n", ver, hel->dcmColorModel); > ok(hel->dcmColorModel == 0, "HAL Device %u hel caps has colormodel %u\n", ver, hel->dcmColorModel); > + > + ok(hal->dwFlags != 0, "HAL Device %u has hardware flags %x\n", ver, hal->dwFlags); > } In principle this patch looks plausible enough. What about hel->dwFlags though, and what about other device types like Ref, MMX, etc.?

1 0

Re: [2/2]dwrite: basic implementation for FontCollectionLoader registration
by Nikolay Sivov Aug. 4, 2014

Aug. 4, 2014

> + IDWriteFontCollectionLoader **CollectionLoaders; > + LONG CollectionLoaderCount; I'd really like a better naming for those, something like 'loaders' and 'loaders_count'. > + if (i == This->CollectionLoaderCount) > + { > + This->CollectionLoaderCount++; > + if (This->CollectionLoaders) > + This->CollectionLoaders = heap_realloc(This->CollectionLoaders, This->CollectionLoaderCount * sizeof(IDWriteFontCollectionLoader*)); > + else > + This->CollectionLoaders = heap_alloc(This->CollectionLoaderCount * sizeof(IDWriteFontCollectionLoader*)); > + if (!This->CollectionLoaders) > + return E_OUTOFMEMORY; > + } > + IDWriteFontCollectionLoader_AddRef(loader); > + This->CollectionLoaders[i] = loader; This should grow more efficiently in my opinion. Also it probably makes sense to have small number of slots allocated on initial factory creation or even use a list (you don't need index access). Another question is do we want to register a default local loader on factory creation or not.

2 3

Re: [2/2]dwrite: basic implementation for FontCollectionLoader registration
by Piotr Caban Aug. 4, 2014

Aug. 4, 2014

On 8/1/14 12:01 PM, Aric Stewart wrote: > + if (i == This->CollectionLoaderCount) > + { > + This->CollectionLoaderCount++; > + if (This->CollectionLoaders) > + This->CollectionLoaders = heap_realloc(This->CollectionLoaders, This->CollectionLoaderCount * sizeof(IDWriteFontCollectionLoader*)); > + else > + This->CollectionLoaders = heap_alloc(This->CollectionLoaderCount * sizeof(IDWriteFontCollectionLoader*)); > + if (!This->CollectionLoaders) > + return E_OUTOFMEMORY; > + } There's a leak in out of memory case. Isn't it better to grow the buffer exponentially?

3 2

What happened to the Fedora packages?
by Neal Gompa Aug. 2, 2014

Aug. 2, 2014

What happened to the Fedora packages? They have not been updated since 0.9.2!!!! Right now it is at 0.9.10!!! Nearly every other Linux distro supported has the up to date packages!!! And why does the Red Hat packages site not go to the SourceForge site as it does for SUSE packages and the others?? I have not really had the guts to ask until now, because I thought that maybe there was a slump, but now, its getting annoying!! And Fedora just released Fedora Core 5 yesterday!!! Please tell me new packages will be ready soon!!! Compiling WINE always crashes my computer, so I prefer to use the RPMs...

10 18