April 2016 - Wine-devel - list.winehq.org

Re: compobj.dll16: Fix implementation of StringFromGUID2.
by Nikolay Sivov 14 Apr '16

14 Apr '16

On 14.04.2016 19:44, Sebastian Lackner wrote: > -INT WINAPI StringFromGUID216(REFGUID id, LPOLESTR str, INT cmax) > +INT WINAPI StringFromGUID216(REFGUID id, LPOLESTR16 str, INT cmax) It also should use INT16, with spec file fixed accordingly.

1 0

Fullscreen upscaling
by David Guillen Fandos 14 Apr '16

14 Apr '16

Hey there! I was looking into doing some automatic upscale for HiDPI screens (mine is 3200*1800). It would be some knob that allows the user to choose a x2 scaling for fullscreen apps and similar stuff. After reading winex11.drv I saw that it wouldn't be that hard to report 1/2 of the actual monitor resolution and then just upscale whatever gets rendered to the gdi_display (so we would have an unscaled surface and then blit that with a 2x scaling to the gdi_display). I guess that some other modifications would be needed though. Any comments on this? I think it could be useful for some people, nowadays that screens just got lots of pixels. Thanks! David G.F.

3 3

Re: [v2 PATCH] dwrite/tests: A test for glyphBox* metrics
by Marvin 14 Apr '16

14 Apr '16

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=22191 Your paranoid android. === w864 (32 bit font) === The test timed out

2 1

Re: [v2 PATCH 2/2] d3d10core: Support NULL RenderTargetView pointer in ClearRenderTargetView
by Marvin 14 Apr '16

14 Apr '16

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=22171 Your paranoid android. === w7u (32 bit device) === device.c:6244: Test failed: Got unexpected value 2.35098856e-038 at (0, 0). === w7pro64 (32 bit device) === device.c:6244: Test failed: Got unexpected value 2.35098856e-038 at (0, 0). === w8 (32 bit device) === device.c:6244: Test failed: Got unexpected value 2.35098856e-038 at (0, 0). === w864 (32 bit device) === device.c:6244: Test failed: Got unexpected value 2.35098856e-038 at (0, 0). === w1064 (32 bit device) === device.c:6244: Test failed: Got unexpected value 2.35098856e-038 at (0, 0). === w7pro64 (64 bit device) === device.c:6244: Test failed: Got unexpected value 2.35098856e-038 at (0, 0). === w864 (64 bit device) === device.c:6244: Test failed: Got unexpected value 2.35098856e-038 at (0, 0). === w1064 (64 bit device) === device.c:6244: Test failed: Got unexpected value 2.35098856e-038 at (0, 0).

1 0

Re: [v2 PATCH 1/2] d3d10core: Support NULL DepthStecilView pointer in ClearDepthStencilView
by Marvin 14 Apr '16

14 Apr '16

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=22170 Your paranoid android. === w7u (32 bit device) === device.c:6235: Test failed: Got unexpected value 2.35098856e-038 at (0, 0). === w7pro64 (32 bit device) === device.c:6235: Test failed: Got unexpected value 2.35098856e-038 at (0, 0). === w8 (32 bit device) === device.c:6235: Test failed: Got unexpected value 2.35098856e-038 at (0, 0). === w864 (32 bit device) === device.c:6235: Test failed: Got unexpected value 2.35098856e-038 at (0, 0). === w1064 (32 bit device) === device.c:6235: Test failed: Got unexpected value 2.35098856e-038 at (0, 0). === w7pro64 (64 bit device) === device.c:6235: Test failed: Got unexpected value 2.35098856e-038 at (0, 0). === w864 (64 bit device) === device.c:6235: Test failed: Got unexpected value 2.35098856e-038 at (0, 0). === w1064 (64 bit device) === device.c:6235: Test failed: Got unexpected value 2.35098856e-038 at (0, 0).

1 0

Re: [PATCH] dwrite/tests: A test for glyphBox* metrics
by Sebastian Lackner 14 Apr '16

14 Apr '16

