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

Re: d3dx8 [6/7] Implement D3DXMatrixStack_Push
by tony.wasserka＠freenet.de 09 Apr '08

09 Apr '08

> > + This->current = This->current +1; > > + HeapReAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, This->matrix, (This->current +1) * sizeof(D3DXMATRIX) ); > > + if ( This->matrix == NULL ) return E_OUTOFMEMORY; > Aside from being a bit suboptimal (doing a realloc on every push), > this probably doesn't do what you want. Consider what happens to the > size of the array when you do something like push/pop/push/pop/...etc. > > It would be better to keep track of the current stack size, and then > grow it by a factor (eg. 2 or 1.5) if a push would overflow the > current stack. You could also consider shrinking the array again if a > pop would make it use less than a certain percentage of the current > size (eg. a third). > > You should also assign the result of HeapReAlloc() to This->matrix > again. Although it's quite possible for HeapReAlloc to just grow the > current block without changing its location, there's no guarantee it > will. The NULL check is useless this way. Just a little addition, I think we should resize the stack differently at different situations, i.e.: - resize it to (This->current+3) if This->currect<5 (something around that is the minimum number of matrices for which anyone would even bother using a MatrixStack) - resize it to (This->current*2) if This->current<12 (when it seems like the application's gonna push many matrices on the stack) - resize it to (This->current+16) if This->current>=12 (when *2 could waste memory) However, I don't know if these numbers are optimal, it's just a suggestion. For the Pop function, I agree that you should free some memory when it's unused, but my suggestion from above fits here, too. However, one third is okay, too, as it isn't that relevant here.

1 0

gdiplus: GdipAddPathArcI, GdipAddPathBezier implementation
by Nikolay Sivov 09 Apr '08

09 Apr '08

