April 2015 - Wine-Devel - list.winehq.org

Re: d3drm: Fix refcounting for IDirect3DRM{1-3}

by Aaryaman Vasishta

On Thu, Apr 30, 2015 at 1:11 PM, Stefan Dösinger <stefandoesinger(a)gmail.com> wrote: > -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA1 > > Am 2015-04-29 um 23:47 schrieb Aaryaman Vasishta: > > - if (!refcount) > > + if (!refcount && !InterlockedDecrement(&d3drm->numIfaces)) > > HeapFree(GetProcessHeap(), 0, d3drm); > This works, but this way you have the code that destroys the object 3 > times. Not much of an issue as long as it is just a HeapFree, but I > think it would be nicer to have a separate function. Either add a > d3drm_destroy function or a pair of d3drm_add_iface and > d3drm_release_iface functions that also take care of > InterlockedIncrement / Decrement for iface_count) > > > - { &IID_IDirect3DRM3, &IID_IDirect3DRM3, > S_OK, TRUE }, > > - { &IID_IDirect3DRM2, &IID_IDirect3DRM2, > S_OK, TRUE }, > > + { &IID_IDirect3DRM3, &IID_IDirect3DRM3, > S_OK, FALSE }, > > + { &IID_IDirect3DRM2, &IID_IDirect3DRM2, > S_OK, FALSE }, > Afaics all the TODOs are gone, so you can remove this field from the > table and the todo_wine code from test_qi(). Either in the same patch > or a separate one. > I was thinking to remove the TODO's in a separate patch, as both the frame tests and d3drm tests use it. Is that okay? Hope AJ agrees with this change :) > > > - LONG ref; > > + LONG ref1, ref2, ref3, numIfaces; > > }; > Please use iface_count. Yeah, it's just a plain style thing, but d3drm > is nicely consistent so far in not using CamelCase (except in cases > where Microsoft determined the name). > > (Yesterday on IRC I said num_ifaces. Henri says xxx_count is the way > he prefers.) > I don't like CamelCase either, glad to use iface_count :) > > > + if (refcount == 1) InterlockedIncrement(&d3drm->numIfaces); > Another minor style issue: Please put the statement on a separate > line, in consistency with the existing code. > -----BEGIN PGP SIGNATURE----- > Version: GnuPG v2 > I'll make the changes and resend the patch as try-2 Thank you! Jam > > iQIcBAEBAgAGBQJVQdzBAAoJEN0/YqbEcdMw5WAP/0H52sqgEXMdSmh6sXH/dZMQ > nTZY9uRhElUbb3r6D6HG0ylit994pPaq887t9BSl8F4vbjsPXLjruDbVSwoScCfp > rIDKy9xET+ek4p5TSFC9cX1BFAiC2VKN7YYWmr/wHKVPZc+aFudU9ykac02bi9p2 > Oe+9wMBcFbwv33UhNL+2dugA4+0yj6egfPJWmju9C6ff6CPGqgXH70rHsrSf1AYs > VkNPRhGpfI9BxojBChVtgc0zwPM1HnfOwfmKWCW75YNgNg9lL0o+bdoy2wCcxiLq > FYwLLT2o9K+RnPQqRDhdJBKWwY8VrJI/fHZEURRau7RW1ia26j4UrrsAdXTQwhPs > Iy8A63pYOxyZsphq5KJ8cwGABYIlBPUG0lXZQRgKh1Jw8f39HIR5cfnu1at/fPSc > Cw6tfL/t+g4KeHaZDWZnROwyp704kIZGgvSaPuJjwkPtHh2MN14cp9KQj389kCUM > 4KfV6FxsAByRuJ8ur1HSaV2OeIOKOJkaDTzaiiMqc581A0awOeCy5SuAUSrQu4IE > 4gAIjryIJXd+kTDw46y6OW5UNXHdNJKGWAMiih25yLBi0BLBfUiW0U7qW5Lcerls > e7zefeUYNT+Umj3WU4sDu6B8B6auSZgKrLu9JAwYH2RUhKX9R+z3uTY25VUbJkoP > 9WJfJJuosIA5zqLbjxu+ > =/BI/ > -----END PGP SIGNATURE----- >

April 30, 2015

Re: d3drm: Fix refcounting for IDirect3DRM{1-3}

