April 2017 - Wine-Devel - list.winehq.org

Re: [PATCH 2/2 v2] ntdll: Zero debug registers after thread creation

by Christian Inci

Thanks. I guess that I should quit coding altogether. On 04/09/2017 07:53 PM, Matteo Bruni wrote: > 2017-04-07 19:22 GMT+02:00 Christian Inci <chris.wine(a)broke-the-inter.net>: >> Thanks, I weren't sure about that one. I thought that every recent compiler will zero-initialize local variables/structures. >> Are there compilers or some special cases which wouldn't let that happen? >> Are they often enough not explicit set to zero (by using non-previously-used memory for the first time), so it'll be assumed to be zero? > > Huh? No, automatic variables aren't implicitly initialized in C and I > wouldn't want a compiler that goes out of its way to initialize them, > slowing down the code for everyone. >

April 9, 2017

Re: [PATCH 2/2 v2] ntdll: Zero debug registers after thread creation

by Matteo Bruni

2017-04-07 19:22 GMT+02:00 Christian Inci <chris.wine(a)broke-the-inter.net>: > Thanks, I weren't sure about that one. I thought that every recent compiler will zero-initialize local variables/structures. > Are there compilers or some special cases which wouldn't let that happen? > Are they often enough not explicit set to zero (by using non-previously-used memory for the first time), so it'll be assumed to be zero? Huh? No, automatic variables aren't implicitly initialized in C and I wouldn't want a compiler that goes out of its way to initialize them, slowing down the code for everyone.

April 9, 2017

Re: [PATCH v2 1/5] wined3d: Save pixel and vertex shader resource views in stateblocks.

by Matteo Bruni

2017-04-05 19:03 GMT+02:00 Henri Verbeet <hverbeet(a)gmail.com>: > BOOL ps_consts_f[224]; /* 176 896 */ > /* --- cacheline 16 boundary (1024 bytes) was 48 bytes ago --- */ > WORD vertexShaderConstantsB; /* 1072 2 */ > WORD vertexShaderConstantsI; /* 1074 2 */ > BOOL vs_consts_f[256]; /* 1076 1024 */ > /* --- cacheline 32 boundary (2048 bytes) was 52 bytes ago --- */ > Incidentally, this also illustrates the issue of the ps_consts_f and > vs_consts_f fields using up way more space than needed. Ugh, that's pretty bad... I couldn't help but write a patch for that. Needs testing though.

April 9, 2017

Re: [resend PATCH 4/4] d3dx9: Effect should preserve states regardless of state manager presence.

by Matteo Bruni

2017-04-06 1:32 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>: > Signed-off-by: Paul Gofman <gofmanp(a)gmail.com> Given that you have to resend anyway... It's generally preferred to write commit subjects in the imperative mood, so in this case it would be "Preserve states regardless of state manager presence." A slightly different, more direct, alternative would be "Ignore state manager when saving and restoring device state." Choose whatever you prefer (including something else entirely).

April 9, 2017

Re: [resend PATCH 2/4] d3dx9/tests: Add tests for effect state manager.

by Matteo Bruni

2017-04-06 1:32 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>: > Signed-off-by: Paul Gofman <gofmanp(a)gmail.com> > --- > + static const struct test_state_manager_update test_effect_states_record[] = expected_updates[]?

April 9, 2017

Re: [resend PATCH 1/4] d3dx9/tests: Add test implementation for ID3DXEffectStateManager interface.

by Matteo Bruni

2017-04-06 1:32 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>: > +struct test_state_manager_update > +{ > + unsigned int state_op; > + DWORD param1; > + DWORD param2; > +}; > + > +struct test_ID3DXEffectStateManagerImpl Sorry for being a PITA but it would be better to avoid this kind of naming, at a first glance it might be confused with a MS structure. Just call it test_manager or similar. > +static HRESULT WINAPI test_ID3DXEffectStateManagerImpl_QueryInterface(ID3DXEffectStateManager *iface, > + REFIID riid, void **object) > +{ > + if (IsEqualGUID(riid, &IID_IUnknown) > + || IsEqualGUID(riid, &IID_ID3DXEffectStateManager)) > + { > + iface->lpVtbl->AddRef(iface); > + *object = iface; > + return S_OK; > + } > + > + return E_NOINTERFACE; > +} Do you know if this is called by d3dx9 at all? Either way, adding some ok() here would be nice. > + state_manager->update_record = new_alloc; > + state_manager->update_record_size = new_size; > + > + } Spurious blank line. Also I'm not sure it makes a lot of sense to use new_alloc / new_size and check that the allocation succeeded with ok() to then always assign the new variables to the struct fields anyway. Memory allocation should never legitimately fail there so I'd remove the vestigial error checking entirely and let the test crash if the allocation somehow fails.

April 9, 2017

Re: [v2 1/2] d3dx9: Support relative addressing in preshader.

by Matteo Bruni