On 14.04.2016 06:13, Nikolay Sivov wrote: > + ok(m1->glyphBoxBottom == expected->glyphBoxBottom, "font %s: glyphBoxLeft %d, expected %d\n", wine_dbgstr_w(nameW), > + m1->glyphBoxBottom, expected->glyphBoxBottom); There is a copy&paste in the ok() message error here.

1 0

Re: [v11 1/4] d3dx9: Implement fxlc constants (expressions) in effect.
by Matteo Bruni 13 Apr '16

13 Apr '16

2016-04-12 11:34 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>: > @@ -369,12 +410,17 @@ static HRESULT get_constants_desc(unsigned int *byte_code, struct d3dx_const_tab > goto err_out; > } > count = 1; > - hr = ID3DXConstantTable_GetConstantDesc(ctab, hc, &cdesc[i], &count); > - if (FAILED(hr)) > + if (FAILED(hr = ID3DXConstantTable_GetConstantDesc(ctab, hc, &cdesc[i], &count))) > { > FIXME("Could not get constant desc, hr %#x.\n", hr); > goto err_out; > } > + if (count != 1) > + { > + FIXME("Unexpected constant descriptors count %u.\n", count); > + hr = D3DERR_INVALIDCALL; > + goto err_out; > + } Notice that, since you set count to 1 before calling GetConstantDesc(), the only possible results are 0 or 1 and the 0 case should be already handled by the "if (FAILED())" branch. You might use a temporary array of 3 elements and pass 3 as count, then only copy the first element to the destination so the rest of the code is unchanged while still giving a chance for the FIXME to trigger. Not sure it's worth it though, maybe just adding support for multiple descriptors (even only in struct d3dx_const_tab and the immediately related code) might be a better plan. > +static HRESULT set_constants_param(struct d3dx_regstore *rs, struct d3dx_const_tab *const_tab, > + D3DXHANDLE hc, struct d3dx_parameter *param) > +{ > + ID3DXConstantTable *ctab = const_tab->ctab; > + D3DXCONSTANT_DESC desc; > + unsigned int const_count, param_count, i, j, n, table, start_offset; > + unsigned int minor, major, major_stride, param_offset; > + BOOL transpose, get_element; > + unsigned int desc_count; > + > + desc_count = 1; > + ID3DXConstantTable_GetConstantDesc(ctab, hc, &desc, &desc_count); > + if (desc_count != 1) > + { > + FIXME("Unexpected constant descriptors count %u.\n", desc_count); > + return D3DERR_INVALIDCALL; > + } Same here, in this case the descriptors are only used locally so both options above should be easier to do. > + if (const_count != param_count) > + { > + FIXME("Number of elements or struct members differs between parameter and constant.\n"); > + return D3DERR_INVALIDCALL; > + } You could print the two counts here. Not a big deal though. Looks good otherwise.

2 2

Re: [v10 1/4] d3dx9: Implement fxlc constants (expressions) in effect.
by Matteo Bruni 13 Apr '16

13 Apr '16

