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

Re: [PATCH 2/4] d3d9/tests: Test that Direct3D9 doesn't modify the pixel format of the window it targets. (try 4)

by Henri Verbeet

On 19 February 2014 04:30, Ken Thomases <ken(a)codeweavers.com> wrote: > I guess this may be less efficient than we'd want. Ideally, as little WGL state as possible would be checked other than when context->level transitions from 0 to 1. context_enter() could check it at that time. Rather than using restore_ctx being non-NULL as a proxy for whether the context needs to be set, we'd use a separate, explicit flag: > > diff --git a/dlls/wined3d/context.c b/dlls/wined3d/context.c > index 4679b25..a0d078c 100644 > --- a/dlls/wined3d/context.c > +++ b/dlls/wined3d/context.c > @@ -1109,6 +1109,7 @@ void context_release(struct wined3d_context *context) > context->restore_ctx = NULL; > context->restore_dc = NULL; > } > + context->needs_set = 0; > } > } > > @@ -1127,7 +1128,10 @@ static void context_enter(struct wined3d_context *context) > current_gl, wglGetCurrentDC()); > context->restore_ctx = current_gl; > context->restore_dc = wglGetCurrentDC(); > + context->needs_set = 1; > } > + else if (context->pixel_format != GetPixelFormat(context->hdc)) > + context->needs_set = 1; > } > } > > @@ -3051,7 +3055,7 @@ struct wined3d_context *context_acquire(const struct wined3d_device *device, str > if (!context_set_current(context)) > ERR("Failed to activate the new context.\n"); > } > - else if (context->restore_ctx) > + else if (context->needs_set) > { > context_set_gl_context(context); > } > diff --git a/dlls/wined3d/wined3d_private.h b/dlls/wined3d/wined3d_private.h > index 0a1511b..0ceb6f1 100644 > --- a/dlls/wined3d/wined3d_private.h > +++ b/dlls/wined3d/wined3d_private.h > @@ -1085,7 +1085,8 @@ struct wined3d_context > DWORD fixed_function_usage_map : 8; /* MAX_TEXTURES, 8 */ > DWORD lowest_disabled_stage : 4; /* Max MAX_TEXTURES, 8 */ > DWORD rebind_fbo : 1; > - DWORD padding : 19; > + DWORD needs_set : 1; > + DWORD padding : 18; > DWORD shader_update_mask; > DWORD constant_update_mask; > DWORD numbered_array_mask; I think that mostly works, but you'll probably want to clear needs_set in context_set_gl_context() instead of in context_release(). I think there's also an argument somewhere that context_update_window() should set needs_set instead of calling context_set_pixel_format() and context_set_gl_context().

Feb. 19, 2014

Patch to fix bug 34324

by Zhenbo Li

Hello, I've been working on http://bugs.winehq.org/show_bug.cgi?id=34324. Till now, I have found two solutions, one is simple and one is complex. Both of them would fix #34324, and I don't know which one is better. As Alexandre Julliard mentioned[1], I would use the value of GetLastError() as the return (error) value in delete_files(). The simple patch would fix part of the 'todo_wine' marks. Meanwhile, the complex patch would fix *all* todo marks about return value in test_delete(). I really appreciate your help (especially for Dmitry Timoshkov, thank you very much). And I'm looking for your feedbacks. Thank you. [1]: http://markmail.org/message/fblet7gadeusyauv#query:+page:1+mid:6eu2a72uuchh… -- Have a nice day! Zhenbo Li

Feb. 19, 2014

Re: msi: Dont crash on installation of services without component KeyPath

by Hans Leidekker

On Wed, 2014-02-19 at 17:57 +0900, Aric Stewart wrote: > Well the image path in the component is optional and looking at the documentation > for CreateServiceW lpBinaryPathName is also optional. So it seemed correct. > > If you feel like we should just exit then that is acceptable as well. What happens on Windows?

Feb. 19, 2014

Re: msi: Dont crash on installation of services without component KeyPath

by Aric Stewart

