September 2017 - Wine-devel

Re: [1/2] ntdll: Implement set_cpu_context for ARM64
by Alexandre Julliard 07 Sep '17

07 Sep '17

André Hentschel <nerv(a)dawncrow.de> writes: > + "ldr x30, [x17, #0xf8]\n\t" > + "ldr x16, [x17, #0x100]\n\t" > + "mov sp, x16\n\t" > + "ldr x16, [x17, #0x108]\n\t" > + "br x16\n\t" You'd also need to restore x16 and x17, this is used in signal handlers. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: [v5 PATCH] wmvcore/tests: add tests for WMReader methods
by Alexandre Julliard 06 Sep '17

06 Sep '17

Jefferson Carpenter <jeffersoncarpenter2(a)gmail.com> writes: > Can somebody explain why this one wasn't committed? I'm not convinced we need a large binary file for this, and it also seems to take a long time to run mainly because it's just a bunch of timeouts at this point. I'd like to see the implementation being fleshed out a bit first. -- Alexandre Julliard julliard(a)winehq.org

2 3

Re: [v2 PATCH] d2d1: Partially implement StrokeContainsPoint() for rectangles
by Henri Verbeet 06 Sep '17

06 Sep '17

On 6 September 2017 at 12:35, Nikolay Sivov <nsivov(a)codeweavers.com> wrote: > -static void d2d_point_normalise(D2D1_POINT_2F *p) > +static float d2d_point_normalise(D2D1_POINT_2F *p) > { > float l; > > if ((l = sqrtf(d2d_point_dot(p, p))) != 0.0f) > d2d_point_scale(p, 1.0f / l); > + > + return l; > } Maybe. I'm not entirely convinced you need this, see below. > +static BOOL d2d_rect_point_on(const D2D1_POINT_2F *point, float tolerance, const D2D1_POINT_2F *points) > +{ > + unsigned int i; > + float dot; > + > + for (i = 0; i < 4; i++) > + { It may improve readability to pull that loop out into the caller, and make this function just about testing whether a point is on a line segment. > + float edge_length, distance; > + D2D1_POINT_2F v, edge; > + > + v.x = point->x - points[i].x; > + v.y = point->y - points[i].y; d2d_point_subtract(&v, point, &points[i]); Or, alternatively, d2d_point_subtract(&v_q, q, p0); > + /* Point to vertex distance. */ > + if (d2d_point_dot(&v, &v) < tolerance * tolerance) > + return TRUE; Maybe. An argument could probably be made for testing joins separately. > + /* Point to edge distance. */ > + edge.x = points[(i + 1) % 4].x - points[i].x; > + edge.y = points[(i + 1) % 4].y - points[i].y; d2d_point_subtract(&edge, &points[(i + 1) % 4], &points[i]); Or, d2d_point_subtract(&v_p, p1, p0); > + if ((edge_length = sqrtf(d2d_point_dot(&edge, &edge))) == 0.0f) > + continue; I guess we'd want to introduce d2d_point_length(). > + distance = fabsf(edge.x * v.y - edge.y * v.x) / edge_length; n.x = -v_p.y; n.y = v_p.x; distance = fabsf(d2d_point_dot(&v_q, &n)) / edge_length; > + d2d_point_normalise(&edge); > + dot = d2d_point_dot(&edge, &v); > + if (dot >= 0.0f && dot <= edge_length) > + return TRUE; If you compare against "edge_length * edge_length", you can drop the normalisation. > +static BOOL d2d_rect_point_inside(const D2D1_POINT_2F *point, const D2D1_POINT_2F *points) > +{ > + D2D1_POINT_2F vec[2]; > + unsigned int i; > + float dot; > + > + for (i = 0; i < 2; i++) > + { > + float l; > + > + vec[0].x = points[i + 1].x - points[i].x; > + vec[0].y = points[i + 1].y - points[i].y; > + vec[1].x = point->x - points[i].x; > + vec[1].y = point->y - points[i].y; > + > + if ((l = d2d_point_normalise(&vec[0])) == 0.0f) > + return FALSE; > + > + if ((dot = d2d_point_dot(&vec[0], &vec[1])) < 0.0f || dot > l) > + return FALSE; > + } > + > + return TRUE; > +} Some of the comments on d2d_rect_point_on() apply here as well. Perhaps more importantly, does this work with shear transformations? > + if (transform) > + { > + D2D1_POINT_2F outer[4], inner[4]; > + > + stroke_width *= 0.5f; > + > + d2d_point_transform(&inner[0], transform, rect->left + stroke_width, rect->top + stroke_width); > + d2d_point_transform(&inner[1], transform, rect->left + stroke_width, rect->bottom - stroke_width); > + d2d_point_transform(&inner[2], transform, rect->right - stroke_width, rect->bottom - stroke_width); > + d2d_point_transform(&inner[3], transform, rect->right - stroke_width, rect->top + stroke_width); > + > + d2d_point_transform(&outer[0], transform, rect->left - stroke_width, rect->top - stroke_width); > + d2d_point_transform(&outer[1], transform, rect->left - stroke_width, rect->bottom + stroke_width); > + d2d_point_transform(&outer[2], transform, rect->right + stroke_width, rect->bottom + stroke_width); > + d2d_point_transform(&outer[3], transform, rect->right + stroke_width, rect->top - stroke_width); This seems suspicious. In general, the stroke width isn't affected by transformations on the geometry. Test coverage for that would be nice in either case.

1 0

Re: [PATCH 2/3] include: Add interfaces IMFMediaTypeHandler, IMFStreamDescriptor
by Henri Verbeet 06 Sep '17

06 Sep '17

On 6 September 2017 at 09:38, Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com> wrote: > +[ > + object, > + uuid(e93dcf6c-4b07-4e1e-8123-aa16ed6eadf5), > +] > +interface IMFMediaTypeHandler : IUnknown > +{ ... > +}; Redundant ';'. Unfortunately we have that issue in a bunch of other IDLs as well, and the Microsoft IDLs don't necessarily always get it right either.

1 0

Re: [PATCH 3/5] wined3d: Store FLUSH flag in the query object.
by Henri Verbeet 06 Sep '17

06 Sep '17

On 6 September 2017 at 01:39, Matteo Bruni <mbruni(a)codeweavers.com> wrote: > So it can be used in the CS thread's query polling. Passing the > appropriate flag to glClientWaitSync() is potentially lighter on > performance than a separate, explicit glFlush(). Maybe. It would be debatable once there are multiple queries in-flight. > @@ -389,8 +389,8 @@ HRESULT CDECL wined3d_query_get_data(struct wined3d_query *query, > } > else if (query->counter_main != query->counter_retrieved) > { > - if (flags & WINED3DGETDATA_FLUSH) > - wined3d_cs_emit_flush(query->device->cs); > + if (flags & WINED3DGETDATA_FLUSH && !(query->flags & WINED3DGETDATA_FLUSH)) > + InterlockedExchange((LONG *)&query->flags, flags); Shouldn't the flag be cleared again when the query is restarted? How about the following: - wined3d_query_get_data() emits a flush when the counters don't match, "flags" contains WINED3DGETDATA_FLUSH, and query->flushed is FALSE. - wined3d_cs_exec_flush() sets query->flushed for all queries on cs->query_poll_list. - query->flushed is cleared when a query is removed from cs->query_poll_list. Would that work? It would leave a small window between wined3d_cs_emit_flush() and wined3d_cs_exec_flush() that could probably be avoided by checking in wined3d_query_get_data() if any queries were issued since the last flush. Not sure whether that's worth it. Alternatively, we could only do the latter. I.e., set cs->queries_flushed in wined3d_cs_emit_flush(), and clear it in wined3d_cs_emit_query_issue(). That wouldn't prevent all redundant flushes, but it would help for applications calling wined3d_query_get_data() in a tight loop.

1 0

Re: [PATCH 1/5] wined3d: Restore support for d3d9 "fake" formats.
by Henri Verbeet 06 Sep '17

06 Sep '17

On 6 September 2017 at 01:39, Matteo Bruni <mbruni(a)codeweavers.com> wrote: > @@ -4114,6 +4114,7 @@ extern enum wined3d_format_id pixelformat_for_depth(DWORD depth) DECLSPEC_HIDDEN > #define WINED3DFMT_FLAG_DEPTH 0x00000004 > #define WINED3DFMT_FLAG_STENCIL 0x00000008 > #define WINED3DFMT_FLAG_RENDERTARGET 0x00000010 > +#define WINED3DFMT_FLAG_FAKE 0x00000020 Could I suggest WINED3DFMT_FLAG_EXTENSION?

1 0

Re: [PATCH 01/10] atl80: Fix processorArchitecture in manifest
by Alexandre Julliard 06 Sep '17

06 Sep '17

Fabian Maurer <dark.shadow4(a)web.de> writes: > processorArchitecture should not be empty string I don't think that's correct for assemblies that can be imported. Note that the processorArchitecture is fixed up when creating the fake dll, so it doesn't end up empty in the resources. -- Alexandre Julliard julliard(a)winehq.org

2 5

Re: [PATCH] wined3d: wined3d_check_surface_capability return TRUE for surfaces with WINED3DFMT_FLAG_TEXTURE.
by Henri Verbeet 05 Sep '17

05 Sep '17

On 4 September 2017 at 18:26, lorenzo Skotos <lorenzofersteam(a)live.it> wrote: > - if (format->glInternal) > + if (format->glInternal || format->flags[WINED3D_GL_RES_TYPE_TEX_2D] & WINED3DFMT_FLAG_TEXTURE) That should fix the regression, but since the ambiguity of WINED3DFMT_FLAG_TEXTURE led to the issue in the first place, I'd rather not simply restore the check. Instead, I think it would be better to explicitly mark these special "extension" formats as such. In theory Matteo has a patch for that.

2 3

Re: [PATCH] ntdll: Make RtlDeregisterWaitEx(handle, INVALID_HANDLE_VALUE) thread safe.
by Stefan Dösinger 05 Sep '17

05 Sep '17

Hi, Is the test I was referring to in the description of the previous patch. It causes seemingly unrelated test failures like https://testbot.winehq.org/JobDetails.pl?Key=32956 (previous iteration of the test that used a different implementation) Stefan Am 2017-09-05 um 17:34 schrieb Stefan Dösinger: > Chromium signals the wait semaphore and calls DeregisterWaitEx with > CompletionHandle = INVALID_HANDLE_VALUE in close succession. Sometimes > the worker thread decides to run the callback, but before it sets > CallbackInProgress RtlDeregisterWaitEx decides that the callback is not > running and returns STATUS_SUCCESS. Chromium then releases resources > that the callback needs to run, resulting in random crashes. > > Signed-off-by: Stefan Dösinger <stefan(a)codeweavers.com> > > --- > > The tests show that we're only supposed to return STATUS_PENDING if the > callback is running. Note that the choice between waiting and returning > STATUS_SUCCESS and not waiting and returning STATUS_PENDING is still > racy, but we'll no longer return STATUS_SUCCESS and run the callback > afterwards. > > I tried to extend the tests to show what happens when both events are > available simultanously. The answer is that it is fairly random. > INVALID_HANDLE_VALUE blocks and runs the callback one last time, the > other ways of calling RtlDeregisterWait[Ex] randomly return STATUS_SUCCESS > and do not run the callback, or they return STATUS_PENDING and may or > may not run the callback. However, in no circumstance does the callback > run after STATUS_SUCCESS is returned. > > I am not including the tests because of the random outcome and because > it plays with SuspendThread and makes the random failures in follow up > tests worse. I'll send it to wine-devel for reference. > --- > dlls/ntdll/threadpool.c | 59 +++++++++++++++++++++++++++---------------------- > 1 file changed, 32 insertions(+), 27 deletions(-) > > diff --git a/dlls/ntdll/threadpool.c b/dlls/ntdll/threadpool.c > index 6063d51d9f..1b0546a781 100644 > --- a/dlls/ntdll/threadpool.c > +++ b/dlls/ntdll/threadpool.c > @@ -541,11 +541,13 @@ static DWORD CALLBACK wait_thread_proc(LPVOID Arg) > break; > } > > - completion_event = wait_work_item->CompletionEvent; > - if (completion_event) NtSetEvent( completion_event, NULL ); > > if (interlocked_inc( &wait_work_item->DeleteCount ) == 2 ) > + { > + completion_event = wait_work_item->CompletionEvent; > delete_wait_work_item( wait_work_item ); > + if (completion_event) NtSetEvent( completion_event, NULL ); > + } > > return 0; > } > @@ -633,44 +635,47 @@ NTSTATUS WINAPI RtlRegisterWait(PHANDLE NewWaitObject, HANDLE Object, > NTSTATUS WINAPI RtlDeregisterWaitEx(HANDLE WaitHandle, HANDLE CompletionEvent) > { > struct wait_work_item *wait_work_item = WaitHandle; > - NTSTATUS status = STATUS_SUCCESS; > + NTSTATUS status; > + HANDLE LocalEvent = NULL; > + BOOLEAN CallbackInProgress; > > - TRACE( "(%p)\n", WaitHandle ); > + TRACE( "(%p %p)\n", WaitHandle, CompletionEvent ); > > if (WaitHandle == NULL) > return STATUS_INVALID_HANDLE; > > - NtSetEvent( wait_work_item->CancelEvent, NULL ); > - if (wait_work_item->CallbackInProgress) > + CallbackInProgress = wait_work_item->CallbackInProgress; > + if (CompletionEvent == INVALID_HANDLE_VALUE || !CallbackInProgress) > { > - if (CompletionEvent != NULL) > - { > - if (CompletionEvent == INVALID_HANDLE_VALUE) > - { > - status = NtCreateEvent( &CompletionEvent, EVENT_ALL_ACCESS, NULL, NotificationEvent, FALSE ); > - if (status != STATUS_SUCCESS) > - return status; > - interlocked_xchg_ptr( &wait_work_item->CompletionEvent, CompletionEvent ); > - if (wait_work_item->CallbackInProgress) > - NtWaitForSingleObject( CompletionEvent, FALSE, NULL ); > - NtClose( CompletionEvent ); > - } > - else > - { > - interlocked_xchg_ptr( &wait_work_item->CompletionEvent, CompletionEvent ); > - if (wait_work_item->CallbackInProgress) > - status = STATUS_PENDING; > - } > - } > - else > - status = STATUS_PENDING; > + status = NtCreateEvent( &LocalEvent, EVENT_ALL_ACCESS, NULL, NotificationEvent, FALSE ); > + if (status != STATUS_SUCCESS) > + return status; > + interlocked_xchg_ptr( &wait_work_item->CompletionEvent, LocalEvent ); > + } > + else if (CompletionEvent != NULL) > + { > + interlocked_xchg_ptr( &wait_work_item->CompletionEvent, CompletionEvent ); > } > > + NtSetEvent( wait_work_item->CancelEvent, NULL ); > + > if (interlocked_inc( &wait_work_item->DeleteCount ) == 2 ) > { > status = STATUS_SUCCESS; > delete_wait_work_item( wait_work_item ); > } > + else if (LocalEvent) > + { > + NtWaitForSingleObject( LocalEvent, FALSE, NULL ); > + status = STATUS_SUCCESS; > + } > + else > + { > + status = STATUS_PENDING; > + } > + > + if (LocalEvent) > + NtClose( LocalEvent ); > > return status; > } >

1 0

Re: [PATCH] wbemprox: Fix CreateInstanceEnum/Async entries.
by Hans Leidekker 05 Sep '17

05 Sep '17

On Tue, 2017-09-05 at 15:19 +0300, Andrey Gusev wrote: > @@ -599,7 +599,7 @@ static HRESULT WINAPI wbem_services_CreateInstanceEnum( > > static HRESULT WINAPI wbem_services_CreateInstanceEnumAsync( > IWbemServices *iface, > - const BSTR strFilter, > + const BSTR strClass, > LONG lFlags, > IWbemContext *pCtx, > IWbemObjectSink *pResponseHandler ) I think the original name is fine.

1 0