2017-04-03 19:50 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>: > On 04/03/2017 06:35 PM, Matteo Bruni wrote: >> >> 2017-03-29 20:17 GMT+02:00 Matteo Bruni <matteo.mystral(a)gmail.com>: >>> >>> Yeah and temporaries are supposed to be written before being read so >>> in theory you can entirely skip the update_table_size() calls for >>> source arguments. >> >> I haven't looked in detail but this still works for me (i.e. >> commenting out those calls doesn't break the tests here, on 32 bits at >> least). How does the accidental CTAB overwriting you mention happen >> exactly? > > Yes, if to just comment out it won't break any test, it will break that > one though if you replace table update with input operand offset range check > with failure return (after moving table size update from input parameters > above the loop of course, otherwise it will break many tests). > I actually spotted the accidental CTAB overwrite before that by watching > warn/trace output of tests and investigating why there are 'uninitialized > input' warnings on preshader operations testing, and had in mind to fix this > test at some point. Overwriting happens in test_preshader_op() function for > 2 argumemts op case. The function thinks there are 4 per-component > operations (which is the case in test blob for 1 argument test preshader but > actually not the case for 2 argument preshader blob), and puts opcode there, > while the last opcode fall after the actual end of preshader code to the > CTAB of the next preshader. > I agree that this part with table size update based on input operands > actual offset looks a bit ugly and can be most likely made better. I just > suspect that the better solution which won't break anything that can work > now same as native needs a bit more investigation of how native code handles > out of bound access in this case. Just dropping it without failing will lead > to crash or undefined behaviour if index register's offset is out of bound > now (which should not normally happen in my current understanding though > without broken bytecode, as well as the need to update table size based on > input operands as well). This of course can be mitigated by also checking > index register offsets at runtime (now the check done for value register > offsets only which can now include index register value), but I am not sure > that moving this extra check to execution time is much nicer. So maybe this > improvement can wait until we get all the remaining major pieces in place > (that pieces are, in my understanding, are state manager support and effect > pools/shared parameters, which make d3dx9 effect framework usable with the > majority of applications)? Or a simple solution is to fix the test and add > the checks instead of table update, ignoring compatibility with native > behaviour for such cases, which we consider possible on broken bytecode > only, at least based on what I've tested so far. I see. It's okay to handle this as native and be robust to inconsistent data in constant tables. >>> Hmm, I've never seen any effect with a number of immediates not >>> multiple of 4. Can you point me to one? >> >> Do you have one of those? > > I somehow was sure that there are such cases in our present tests, > though could not find them now. I pretty sure I saw the values of '6' as > immediate constant table size, maybe they were present in some previous > versions of blobs when there was less immediate constants used in relative > addressing test, or during my extensive testing of out of bounds indexes. > Anyway, I found such a case (18 immediate constants) in one of real apps I > am also testing this on, I am attaching a text file with that preshader dump > (it also has an immediate constants count display which I temporary added to > the preshader dump code, the same way you do in your patch, without the > values dump though). It looks to me as the CLIT section in the bytecode you attached contains 16 as the constants count. Not sure why the print below doesn't match that. Either way, it doesn't matter much. Even if immediate counts non-multiple-of-4 can actually exist we could (and I argue, should) bump that up to the next multiple, zeroing the extra components.

April 9, 2017

Re: [PATCH] d3dx11: Add D3DX11FilterTexture stub.

by Matteo Bruni

2017-04-03 18:49 GMT+02:00 Andrey Gusev <andrey.goosev(a)gmail.com>: > Signed-off-by: Andrey Gusev <andrey.goosev(a)gmail.com> > diff --git a/include/d3dx11tex.h b/include/d3dx11tex.h > index 16b280a..81a43cc 100644 > --- a/include/d3dx11tex.h > +++ b/include/d3dx11tex.h > @@ -93,6 +93,8 @@ HRESULT WINAPI D3DX11CreateShaderResourceViewFromMemory(ID3D11Device *device, co > HRESULT WINAPI D3DX11CreateTextureFromMemory(ID3D11Device *device, const void *src_data, SIZE_T src_data_size, > D3DX11_IMAGE_LOAD_INFO *loadinfo, ID3DX11ThreadPump *pump, ID3D11Resource **texture, HRESULT *hresult); > > +HRESULT WINAPI D3DX11FilterTexture(ID3D11DeviceContext *context, ID3D11Resource *texture, UINT src_level, UINT filter); > + > HRESULT WINAPI D3DX11GetImageInfoFromMemory(const void *src_data, SIZE_T src_data_size, ID3DX11ThreadPump *pump, > D3DX11_IMAGE_INFO *img_info, HRESULT *hresult); > > -- > 2.7.4 Could you please also add the D3DX11_FILTER_FLAG definition to d3dx11tex.h? Looks okay otherwise.

April 9, 2017

GSoC Mentors

by André Hentschel

Hi, for those who had not had a look at the proposals yet, please review them and comment. Thanks!

April 9, 2017

Re: [PATCH v5] ntdll/tests: Check debug registers of a newly created thread

by Marvin

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=30054 Your paranoid android. === build (build) === Make failed

April 9, 2017