by Stefan Dösinger

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Am 2015-04-29 um 23:47 schrieb Aaryaman Vasishta: > - if (!refcount) > + if (!refcount && !InterlockedDecrement(&d3drm->numIfaces)) > HeapFree(GetProcessHeap(), 0, d3drm); This works, but this way you have the code that destroys the object 3 times. Not much of an issue as long as it is just a HeapFree, but I think it would be nicer to have a separate function. Either add a d3drm_destroy function or a pair of d3drm_add_iface and d3drm_release_iface functions that also take care of InterlockedIncrement / Decrement for iface_count) > - { &IID_IDirect3DRM3, &IID_IDirect3DRM3, S_OK, TRUE }, > - { &IID_IDirect3DRM2, &IID_IDirect3DRM2, S_OK, TRUE }, > + { &IID_IDirect3DRM3, &IID_IDirect3DRM3, S_OK, FALSE }, > + { &IID_IDirect3DRM2, &IID_IDirect3DRM2, S_OK, FALSE }, Afaics all the TODOs are gone, so you can remove this field from the table and the todo_wine code from test_qi(). Either in the same patch or a separate one. > - LONG ref; > + LONG ref1, ref2, ref3, numIfaces; > }; Please use iface_count. Yeah, it's just a plain style thing, but d3drm is nicely consistent so far in not using CamelCase (except in cases where Microsoft determined the name). (Yesterday on IRC I said num_ifaces. Henri says xxx_count is the way he prefers.) > + if (refcount == 1) InterlockedIncrement(&d3drm->numIfaces); Another minor style issue: Please put the statement on a separate line, in consistency with the existing code. -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBAgAGBQJVQdzBAAoJEN0/YqbEcdMw5WAP/0H52sqgEXMdSmh6sXH/dZMQ nTZY9uRhElUbb3r6D6HG0ylit994pPaq887t9BSl8F4vbjsPXLjruDbVSwoScCfp rIDKy9xET+ek4p5TSFC9cX1BFAiC2VKN7YYWmr/wHKVPZc+aFudU9ykac02bi9p2 Oe+9wMBcFbwv33UhNL+2dugA4+0yj6egfPJWmju9C6ff6CPGqgXH70rHsrSf1AYs VkNPRhGpfI9BxojBChVtgc0zwPM1HnfOwfmKWCW75YNgNg9lL0o+bdoy2wCcxiLq FYwLLT2o9K+RnPQqRDhdJBKWwY8VrJI/fHZEURRau7RW1ia26j4UrrsAdXTQwhPs Iy8A63pYOxyZsphq5KJ8cwGABYIlBPUG0lXZQRgKh1Jw8f39HIR5cfnu1at/fPSc Cw6tfL/t+g4KeHaZDWZnROwyp704kIZGgvSaPuJjwkPtHh2MN14cp9KQj389kCUM 4KfV6FxsAByRuJ8ur1HSaV2OeIOKOJkaDTzaiiMqc581A0awOeCy5SuAUSrQu4IE 4gAIjryIJXd+kTDw46y6OW5UNXHdNJKGWAMiih25yLBi0BLBfUiW0U7qW5Lcerls e7zefeUYNT+Umj3WU4sDu6B8B6auSZgKrLu9JAwYH2RUhKX9R+z3uTY25VUbJkoP 9WJfJJuosIA5zqLbjxu+ =/BI/ -----END PGP SIGNATURE-----

April 30, 2015

Re: kernel32/tests: Add tests for deleting readonly files with NtCreateFile.

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=13137 Your paranoid android. === wvistau64 (32 bit file) === file.c:444: Test failed: attributes of file "testfile.xxx" are 0x0820 === w7u (32 bit file) === file.c:4052: Test failed: SetFileValidData failed 87 file.c:4055: Test failed: SetFileValidData failed 87 === wvistau64 (64 bit file) === file.c:444: Test failed: attributes of file "testfile.xxx" are 0x0820

April 30, 2015

Re: [3/3] ws2_32: Properly handle closing sockets during a select call.

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=13135 Your paranoid android. === w2008s64 (64 bit sock) === No test summary line found

April 30, 2015

Re: shell32: Add caption to run dialog (resend)

by Alex Henrie

Thanks for your interest in developing Wine! A few pointers: You are supposed to use your name when submitting patches. I don't think Alexandre will accept just your initials. Also, you should avoid putting all that HTML garbage into the patch by using `git format-patch` and `git send-email`. If you cannot use git send-email, send the patch as an attachment. Please read http://wiki.winehq.org/SubmittingPatches Finally, your patch is more likely to be accepted if you can give an example of a Windows program that demonstrates the problem you're trying to fix. -Alex

April 30, 2015

Re: [PATCH 2/5] ddraw: Release the surface clipper (if any) in ddraw_surface_wined3d_object_destroyed() (Valgrind).

by Matteo Bruni

2015-04-29 20:23 GMT+02:00 Marvin <testbot(a)winehq.org>: > 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=13125 > > Your paranoid android. > > > === w8 (32 bit ddraw1) === > ddraw1.c:1373: Test failed: Expected color 0x00ff0000 for test 0, got 0x0000ff00. > ddraw1.c:1392: Test failed: Expected color 0x000000ff for test 0, got 0x0000ff00. > ddraw1.c:6666: Test failed: Got unexpected color 0x000000ff. > ddraw1.c:6670: Test failed: Got unexpected color 0x00000080. FWIW, I have a number of "fixes" for those intermittent test failures, I'll eventually get to them...

April 29, 2015

Re: Strange wined3d crash

by Henri Verbeet

