June 2012 - Wine-Devel - list.winehq.org

Re: [PATCH 5/5] d3dcompiler: Parse variable declarations.

by Matteo Bruni

2012/6/5 Matteo Bruni <matteo.mystral(a)gmail.com>: > 2012/6/5 Rico Schüller <kgbricola(a)web.de>: >> Am 05.06.2012 13:08, schrieb Matteo Bruni: >> >>> 2012/6/5 Stefan Dösinger<stefandoesinger(a)gmx.at>: >>>> >>>> Am Montag, 4. Juni 2012, 17:58:24 schrieb Matteo Bruni: >>>>> >>>>> +struct hlsl_type >>>>> +{ >>>>> ... >>>>> + unsigned int dimx; >>>>> + unsigned int dimy; >>>>> ... >>>>> +}; >>>> >>>> One thing I noticed when I wrote my compiler was that a float1x1 is not >>>> the >>>> same as a float1 or float. I did not implement this difference nor do I >>>> know >>>> if any games depend on it. >>>> >>>> (That's no reason to hold up this patch, just something to keep in mind >>>> for >>>> further work) >>>> >>> >>> Yeah, I noticed that too, but it doesn't seem to make much of a >>> difference in practice. Of course, if later on it shows that it >>> actually matters for something relevant, I'll change things >>> accordingly. >>> >>> >>> >> For the effect interface, it makes a difference. See >> http://source.winehq.org/git/wine.git/blob/90f93e3819dcaa0cea16bd297fa69a35… >> and >> http://source.winehq.org/git/wine.git/blob/90f93e3819dcaa0cea16bd297fa69a35… >> . The difference is the type D3DXPC_SCALAR vs D3DXPC_VECTOR vs >> D3DXPC_MATRIX_ROWS and of course some different handling, see >> http://source.winehq.org/git/wine.git/blob/90f93e3819dcaa0cea16bd297fa69a35… >> . A test may help to check how variables in shaders are handled. >> >> Cheers >> Rico Oops, I misclicked on the send button, sorry for the spam... I just did a quick test. It looks like that the actual generated code does not change but the constant table is affected by the scalar/vector/matrix type similarly to the effect interface stuff. Since the compiler seems to preserve the type of the constant, I guess in general this may matter, after all... So I presume I'll add another field to the hlsl_type structure in the near future to keep track of the scalar/vector/matrix type or something like that. Anyway this patch is not affected by this detail, so I think it is still good to go. BTW, thanks for the comments :)

June 5, 2012

Re: [PATCH 5/5] d3dcompiler: Parse variable declarations.

by Matteo Bruni

