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

Re: d3dx9_36 [patch 4/5]: Implement D3DXSHEvalConeLight
by Rico Schüller Oct. 9, 2012

Oct. 9, 2012

On 09.10.2012 09:50, Nozomi Kodama wrote: > +static void CapIntegrale(FLOAT *out, FLOAT order, FLOAT angle) > +{ I'd use a lower case name. > + if (order == 5) > + return; Indentation. If you break to a new line, please indent 8 spaces (in the case for ok(...)). > + 2.0f * sqrtf(D3DX_PI / 7.0f), 2.0f / 3.0f * sqrtf(D3DX_PI), 2.0f * sqrt (D3DX_PI / 11.0f) }; sqrtf. > + FLOAT cap[6], clamped_angle, norm, scale, temp[36]; No need for temp/scale? (see below) > + TRACE("order %u, dir %p, radius %f, red %f, green %f, blue %f, rout %p, gout %p, bout %p\n", order, dir, radius, Rintensity, Gintensity, Bintensity, rout, gout, bout); Long line. Please break. > + for (i = 0; i < order ; i++) Space. > + if (gout) > + for (i = 0; i < order; i++) > + { > + scale = cap[i] * Gintensity * coeff[i] / norm; > + for (j = 0; j < 2 * i + 1; j++) > + gout[i * i + j] = temp[i * i + j] * scale; > + } > + > + if (bout) > + for (i = 0; i < order; i++) > + { > + scale = cap[i] * Bintensity * coeff[i] / norm; > + for (j = 0; j < 2 * i + 1; j++) > + bout[i * i + j] = temp[i * i + j] * scale; > + } Pretty much the same calculation is done again and again. What happens if order is 100? You'll get a access violation. This is also true for cap and coeff in my sample here (It needs some tests ... Order may be clamped to D3DXSH_MAXORDER ... Does native crash in that case? Should we touch the values for order >= D3DXSH_MAXORDER at all?): D3DXSHEvalDirection(rout, order, dir); for (i = 0; i < order; i++) { FLOAT scale = cap[i] * coeff[i] / norm; for (j = 0; j < 2 * i + 1; j++) { FLOAT temp = rout[i * i + j] * scale; rout[i * i + j] = temp * Rintensity; if (gout) gout[i * i + j] = temp * Gintensity; if (bout) bout[i * i + j] = temp * Bintensity; } } Cheers Rico

1 0

Re: d3dx9_36 [patch 3/5]: Implement handy computations for small orders in D3DXSHRotate
by Rico Schüller Oct. 9, 2012

Oct. 9, 2012

On 09.10.2012 09:49, Nozomi Kodama wrote: > + FLOAT temp[25]; > + CONST FLOAT coeff[]={ > + matrix->u.m[1][0] * matrix->u.m[0][0], matrix->u.m[1][1] * matrix->u.m[0][1], > + matrix->u.m[1][1] * matrix->u.m[2][1], matrix->u.m[1][0] * matrix->u.m[2][0], > + matrix->u.m[2][0] * matrix->u.m[2][0], matrix->u.m[2][2] * matrix->u.m[2][2], > + matrix->u.m[2][1] * matrix->u.m[2][1], matrix->u.m[0][0] * matrix->u.m[2][0], > + matrix->u.m[0][1] * matrix->u.m[2][1], matrix->u.m[0][1] * matrix->u.m[0][1], > + matrix->u.m[1][0] * matrix->u.m[1][0], matrix->u.m[1][1] * matrix->u.m[1][1], > + matrix->u.m[0][0] * matrix->u.m[0][0], matrix->u.m[1][2] * matrix->u.m[1][2], }; > > + out[4] = temp[0] * in[4] + temp[1] * in[5] + temp[2] * in[6] + temp[3] * in[7] + temp[4] * in[8]; Do we really need that much temps? They aren't reused? What are the coeff used for and why are they calculated separately? Are those variables used more than once (coeff[5] and [13] are only used once), maybe it's a mathematical background? What about something like: temp = (matrix->u.m[1][1] * matrix->u.m[0][0] + matrix->u.m[0][1] * matrix->u.m[1][0]) * in[4]; temp -= (matrix->u.m[1][0] * matrix->u.m[2][1] + matrix->u.m[1][1] * matrix->u.m[2][0]) * in[5]; temp += ... out[4] = temp; (using out[4] += may also work, but I'm not sure for cases where in = out, if it's working, we don't need the temp at all, it may be needed to reorder the calculation in that case to start with the index needed in the result ... out[5] = y * in[5]; out[5] += x * in[4]; out[5] += z * in[6]; ...). Cheers Rico

1 0

Re: gdiplus: Add more font transformation tests.
by Vincent Povirk Oct. 9, 2012

Oct. 9, 2012

> +todo_wine > + expectf(133.203125, bounds.Height); Should we have this level of precision in our font tests?

2 1

Re: d3dx9_36 [patch 2/5]: Implement D3DXSHEvalHemisphereLight
by Rico Schüller Oct. 9, 2012

Oct. 9, 2012

On 09.10.2012 09:49, Nozomi Kodama wrote: > +HRESULT WINAPI D3DXSHEvalHemisphereLight(UINT order, CONST D3DXVECTOR3 *dir, D3DXCOLOR top, D3DXCOLOR bottom, FLOAT *rout, FLOAT *gout, FLOAT *bout) The number of columns used for the code seems to be a little high again. > + if (j >= 4 && j < order * order) > + expected = 0.0f; > + else if ( j >= order * order ) I'd use "if (x == y)" without spaces on all occurrences... there are a couple of them in the patch. Cheers Rico

1 0

Fwd: Re: ole32: Fix dwClsContext parameter of a CoCreateInstance call in DefaultHandler_Run.
by Nikolay Sivov Oct. 9, 2012

Oct. 9, 2012

Forwarding to wine-devel as well P.S. Roman, please reply to wine-devel list next time. -------- Original Message -------- Subject: Re: ole32: Fix dwClsContext parameter of a CoCreateInstance call in DefaultHandler_Run. Date: Tue, 09 Oct 2012 15:27:10 +0400 From: Roman Dadkov <romand(a)etersoft.ru> Organization: Etersoft To: Nikolay Sivov <bunglehead(a)gmail.com> Yes, it's my fail, I forgot, that can be use one enumeration CLSCTX_ALL in this case. Unfortunately, the test for this case can't be added to Wine, because it leads a crash. But I have attached a small test program that shows a problem. It uses CLSID which entered into the registry after installing Office using Wine. Nikolay Sivov 08.10.2012 10:07: > On 10/6/2012 19:29, Roman Dadkov wrote: >> This patch change dwClsContext parameter of a CoCreateInstance call >> in function DefaultHandler_Run. >> Because even if there is some clsid in the registry, the function >> will not be able to run the newly created object. >> >> >> >> - hr = CoCreateInstance(&This->clsid, NULL, CLSCTX_LOCAL_SERVER, >> + hr = CoCreateInstance(&This->clsid, NULL, >> + CLSCTX_INPROC_SERVER|CLSCTX_INPROC_HANDLER| >> + CLSCTX_LOCAL_SERVER|CLSCTX_REMOTE_SERVER, >> &IID_IOleObject, (void **)&This->pOleDelegate); > This could be replaced with CLSCTX_ALL. > > Is it possible to add a test for that?

1 0

Re: ole32: Fix dwClsContext parameter of a CoCreateInstance call in DefaultHandler_Run.
by Roman Dadkov Oct. 9, 2012

Oct. 9, 2012

Unfortunately, the test for this case can't be added to Wine, because it leads a crash. But I have attached a small test program that shows a problem. It uses CLSID which entered into the registry after installing Office using Wine.

1 0

Re: [3/3] winemaker: Generate def file from spec file with winebuild
by Alexandre Julliard Oct. 9, 2012

Oct. 9, 2012

André Hentschel <nerv(a)dawncrow.de> writes: > @@ -2576,7 +2577,9 @@ sub generate_project_files($) > if (@$target[$T_TYPE] == $TT_DLL && (@$project_settings[$T_FLAGS] & $TF_HASDEF)) { > print FILEO "\$(${canon}_MODULE).so: \$(${canon}_OBJS) \$(${canon}_MODULE:.dll=.def)\n"; > } elsif (@$target[$T_TYPE] == $TT_DLL) { > - print FILEO "\$(${canon}_MODULE).so: \$(${canon}_OBJS) \$(${canon}_MODULE:.dll=.spec)\n"; > + print FILEO "lib\$(${canon}_MODULE:.dll=.def): \$(${canon}_MODULE:.dll=.spec)\n"; > + print FILEO "\t\$(WINEBUILD) --def -E \$(${canon}_MODULE:.dll=.spec) -o lib\$(${canon}_MODULE:.dll=.def)\n\n"; > + print FILEO "\$(${canon}_MODULE).so: \$(${canon}_OBJS) \$(${canon}_MODULE:.dll=.spec) lib\$(${canon}_MODULE:.dll=.def)\n"; The .so doesn't depend on the .def. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: user32: Fix the edit class extra byte count on non-x86 platforms (try 2)
by Alexandre Julliard Oct. 8, 2012

Oct. 8, 2012

André Hentschel <nerv(a)dawncrow.de> writes: > @@ -5173,10 +5173,10 @@ const struct builtin_class_descr EDIT_builtin_class = > editW, /* name */ > CS_DBLCLKS | CS_PARENTDC, /* style */ > WINPROC_EDIT, /* proc */ > -#ifdef __i386__ > - sizeof(EDITSTATE *) + sizeof(WORD), /* extra */ > -#else > +#ifdef _WIN64 > sizeof(EDITSTATE *), /* extra */ > +#else > + sizeof(EDITSTATE *) + sizeof(WORD), /* extra */ > #endif Why do you need that? -- Alexandre Julliard julliard(a)winehq.org

2 3

Re: [PATCH 2/2] gdi32/tests: Fixed broken tests on non English system Part 2.
by Qian Hong Oct. 8, 2012

Oct. 8, 2012

Hello Dmitry, Any feedback for this patch? Thanks a lot for any comments! On Thu, Oct 4, 2012 at 3:02 PM, Qian Hong <fracting(a)gmail.com> wrote: > Dmitry, could you review this patch? Thanks a lot ;-) > > This series of patch fix some broken tests on Japanese Windows and Chinese Windows: -- Regards, Qian Hong - Sent from Ubuntu http://www.ubuntu.com/

2 2

Re: [1/3] scrrun: Implement IDictionary_Add and Count
by Alexandre Julliard Oct. 8, 2012

Oct. 8, 2012

Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com> writes: > static HRESULT WINAPI dictionary_get_Count(IDictionary *iface, LONG *pCount) > { > dictionary *This = impl_from_IDictionary(iface); > > - FIXME("(%p)->(%p)\n", This, pCount); > + TRACE("(%p)->(%p)\n", This, pCount); > > - *pCount = 0; > + *pCount = list_count(&This->items); You don't want to use list_count(). -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: [1/4] gdiplus: Add a font transformation test.
by Vincent Povirk Oct. 8, 2012

Oct. 8, 2012

Series looks good to me.

1 0

Re: [3/4] ole32: Move the propvariant serialization into a helper function.
by Alexandre Julliard Oct. 8, 2012

Oct. 8, 2012

Vincent Povirk <madewokherd(a)gmail.com> writes: > From eb900b2807167f25ad8f4e97c558dec8b71b070c Mon Sep 17 00:00:00 2001 > From: Vincent Povirk <vincent(a)codeweavers.com> > Date: Thu, 4 Oct 2012 16:58:48 -0500 > Subject: [PATCH 3/4] ole32: Move the propvariant serialization into a helper > function. It doesn't work here: ../../../tools/runtest -q -P wine -M msi.dll -T ../../.. -p msi_test.exe.so suminfo.c && touch suminfo.ok suminfo.c:441: Test succeeded inside todo block: value incorrect suminfo.c:450: Test failed: MsiSummaryInfoSetProperty failed 0 make: *** [suminfo.ok] Error 2 -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: wined3d: Add retail NVIDIA GeForce GTX 660Ti/660/650
by Henri Verbeet Oct. 8, 2012

Oct. 8, 2012

On 8 October 2012 03:52, Jimmy Hon <honyuenkwun(a)gmail.com> wrote: > + CARD_NVIDIA_GEFORCE_GTX650 = 0x0FC6, > + CARD_NVIDIA_GEFORCE_GTX660 = 0x11C0, > + CARD_NVIDIA_GEFORCE_GTX660TI = 0x1183, Please use lower case constants like everywhere else.

1 0

Re: ole32: Fix dwClsContext parameter of a CoCreateInstance call in DefaultHandler_Run.
by Nikolay Sivov Oct. 8, 2012

Oct. 8, 2012

On 10/6/2012 19:29, Roman Dadkov wrote: > This patch change dwClsContext parameter of a CoCreateInstance call in > function DefaultHandler_Run. > Because even if there is some clsid in the registry, the function will > not be able to run the newly created object. > > > - hr = CoCreateInstance(&This->clsid, NULL, CLSCTX_LOCAL_SERVER, > + hr = CoCreateInstance(&This->clsid, NULL, > + CLSCTX_INPROC_SERVER|CLSCTX_INPROC_HANDLER| > + CLSCTX_LOCAL_SERVER|CLSCTX_REMOTE_SERVER, > &IID_IOleObject, (void **)&This->pOleDelegate); This could be replaced with CLSCTX_ALL. Is it possible to add a test for that?

1 0

Re: iphlpapi/tests: Skip tests if functions are not supported
by Dmitry Timoshkov Oct. 8, 2012

Oct. 8, 2012

André Hentschel <nerv(a)dawncrow.de> wrote: > For Mac, BSD, Solaris If it's Wine specific, then probably there shouild be a todo_wine somewhere to make it clear that under Windows there is no skips. -- Dmitry.

1 0

Please remove this email address
by Ken Sharp Oct. 7, 2012

Oct. 7, 2012

Hi, I have been receiving daily digests from this mailbox for a while. I tried emailing the "list owner" according to the website but that had no effect. I have been filtering the digests away so I didn't see that people had replied (thanks to everyone who did, sorry I haven't replied). I have tried changing the preferences but it just tells me that this email address doesn't exist. I can't do anything with this. Could someone with relevant access please remove me so that I can add myself again, properly? Thanks, Ken

1 0

Help getting amd64 assembly patch into wine?
by Joerg-Cyril.Hoehle＠t-systems.com Oct. 6, 2012

Oct. 6, 2012

Dan, I found your message very unclear. >The patch adds support for OpenMP programs like this: And then you start talking about vcomp_fork without telling us where it comes from and what it should do. So I'll guess from the names. - vcomp_for_static/sections_init sound like startup code initialisation of BSS and DATA segments. - fork sounds like running code with thread-local copies to BSS and DATA I guess vcomp eats up one register or parameter to keep a pointer to the thread-local storage. > p_vcomp_fork(0, 1, _test_vcomp_fork_worker1, &ncalls); Your code does not explain what the first parameter is. I believe that va_list etc. is not going to lead you anywhere. First, va_* is only one side. E.g. include/wine/test.h uses va_list and __builtin_ms_va_list You need the other side too, namely, given a va_* structure, call a function X with the parameters given in that structure. The GNU ffcall library http://www.gnu.org/software/libffcall/ distinguishes both. One half is called vacall, the other one avcall :-) Your tests show that you need 2 different va_lists: > p_vcomp_fork(0, 5, _test_vcomp_fork_worker5, 1, 2, 3, 4, 5); 1. On the receiver side, va_start would create a va_list for the parameter set (0, 5, function pointer, 1, 2, 3, 4, 5) 2. But now you need to call something with the apparent parameter set (1, 2, 3, 4, 5) (I say apparent because you don't explain the vcomp execution model. Is a hidden parameter added somewhere? What's that first parameter 0 to fork?) Obviously, if you don't know the internals of a va_list, you'll not be able to transform one structure into the other. So bad, now what is actually needed? Think assembly. Using your test example: + p_vcomp_fork(0, 5, _test_worker5, 1, 2, 3, 4, 5); _vcomp_fork finds data: 1. on the stack 2. in registers 3. in FP registers The C stack layout is (topmost first): 0, then 5, &_test_worker5, 1, 2, etc. What you obviously need to do is, supposing the first parameters are duplicated in registers: - Save registers and possibly FP registers into a structure - Remember the stack pointer and number of elements, -- hereby creating a va_args structure whose layout you know. - CreateThread?? etc. - Restore registers - Shift register parameters by 3 items: (assuming 5 such registers) register-for-param-1 (1) <- register-for-param-4 register-for-param-2 (2) <- register-for-param-5 - Copy from stack into registers register-for-param-3 (3) <- from stack register-for-param-4 (4) <- from stack register-for-param-5 (5) <- from stack - POP 0 and 5 - POP &_test_worker5 into scratch register - JMPTO _test_worker5 via scratch register This assumes there's nothing like CreateThread so you can run from the original stack. If there were, you'd need to copy the 5 elements from the stack to the new one (which is presumably why vcomp_fork receives their number as parameter). Does this help? Jörg Höhle

3 3

Windows audio architecture
by Saulius Krasuckas Oct. 6, 2012

Oct. 6, 2012

One link catched my eye here: "The evolution of a data structure – the WAVEFORMAT" https://www.assembla.com/spaces/portaudio/wiki/Win32AudioBackgroundInfo Maybe it is of some use for Wine developers too:) Some URLs need adjustment, though. Eg. Creative Labs description of Vista audio architecture changed to: http://forums.creative.com/showthread.php?t=302341 S.

1 0

Re: [patch 1/1] dlls/windowscodecs/pngformat.c: implemented 'PngDecoder_Block_GetCount'
by Nikolay Sivov Oct. 5, 2012

Oct. 5, 2012

On 10/5/2012 06:43, max+git(a)mtew.isa-geek.net wrote: > From: Max TenEyck Woodbury <max+git(a)mtew.isa-geek.net> > > --- > dlls/windowscodecs/pngformat.c | 6 ++++-- > 1 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/dlls/windowscodecs/pngformat.c b/dlls/windowscodecs/pngformat.c > index 686f9c6..e8e7cbe 100644 > --- a/dlls/windowscodecs/pngformat.c > +++ b/dlls/windowscodecs/pngformat.c > @@ -899,8 +899,10 @@ static HRESULT WINAPI PngDecoder_Block_GetContainerFormat(IWICMetadataBlockReade > static HRESULT WINAPI PngDecoder_Block_GetCount(IWICMetadataBlockReader *iface, > UINT *pcCount) > { > - FIXME("%p,%p: stub\n", iface, pcCount); > - return E_NOTIMPL; > + PngDecoder *This = impl_from_IWICMetadataBlockReader(iface); > + if (!pcCount) return E_INVALIDARG; > + *pcCount = This->ref; > + return S_OK; > } > > static HRESULT WINAPI PngDecoder_Block_GetReaderByIndex(IWICMetadataBlockReader *iface, Return reference count as block count?

3 6

Re: [PATCH 2/3] ntoskrnl.exe: Improve IoGetCurrentProcess stub.
by Dmitry Timoshkov Oct. 5, 2012

Oct. 5, 2012

Christian Costa <titan.costa(a)gmail.com> wrote: > +EPROCESS process_info; > + > #ifdef __i386__ > #define DEFINE_FASTCALL1_ENTRYPOINT( name ) \ > __ASM_STDCALL_FUNC( name, 4, \ > @@ -1200,8 +1203,11 @@ NTSTATUS WINAPI FsRtlRegisterUncProvider(PHANDLE MupHandle, PUNICODE_STRING Redi > */ > PEPROCESS WINAPI IoGetCurrentProcess(void) > { > - FIXME("() stub\n"); > - return NULL; > + FIXME("(): partial stub\n"); > + > + process_info.UniqueProcessId = (PVOID)PsGetCurrentProcessId(); > + > + return &process_info; > } Why do you think that returning the structure filled with garbage is better than returning NULL? -- Dmitry.

3 7

Re: [PATCH 1/3] ntoskrnl.exe: Add include/ddk/ntifs.h header with EPROCESS struct definition.
by Paul Chitescu Oct. 5, 2012

Oct. 5, 2012

On Friday 05 October 2012 10:00:00 am Christian Costa wrote: > --- > include/ddk/ntifs.h | 555 > +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 555 > insertions(+) > create mode 100644 include/ddk/ntifs.h > > diff --git a/include/ddk/ntifs.h b/include/ddk/ntifs.h > new file mode 100644 > [...] What version of Windows were these extracted from?

2 2

Re: [PATCH 1/3] ntoskrnl.exe: Add include/ddk/ntifs.h header with EPROCESS struct definition.
by Dmitry Timoshkov Oct. 5, 2012

Oct. 5, 2012

Did you check the DDK where it's supposed to be declared? -- Dmitry.

1 0

Help getting amd64 assembly patch into wine?
by Dan Kegel Oct. 4, 2012

Oct. 4, 2012

As you might know from watching too many of my patches scroll by, I've been working on adding support for OpenMP to wine. (A handful of games, and a lot of serious apps, seem to use that api.) After getting it nicely organized and cleaned up to the point where it passed all the tests I could throw at it, I submitted it as http://source.winehq.org/patches/data/90524, Alexandre rejected the patch, saying it had multiple problems, but declining to provide any further hints. He suggested I find someone to work with to get it in... so here I am, hat in hand, asking for help. The patch adds support for OpenMP programs like this: // Compile with "cl test.c /openmp" #include <omp.h> #include <stdio.h> int main (int argc, char **argv) { #pragma omp parallel { printf("%d\n", omp_get_thread_num()); } } Later patches add support for other OpenMP constructs. My patch doesn't actually parallelize the app, but it does let it run rather than crash on unimplemented _vcomp_fork. Visual C will happily provide an assembly listing of this program showing how _vcomp_fork is called, so understanding that interface well enough to pass simple tests wasn't hard. The main challenge was figuring out how to get the variable list of arguments off the stack, and then put them back onto the stack when calling the provided function pointer. This bit of varargs hackery can't be done in pure C as far as I can tell, so I used assembly. I started programming in machine language long ago, so getting that working wasn't too hard once I realized that's what was needed. Getting it to pass the Alexandre test is another matter. Alexandre did give feedback on one earlier iteration of the patch, telling me I was re-inventing the wheel, so I tossed my old assembly and copied the helper-calling code nearly verbatim from call_method in oleaut32/typelib.c. Maybe he objects to not passing copies of the arguments in the floating point registers on amd64... but the arguments are always pointers, so that shouldn't be needed. I'm definitely not yet comfortable with the ASM_CFI stuff, so perhaps there's a problem there, but I did at least verify that injecting a fault into _test_vcomp_fork_worker5() still yielded a good stack trace on both x86 and amd64 in winedbg. There aren't many comments, and the ones I did add aren't the greatest, but I doubt that's the issue. Perhaps he objects to breaking up _vcomp_fork into three functions, but I couldn't see any other way to write it without making later patches like http://source.winehq.org/patches/data/90527 (and a future patch that adds actual multiple threads) put more logic than seems wise into assembly. So maybe it's none of the above. If someone can suggest what I might be missing, I'd appreciate it. Even just "It will break if..." or "Better compare how this was done in..." would be very helpful. Thanks!

2 2

Re: [PATCH 2/2] mshtml: Added ActiveX event binding test
by Marvin Oct. 4, 2012

Oct. 4, 2012

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=22013 Your paranoid android. === WINEBUILD (build) === Patch failed to apply

2 1

Re: [PATCH 5/5] winex11: Don't send SC_RESTORE when a maximized window switches to fullscreen.
by Dmitry Timoshkov Oct. 4, 2012

Oct. 4, 2012

Henri Verbeet <hverbeet(a)codeweavers.com> wrote: > - if (style & WS_MAXIMIZE) > + if ((style & WS_MAXIMIZE) && (style & WS_CAPTION) == WS_CAPTION) Probably you should test for WS_MAXIMIZEBOX instead. -- Dmitry.

1 0