Well the image path in the component is optional and looking at the documentation for CreateServiceW lpBinaryPathName is also optional. So it seemed correct. If you feel like we should just exit then that is acceptable as well. -aric On 2014/02/19 17:51, Hans Leidekker wrote: > On Wed, 2014-02-19 at 14:49 +0900, Aric Stewart wrote: >> diff --git a/dlls/msi/action.c b/dlls/msi/action.c >> index 07322fe..a004225 100644 >> --- a/dlls/msi/action.c >> +++ b/dlls/msi/action.c >> @@ -5794,27 +5794,30 @@ static UINT ITERATE_InstallService(MSIRECORD *rec, LPVOID param) >> goto done; >> } >> key = MSI_RecordGetString(row, 6); >> - file = msi_get_loaded_file(package, key); >> - msiobj_release(&row->hdr); >> - if (!file) >> - { >> - ERR("Failed to load the service file\n"); >> - goto done; >> - } >> - >> - if (!args || !args[0]) image_path = file->TargetPath; >> - else >> + if (key) >> { >> - int len = strlenW(file->TargetPath) + strlenW(args) + 2; >> - if (!(image_path = msi_alloc(len * sizeof(WCHAR)))) >> + file = msi_get_loaded_file(package, key); >> + msiobj_release(&row->hdr); >> + if (!file) >> { >> - ret = ERROR_OUTOFMEMORY; >> + ERR("Failed to load the service file\n"); >> goto done; >> } >> >> - strcpyW(image_path, file->TargetPath); >> - strcatW(image_path, szSpace); >> - strcatW(image_path, args); >> + if (!args || !args[0]) image_path = file->TargetPath; >> + else >> + { >> + int len = strlenW(file->TargetPath) + strlenW(args) + 2; >> + if (!(image_path = msi_alloc(len * sizeof(WCHAR)))) >> + { >> + ret = ERROR_OUTOFMEMORY; >> + goto done; >> + } >> + >> + strcpyW(image_path, file->TargetPath); >> + strcatW(image_path, szSpace); >> + strcatW(image_path, args); >> + } >> } >> service = CreateServiceW(hscm, name, disp, GENERIC_ALL, serv_type, >> start_type, err_control, image_path, load_order, >> @@ -5831,7 +5834,7 @@ static UINT ITERATE_InstallService(MSIRECORD *rec, LPVOID param) >> WARN("failed to set service description %u\n", GetLastError()); >> } >> >> - if (image_path != file->TargetPath) msi_free(image_path); >> + if (image_path && image_path != file->TargetPath) msi_free(image_path); >> done: >> CloseServiceHandle(service); >> CloseServiceHandle(hscm); > > Should we still create the service if the installer doesn't specify the image path? > >

Feb. 19, 2014

Re: msi: Dont crash on installation of services without component KeyPath

by Hans Leidekker

On Wed, 2014-02-19 at 14:49 +0900, Aric Stewart wrote: > diff --git a/dlls/msi/action.c b/dlls/msi/action.c > index 07322fe..a004225 100644 > --- a/dlls/msi/action.c > +++ b/dlls/msi/action.c > @@ -5794,27 +5794,30 @@ static UINT ITERATE_InstallService(MSIRECORD *rec, LPVOID param) > goto done; > } > key = MSI_RecordGetString(row, 6); > - file = msi_get_loaded_file(package, key); > - msiobj_release(&row->hdr); > - if (!file) > - { > - ERR("Failed to load the service file\n"); > - goto done; > - } > - > - if (!args || !args[0]) image_path = file->TargetPath; > - else > + if (key) > { > - int len = strlenW(file->TargetPath) + strlenW(args) + 2; > - if (!(image_path = msi_alloc(len * sizeof(WCHAR)))) > + file = msi_get_loaded_file(package, key); > + msiobj_release(&row->hdr); > + if (!file) > { > - ret = ERROR_OUTOFMEMORY; > + ERR("Failed to load the service file\n"); > goto done; > } > > - strcpyW(image_path, file->TargetPath); > - strcatW(image_path, szSpace); > - strcatW(image_path, args); > + if (!args || !args[0]) image_path = file->TargetPath; > + else > + { > + int len = strlenW(file->TargetPath) + strlenW(args) + 2; > + if (!(image_path = msi_alloc(len * sizeof(WCHAR)))) > + { > + ret = ERROR_OUTOFMEMORY; > + goto done; > + } > + > + strcpyW(image_path, file->TargetPath); > + strcatW(image_path, szSpace); > + strcatW(image_path, args); > + } > } > service = CreateServiceW(hscm, name, disp, GENERIC_ALL, serv_type, > start_type, err_control, image_path, load_order, > @@ -5831,7 +5834,7 @@ static UINT ITERATE_InstallService(MSIRECORD *rec, LPVOID param) > WARN("failed to set service description %u\n", GetLastError()); > } > > - if (image_path != file->TargetPath) msi_free(image_path); > + if (image_path && image_path != file->TargetPath) msi_free(image_path); > done: > CloseServiceHandle(service); > CloseServiceHandle(hscm); Should we still create the service if the installer doesn't specify the image path?