2012/6/5 Rico Schüller <kgbricola(a)web.de>: > Am 05.06.2012 13:08, schrieb Matteo Bruni: > >> 2012/6/5 Stefan Dösinger<stefandoesinger(a)gmx.at>: >>> >>> Am Montag, 4. Juni 2012, 17:58:24 schrieb Matteo Bruni: >>>> >>>> +struct hlsl_type >>>> +{ >>>> ... >>>> + unsigned int dimx; >>>> + unsigned int dimy; >>>> ... >>>> +}; >>> >>> One thing I noticed when I wrote my compiler was that a float1x1 is not >>> the >>> same as a float1 or float. I did not implement this difference nor do I >>> know >>> if any games depend on it. >>> >>> (That's no reason to hold up this patch, just something to keep in mind >>> for >>> further work) >>> >> >> Yeah, I noticed that too, but it doesn't seem to make much of a >> difference in practice. Of course, if later on it shows that it >> actually matters for something relevant, I'll change things >> accordingly. >> >> >> > For the effect interface, it makes a difference. See > http://source.winehq.org/git/wine.git/blob/90f93e3819dcaa0cea16bd297fa69a35… > and > http://source.winehq.org/git/wine.git/blob/90f93e3819dcaa0cea16bd297fa69a35… > . The difference is the type D3DXPC_SCALAR vs D3DXPC_VECTOR vs > D3DXPC_MATRIX_ROWS and of course some different handling, see > http://source.winehq.org/git/wine.git/blob/90f93e3819dcaa0cea16bd297fa69a35… > . A test may help to check how variables in shaders are handled. > > Cheers > Rico

June 5, 2012

Re: [4/5] msi/tests: Use only uppercase characters for the PID_REVNUMBER property.

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 http://testbot.winehq.org/JobDetails.pl?Key=18792 Your paranoid android. === WNT4WSSP6 (32 bit install) === install.c:6053: Test failed: Per-user icon file isn't where it's expected (C:\WINNT\Profiles\Default User\Application Data\Microsoft\Installer\{7DF88A49-996F-4EC8-A022-BF956F9B2CBB}\testicon)

June 5, 2012

Re: [PATCH 5/5] d3dcompiler: Parse variable declarations.

by Rico Schüller

Am 05.06.2012 13:08, schrieb Matteo Bruni: > 2012/6/5 Stefan Dösinger<stefandoesinger(a)gmx.at>: >> Am Montag, 4. Juni 2012, 17:58:24 schrieb Matteo Bruni: >>> +struct hlsl_type >>> +{ >>> ... >>> + unsigned int dimx; >>> + unsigned int dimy; >>> ... >>> +}; >> One thing I noticed when I wrote my compiler was that a float1x1 is not the >> same as a float1 or float. I did not implement this difference nor do I know >> if any games depend on it. >> >> (That's no reason to hold up this patch, just something to keep in mind for >> further work) >> > > Yeah, I noticed that too, but it doesn't seem to make much of a > difference in practice. Of course, if later on it shows that it > actually matters for something relevant, I'll change things > accordingly. > > > For the effect interface, it makes a difference. See http://source.winehq.org/git/wine.git/blob/90f93e3819dcaa0cea16bd297fa69a35… and http://source.winehq.org/git/wine.git/blob/90f93e3819dcaa0cea16bd297fa69a35… . The difference is the type D3DXPC_SCALAR vs D3DXPC_VECTOR vs D3DXPC_MATRIX_ROWS and of course some different handling, see http://source.winehq.org/git/wine.git/blob/90f93e3819dcaa0cea16bd297fa69a35… . A test may help to check how variables in shaders are handled. Cheers Rico

June 5, 2012

Re: [1/5] msi: Add support for 64-bit registry components.

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 http://testbot.winehq.org/JobDetails.pl?Key=18791 Your paranoid android. === WNT4WSSP6 (32 bit install) === install.c:6053: Test failed: Per-user icon file isn't where it's expected (C:\WINNT\Profiles\Default User\Application Data\Microsoft\Installer\{7DF88A49-996F-4EC8-A022-BF956F9B2CBB}\testicon)

June 5, 2012

Re: oleaut32: Add a test for BSTR cache. Take 3.

by Dmitry Timoshkov

Jacek Caban <jacek(a)codeweavers.com> wrote: > >> BTW, did you test the app on Windows with disabled BSTR cache? It's easy > >> to do by setting OANOCACHE environment variable before running the app. > >> If that breaks the app, then indeed the app is broken in a way that it > >> depends on BSTR cache behaviour. > > It's easy to test under Wine too. Yes, the app depends on enabled BSTR cache. > > Then you don't eliminate possibility of use-after-free bugs in Wine code > base. Of course. But if that wasn't clear enough I'll rephrase it: yes, I tested the affected application under Windows, and setting OANOCACHE breaks it. With native oleaut32.dll the app works perfectly under Wine, setting OANOCACHE breaks it same way as under Windows. -- Dmitry.

June 5, 2012

Re: oleaut32: Add a test for BSTR cache. Take 3.

by Jacek Caban

On 06/05/12 13:31, Dmitry Timoshkov wrote: > Jacek Caban <jacek(a)codeweavers.com> wrote: > >> BTW, did you test the app on Windows with disabled BSTR cache? It's easy >> to do by setting OANOCACHE environment variable before running the app. >> If that breaks the app, then indeed the app is broken in a way that it >> depends on BSTR cache behaviour. > It's easy to test under Wine too. Yes, the app depends on enabled BSTR cache. Then you don't eliminate possibility of use-after-free bugs in Wine code base. Jacek

June 5, 2012

Re: oleaut32: Add a test for BSTR cache. Take 3.

by Alexandre Julliard

Dmitry Timoshkov <dmitry(a)baikal.ru> writes: > Alexandre Julliard <julliard(a)winehq.org> wrote: > >> That doesn't seem very useful. What use case would there be for an app >> to rely on some random 75% of its strings to remain valid? > > Although the test is about statistics, my intention was to show that BSTR > cache in Wine corrupts cached strings. I could try to figure out exact > steps which lead to corruption of real strings the application cares > about, but that's pretty hard to do, there are enourmous amount of lines in > the log that are related to BSTR creation/destruction, and finally the string > the app cares about and tries to use is corrupted. I thought that adding > a statistical test would hint Jacek what might be wrong with current cache > implementation. Any cache is going to reuse space sooner or later, that doesn't mean there's something wrong with it. If an app relies on a specific string not being reused, we need a test for that exact allocation pattern so that we can make sure our cache works the same way for that case. -- Alexandre Julliard julliard(a)winehq.org

June 5, 2012

Re: oleaut32: Add a test for BSTR cache. Take 3.

by Dmitry Timoshkov

Jacek Caban <jacek(a)codeweavers.com> wrote: > BTW, did you test the app on Windows with disabled BSTR cache? It's easy > to do by setting OANOCACHE environment variable before running the app. > If that breaks the app, then indeed the app is broken in a way that it > depends on BSTR cache behaviour. It's easy to test under Wine too. Yes, the app depends on enabled BSTR cache. -- Dmitry.

June 5, 2012

Re: oleaut32: Add a test for BSTR cache. Take 3.

by Jacek Caban

Hi Dmitry, On 06/05/12 13:05, Dmitry Timoshkov wrote: > Alexandre Julliard <julliard(a)winehq.org> wrote: > >> That doesn't seem very useful. What use case would there be for an app >> to rely on some random 75% of its strings to remain valid? > Although the test is about statistics, my intention was to show that BSTR > cache in Wine corrupts cached strings. I could try to figure out exact > steps which lead to corruption of real strings the application cares > about, but that's pretty hard to do, there are enourmous amount of lines in > the log that are related to BSTR creation/destruction, and finally the string > the app cares about and tries to use is corrupted. I thought that adding > a statistical test would hint Jacek what might be wrong with current cache > implementation. Unfortunately, statistical test isn't much helpful. The corruption you can see is probably due to the fact that not all strings are cached. Once the cache is full (for strings of similar size), the string is immediately freed. We can't cache all of them, that would be a massive leak. When I wrote the patch, I've seen the exact app using BSTRs in a pattern that I've verified that should work with the cache (or better said, the bug is hidden by the cache). I understand that it may be not easy in some cases, but I don't see how we could fix the implementation without understanding the cause of the problem. BTW, did you test the app on Windows with disabled BSTR cache? It's easy to do by setting OANOCACHE environment variable before running the app. If that breaks the app, then indeed the app is broken in a way that it depends on BSTR cache behaviour. Cheers, Jacek

June 5, 2012