On 29 April 2015 at 19:02, Matteo Bruni <matteo.mystral(a)gmail.com> wrote: > Essentially it crashes in shader_vaddline() as soon as we have to > resize the buffer because vsnprintf() tries to use "args" for a second > time. Oops. I guess this really got broken with cf12f51b1520546dc855e8e8e43135a06e7a0cbf already, but the larger initial buffer size made it work most of the time. > I attached a fix. Henri, is something like that okay with you? In principle, but I think va_copy() is C99; I'm not sure we can rely on it being present. Since shader_vaddline() only has a couple of callers, it may be better to just move the resizing code into a helper function and use that from the callers.

April 29, 2015

Re: [PATCH 2/5] ddraw: Release the surface clipper (if any) in ddraw_surface_wined3d_object_destroyed() (Valgrind).

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=13125 Your paranoid android. === w8 (32 bit ddraw1) === ddraw1.c:1373: Test failed: Expected color 0x00ff0000 for test 0, got 0x0000ff00. ddraw1.c:1392: Test failed: Expected color 0x000000ff for test 0, got 0x0000ff00. ddraw1.c:6666: Test failed: Got unexpected color 0x000000ff. ddraw1.c:6670: Test failed: Got unexpected color 0x00000080.

April 29, 2015

Re: Wine 64 bit

by Charles Davis

> On Apr 29, 2015, at 7:25 AM, Josh DuBois <duboisj(a)codeweavers.com> wrote: > > On 4/29/15 12:40 AM, Charles Davis wrote: >> I don’t use Apple’s broken as(1), because I patch Clang, whose integrated assembler is decidedly un-broken. That’s why I never submitted a patch for that. (In fact, it’s possible to make as(1) invoke Clang to do the assembly, if you pass it the ‘-q’ switch.) > Ah. Passing -q to as gave me a raft of errors the first time I tried. Clang's assembler choked on unrecognized .stabs directives and would not assemble, but removing -gstabs+ from the compile line allows clang to assemble relay.s without changing the movq instructions by passing -q to as. I'm not familiar with the impact, if any, that will have on debugging. The problem is that dbghelp doesn’t yet know how to find dSYM companions, or that it can read the DWARF data from the object files. (That’s why we don’t use DWARF yet on Mac OS.) The latter is somewhat harder, because any addresses in the DWARF data need to be fixed up to match up with their final locations. For this reason, the linker leaves a small amount of stabs data in the final binary. (Ironic, isn’t it?) > > It still seems useful to me to be able to assemble with Apples as and compile with gcc. Is there harm in changing the various movq instructions to be compatible with Apple's as? I’m tempted to say that your fix will reduce the performance of the relays slightly (since we’re loading the XMM registers from memory instead of from registers), but then it occurred to me that since we loaded that chunk of memory recently anyway, it should be cached. My other concern is that as(1) is basically in maintenance mode now that we have Clang. (That’s why we still have bugs in it like the issue with ‘movq’ from registers.) I think Apple at some point wants to kill it off—or at least make -q the default. >> That’s great! In fact, that’s actually further than I get building with Clang—even with my patches applied to it. (It doesn’t help that Clang produces broken DWARF unwind info for Win64 ABI functions, which I really need to fix…) >> >> By the way, I can tell you right now that WoW64 won’t work quite yet. > Someone on bug 38380 claims to have gotten even father and successfully run 64-bit windows code. All of the 64-bit windows apps I tried seemed to have 32-bit installers, so I guess WoW64 is really necessary to give it a good try. All right. But even with AJ’s suggestion (using separate wine32/wine64 directories for our DLLs), libwine itself still needs to be built fat, because it’s installed directly to $(libdir). For now, I think I’ll submit my patch to configure. Chip

April 29, 2015

Re: Strange wined3d crash

by Matteo Bruni

2015-04-29 18:03 GMT+02:00 Francois Gouget <fgouget(a)free.fr>: > On Wed, 29 Apr 2015, Henri Verbeet wrote: > >> On 29 April 2015 at 12:40, Francois Gouget <fgouget(a)free.fr> wrote: >> > I am getting a strange wined3d crash during the d3d8:device test. >> > >> Is that a regression? We did touch the shader generation code recently. > > It is actually. This is the patch that introduced the crash: > > commit 28343db2083de7b4afe4079da090520f44b5544d > Author: Matteo Bruni <mbruni(a)codeweavers.com> > Date: Mon Apr 27 17:39:12 2015 +0200 > > wined3d: Don't use the builtin FFP uniforms for the lights. > > While at it, use D3D-like uniforms for the light parameters and > (consequently) implement range and proper D3D spotlights support. > > > I can test patches but I'm not sure I'll have time to debug this much > further for a while :-( Note that this is with an Intel HD Graphics 4600 > GPU. Thanks Francois, I can reproduce. Essentially it crashes in shader_vaddline() as soon as we have to resize the buffer because vsnprintf() tries to use "args" for a second time. I attached a fix. Henri, is something like that okay with you?

April 29, 2015