Feb. 19, 2014

Re: Failing dpnet tests

by Alistair Leslie-Hughes

Hi Francois, The issue is when it run on the test bot. This patch removed all tests and traces around CoCreateInstance. https://newtestbot.winehq.org/JobDetails.pl?Key=4949&log_201=1#k201 Its timing out. I've seen this on Linux when mixing native dpnsvr.exe and builtin dpnet.dll due to the missing callback in dpnet.dll (not a windows issue). If dpnsvr uses the local address instead of the loopback then this cause the timeout. Would it be possible to test windows 8 without the firewall enabled? It's going to be hard to get patches accepted if win8 keeps failing. Best Regards Alistair Leslie-Hughes

Feb. 19, 2014

Re: [PATCH 2/4] d3d9/tests: Test that Direct3D9 doesn't modify the pixel format of the window it targets. (try 4)

by Ken Thomases

On Feb 18, 2014, at 7:53 PM, Ken Thomases wrote: > This fixes it, although it may be more crude than we'd like: > - else if (context->restore_ctx) > + else if (context->restore_ctx || context->pixel_format != GetPixelFormat(wglGetCurrentDC())) On second thought, that would be unreliable. It depends on the app's DC still being valid, which it may not be. We'd have to check using a DC for the window that would actually be used if the context were to be made current. (I think we can ignore the swapchain backup DC, since that's used when the pixel format can't be set on context->hdc and the condition will be true, which is what we want.) So: else if (context->restore_ctx || context->pixel_format != GetPixelFormat(context->hdc)) I guess this may be less efficient than we'd want. Ideally, as little WGL state as possible would be checked other than when context->level transitions from 0 to 1. context_enter() could check it at that time. Rather than using restore_ctx being non-NULL as a proxy for whether the context needs to be set, we'd use a separate, explicit flag: diff --git a/dlls/wined3d/context.c b/dlls/wined3d/context.c index 4679b25..a0d078c 100644 --- a/dlls/wined3d/context.c +++ b/dlls/wined3d/context.c @@ -1109,6 +1109,7 @@ void context_release(struct wined3d_context *context) context->restore_ctx = NULL; context->restore_dc = NULL; } + context->needs_set = 0; } } @@ -1127,7 +1128,10 @@ static void context_enter(struct wined3d_context *context) current_gl, wglGetCurrentDC()); context->restore_ctx = current_gl; context->restore_dc = wglGetCurrentDC(); + context->needs_set = 1; } + else if (context->pixel_format != GetPixelFormat(context->hdc)) + context->needs_set = 1; } } @@ -3051,7 +3055,7 @@ struct wined3d_context *context_acquire(const struct wined3d_device *device, str if (!context_set_current(context)) ERR("Failed to activate the new context.\n"); } - else if (context->restore_ctx) + else if (context->needs_set) { context_set_gl_context(context); } diff --git a/dlls/wined3d/wined3d_private.h b/dlls/wined3d/wined3d_private.h index 0a1511b..0ceb6f1 100644 --- a/dlls/wined3d/wined3d_private.h +++ b/dlls/wined3d/wined3d_private.h @@ -1085,7 +1085,8 @@ struct wined3d_context DWORD fixed_function_usage_map : 8; /* MAX_TEXTURES, 8 */ DWORD lowest_disabled_stage : 4; /* Max MAX_TEXTURES, 8 */ DWORD rebind_fbo : 1; - DWORD padding : 19; + DWORD needs_set : 1; + DWORD padding : 18; DWORD shader_update_mask; DWORD constant_update_mask; DWORD numbered_array_mask; -Ken

