November 2016 - Wine-Devel - list.winehq.org

Re: [v2 PATCH 1/3] dpnet/test: Add IDirectPlay8Client EnumHost tests
by Alexandre Julliard Nov. 16, 2016

Nov. 16, 2016

Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com> writes: > +static HRESULT lastAsyncCode = E_FAIL; > +static DPNHANDLE lastAsyncHandle = 0xdeadbeef; > + > +#define CHECK_LAST_ASYNC_OP(hr, handle) \ > + ok(lastAsyncCode == hr, "got 0x%08x\n", lastAsyncCode); \ > + ok(lastAsyncHandle == handle, "got 0x%08x\n", handle); \ > + lastAsyncCode = E_FAIL; lastAsyncHandle = 0xdeadbeef; Please don't define such macros. In this case you should simply spell it out explicitly, it's not a lot of code. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: [v3 PATCH] winex11.drv: import X11's "text/html" as "HTML Format"
by Alexandre Julliard Nov. 16, 2016

Nov. 16, 2016

Damjan Jovanovic <damjan.jov(a)gmail.com> writes: > +static char *standardize_text_html( const char *data, size_t size ) > +{ > + char *textHtml = NULL; > + > + /* Firefox uses UTF-16LE with byte order mark. Convert to UTF-8 without the BOM. */ > + if (size >= 2 && ((BYTE*)data)[0] == 0xff && ((BYTE*)data)[1] == 0xfe) > + { > + INT textHtmlSize = WideCharToMultiByte(CP_UTF8, 0, (LPCWSTR)&data[2], (size-2)/2, NULL, 0, NULL, NULL); > + textHtml = HeapAlloc(GetProcessHeap(), 0, textHtmlSize); > + if (textHtml) > + WideCharToMultiByte(CP_UTF8, 0, (LPCWSTR)&data[2], (size-2)/2, textHtml, textHtmlSize, NULL, NULL); > + else > + ERR("out of memory\n"); > + } > + return textHtml; It seems to me that you should still import it even if it's not in Firefox format. -- Alexandre Julliard julliard(a)winehq.org

1 0

Question about wineconf 2016 decisions
by CARLOS RODRIGUEZ Nov. 16, 2016

Nov. 16, 2016

Hi After see alexander julliard wineconf 2016 decisions Only i have ask if CSMT can arrive in wine 2.0 or will be postponed to next wine development cicle wine 2.1 Thanks

3 2

build error on wininet.h on wine testbot
by Daniel Lehman Nov. 16, 2016

Nov. 16, 2016

I'm getting the errors below when submitting a patch to test bot, even though the patch doesn't modify wininet.h full log is https://urldefense.proofpoint.com/v2/url?u=http-3A__testbot.winehq.org_JobD… am I doing something wrong? Thanks daniel Applying patch + git apply --verbose /home/winetest/tools/testbot/var/../var/staging/patch.diff Checking patch dlls/urlmon/tests/uri.c... Checking patch dlls/urlmon/uri.c... Applied patch dlls/urlmon/tests/uri.c cleanly. Applied patch dlls/urlmon/uri.c cleanly. Making test executable + time make -j4 -C dlls/urlmon/tests make: Entering directory `/home/winetest/tools/testbot/var/build-mingw32/dlls/urlmon/tests' i686-w64-mingw32-gcc -c -o uri.o ../../../../wine/dlls/urlmon/tests/uri.c -I. -I../../../../wine/dlls/urlmon/tests \ -I../../../include -I../../../../wine/include -D__WINESRC__ -D_REENTRANT -Wall -pipe \ -fno-strict-aliasing -Wdeclaration-after-statement -Wempty-body -Wignored-qualifiers \ -Wstrict-prototypes -Wtype-limits -Wunused-but-set-parameter -Wvla -Wwrite-strings -Wpointer-arith \ -Wlogical-op -gdwarf-2 -gstrict-dwarf -fno-omit-frame-pointer -g -O2 -D_WIN32 In file included from ../../../../wine/dlls/urlmon/tests/uri.c:33:0: ../../../../wine/include/wininet.h:1:1: error: stray '\35' in program ../../../../wine/include/wininet.h:1:3: warning: null character(s) ignored [enabled by default] ../../../../wine/include/wininet.h:1:1: error: stray '\210' in program ../../../../wine/include/wininet.h:1:1: error: stray '\377' in program ../../../../wine/include/wininet.h:1:1: error: stray '\377' in program ../../../../wine/include/wininet.h:1:1: error: stray '\200' in program ../../../../wine/include/wininet.h:1:1: error: unknown type name 'b' ../../../../wine/include/wininet.h:1:1: error: stray '\36' in program ../../../../wine/include/wininet.h:1:11: warning: null character(s) ignored [enabled by default] ../../../../wine/include/wininet.h:1:1: error: stray '\210' in program ../../../../wine/include/wininet.h:1:1: error: stray '\377' in program ...

2 1

Steam in Windows 7 mode
by Alex Henrie Nov. 16, 2016

Nov. 16, 2016

Hi all, Everybody at WineConf (myself included) seemed pretty happy about the idea to have Wine use Windows 7 mode by default. However, it looks like Steam does not accept the -no-cef-sandbox parameter in Windows Vista and later. This means that if we switch to Windows 7, most parts of Steam will crash. I see a few options: 1. Ask the Steam developers to make -no-cef-sandbox work in all versions of Windows. 2. Tell users to switch to Windows XP mode before using Steam. 3. Commit some nasty hack to handle the syscall and continue normal processing. This needs further discussion. -Alex

2 1

Re: [17/23] wined3d: Implement return instruction.
by Andrew Wesie Nov. 16, 2016

Nov. 16, 2016

Is the correct thing to add a test to dlls/d3d11/tests/d3d11.c that illustrates the bug and shows that the patch fixes the bug? -Andrew On Mon, Nov 14, 2016 at 5:07 PM, Marvin <testbot(a)winehq.org> wrote: > Thank you for your contribution to Wine! > > This is an automated notification to let you know that your patch has > been reviewed and its status set to "Rejected". > > This means that the patch has been rejected by a reviewer. You should > have received a mail explaining why it was rejected. You need to fix > the issue and resend the patch, or if you are convinced that your > patch is good as is, you should reply to the rejection message with > your counterarguments. > > If you do not understand the reason for this status, disagree with our > assessment, or are simply not sure how to proceed next, please ask for > clarification by replying to this email. >

2 1

Debugging wine and finding the cause of a crash
by Fabian Maurer Nov. 15, 2016

Nov. 15, 2016

I'm currently trying to get a program to work that doesn't work with wine, causing an access-violation at some unreadable address. The "+seh" output shows that it's the first crash, and "+relay" doesn't help much, since the last calls are "user32.GetKeyState" and similar. I already tried debugging the program, but at the place of the crash there aren't any wine functions called, so I assume it's some earlier parts (e.g. during loading) causing the issue. How do I go about finding which wine functions are the culprit? Any tips for that, how do you investigate such issues? It would be easy if it crashed directly after calling a WinApi function, but since it seems to crash much later in a completely different part of the code, I don't really know how to proceed.

2 2

Re: [PATCH 2/2] ntdll: Added TokenIsAppContainer semi-stub implementation.
by Marvin Nov. 15, 2016

Nov. 15, 2016

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=26665 Your paranoid android. === w1064 (32 bit security) === security.c:4035: Test failed: Builtin Users ACE has unexpected flags (0x12 != 0xa) security.c:4039: Test failed: Builtin Users ACE has unexpected mask (0x20019 != 0x80000000) security.c:4048: Test failed: Builtin Admins ACE has unexpected flags (0x12 != 0x0) === w1064 (64 bit security) === security.c:4035: Test failed: Builtin Users ACE has unexpected flags (0x2 != 0xa) security.c:4039: Test failed: Builtin Users ACE has unexpected mask (0x20019 != 0x80000000) security.c:4048: Test failed: Builtin Admins ACE has unexpected flags (0x2 != 0x0)

1 0

Re: [PATCH 1/2] advapi32: Added TokenIsAppContainer tests.
by Marvin Nov. 15, 2016

Nov. 15, 2016

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=26664 Your paranoid android. === w1064 (32 bit security) === security.c:4036: Test failed: Builtin Users ACE has unexpected flags (0x12 != 0xa) security.c:4040: Test failed: Builtin Users ACE has unexpected mask (0x20019 != 0x80000000) security.c:4049: Test failed: Builtin Admins ACE has unexpected flags (0x12 != 0x0) === w1064 (64 bit security) === security.c:4036: Test failed: Builtin Users ACE has unexpected flags (0x2 != 0xa) security.c:4040: Test failed: Builtin Users ACE has unexpected mask (0x20019 != 0x80000000) security.c:4049: Test failed: Builtin Admins ACE has unexpected flags (0x2 != 0x0)

1 0

Re: [PATCH] winegstreamer: Fix memory leak in amt_from_gst_caps_video.
by Andrew Eikum Nov. 15, 2016

Nov. 15, 2016

This is an improvement, but I think we should go further and fix leaks in all of the failure cases. The problem is we're using the amt_from_gst_caps_video return value inconsistently. In accept_caps_sink, we unconditionally free the resulting AM_MEDIA_TYPE. But in setcaps_sink, we only free it, via the pin->pmt assignment, if amt_from_gst_caps_video succeeds. I think we should always free the allocated memory in the failure cases in amt_from_gst_caps_video, and then only free the resulting AMT in the calling functions if amt_from_gst_caps_video succeeded. The resulting AMT should never be used or freed if amt_from_gst_caps_video fails. Note that amt_from_gst_caps_audio has the same inconsistent return type handling. I would accept a separate patch to fix this handling, too. (Unlike for video, it doesn't currently matter in practice because of amt_from_gst_caps_audio's implementation.) Does that make sense? Andrew On Mon, Nov 14, 2016 at 11:43:21PM -0700, Alex Henrie wrote: > Cc: Andrew Eikum <aeikum(a)codeweavers.com> > > Coverity #713781, "Variable vih going out of scope leaks the storage it > points to." > > Signed-off-by: Alex Henrie <alexhenrie24(a)gmail.com> > --- > dlls/winegstreamer/gstdemux.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/dlls/winegstreamer/gstdemux.c b/dlls/winegstreamer/gstdemux.c > index a6a41ab..5d97510 100644 > --- a/dlls/winegstreamer/gstdemux.c > +++ b/dlls/winegstreamer/gstdemux.c > @@ -179,18 +179,21 @@ static gboolean amt_from_gst_caps_audio(GstCaps *caps, AM_MEDIA_TYPE *amt) > > static gboolean amt_from_gst_caps_video(GstCaps *caps, AM_MEDIA_TYPE *amt) > { > - VIDEOINFOHEADER *vih = CoTaskMemAlloc(sizeof(*vih)); > - BITMAPINFOHEADER *bih = &vih->bmiHeader; > + VIDEOINFOHEADER *vih; > + BITMAPINFOHEADER *bih; > gint32 width = 0, height = 0, nom = 0, denom = 0; > GstVideoInfo vinfo; > > if (!gst_video_info_from_caps (&vinfo, caps)) > return FALSE; > width = vinfo.width; > height = vinfo.height; > nom = vinfo.fps_n; > denom = vinfo.fps_d; > > + vih = CoTaskMemAlloc(sizeof(*vih)); > + bih = &vih->bmiHeader; > + > amt->formattype = FORMAT_VideoInfo; > amt->pbFormat = (BYTE*)vih; > amt->cbFormat = sizeof(*vih); > -- > 2.10.2 >

2 1

Re: [resend PATCH 2/4] oledb32: IErrorRecords uses 0-based index to access record info
by Alistair Leslie-Hughes Nov. 15, 2016

Nov. 15, 2016

Hi Nikolay, On 15/11/16 03:56, Nikolay Sivov wrote: > Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> > > + hr = IErrorRecords_GetRecordCount(errrecs, &cnt); > + ok(hr == S_OK, "got %08x\n", hr); > + ok(cnt == 0, "expected 2 got %d\n", cnt); The comment here is incorrect. Best Regards Alistair.

2 1

Re: [PATCH] cabinet: Properly initialize our internal fci structure v2(bug 36115)(valgrind)
by Huw Davies Nov. 14, 2016

Nov. 14, 2016

On 14 Nov 2016, at 16:46, Detlef Riekenberg <wine.dev(a)web.de> wrote: > > We did not initialize all items for our internal fci structure. > This should fix bug 36115 > > v2: > Remove comments, as suggested by Huw > > -- > bye bye ... Detlef > > Signed-off-by: Detlef Riekenberg <wine.dev(a)web.de> > --- > dlls/cabinet/fci.c | 19 +------------------ > 1 file changed, 1 insertion(+), 18 deletions(-) > > diff --git a/dlls/cabinet/fci.c b/dlls/cabinet/fci.c > index 70d4f3f..03fbf46 100644 > --- a/dlls/cabinet/fci.c > +++ b/dlls/cabinet/fci.c > @@ -1036,6 +1036,7 @@ HFCI __cdecl FCICreate( > return NULL; > } > > + memset(p_fci_internal, 0, sizeof(FCI_Int)); I also suggested changing the sizeof(FCI_Int) -> sizeof(*p_fci_internal), but maybe that wasn’t very clear. Huw.

1 0

WineConf decisions
by Alexandre Julliard Nov. 14, 2016

Nov. 14, 2016

At this year's WineConf we again had good constructive discussions about the development process, and decided a number of things: - Version numbering will change to a sligthly shorter scheme. The next stable release will be called 2.0. Stable release updates will follow the existing scheme: 2.0.1, 2.0.2, etc. For the developemnt branch we will drop the intermediate number: releases will be numbered 2.1, 2.2, 2.3, etc. and the next stable release in late 2017 will be called 3.0. - Code freeze for the 2.0 release will start after 1.9.24, in 3-4 weeks, with a final release targeted around the end of the year. - Michael Stefaniuc has agreed to continue maintaining the stable branch, and will be in charge of the 2.0.x releases. - Since it sometimes takes months for the stable release to get into distributions, we'll look into providing binary packages of the stable branch to make it easier for users to stay up to date. - The default reported Windows version will be upgraded to Windows 7. This will be done on the development branch soon after 2.0 is released, so that we have the time to catch regressions and address them before the next stable release. - To further the integration of WineHQ and Wine-Staging, we'll look into merging the Staging website into the Wine Wiki. We'll also merge the IRC channels, and encourage people to discuss Staging issues on #winehq and #winehackers, with #wine-staging being retired at some point. - To make sure that no patch falls off the list unreviewed, we are going to implement a mechanism to automatically assign a reviewer from the MAINTAINERS pool when a patch has been in New state for a week without action. - Since reviewers assigned at random may not always be comfortable with fully signing off on a patch, we discussed adding an Acked-by header as a weaker version of Signed-off-by. We finally decided that for now, an informal mail to wine-devel would be good enough. We may revisit that decision if a formal header makes automation easier. I'm hoping that these improvements will make things better for everybody. As always, feedback is encouraged, especially from people who couldn't make it to WineConf. As we implement these changes, please let us know how we are doing, and where we can do better. -- Alexandre Julliard julliard(a)winehq.org

1 0

AppDB Maintainership and Updates
by Jeremy Newman Nov. 14, 2016

Nov. 14, 2016

Hey all! Just wanted to let you all know that at WineConf we decided that I should once again take up the mantle of AppDB codebase maintainer. Woo! I'll be in charge of reviewing and approving AppDB patches from now on. If you want to submit patches, please send them to the wine-patches mailing list with the subject prefix of [AppDB]. Bugs and feature requests should be filed on Bugzilla as usual. Discussion can happen here on wine-devel, IRC #winehackers, or preferably on the Forums in the new "Website Issues" forum. I've started the process of cleaning up some of the code and moving things around. Please bear with me as I'll be doing this in whatever spare time I can find. I created a ToDo page in the Wiki that will track the various things that I've agreed to work on. If it is not listed there, then it is open for someone else to jump on. https://wiki.winehq.org/AppDB_ToDo_List -N

1 0

Re: [17/23] wined3d: Implement return instruction.
by Matteo Bruni Nov. 14, 2016

Nov. 14, 2016

2016-11-13 12:35 GMT-06:00 Andrew Wesie <awesie(a)gmail.com>: > @@ -6079,8 +6101,26 @@ static GLuint shader_glsl_generate_vshader(const struct wined3d_context *context > /* Base Shader Body */ > shader_generate_main(shader, buffer, reg_maps, function, &priv_ctx); > > + shader_addline(buffer, "}\n"); > + > + TRACE("Compiling shader object %u.\n", shader_id); > + shader_glsl_compile(gl_info, shader_id, buffer->buffer); > + > + return shader_id; > +} This breaks < SM4 shaders, those don't have an explicit return instruction at the end. In general, each patch should pass the wine tests. Please run the d3d tests and make sure that none are broken by your patchset. Specifically, this patch breaks the d3d9 visual test pretty badly. It looks like there are more failures introduced by other patches in the series but I haven't tracked them down. > +static void shader_glsl_prologue_vs(const struct wined3d_shader_context *ctx) I think this should actually be "epilogue".

1 0

Re: [11/23] wined3d: Implement primitive restart index.
by Matteo Bruni Nov. 14, 2016

Nov. 14, 2016

2016-11-13 12:35 GMT-06:00 Andrew Wesie <awesie(a)gmail.com>: > Signed-off-by: Andrew Wesie <awesie(a)gmail.com> > --- > dlls/wined3d/directx.c | 3 +++ > dlls/wined3d/glsl_shader.c | 14 ++++++++++++++ > dlls/wined3d/wined3d_gl.h | 2 ++ > 3 files changed, 19 insertions(+) > > diff --git a/dlls/wined3d/directx.c b/dlls/wined3d/directx.c > index a8c8c9f..c930cf6 100644 > --- a/dlls/wined3d/directx.c > +++ b/dlls/wined3d/directx.c > @@ -121,6 +121,7 @@ static const struct wined3d_extension_map gl_extension_map[] = > {"GL_ARB_draw_elements_base_vertex", ARB_DRAW_ELEMENTS_BASE_VERTEX }, > {"GL_ARB_draw_instanced", ARB_DRAW_INSTANCED }, > {"GL_ARB_ES2_compatibility", ARB_ES2_COMPATIBILITY }, > + {"GL_ARB_ES3_compatibility", ARB_ES3_COMPATIBILITY }, > {"GL_ARB_explicit_attrib_location", ARB_EXPLICIT_ATTRIB_LOCATION }, > {"GL_ARB_fragment_coord_conventions", ARB_FRAGMENT_COORD_CONVENTIONS}, > {"GL_ARB_fragment_program", ARB_FRAGMENT_PROGRAM }, > @@ -228,6 +229,7 @@ static const struct wined3d_extension_map gl_extension_map[] = > {"GL_NV_half_float", NV_HALF_FLOAT }, > {"GL_NV_light_max_exponent", NV_LIGHT_MAX_EXPONENT }, > {"GL_NV_point_sprite", NV_POINT_SPRITE }, > + {"GL_NV_primitive_restart", NV_PRIMITIVE_RESTART }, > {"GL_NV_register_combiners", NV_REGISTER_COMBINERS }, > {"GL_NV_register_combiners2", NV_REGISTER_COMBINERS2 }, > {"GL_NV_texgen_reflection", NV_TEXGEN_REFLECTION }, > @@ -3125,6 +3127,7 @@ static void load_gl_funcs(struct wined3d_gl_info *gl_info) > USE_GL_FUNC(glMapBuffer) /* OpenGL 1.5 */ > USE_GL_FUNC(glPointParameteri) /* OpenGL 1.4 */ > USE_GL_FUNC(glPointParameteriv) /* OpenGL 1.4 */ > + USE_GL_FUNC(glPrimitiveRestartIndex) > USE_GL_FUNC(glShaderSource) /* OpenGL 2.0 */ > USE_GL_FUNC(glStencilFuncSeparate) /* OpenGL 2.0 */ > USE_GL_FUNC(glStencilOpSeparate) /* OpenGL 2.0 */ AFAICS this function was added in OpenGL 3.1 and there is no specific ARB extension for it. Maybe adding a new WINED3D_GL_ entry to flag the availability of the function is a good idea. Also you probably want to MAP_GL_FUNCTION() the NV variant to the core one and add an entry for GL_ARB_ES3_compatibility to core_extensions. > diff --git a/dlls/wined3d/glsl_shader.c b/dlls/wined3d/glsl_shader.c > index f136536..27a5533 100644 > --- a/dlls/wined3d/glsl_shader.c > +++ b/dlls/wined3d/glsl_shader.c > @@ -8613,6 +8613,20 @@ static void shader_glsl_init_context_state(struct wined3d_context *context) > > gl_info->gl_ops.gl.p_glEnable(GL_PROGRAM_POINT_SIZE); > checkGLcall("GL_PROGRAM_POINT_SIZE"); > + if (gl_info->supported[ARB_ES3_COMPATIBILITY]) > + { > + /* We prefer this method because it correctly handles 16-bit and 32-bit indices. */ > + gl_info->gl_ops.gl.p_glEnable(GL_PRIMITIVE_RESTART_FIXED_INDEX); > + checkGLcall("GL_PRIMITIVE_RESTART_FIXED_INDEX"); > + } > + else if (gl_info->supported[NV_PRIMITIVE_RESTART] && GL_EXTCALL(glPrimitiveRestartIndex)) > + { > + /* FIXME: Does this handle 16-bit indices correctly? */ > + GL_EXTCALL(glPrimitiveRestartIndex(0xFFFFFFFF)); > + checkGLcall("glPrimitiveRestartIndex"); > + gl_info->gl_ops.gl.p_glEnable(GL_PRIMITIVE_RESTART); > + checkGLcall("GL_PRIMITIVE_RESTART"); Probably not, but nothing prevents you to use 0xffff for 16-bit indices and force re-setting the restart index when changing between 16-bit and 32-bit indices.

2 2

Re: [08/23] wined3d: Allow viewport to be out-of-bounds if using offscreen target.
by Matteo Bruni Nov. 14, 2016

Nov. 14, 2016

2016-11-13 12:35 GMT-06:00 Andrew Wesie <awesie(a)gmail.com>: > Signed-off-by: Andrew Wesie <awesie(a)gmail.com> > --- > dlls/wined3d/state.c | 42 ++++++++++++++++++++++-------------------- > 1 file changed, 22 insertions(+), 20 deletions(-) > > diff --git a/dlls/wined3d/state.c b/dlls/wined3d/state.c > index cd3763c..ed20e325 100644 > --- a/dlls/wined3d/state.c > +++ b/dlls/wined3d/state.c > @@ -4617,26 +4617,6 @@ static void viewport_miscpart(struct wined3d_context *context, const struct wine > struct wined3d_viewport vp = state->viewport; > unsigned int width, height; > > - if (target) > - { > - if (vp.width > target->width) > - vp.width = target->width; > - if (vp.height > target->height) > - vp.height = target->height; > - > - wined3d_rendertarget_view_get_drawable_size(target, context, &width, &height); > - } > - else if (depth_stencil) > - { > - width = depth_stencil->width; > - height = depth_stencil->height; > - } > - else > - { > - FIXME("No attachments draw calls not supported.\n"); > - return; > - } > - > gl_info->gl_ops.gl.p_glDepthRange(vp.min_z, vp.max_z); > checkGLcall("glDepthRange"); > /* Note: GL requires lower left, DirectX supplies upper left. This is > @@ -4644,7 +4624,29 @@ static void viewport_miscpart(struct wined3d_context *context, const struct wine > if (context->render_offscreen) > gl_info->gl_ops.gl.p_glViewport(vp.x, vp.y, vp.width, vp.height); > else > + { > + if (target) > + { > + if (vp.width > target->width) > + vp.width = target->width; > + if (vp.height > target->height) > + vp.height = target->height; > + > + wined3d_rendertarget_view_get_drawable_size(target, context, &width, &height); > + } > + else if (depth_stencil) > + { > + width = depth_stencil->width; > + height = depth_stencil->height; > + } > + else > + { > + FIXME("No attachments draw calls not supported.\n"); > + return; > + } > + > gl_info->gl_ops.gl.p_glViewport(vp.x, (height - (vp.y + vp.height)), vp.width, vp.height); > + } > checkGLcall("glViewport"); > } You need to make the same change to viewport_miscpart_cc() (you can blame me for that). Again, it would be very nice to have a testcase for this. Extending viewport_test() in d3d9/tests/visual.c and adding similar tests to the other d3d versions shouldn't be too hard.

2 2

Re: [PATCH] hidclass: All reports read or written to user space lead with a reportId
by Sebastian Lackner Nov. 14, 2016

Nov. 14, 2016

On 14.11.2016 14:35, Aric Stewart wrote: > This is a little confusing because reports to and from the device > itself only have a leading reportId if the reportid is not 0. > Otherwise there is no leading reportId. So we need to add a 0 reportId > to reports coming from such a device and strip the leading 0 from > reports written to the device. > > Signed-off-by: Aric Stewart <aric(a)codeweavers.com> > --- > dlls/hidclass.sys/descriptor.c | 31 ++++----------- > dlls/hidclass.sys/device.c | 90 ++++++++++++++++++++++++++++-------------- > 2 files changed, 67 insertions(+), 54 deletions(-) > > > > 0001-hidclass-All-reports-read-or-written-to-user-space-lea.txt > > > diff --git a/dlls/hidclass.sys/descriptor.c b/dlls/hidclass.sys/descriptor.c > index 9a26f0f4..8bcda43 100644 > --- a/dlls/hidclass.sys/descriptor.c > +++ b/dlls/hidclass.sys/descriptor.c > @@ -887,10 +887,8 @@ static WINE_HIDP_PREPARSED_DATA* build_PreparseData( > new_report(wine_report, input_features[0]); > data->dwInputReportCount++; > > - if (input_features[0]->caps.ReportID != 0) > - bitOffset = 8; > - else > - bitOffset = 0; > + /* Room for the reportID */ > + bitOffset = 8; > > for (i = 0; i < i_count; i++) > { > @@ -901,10 +899,7 @@ static WINE_HIDP_PREPARSED_DATA* build_PreparseData( > new_report(wine_report, input_features[i]); > data->dwInputReportCount++; > bitLength = max(bitOffset, bitLength); > - if (input_features[i]->caps.ReportID != 0) > - bitOffset = 8; > - else > - bitOffset = 0; > + bitOffset = 8; > } > build_elements(wine_report, input_features[i], &bitOffset); > count_elements(input_features[i], &data->caps.NumberInputButtonCaps, > @@ -922,10 +917,7 @@ static WINE_HIDP_PREPARSED_DATA* build_PreparseData( > data->dwOutputReportOffset = (BYTE*)wine_report - (BYTE*)data->InputReports; > new_report(wine_report, output_features[0]); > data->dwOutputReportCount++; > - if (output_features[0]->caps.ReportID != 0) > - bitOffset = 8; > - else > - bitOffset = 0; > + bitOffset = 8; > > for (i = 0; i < o_count; i++) > { > @@ -936,10 +928,7 @@ static WINE_HIDP_PREPARSED_DATA* build_PreparseData( > new_report(wine_report, output_features[i]); > data->dwOutputReportCount++; > bitLength = max(bitOffset, bitLength); > - if (output_features[0]->caps.ReportID != 0) > - bitOffset = 8; > - else > - bitOffset = 0; > + bitOffset = 8; > } > build_elements(wine_report, output_features[i], &bitOffset); > count_elements(output_features[i], &data->caps.NumberOutputButtonCaps, > @@ -957,10 +946,7 @@ static WINE_HIDP_PREPARSED_DATA* build_PreparseData( > data->dwFeatureReportOffset = (BYTE*)wine_report - (BYTE*)data->InputReports; > new_report(wine_report, feature_features[0]); > data->dwFeatureReportCount++; > - if (feature_features[0]->caps.ReportID != 0) > - bitOffset = 8; > - else > - bitOffset = 0; > + bitOffset = 8; > > for (i = 0; i < f_count; i++) > { > @@ -971,10 +957,7 @@ static WINE_HIDP_PREPARSED_DATA* build_PreparseData( > new_report(wine_report, feature_features[i]); > data->dwFeatureReportCount++; > bitLength = max(bitOffset, bitLength); > - if (feature_features[0]->caps.ReportID != 0) > - bitOffset = 8; > - else > - bitOffset = 0; > + bitOffset = 8; > } > build_elements(wine_report, feature_features[i], &bitOffset); > count_elements(feature_features[i], &data->caps.NumberFeatureButtonCaps, > diff --git a/dlls/hidclass.sys/device.c b/dlls/hidclass.sys/device.c > index 5dd5ebb..6454b67 100644 > --- a/dlls/hidclass.sys/device.c > +++ b/dlls/hidclass.sys/device.c > @@ -197,6 +197,28 @@ void HID_DeleteDevice(HID_MINIDRIVER_REGISTRATION *driver, DEVICE_OBJECT *device > IoDeleteDevice(device); > } > > +static NTSTATUS copy_packet_into_buffer(HID_XFER_PACKET *packet, BYTE* buffer, ULONG buffer_length, ULONG *out_length) > +{ > + *out_length = 0; > + if (buffer_length > packet->reportBufferLen) > + { > + if (packet->reportId != 0) > + { > + memcpy(buffer, packet->reportBuffer, packet->reportBufferLen); > + *out_length = packet->reportBufferLen; The length check is a bit too strict in this case. You are returning STATUS_BUFFER_OVERFLOW even when the buffer is sufficient (buffer_length == packet->reportBufferLen). > + } > + else > + { > + buffer[0] = 0; > + memcpy(&buffer[1], packet->reportBuffer, packet->reportBufferLen); > + *out_length = packet->reportBufferLen + 1; > + } > + return STATUS_SUCCESS; > + } > + else > + return STATUS_BUFFER_OVERFLOW; > +} > + > static void HID_Device_processQueue(DEVICE_OBJECT *device) > { > LIST_ENTRY *entry; > @@ -217,20 +239,14 @@ static void HID_Device_processQueue(DEVICE_OBJECT *device) > RingBuffer_Read(ext->ring_buffer, ptr, packet, &buffer_size); > if (buffer_size) > { > + NTSTATUS hr; Maybe its just me, but I think "hr" is really a very bad name for NTSTATUS codes. ;) > + ULONG out_length; > IO_STACK_LOCATION *irpsp = IoGetCurrentIrpStackLocation(irp); > packet->reportBuffer = (BYTE *)packet + sizeof(*packet); > TRACE_(hid_report)("Processing Request (%i)\n",ptr); > - if (irpsp->Parameters.Read.Length >= packet->reportBufferLen) > - { > - memcpy(irp->AssociatedIrp.SystemBuffer, packet->reportBuffer, packet->reportBufferLen); > - irp->IoStatus.Information = packet->reportBufferLen; > - irp->IoStatus.u.Status = STATUS_SUCCESS; > - } > - else > - { > - irp->IoStatus.Information = 0; > - irp->IoStatus.u.Status = STATUS_BUFFER_OVERFLOW; > - } > + hr = copy_packet_into_buffer(packet, irp->AssociatedIrp.SystemBuffer, irpsp->Parameters.Read.Length, &out_length); > + irp->IoStatus.u.Status = hr; > + irp->IoStatus.Information = out_length; > } > else > { > @@ -331,7 +347,10 @@ static DWORD CALLBACK hid_device_thread(void *args) > if (!exit_now && irp->IoStatus.u.Status == STATUS_SUCCESS) > { > packet->reportBufferLen = irp->IoStatus.Information; > - packet->reportId = packet->reportBuffer[0]; > + if (ext->preparseData->InputReports[0].reportID) > + packet->reportId = packet->reportBuffer[0]; > + else > + packet->reportId = 0; > RingBuffer_Write(ext->ring_buffer, packet); > HID_Device_processQueue(device); > } > @@ -461,9 +480,17 @@ static NTSTATUS HID_set_to_device(DEVICE_OBJECT *device, IRP *irp) > NTSTATUS rc; > > TRACE_(hid_report)("Device %p Buffer length %i Buffer %p\n", device, irpsp->Parameters.DeviceIoControl.InputBufferLength, irp->AssociatedIrp.SystemBuffer); > - packet.reportBuffer = irp->AssociatedIrp.SystemBuffer; > - packet.reportId = ((char*)irp->AssociatedIrp.SystemBuffer)[0]; > - packet.reportBufferLen = irpsp->Parameters.DeviceIoControl.InputBufferLength; > + packet.reportId = ((BYTE*)irp->AssociatedIrp.SystemBuffer)[0]; > + if (packet.reportId == 0) > + { > + packet.reportBuffer = &((BYTE*)irp->AssociatedIrp.SystemBuffer)[1]; > + packet.reportBufferLen = irpsp->Parameters.DeviceIoControl.InputBufferLength - 1; > + } > + else > + { > + packet.reportBuffer = irp->AssociatedIrp.SystemBuffer; > + packet.reportBufferLen = irpsp->Parameters.DeviceIoControl.InputBufferLength; > + } > TRACE_(hid_report)("(id %i, len %i buffer %p)\n", packet.reportId, packet.reportBufferLen, packet.reportBuffer); > > rc = call_minidriver(irpsp->Parameters.DeviceIoControl.IoControlCode, > @@ -636,20 +663,15 @@ NTSTATUS WINAPI HID_Device_read(DEVICE_OBJECT *device, IRP *irp) > if (buffer_size) > { > IO_STACK_LOCATION *irpsp = IoGetCurrentIrpStackLocation( irp ); > + NTSTATUS hr; Same here. > + ULONG out_length; > packet->reportBuffer = (BYTE *)packet + sizeof(*packet); > TRACE_(hid_report)("Got Packet %p %i\n", packet->reportBuffer, packet->reportBufferLen); > - if (irpsp->Parameters.Read.Length >= packet->reportBufferLen) > - { > - memcpy(irp->AssociatedIrp.SystemBuffer, packet->reportBuffer, packet->reportBufferLen); > - irp->IoStatus.Information = packet->reportBufferLen; > - irp->IoStatus.u.Status = STATUS_SUCCESS; > - } > - else > - { > - irp->IoStatus.Information = 0; > - irp->IoStatus.u.Status = STATUS_BUFFER_OVERFLOW; > - } > - IoCompleteRequest( irp, IO_NO_INCREMENT ); > + > + hr = copy_packet_into_buffer(packet, irp->AssociatedIrp.SystemBuffer, irpsp->Parameters.Read.Length, &out_length); > + irp->IoStatus.Information = out_length; > + irp->IoStatus.u.Status = hr; > + IoCompleteRequest(irp, IO_NO_INCREMENT); > } > else > { > @@ -671,9 +693,17 @@ NTSTATUS WINAPI HID_Device_write(DEVICE_OBJECT *device, IRP *irp) > irp->IoStatus.Information = 0; > > TRACE_(hid_report)("Device %p Buffer length %i Buffer %p\n", device, irpsp->Parameters.Write.Length, irp->AssociatedIrp.SystemBuffer); > - packet.reportBuffer = irp->AssociatedIrp.SystemBuffer; > - packet.reportId = ((char*)irp->AssociatedIrp.SystemBuffer)[0]; > - packet.reportBufferLen = irpsp->Parameters.Write.Length; > + packet.reportId = ((BYTE*)irp->AssociatedIrp.SystemBuffer)[0]; > + if (packet.reportId == 0) > + { > + packet.reportBuffer = &((BYTE*)irp->AssociatedIrp.SystemBuffer)[1]; > + packet.reportBufferLen = irpsp->Parameters.Write.Length - 1; Besides the fact that the code looks suspicious (couldn't a 0 byte also have a different meaning?) this will probably break the udev backend. As documented in https://www.kernel.org/doc/Documentation/hid/hidraw.txt, the write() command expects that the first byte is 0 if the device does not use numbered reports. > + } > + else > + { > + packet.reportBuffer = irp->AssociatedIrp.SystemBuffer; > + packet.reportBufferLen = irpsp->Parameters.Write.Length; > + } > TRACE_(hid_report)("(id %i, len %i buffer %p)\n", packet.reportId, packet.reportBufferLen, packet.reportBuffer); > > rc = call_minidriver(IOCTL_HID_WRITE_REPORT, device, NULL, 0, &packet, sizeof(packet)); > > >

2 4

Re: [12/23] wined3d: Clear WINED3D_BUFFER_DISCARD flag in unmap.
by Henri Verbeet Nov. 14, 2016

Nov. 14, 2016

On 13 November 2016 at 19:35, Andrew Wesie <awesie(a)gmail.com> wrote: > @@ -1248,6 +1248,7 @@ static void wined3d_buffer_unmap(struct wined3d_buffer *buffer) > context_release(context); > > buffer_clear_dirty_areas(buffer); > + buffer->flags &= ~WINED3D_BUFFER_DISCARD; Does filtering redundant WINED3D_MAP_DISCARD maps still work after this? Why is this change needed?

1 0

Re: [10/23] d3d11: Reset base vertex index to zero when not using indexed draw.
by Henri Verbeet Nov. 14, 2016

Nov. 14, 2016

On 13 November 2016 at 19:35, Andrew Wesie <awesie(a)gmail.com> wrote: > Signed-off-by: Andrew Wesie <awesie(a)gmail.com> > --- > dlls/d3d11/device.c | 4 ++++ > 1 file changed, 4 insertions(+) > I think this shouldn't be needed when you do the state invalidation in wined3d_cs_exec_draw(). Ideally we'd probably just get rid of wined3d_device_set_base_vertex_index(), but d3d8 stateblocks complicate that.

1 0

Re: [09/23] wined3d: Fix mapping of SV_VertexID to gl_VertexID.
by Henri Verbeet Nov. 14, 2016

Nov. 14, 2016

On 13 November 2016 at 19:35, Andrew Wesie <awesie(a)gmail.com> wrote: > @@ -481,7 +481,7 @@ void wined3d_cs_emit_draw(struct wined3d_cs *cs, int base_vertex_idx, unsigned i > > op = cs->ops->require_space(cs, sizeof(*op)); > op->opcode = WINED3D_CS_OP_DRAW; > - op->base_vertex_idx = base_vertex_idx; > + op->base_vertex_idx = indexed ? base_vertex_idx : 0; This change seems questionable. Why is it needed? > @@ -1988,7 +1988,11 @@ void CDECL wined3d_device_set_base_vertex_index(struct wined3d_device *device, I > { > TRACE("device %p, base_index %d.\n", device, base_index); > > - device->update_state->base_vertex_index = base_index; > + if (device->update_state->base_vertex_index != base_index) > + { > + device->update_state->base_vertex_index = base_index; > + device_invalidate_state(device, STATE_BASEVERTEXINDEX); > + } > } This should happen in wined3d_cs_exec_draw(). (The background is that we're working on executing that on a different thread, and state invalidation should generally happen in that same thread.) > @@ -1424,6 +1425,12 @@ static void shader_glsl_load_constants(void *shader_priv, struct wined3d_context > checkGLcall("glUniform4fv"); > } > > + if (update_mask & WINED3D_SHADER_CONST_BASE_VERTEX) > + { > + GL_EXTCALL(glUniform1i(prog->vs.base_vertex_location, state->base_vertex_index)); > + checkGLcall("glUniform1i"); > + } > @@ -1793,7 +1800,8 @@ static void shader_glsl_declare_generic_vertex_attribute(struct wined3d_string_b > > if (e->sysval_semantic == WINED3D_SV_VERTEX_ID) > { > - shader_addline(buffer, "vec4 vs_in%u = vec4(intBitsToFloat(gl_VertexID), 0.0, 0.0, 0.0);\n", > + shader_addline(buffer, "uniform int base_vertex;\n"); > + shader_addline(buffer, "vec4 vs_in%u = vec4(intBitsToFloat(gl_VertexID - base_vertex), 0.0, 0.0, 0.0);\n", > index); > return; > } Does this do the right thing when ARB_draw_elements_base_vertex is not supported?

1 0

Re: [00/23] Fixes to support Overwatch
by Matteo Bruni Nov. 14, 2016

Nov. 14, 2016

2016-11-13 12:35 GMT-06:00 Andrew Wesie <awesie(a)gmail.com>: > This is the first set of patches to support running Overwatch under WINE. > There are additional patches necessary but I am still preparing those for > release. Wow, that's very cool! Thank you for your work! You can expect your patches to be reviewed over the next few days and get either sign-offs or comments about things to improve. Going forward, in general I suggest you to limit your patch series to 5 patches or so (depending on how it makes the most sense), it makes it easier to the reviewers. I'm going to reply to a few patches in your series with the things I noticed at a quick look...

2 1

Re: [PATCH] wined3d: Add Polaris GPUs and their driver.
by Henri Verbeet Nov. 14, 2016

Nov. 14, 2016

On 13 November 2016 at 23:10, Grazvydas Ignotas <notasas(a)gmail.com> wrote: > - {HW_VENDOR_AMD, CARD_AMD_RADEON_R9, "AMD Radeon R9 290", DRIVER_AMD_R600, 4096}, > + {HW_VENDOR_AMD, CARD_AMD_RADEON_R9, "AMD Radeon R9 290", DRIVER_AMD_RX, 4096}, Is that change intentional?

2 4

Re: [PATCH v2] hidclass.sys: Handle POP and PUSH in device descriptors
by Sebastian Lackner Nov. 14, 2016

Nov. 14, 2016

On 14.11.2016 15:08, Aric Stewart wrote: > + if (!list_empty(&caps_stack)) > + { > + struct caps_stack *entry, *cursor; > + ERR("%i unpopped device caps on the stack\n", list_count(&caps_stack)); > + LIST_FOR_EACH_ENTRY_SAFE(entry, cursor, &caps_stack, struct caps_stack, entry) > + list_remove(&entry->entry); I assume you also wanted to deallocate the remaining blocks? > + }

2 1

Status of HID in Wine
by Aric Stewart Nov. 14, 2016

Nov. 14, 2016

Hello everyone, So with this the last few commits I can officially say that WINE now has a fully functioning HID stack for Linux hidraw devices. Thanks to Sebastian Lackner and Alexandre for all the help, reviews and attention paid. It is working and in the state it is now because of their assistance. This means that applications that use hid.dll or direct device access to HID devices will start seeing and being able to interact with devices. This is very exciting but is just the very first step. Of course any application trying to do HID directly would be a wonderful test case, I am sure there are many many things that will need to be refined and added before a real application starts being happy. I do have a very rough, quick, poorly coded test case for HID controllers available to the curious. https://www.codeweavers.com/xfer/aric/HID/hid.exe There are still manual steps required before things will start working. 1) By default no hidraw devices on Linux are given any access to users. So you will need to had rw permission to the /dev/hidraw<X> device that you want to have accessible in Wine. Or if you want to be wild and crazy you can add a new udev rule to add permission to all the devices as they are added. 2) The winebus service is not started by default yet. so you will need to start winebus. (wine net start winebus) This makes things a little complicated. I would recommend testing with a process like this: wine cmd net start winebus <test application> Or you an change the service start for winebus in the registry [System\\CurrentControlSet\\Services\\WineBus] "Start"=dword:00000002 I would love comment, reports, anything as people test various devices and programs. As for more platforms, patches for OS/X have just been submitted so that IOHID devices will work on Mac. Then I have a set of patches adding linux event devices as well. Looking forward, the hope is that then we will be able to build RawInput support on top of HID, and also dinput and xinput support as well. As a fun challenge I have been working to get native dinput from DirectX 9 to work on Wine on top of HID and it is moving forward much better than I expect, with a lot of hacks at present. (native Dinput sees the gamepads and even start trying to read them, but current is not getting reports properly) So tell me how things work! thanks, -aric

2 2