June 2017 - Wine-devel - list.winehq.org

Re: [PATCH] msacm32: Fix bytes written in PCM converter.
by Andrew Eikum 12 Jun '17

12 Jun '17

On Tue, Jun 06, 2017 at 06:17:46PM -0500, Zebediah Figura wrote: > diff --git a/dlls/msacm32/pcmconverter.c b/dlls/msacm32/pcmconverter.c > index e4bc5e7777..51f5eec8ee 100644 > --- a/dlls/msacm32/pcmconverter.c > +++ b/dlls/msacm32/pcmconverter.c > @@ -561,6 +561,7 @@ static void cvtSS88C(DWORD srcRate, const unsigned char* src, LPDWORD nsrc, > error = error - dstRate; > } > } > + *ndst = 0; > } > I'm confused how this makes a difference. Isn't ndst already 0 at this point since it exited the while loop? Andrew

3 3

Re: [PATCH 4/5] d3drm: Make it possible to create frame with CreateObject()
by Henri Verbeet 12 Jun '17

12 Jun '17

On 11 June 2017 at 17:52, Nikolay Sivov <nsivov(a)codeweavers.com> wrote: > Subject: [PATCH 4/5] d3drm: Make it possible to create frame with CreateObject() Technically that should be "frames" or "frame objects" instead of "frame". Likewise in patch 5/5.

1 0

Re: [PATCH 3/5] d3drm/tests: Some tests for mesh builder and faces interaction
by Henri Verbeet 12 Jun '17

12 Jun '17

On 11 June 2017 at 17:52, Nikolay Sivov <nsivov(a)codeweavers.com> wrote: > + /* CreateFace() already added this face. */ > + hr = IDirect3DRMMeshBuilder_AddFace(mesh_builder, face); > + ok(hr == D3DRMERR_FACEUSED, "Got unexpected hr %#x.\n", hr); > + > + count = IDirect3DRMMeshBuilder_GetFaceCount(mesh_builder); > + ok(count == 1, "Unexpected face count %d.\n", count); > + > + hr = IDirect3DRMFace_AddVertex(face, 1.0f, 2.0f, 3.0f); > + ok(SUCCEEDED(hr), "Failed to add a vertex, hr %#x.\n", hr); > + > + ref = IDirect3DRMFace_Release(face); > + ok(ref == 0, "Unexpected ref count %u.\n", ref); > + > + count = IDirect3DRMMeshBuilder_GetFaceCount(mesh_builder); > + ok(count == 1, "Unexpected face count %d.\n", count); > + > + hr = IDirect3DRMMeshBuilder2_GetFace(mesh_builder2, 0, &face1); > + ok(SUCCEEDED(hr), "Failed to retrieve face, hr %#x.\n", hr); > + ok(face1 != face, "Unexpected face pointer.\n"); This fails for me on Windows: d3drm.c:606: Test failed: Unexpected face pointer. > +static void test_add_vertex_normal_indexed(void) > +{ ... > + hr = IDirect3DRMMeshBuilder3_DeleteVertices(mesh_builder3, 0, 1); > + ok(SUCCEEDED(hr), "Failed to delete vertices, hr %#x.\n", hr); > +} > + IDirect3DRMMeshBuilder3_Release(mesh_builder3); > + IDirect3DRMMeshBuilder_Release(mesh_builder); > + > + IDirect3DRM_Release(d3drm); > +} This sometimes fails for me on Windows: d3drm.c:6787: this is the last test seen before the exception 0cec:d3drm: unhandled exception c0000005 at 77532A62

1 0

Re: [PATCH 1/5] d3drm/tests: Add helpers to test vectors
by Henri Verbeet 12 Jun '17

12 Jun '17

On 11 June 2017 at 17:52, Nikolay Sivov <nsivov(a)codeweavers.com> wrote: > Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> > --- > dlls/d3drm/tests/d3drm.c | 144 +++++++++++++++++++---------------------------- > 1 file changed, 57 insertions(+), 87 deletions(-) > For what it's worth, 32 seems fairly loose for values that are either loaded directly or trivially computed. I suspect it should be possible to tighten that a little.

1 0

Re: [PATCH] user32: Return desktop as shell window if shell is not set.
by Nikolay Sivov 12 Jun '17

12 Jun '17

On 10.06.2017 20:22, Ilya Konovalov wrote: > This should fix https://bugs.winehq.org/show_bug.cgi?id=38196 Explorer seems like a good candidate to call SetShellWindow() from.