2016-04-07 17:41 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>: > switch (state->type) > { > case ST_CONSTANT: > + *param_value = param->data; > + *out_param = param; > + return D3D_OK; > case ST_PARAMETER: > + param = param->referenced_param; > *param_value = param->data; > *out_param = param; > return D3D_OK; It doesn't matter (this is perfectly fine) but you could have kept the fallthrough by switching these two cases around. > +static HRESULT set_constants_param(struct d3dx_regstore *rs, struct d3dx_const_tab *const_tab, > + D3DXHANDLE hc, struct d3dx_parameter *param) > +{ > + ID3DXConstantTable *ctab = const_tab->ctab; > + D3DXCONSTANT_DESC desc; > + unsigned int const_count, param_count, i, j, n, table, start_offset; > + unsigned int minor, major, major_stride, param_offset; > + BOOL transpose, get_element; > + > + ID3DXConstantTable_GetConstantDesc(ctab, hc, &desc, &const_count); This isn't correct, although it currently works. const_count is supposed to be an inout parameter for GetConstantDesc() (you could potentially have multiple instances of the same constant in different register sets). The current implementation of GetConstantDesc() doesn't check the parameter and blindly returns only the first descriptor, also setting const_count to 1 (actually the following descriptors aren't even parsed currently by the constant table code IIRC) so this ends up working, although that's a bug and at some point it might get fixed which would in turn break this. Also it's a bit of an "overload" to use the same variable here and as the count of constant elements / members in the following lines, it's probably nicer to use two separate variables. BTW you're using GetConstantDesc() correctly in get_constants_desc(). > + table = const_tab->regset2table[desc.RegisterSet]; > + if (table >= PRES_REGTAB_COUNT) > + { > + FIXME("Unexpected register set %u.\n", desc.RegisterSet); > + return D3DERR_INVALIDCALL; > + } Since regset2table is defined by us ERR() is maybe more appropriate here. It might make sense to also check desc.RegisterSet before accessing the array. > + hc = ID3DXConstantTable_GetConstant(const_tab->ctab, NULL, i); > + if (!hc) > + { > + FIXME("Could not get constant.\n"); > + hr = D3DERR_INVALIDCALL; You could probably also print the value of 'i' in the FIXME. Up to you, but you could switch the then and else branches and drop the '!' in the if condition. > + if (oi->func_all_comps) > + { > + if (oi->input_count * ins->component_count > ARGS_ARRAY_SIZE) > + { > + FIXME("Too many arguments (%u) for one instruction.\n", oi->input_count * ins->component_count); > + return E_FAIL; > + } I think this check should be outside of the if (oi->func_all_comps) block. > + hr = set_constants(&peval->pres.regs, &peval->pres.inputs); > + if (FAILED(hr)) > + return hr; > + > + hr = execute_preshader(&peval->pres); > + if (FAILED(hr)) > + return hr; Not a big deal but since you're there... You could merge the function calls with the respective if, like you're doing in some other patches of the series.

2 6

Re: [v11 4/4] d3dx9: Implement setting named shader constants in effect.
by Matteo Bruni 13 Apr '16

13 Apr '16

2016-04-12 11:34 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>: > static HRESULT d3dx9_apply_state(struct ID3DXEffectImpl *effect, struct d3dx_pass *pass, > - struct d3dx_state *state, int parent_index) > + struct d3dx_state *state, unsigned int parent_index); > + > +HRESULT d3dx_set_shader_const_fxlc(struct ID3DXEffectImpl *effect, struct d3dx_pass *pass, > + struct d3dx_parameter *param, BOOL vs) Nitpick, the _fxlc suffix doesn't seem correct at this point I think. Just call it d3dx_effect_set_shader_constants() or something similar? The function can probably be static too. > +static HRESULT set_shader_constants_device(struct IDirect3DDevice9 *device, struct d3dx_regstore *rs, > + D3DXPARAMETER_TYPE type, enum pres_reg_tables table) > +{ > + unsigned int start, n; Maybe call it "count" instead of "n"? It's up to you though.

1 0

Re: [v11 2/4] d3dx9: Add basic opcodes to preshader.
by Matteo Bruni 13 Apr '16

13 Apr '16

2016-04-12 11:34 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>: > @@ -3834,11 +3831,16 @@ static void test_effect_preshader(IDirect3DDevice9 *device) > v = i < 8 ? (unsigned int *)&light.Diffuse : (unsigned int *)&light.Ambient; > ok(hr == D3D_OK, "Got result %#x.\n", hr); > for (j = 0; j < 4; ++j) > + { > + unsigned int mask; > + > + mask = ~0u << test_effect_preshader_op_results[i].tolerance_bit_count; > todo_wine_if(test_effect_preshader_op_results[i].todo[j]) > - ok(v[j] == test_effect_preshader_op_results[i].result[j], > + ok((v[j] & mask) == (test_effect_preshader_op_results[i].result[j] & mask), That doesn't seem right. You can just use compare_float() to do a correct float comparison.

1 0