Is it something wrong with this patch? Changelog: - GdipAddPathArcI, GdipAddPathBezier implemented using existing calls Index: dlls/gdiplus/gdiplus.spec =================================================================== RCS file: /home/wine/wine/dlls/gdiplus/gdiplus.spec,v retrieving revision 1.170 diff -u -r1.170 gdiplus.spec --- dlls/gdiplus/gdiplus.spec 24 Mar 2008 12:57:16 -0000 1.170 +++ dlls/gdiplus/gdiplus.spec 7 Apr 2008 18:57:29 -0000 @@ -1,6 +1,6 @@ @ stdcall GdipAddPathArc(ptr long long long long long long) -@ stub GdipAddPathArcI -@ stub GdipAddPathBezier +@ stdcall GdipAddPathArcI(ptr long long long long long long) +@ stdcall GdipAddPathBezier(ptr long long long long long long long long) @ stdcall GdipAddPathBezierI(ptr long long long long long long long long) @ stdcall GdipAddPathBeziers(ptr ptr long) @ stub GdipAddPathBeziersI Index: dlls/gdiplus/graphicspath.c =================================================================== RCS file: /home/wine/wine/dlls/gdiplus/graphicspath.c,v retrieving revision 1.33 diff -u -r1.33 graphicspath.c --- dlls/gdiplus/graphicspath.c 4 Apr 2008 11:24:06 -0000 1.33 +++ dlls/gdiplus/graphicspath.c 7 Apr 2008 18:56:45 -0000 @@ -97,6 +97,13 @@ return Ok; } +GpStatus WINGDIPAPI GdipAddPathArcI(GpPath *path, INT x1, INT y1, INT x2, + INT y2, REAL startAngle, REAL sweepAngle) +{ + TRACE("%p %d %d %d %d %f %f\n",path,x1,y1,x2,y2,startAngle,sweepAngle); + return GdipAddPathArc(path, (REAL)x1, (REAL)y1, (REAL)x2, {REAL)y2, startAngle, sweepAngle); +} + GpStatus WINGDIPAPI GdipAddPathBezierI(GpPath *path, INT x1, INT y1, INT x2, INT y2, INT x3, INT y3, INT x4, INT y4) { @@ -158,6 +165,23 @@ return Ok; } +GpStatus WINGDIPAPI GdipAddPathBezier(GpPath *path, REAL x1, REAL y1, REAL x2, REAL y2, REAL x3, REAL y3, REAL x4, REAL y4) +{ + GpPointF points[3]; + + points[0].X = x1; + points[0].Y = y1; + points[1].X = x2; + points[1].Y = y2; + points[2].X = x3; + points[2].Y = y3; + points[3].X = x4; + points[3].Y = y4; + + TRACE("%p %f %f %f %f %f %f\n",path,x1,y1,x2,y2,x3,y3,x4,y4); + return GdipAddPathBeziers(path, points, 4); +} + GpStatus WINGDIPAPI GdipAddPathEllipse(GpPath *path, REAL x, REAL y, REAL width, REAL height) { Index: include/gdiplusflat.h =================================================================== RCS file: /home/wine/wine/include/gdiplusflat.h,v retrieving revision 1.155 diff -u -r1.155 gdiplusflat.h --- include/gdiplusflat.h 24 Mar 2008 12:57:16 -0000 1.155 +++ include/gdiplusflat.h 7 Apr 2008 18:55:25 -0000 @@ -182,6 +182,8 @@ GpStatus WINGDIPAPI GdipSetTextureTransform(GpTexture *,GDIPCONST GpMatrix*); GpStatus WINGDIPAPI GdipAddPathArc(GpPath*,REAL,REAL,REAL,REAL,REAL,REAL); +GpStatus WINGDIPAPI GdipAddPathArcI(GpPath*,INT,INT,INT,INT,REAL,REAL); +GpStatus WINGDIPAPI GdipAddPathBezier(GpPath*,REAL,REAL,REAL,REAL,REAL,REAL,REAL,REAL); GpStatus WINGDIPAPI GdipAddPathBezierI(GpPath*,INT,INT,INT,INT,INT,INT,INT,INT); GpStatus WINGDIPAPI GdipAddPathBeziers(GpPath*,GDIPCONST GpPointF*,INT); GpStatus WINGDIPAPI GdipAddPathEllipse(GpPath*,REAL,REAL,REAL,REAL);

1 0

Re: [PATCH 1/2] user32: add edit control check to see if its in a dialog on WM_CHAR/VK_RETURN.
by Alexandre Julliard 09 Apr '08

09 Apr '08

"Lei Zhang" <thestig(a)google.com> writes: > +static BOOL EDIT_IsInsideDialog(EDITSTATE *es) > +{ > + static WCHAR dialog_class[] = {'#','3','2','7','7','0',0}; > + WCHAR name[8]; > + int r; > + > + r = GetClassNameW(es->hwndParent, name, 8); > + if (r > 0) > + if (!lstrcmpW(name, dialog_class)) > + return TRUE; > + return FALSE; > +} You should test the WIN_ISDIALOG flag instead. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: [10/10] d3d9: Some Windows drivers set undefined attributes to 0.0
by H. Verbeet 09 Apr '08

09 Apr '08

On 08/04/2008, Stefan Dösinger <stefan(a)codeweavers.com> wrote: > + Sleep(100000); Is that Sleep() really needed?

1 0

Re: d3dx9: Implement D3DXGetShaderVersion with tests 4/4
by H. Verbeet 09 Apr '08

09 Apr '08

On 08/04/2008, Luis Busquets <luis.busquets(a)ilidium.com> wrote: > This patch implements D3DXGetShaderVersion and adds tests to two simple > shader which have been extrated from d3d9/tests/shader.c > > + compare_shader_versions (simple_vs, 0xFFFE0101); > + compare_shader_versions (simple_ps, 0xFFFF0101); D3DVS_VERSION(1, 1) and D3DPS_VERSION(1, 1) would be more appropriate than 0xFFFE0101 and 0xFFFF0101 here.

1 0

Re: d3dx9: Implement D3DXGetShaderSize with tests 3/4
by H. Verbeet 09 Apr '08

09 Apr '08

On 08/04/2008, Luis Busquets <luis.busquets(a)ilidium.com> wrote: > This patch implements the function D3DXGetShaderSize and adds tests to two > simple shader which have been extrated from d3d9/tests/shader.c > > + /* We have to look for the end token 0x0000ffff which is D3DVS_END or D3DPS_END as both values are the same*/ > + while (*address!=D3DVS_END()) { The comment about D3DSIO_END still stands. > +#define compare_shader_sizes(shader, exp) \ > + shade=(DWORD *)shader; \ > + got=D3DXGetShaderSize(shader); \ > + ok(got==exp, "Expected: %d, Got: %d\n", exp, got); This is pretty ugly, but I guess it works. > + compare_shader_sizes (simple_vs, 84); > + compare_shader_sizes (simple_ps, 88); Please also test passing a NULL shader. (FWIW, sizeof(simple_vs) should also work instead of hardcoding 84 here, same for simple_ps)

1 0

Re: d3dx8: [7/7] Add tests for MatrixStack
by H. Verbeet 09 Apr '08

09 Apr '08

On 08/04/2008, David Adam <david.adam.cnrs(a)gmail.com> wrote: > While it's certainly good to test the normal usage of the API, it's also useful to test the behaviour of error conditions (Pop/LoadMatrix/GetTop on an empty stack, etc).

1 0

Re: d3dx8 {5/7] Implement D3DXMatrixStack_Pop
by H. Verbeet 09 Apr '08

09 Apr '08

On 08/04/2008, David Adam <david.adam.cnrs(a)gmail.com> wrote: > + if ( This->current == 0 ) > + { > + HeapFree(GetProcessHeap(), 0, This->matrix); > + return D3DERR_INVALIDCALL; > + } > + This->current = This->current -1; If This->current was 0 when calling Pop, it now points at a non-existing item. This would probably break things like LoadMatrix, GetTop, etc. Also, calling Pop again would result in a double free of This->matrix.

1 0

Re: d3dx8 [6/7] Implement D3DXMatrixStack_Push
by H. Verbeet 09 Apr '08

09 Apr '08

On 08/04/2008, David Adam <david.adam.cnrs(a)gmail.com> wrote: > + This->current = This->current +1; > + HeapReAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, This->matrix, (This->current +1) * sizeof(D3DXMATRIX) ); > + if ( This->matrix == NULL ) return E_OUTOFMEMORY; Aside from being a bit suboptimal (doing a realloc on every push), this probably doesn't do what you want. Consider what happens to the size of the array when you do something like push/pop/push/pop/...etc. It would be better to keep track of the current stack size, and then grow it by a factor (eg. 2 or 1.5) if a push would overflow the current stack. You could also consider shrinking the array again if a pop would make it use less than a certain percentage of the current size (eg. a third). You should also assign the result of HeapReAlloc() to This->matrix again. Although it's quite possible for HeapReAlloc to just grow the current block without changing its location, there's no guarantee it will. The NULL check is useless this way.

1 0

Re: Reece Dunn : comdlg32: Fix building the tests with MSVC headers.
by Francois Gouget 09 Apr '08

09 Apr '08

On Mon, 24 Mar 2008, Alexandre Julliard wrote: [...] > Commit: ae32a7fe0473aebb875b26083c1567ff046c3b80 > URL: http://source.winehq.org/git/wine.git/?a=commit;h=ae32a7fe0473aebb875b26083… > > Author: Reece Dunn <msclrhd(a)googlemail.com> > Date: Sat Mar 22 22:50:43 2008 +0000 > > comdlg32: Fix building the tests with MSVC headers. [...] > +++ b/dlls/comdlg32/tests/printdlg.c > @@ -26,6 +26,7 @@ > #include "winerror.h" > #include "wingdi.h" > #include "winuser.h" > +#include "objbase.h" The question is why didn't this fail in Wine in the first place? The Wine headers must be different in some way which causes the compile to succeed when it should have failed. Thus they should be fixed. -- Francois Gouget <fgouget(a)free.fr> http://fgouget.free.fr/ $live{free} || die "";

2 1