Feb. 19, 2014

Re: Advice fixing #13683 (DirectX mouse overlay?)

by Christopher Thielen

Good point. Maybe knowing the offset isn't too useful for now. I still haven't figured out through what mechanism the game draws this missing cursor. The D3D9 cursor functions never make a sound in the logs and while the User32 LoadCursor/SetCursor/clipping stuff is called an awful lot on initialization, it looks like it's always for the system cursors. I also pasted in a method to dump to a .bmp file anything that goes through LoadIcon and wound up with one icon I don't recall and another which was simply Wine's icon. So, my hunch now is that the game draws the cursor using DirectX and uses User32 only for setting mouse clipping. Is there a decent way to dump as a file every DX resource loaded that's under a certain size? I'd like to confirm my theory and maybe if I'm lucky figure out which DX surface/texture is being drawn incorrectly (if any). - Christopher Thielen On 02/18/2014 12:16 AM, Stefan Dösinger wrote: > > Am 18.02.2014 um 06:12 schrieb Christopher Thielen <christopher(a)thielen.co>: >> Two questions: >> 1) Is there a way to add timestamps to the TRACE statements in Wine? > WINEDEBUG=+timestamp > > This adds timestamps to other traces, so you can do e.g. WINEDEBUG=d3d,timestamp to get a d3d trace with timestamps. > >> 2) If I know which file is opened by the game, is there a way to debug from that point on, i.e. capture the fopen() and enable +cursor,+resource,+d3d TRACES for say, only 5 seconds after that point? Something along those lines? > Afaik no. You could use some search and replace tricks in text editors or with command line text tools though. >> >> I can find the exact offset within the data file used to load the missing resource - but I'd need a way to know when a fread() was that far along. I figure if I can determine that I can determine if the cursor is loaded as a Windows cursor or as a D3D surface and go from there. > Note that there are plenty of ways to open and read files. fopen and fread are just one of them. There are win32 equivalents of those C functions, and there are specific resource loading functions which you’ve probably already seen. >

Feb. 19, 2014

Re: [PATCH 2/4] d3d9/tests: Test that Direct3D9 doesn't modify the pixel format of the window it targets. (try 4)

by Ken Thomases

On Feb 18, 2014, at 5:40 PM, Henri Verbeet wrote: > After thinking about it for a bit, I think the issue is essentially > that context_acquire() depends on context_enter() to set > context->restore_ctx when context_set_gl_context() needs to be called. > If only the pixel format is different context_set_gl_context() will > not be called. OK, I can see how that would happen. However, my wined3d patch didn't change that part of the logic. I think my test just exposed a pre-existing bug. It doesn't show up without my wined3d patch applied because, in that case, wined3d fails to restore the window's pixel format and leaves it with a double-buffered pixel format. The two bugs cancel each other out and fixing one allowed the other to be revealed. This fixes it, although it may be more crude than we'd like: diff --git a/dlls/wined3d/context.c b/dlls/wined3d/context.c index 4679b25..3d11c16 100644 --- a/dlls/wined3d/context.c +++ b/dlls/wined3d/context.c @@ -3051,7 +3056,7 @@ struct wined3d_context *context_acquire(const struct wined3d_device *device, str if (!context_set_current(context)) ERR("Failed to activate the new context.\n"); } - else if (context->restore_ctx) + else if (context->restore_ctx || context->pixel_format != GetPixelFormat(wglGetCurrentDC())) { context_set_gl_context(context); } -Ken

Feb. 19, 2014

Re: [PATCH 2/4] d3d9/tests: Test that Direct3D9 doesn't modify the pixel format of the window it targets. (try 4)

by Henri Verbeet

After thinking about it for a bit, I think the issue is essentially that context_acquire() depends on context_enter() to set context->restore_ctx when context_set_gl_context() needs to be called. If only the pixel format is different context_set_gl_context() will not be called.

Feb. 18, 2014