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