2 1

Re: [PATCH v2] msxml3: Remove CRs in domtext_put_data and add them in domtext_get_xml.
by Marvin 12 Jun '17

12 Jun '17

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=31588 Your paranoid android. === wxppro (32 bit domdoc) === 0a20:domdoc: unhandled exception c0000005 at 749B3845 === w2003std (32 bit domdoc) === 05f0:domdoc: unhandled exception c0000005 at 72EAD79F === wvistau64 (32 bit domdoc) === 01f4:domdoc: unhandled exception c0000005 at 737A9A31 === w2008s64 (32 bit domdoc) === 08f4:domdoc: unhandled exception c0000005 at 741B9A31 === w7u (32 bit domdoc) === 0f30:domdoc: unhandled exception c0000005 at 6BF5ADFC === w7pro64 (32 bit domdoc) === 084c:domdoc: unhandled exception c0000005 at 737AADA9 === w8 (32 bit domdoc) === 0ecc:domdoc: unhandled exception c0000005 at 68D2B47E === w864 (32 bit domdoc) === 0f98:domdoc: unhandled exception c0000005 at 7445B47E === w1064 (32 bit domdoc) === 0eb4:domdoc: unhandled exception c0000005 at 6FFD7894 === wvistau64 (64 bit domdoc) === 0504:domdoc: unhandled exception c0000005 at 000007FEF9FDA46A === w2008s64 (64 bit domdoc) === 0a44:domdoc: unhandled exception c0000005 at 000007FEF92DA46A === w7pro64 (64 bit domdoc) === 06ec:domdoc: unhandled exception c0000005 at 000007FEF3D1A0FC === w864 (64 bit domdoc) === 0fa8:domdoc: unhandled exception c0000005 at 00007FFB1B6148C4 === w1064 (64 bit domdoc) === 0e28:domdoc: unhandled exception c0000005 at 00007FFC241D2166

1 0

Re: [PATCH v3] comctl32/taskdialog: Implement buttons.
by Nikolay Sivov 11 Jun '17

11 Jun '17

Thanks. I attached what I believe to be improved version, while keeping your positioning logic. It fixes compiler warning, loading custom button text from resources, naming issues, and minor formatting inconsistencies. If you're okay with it, I'll submit under your name, or you can do it yourself. Nikolay

3 2

Re: [PATCH 3/5] d3dx9: Turn a "Out of bounds" FIXME into a WARN.
by Paul Gofman 11 Jun '17

11 Jun '17

