September 2017 - Wine-Devel

Re: [PATCH] secur32: Split push/pull schannel transports

by Alexandre Julliard

Anton Romanov <theli.ua(a)gmail.com> writes: > So, it's been a month now. Is there something I could do to have this accepted? It's assigned to Jacek for review, but he's on vacation until tomorrow I believe, so it may be a few more days until he gets around to it. Sorry about the delay. -- Alexandre Julliard julliard(a)winehq.org

Sept. 6, 2017

Re: [v2 PATCH] d2d1: Partially implement StrokeContainsPoint() for rectangles

by Henri Verbeet

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.

Sept. 6, 2017

Re: [PATCH 2/3] include: Add interfaces IMFMediaTypeHandler, IMFStreamDescriptor

by Henri Verbeet

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.

Sept. 6, 2017

Re: [PATCH 3/5] wined3d: Store FLUSH flag in the query object.

by Henri Verbeet

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.

Sept. 6, 2017

Re: [PATCH 1/5] wined3d: Restore support for d3d9 "fake" formats.

by Henri Verbeet

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?

Sept. 6, 2017

Re: [PATCH 01/10] atl80: Fix processorArchitecture in manifest

by Fabian Maurer

> Yes, they would be affected too. > Sure, but this would need a more complicated process. Using "*" is not > correct here. I see, thanks for explaining. Regards, Fabian Maurer

Sept. 6, 2017

Re: [PATCH 01/10] atl80: Fix processorArchitecture in manifest

by Alexandre Julliard

Fabian Maurer <dark.shadow4(a)web.de> writes: >> All the files get a proper resource when built for Wine. It's only an >> issue when cross-compiling to a Windows binary, since the PE compiler >> doesn't do the same magic. > So, the dlls would be affected too, when cross-compiled? Or am I > misunderstanding something here? Yes, they would be affected too. > Well, we don't really need to have it work when cross-compiled, I just thought > it would be nice to have. Sure, but this would need a more complicated process. Using "*" is not correct here. -- Alexandre Julliard julliard(a)winehq.org

Sept. 6, 2017

Re: [PATCH 01/10] atl80: Fix processorArchitecture in manifest

by Fabian Maurer

> All the files get a proper resource when built for Wine. It's only an > issue when cross-compiling to a Windows binary, since the PE compiler > doesn't do the same magic. So, the dlls would be affected too, when cross-compiled? Or am I misunderstanding something here? Well, we don't really need to have it work when cross-compiled, I just thought it would be nice to have. Regards, Fabian Maurer

Sept. 6, 2017

Re: [PATCH 01/10] atl80: Fix processorArchitecture in manifest

by Alexandre Julliard

Fabian Maurer <dark.shadow4(a)web.de> writes: >> 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. > > Oh, you mean dlls are handled differently? I just went by the manifest itself, > I figured that when it's wrong in one place it would be wrong in the other, > too. > If you can guarantee that the dlls get a proper resource, then never mind > that. All the files get a proper resource when built for Wine. It's only an issue when cross-compiling to a Windows binary, since the PE compiler doesn't do the same magic. -- Alexandre Julliard julliard(a)winehq.org

Sept. 6, 2017

Re: [PATCH 01/10] atl80: Fix processorArchitecture in manifest

by Fabian Maurer

> 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. Oh, you mean dlls are handled differently? I just went by the manifest itself, I figured that when it's wrong in one place it would be wrong in the other, too. If you can guarantee that the dlls get a proper resource, then never mind that. Regards, Fabian Maurer

Sept. 6, 2017