Matteo, I am curious why would you want to change this FIXME to WARN? We know that native implementation does not fail effect creation in this case. Regards, Paul. On 06/09/2017 10:19 PM, Matteo Bruni wrote: > Signed-off-by: Matteo Bruni <mbruni(a)codeweavers.com> > --- > It could be triggered by broken preshaders. > > dlls/d3dx9_36/preshader.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/dlls/d3dx9_36/preshader.c b/dlls/d3dx9_36/preshader.c > index 522d4841f9..d0cff22e4a 100644 > --- a/dlls/d3dx9_36/preshader.c > +++ b/dlls/d3dx9_36/preshader.c > @@ -868,7 +868,7 @@ static HRESULT parse_preshader(struct d3dx_preshader *pres, unsigned int *ptr, u > } > if (reg_idx >= pres->regs.table_sizes[table]) > { > - FIXME("Out of bounds register index, i %u, j %u, table %u, reg_idx %u.\n", > + WARN("Out of bounds register index, i %u, j %u, table %u, reg_idx %u.\n", > i, j, table, reg_idx); > return D3DXERR_INVALIDDATA; > }

2 1

Re: [PATCH 1/5] d3dx9: Avoid casting each value separately in set_constants().
by Matteo Bruni 09 Jun '17

09 Jun '17

2017-06-06 14:28 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>: > +static void pres_float_to_int(void *in, void *out, unsigned int count) "in" should probably be const in all of these helpers. > +static void pres_bool_to_int(void *in, void * out, unsigned int count) Stray whitespace after *. > +{ > + unsigned int i; > + const float *in_bool = in; > + int *out_int = out; > + > + for (i = 0; i < count; ++i) > + out_int[i] = !!in_bool[i]; Is the double negation necessary here (or above)? > +static void regstore_set_data(struct d3dx_regstore *rs, unsigned int table, > + unsigned int offset, unsigned int *in, unsigned int count, enum pres_value_type param_type) > +{ > + typedef void (*conv_func)(void *in, void *out, unsigned int count); > + static conv_func set_const_funcs[PRES_VT_COUNT][PRES_VT_COUNT] = Missing const. > + { > + {NULL, NULL, pres_float_to_int, pres_value_to_bool}, > + {NULL, NULL, NULL, NULL}, > + {pres_int_to_float, NULL, NULL, pres_value_to_bool}, > + {pres_bool_to_float, NULL, pres_bool_to_int, NULL} > + }; > + enum pres_value_type table_type = table_info[table].type; > + > + if (table_type == param_type) > + { > + regstore_set_values(rs, table, in, offset, count); > + return; > + } IIUC in this case regstore_set_values() will have in == out. That's a problem since it means memcpy()ing the data onto itself, which is undefined. It's also unnecessary, regstore_set_modified() would be enough, but only if in == out which is something the current regstore_set_data() interface doesn't enforce (and indeed patch 2/5 doesn't respect). I guess the root issue is that you're now using the regstore also as temporary storage and none of the existing (and new) code expects it. So it's a matter of fixing the code as necessary to make it work fine or to avoid this "new" usage of the regstore e.g. by means of a separate temporary buffer. > for (element = 0; element < const_set->element_count; ++element) > { > + unsigned int *out = (unsigned int *)rs->tables[table] + start_offset; Aside from the issue I mentioned above, this works only because all the formats we're interesting in converting from / to happen be 4 bytes wide. It's somewhat assumed in a bunch of places already but adding more isn't too exciting. I'd like at least an assert checking for the precondition, if possible. > @@ -1083,23 +1155,15 @@ static void set_constants(struct d3dx_regstore *rs, struct d3dx_const_tab *const > WARN("Parameter data is too short, name %s, component %u.\n", debugstr_a(param->name), i); > break; > } > - > - in = (unsigned int *)data + param_offset; > - switch (table_type) > - { > - case PRES_VT_FLOAT: set_number(&out, D3DXPT_FLOAT, in, param->type); break; > - case PRES_VT_INT: set_number(&out, D3DXPT_INT, in, param->type); break; > - case PRES_VT_BOOL: set_number(&out, D3DXPT_BOOL, in, param->type); break; > - default: > - FIXME("Unexpected type %#x.\n", table_info[table].type); > - break; > - } > - regstore_set_values(rs, table, &out, offset, 1); > + out[offset] = data[param_offset]; I think separating the matrix transposing / "reshaping" from the data conversion is okay, if that allows further changes and optimizations like patch 2/5. You might want to add a small comment around here to clarify the split. > } > } > start_offset += get_offset_reg(table, const_set->register_count); > data += param->rows * param->columns; > } > + start_offset = get_offset_reg(table, const_set->register_index); > + regstore_set_data(rs, table, start_offset, (unsigned int *)rs->tables[table] + start_offset, > + get_offset_reg(table, const_set->register_count) * const_set->element_count, param_type); I understand that you want to avoid to use some separate storage for the "to be converted" data but, like above, this regstore table pointer cast makes me a bit uncomfortable.

2 4

Re: [PATCH 4/5] d3dx9: Get rid of constant's length checking for matrix in set_constants().
by Matteo Bruni 09 Jun '17

09 Jun '17

2017-06-06 14:28 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>: > @@ -1026,10 +1027,22 @@ static void get_const_upload_info(struct d3dx_const_param_eval_output *const_set > info->major = param->rows; > info->minor = param->columns; > } > - info->major_stride = max(info->minor, get_offset_reg(table, 1)); > - info->major_count = min(info->major * info->major_stride, > - get_offset_reg(table, const_set->register_count) + info->major_stride - 1) / info->major_stride; > - info->count = info->major_count * info->minor; > + > + if (table == PRES_REGTAB_OBCONST) Minor, but you could key this on component_count == 1 instead of explicitly checking PRES_REGTAB_OBCONST. > + { > + unsigned int const_length = get_offset_reg(table, const_set->register_count); > + > + info->major_stride = info->minor; > + info->major_count = const_length / info->major_stride; > + info->minor_remainder = const_length % info->major_stride; > + } > + else > + { > + info->major_stride = get_offset_reg(table, 1); Another potential use for the get_reg_components(table) helper :)

2 2