Re: GSoC-2011: Implement Missing Mesh Functions in Wine’s D3DX9
On Wed, Jun 22, 2011 at 4:08 PM, Stefan Dösinger wrote: > I'll have to do some more background reading on the kind of mesh data > structures used here before I can give qualified comments on your patches, so > for now I'll yield to Dylan :-) I looked over the latest patches, and the code looks good now, but I have a couple of nitpicks for the comments. +/* ConvertAdjacencyToPointReps helper function. + * + * Goes around the edges of a face and replaces the vertices in any adjacent (changed word is in caps, but capitalization not needed) * Goes around the edges of EACH face... + * The vertices in a point representation must be ordered sequentially, e.g. + * index 3 holds the index of the vertex that replaces vertex 3, i.e. if + * vertex 3 is replaced by vertex 5 then index 3 would contain 5. If no vertex + * replaces it, then it contains the same number as the index itself, e.g. + * index 3 would contain 3. */ Your example has vertex 5 replacing vertex 3, which doesn't seem possible if lower vertex indices always replace higher vertex indices.
Re: GSoC-2011: Implement Missing Mesh Functions in Wine’s D3DX9
On Wed, Jun 22, 2011 at 4:08 PM, Stefan Dösinger wrote: > I'll have to do some more background reading on the kind of mesh data > structures used here before I can give qualified comments on your patches, so > for now I'll yield to Dylan :-) I couldn't actually find information on point representative data (aka PointReps), but a quick search found others wondering what it is. Basically it is an array of vertex indices, one for each vertex, each with either its own index or the index of an co-located vertex. e.g. If you have triangles ABC and DEF (as shown below), with two sets of points co-located, and the vertex buffer stored in alphabetical order. Then the point_reps array would be [0, 1, 2, 2, 1, 5] to indicate that vertices B & C can replace E & D respectively. A---(B/E) | / | | / | (C/D)--F The adjacency buffer is documented well enough on MSDN for GenerateAdjacency (http://msdn.microsoft.com/en-us/library/bb205737(v=VS.85).aspx) and seems to be used in this method for determining which vertices are co-located. I hope that helps.
Re: Need help at collecting ActionMapping data
> Hi, > so you are the One who can finish off the bug 8754?? :-D I hope so. As soon as my first patches get in I hope things should be working with the keyboard, so if you have access to the NFSU games you should test it. > I own force feedback wheel: "Logitech Wingman Formula Force GP" and > did three of this piece of HW on: Awesome! Thank you. > Results are attached. I also have access to some Microsoft Force > Feedback wheel (Sidewinder?), so if interested i could try to make > some log during the weeken. Also Win7 should not be a big problem. > It's not vital but if it's not a problem for you I'd like this data too. Thanks again
Re: [PATCH 2/2] d3dx9_36/tests: Added tests for constant table samplers (Try 2)
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=11904 Your paranoid android. === WINEBUILD (build) === Patch failed to apply
Re: GSoC-2011: Implement Missing Mesh Functions in Wine’s D3DX9
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Am 22.06.2011 um 21:48 schrieb Michael Mc Donnell: >> HeapFree checks for NULL for you, so remove the NULL check before >> calling HeapFree. > > Ok. I see that now in the Wine source. That's a lot nicer than on Windows. Windows also checks for NULL pointers in HeapFree. Otherwise Wine wouldn't do so, and we'd need the NULL checks to be able to run our code on Windows. > My UpdateSemantics patch finally got applied Congratulations! I'll have to do some more background reading on the kind of mesh data structures used here before I can give qualified comments on your patches, so for now I'll yield to Dylan :-) -BEGIN PGP SIGNATURE- Version: GnuPG/MacGPG2 v2.0.17 (Darwin) iQIcBAEBAgAGBQJOAkvaAAoJEN0/YqbEcdMwxssP/j1BOavQHLCSJdarIwV6B+NL wkIB9W6L6EsvGxr/8we6mTSVAQ3VMBK74F1yB5BBDNls1YXVugDHm7GI5Yrku4lR mE8YhwudJVZ3i3eKeHBvjRIzkGPW+eUpNkkmJQtQVkpU/Lu/igVDB+pxiFSRxhc4 aAA26bIZ3Uer/wm/1YdIi3geJ05CH4uZpDpWaslARURSP4zofsXiGj5m4KI0WCup 7BtlDdwA9SzamxVHRBTXld0Y4vcg80jBOlmvd6soifD+VI4LCXA31xHBCLR//BAK 8mbdDcPF00mJGRwJKXbL/FJunXRocaOSu0ujtc9Tx9BQbtLGAs0C6VWKgfvvh47M jZ0RFc5ACStWQTkSNPrhvycS188oJ/5mW3aISG+AVuTme3H9zSrNccJYnBpjR+iy GRbn2DmmxRziq/XS4GjW7+oCapHwsESoyZNDxO1fqI0k+gODmGWvR41oONN52/wv QbvAxjsLknwMKvX2MTNVAUoeQWsPB9OIV8HxIPjwxNGFsvM8/ETOprHRyZP2dgNc XcnDQM5/5O2amNRCrRIA4tQaE8QbhxQfe0M9fwqf4XatzYx8tzW1kus9lm9HMEp5 j/FZjBnlU+RjSa+u5TDZxeW5nYwPbrw2ygWcTRom4taZydvWHxWBY4t6IkMfSe+g hBPbh6pFtYZwhpFKWD7u =/xC4 -END PGP SIGNATURE-
Re: GSoC-2011: Implement Missing Mesh Functions in Wine’s D3DX9
On Wed, Jun 22, 2011 at 4:53 PM, Dylan Smith wrote:
> On Wed, Jun 22, 2011 at 10:12 AM, Michael Mc Donnell
> wrote:
>> Thanks your comments. I've attached an updated version.
>
>> + if (indices) HeapFree(GetProcessHeap(), 0, indices);
>> + }
>> + if (new_indices) HeapFree(GetProcessHeap(), 0, new_indices);
>
> HeapFree checks for NULL for you, so remove the NULL check before
> calling HeapFree.
Ok. I see that now in the Wine source. That's a lot nicer than on Windows.
>> + struct
>> + {
>> + ID3DXMesh *meshes[ARRAY_SIZE(vertices)];
>> + const DWORD *indices[ARRAY_SIZE(vertices)];
>> + const DWORD num_vertices[ARRAY_SIZE(vertices)];
>> + const DWORD *adjacencies[ARRAY_SIZE(vertices)];
>> + DWORD *point_reps[ARRAY_SIZE(vertices)];
>> + const DWORD *exp_point_reps[ARRAY_SIZE(vertices)];
>> + const DWORD options[ARRAY_SIZE(vertices)];
>> + }
>> + tc =
>> + {
>> + {NULL, NULL, NULL, NULL, NULL, NULL, NULL},
>> + {indices0, indices1, indices2, indices3, indices4, indices5,
>> indices6, (DWORD*)indices6_16bit},
>> + {num_vertices0, num_vertices1, num_vertices2, num_vertices3,
>> num_vertices4, num_vertices5, num_vertices6, num_vertices6},
>> + {adjacency0, adjacency1, adjacency2, adjacency3, adjacency4,
>> adjacency5, adjacency6, adjacency6},
>> + {point_rep0, point_rep1, point_rep2, point_rep3, point_rep4,
>> point_rep5, point_rep6, point_rep6},
>> + {exp_point_rep0, exp_point_rep1, exp_point_rep2, exp_point_rep3,
>> exp_point_rep4, exp_point_rep5, exp_point_rep6, exp_point_rep6},
>> + {options, options, options, options, options, options, options,
>> options_16bit}
>> + };
>
> I actually meant something more like:
>
> --- snip ---
> struct {
> const DWORD *indices,
> const DWORD num_vertices,
> const DWORD *adjacencies,
> DWORD *point_reps,
> const DWORD *exp_point_reps,
> const DWORD options,
> } tc[] = {
> {indices0, num_vertices0, adjacency0, point_rep0, exp_point_rep0, options},
> {indices1, num_vertices1, adjacency1, point_rep1, exp_point_rep1, options},
> ...
> }
Ah ok :-) That makes more sense.
> ...
>
> for (i = 0; i < ARRAY_SIZE(tc); i++)
> {
> ...
> }
> --- snip ---
>
> I don't think storing an array of meshes is necessary. You can just
> release the mesh and set the mesh variable to NULL (as needed to avoid
> re-releasing) before creating another one.
I've changed it so that it only keeps the first mesh around for the NULL checks.
My UpdateSemantics patch finally got applied so I've removed the
duplicate functions too.
From 039b27048842da6823f5f8a621cbd22a3c632c6c Mon Sep 17 00:00:00 2001
From: Michael Mc Donnell
Date: Fri, 27 May 2011 16:24:18 +0200
Subject: d3dx9/tests: Implemented ConvertAdjacencyToPointReps test.
---
dlls/d3dx9_36/tests/mesh.c | 332
1 files changed, 332 insertions(+), 0 deletions(-)
diff --git a/dlls/d3dx9_36/tests/mesh.c b/dlls/d3dx9_36/tests/mesh.c
index 26a0c10..ee9ad67 100644
--- a/dlls/d3dx9_36/tests/mesh.c
+++ b/dlls/d3dx9_36/tests/mesh.c
@@ -4676,6 +4676,337 @@ cleanup:
free_test_context(test_context);
}
+static void test_convert_adjacency_to_point_reps(void)
+{
+HRESULT hr;
+struct test_context *test_context = NULL;
+const DWORD options = D3DXMESH_32BIT | D3DXMESH_SYSTEMMEM;
+const DWORD options_16bit = D3DXMESH_SYSTEMMEM;
+const D3DVERTEXELEMENT9 declaration[] =
+{
+{0, 0, D3DDECLTYPE_FLOAT3, D3DDECLMETHOD_DEFAULT, D3DDECLUSAGE_POSITION, 0},
+{0, 12, D3DDECLTYPE_FLOAT3, D3DDECLMETHOD_DEFAULT, D3DDECLUSAGE_NORMAL, 0},
+{0, 24, D3DDECLTYPE_D3DCOLOR, D3DDECLMETHOD_DEFAULT, D3DDECLUSAGE_COLOR, 0},
+D3DDECL_END()
+};
+const unsigned int VERTS_PER_FACE = 3;
+void *vertex_buffer;
+void *index_buffer;
+DWORD *attributes_buffer;
+int i, j;
+struct vertex_pnc
+{
+FLOAT x, y, z; /* Position */
+FLOAT nx, ny, nz; /* Normal */
+DWORD color; /* In case of manual visual inspection */
+};
+/* mesh0 (one face) */
+const struct vertex_pnc vertices0[] =
+{
+{0.0f, 1.0f, 0.f, 0.0f, 0.0f, 1.0f, 0x},
+{1.0f, -1.0f, 0.f, 0.0f, 0.0f, 1.0f, 0xff00ff00},
+{-1.0f, -1.0f, 0.f, 0.0f, 0.0f, 1.0f, 0xffff},
+};
+const DWORD indices0[] = {0, 1, 2};
+const unsigned int num_vertices0 = ARRAY_SIZE(vertices0);
+const DWORD adjacency0[] = {-1, -1, -1};
+DWORD point_rep0[num_vertices0];
+const DWORD exp_point_rep0[] = {0, 1, 2};
+/* mesh1 (right) */
+const struct vertex_pnc vertices1[] =
+{
+{0.0f, 1.0f, 0.f, 0.0f, 0.0f, 1.0f, 0x},
+{1.0f, -1.0f, 0.f, 0.0f, 0.0f, 1.0f, 0xff00ff00},
+{-1.0f, -1.0f, 0.f, 0.0f, 0.0f, 1.0f, 0xffff},
+
+{0.1f, 1.0f, 0.f, 0.0f, 0.0f, 1.0f, 0x},
+{2.1f, 1.0f, 0.f, 0.0f, 0.0f, 1.0f, 0xffff},
+
Re: GSoC-2011: Implement Missing Mesh Functions in Wine’s D3DX9
On Wed, Jun 22, 2011 at 10:12 AM, Michael Mc Donnell
wrote:
> Thanks your comments. I've attached an updated version.
> +if (indices) HeapFree(GetProcessHeap(), 0, indices);
> +}
> +if (new_indices) HeapFree(GetProcessHeap(), 0, new_indices);
HeapFree checks for NULL for you, so remove the NULL check before
calling HeapFree.
> +struct
> +{
> + ID3DXMesh *meshes[ARRAY_SIZE(vertices)];
> + const DWORD *indices[ARRAY_SIZE(vertices)];
> + const DWORD num_vertices[ARRAY_SIZE(vertices)];
> + const DWORD *adjacencies[ARRAY_SIZE(vertices)];
> + DWORD *point_reps[ARRAY_SIZE(vertices)];
> + const DWORD *exp_point_reps[ARRAY_SIZE(vertices)];
> + const DWORD options[ARRAY_SIZE(vertices)];
> +}
> +tc =
> +{
> +{NULL, NULL, NULL, NULL, NULL, NULL, NULL},
> +{indices0, indices1, indices2, indices3, indices4, indices5,
> indices6, (DWORD*)indices6_16bit},
> +{num_vertices0, num_vertices1, num_vertices2, num_vertices3,
> num_vertices4, num_vertices5, num_vertices6, num_vertices6},
> +{adjacency0, adjacency1, adjacency2, adjacency3, adjacency4,
> adjacency5, adjacency6, adjacency6},
> +{point_rep0, point_rep1, point_rep2, point_rep3, point_rep4,
> point_rep5, point_rep6, point_rep6},
> +{exp_point_rep0, exp_point_rep1, exp_point_rep2, exp_point_rep3,
> exp_point_rep4, exp_point_rep5, exp_point_rep6, exp_point_rep6},
> +{options, options, options, options, options, options, options,
> options_16bit}
> +};
I actually meant something more like:
--- snip ---
struct {
const DWORD *indices,
const DWORD num_vertices,
const DWORD *adjacencies,
DWORD *point_reps,
const DWORD *exp_point_reps,
const DWORD options,
} tc[] = {
{indices0, num_vertices0, adjacency0, point_rep0, exp_point_rep0, options},
{indices1, num_vertices1, adjacency1, point_rep1, exp_point_rep1, options},
...
}
...
for (i = 0; i < ARRAY_SIZE(tc); i++)
{
...
}
--- snip ---
I don't think storing an array of meshes is necessary. You can just
release the mesh and set the mesh variable to NULL (as needed to avoid
re-releasing) before creating another one.
Re: GSoC-2011: Implement Missing Mesh Functions in Wine’s D3DX9
On Tue, Jun 21, 2011 at 5:36 PM, Dylan Smith wrote:
> On Tue, Jun 21, 2011 at 6:32 AM, Michael Mc Donnell
> wrote:
>> Here is my test and implementation of ConvertAdjacencyToPointReps.
>
>
>> + hr = iface->lpVtbl->LockIndexBuffer(iface, D3DLOCK_READONLY,
>> (void**)&indices);
>> + if (FAILED(hr)) goto cleanup;
>> + memcpy(new_indices, indices, This->numvertices * sizeof(*indices));
>
> I think you want the number of faces * 3 rather than the number of
> vertices. And the size of an index is not constant, it is 32-bit if
> This->options has the D3DXMESH_32BIT bit set and 16-bit if the bit
> isn't set.
Yes you're right, I'll change that. I had completely forgotten about
16-bit indices, so that required a lot more changes and a test.
>> +static void test_convert_adjacency_to_point_reps(void)
>> +{
> ...
>> + /* All mesh data */
>> + struct vertex_pnc *vertices[] = {vertices0, vertices1, vertices2,
>> vertices3, vertices4, vertices5, vertices6};
>> + ID3DXMesh *meshes[ARRAY_SIZE(vertices)];
>> + DWORD *indices[] = {indices0, indices1, indices2, indices3, indices4,
>> indices5, indices6};
>> + DWORD num_vertices[] = {num_vertices0, num_vertices1, num_vertices2,
>> num_vertices3, num_vertices4, num_vertices5, num_vertices6};
>> + DWORD num_faces[] = {num_faces0, num_faces1, num_faces2, num_faces3,
>> num_faces4, num_faces5, num_faces6};
>> + DWORD *adjacencies[] = {adjacency0, adjacency1, adjacency2, adjacency3,
>> adjacency4, adjacency5, adjacency6};
>> + DWORD *point_reps[] = {point_rep0, point_rep1, point_rep2, point_rep3,
>> point_rep4, point_rep5, point_rep6};
>> + DWORD *exp_point_reps[] = {exp_point_rep0, exp_point_rep1,
>> exp_point_rep2, exp_point_rep3, exp_point_rep4, exp_point_rep5,
>> exp_point_rep6};
>
> I think it would make sense to put all these arrays of the same length
> into a structure (e.g. struct test_case) array. This would make it
> clear what needs to be updated to add another test case, and less
> error prone than making sure each relevant individual array is
> updated.
Ok I've tried to make a struct for them.
> Also, several of these and similar array seem like they can be made constant.
Check, I've const'ified everything I could find.
>> +static void test_convert_adjacency_to_point_reps(void)
>> +{
> ...
>> + for (i = 0; i < ARRAY_SIZE(meshes); i++)
>> + {
> ...
>> + hr = meshes[i]->lpVtbl->ConvertAdjacencyToPointReps(meshes[i],
>> adjacencies[i], point_reps[i]);
>> + todo_wine ok(hr == D3D_OK, "ConvertAdjacencyToPointReps failed. "
>> + "Got %x expected D3D_OK\n", hr);
>
> The value i should probably be printed in this error message.
> Otherwise, if you see a failure on test.winehq.org that you can't
> reproduce, it will be hard to figure out what might have caused the
> failure.
Yes, good idea.
> That's it for now.
Thanks your comments. I've attached an updated version.
From be56fecfb515d3407d7fe133ba00a8463745a973 Mon Sep 17 00:00:00 2001
From: Michael Mc Donnell
Date: Fri, 27 May 2011 16:24:18 +0200
Subject: d3dx9/tests: Implemented ConvertAdjacencyToPointReps test.
---
dlls/d3dx9_36/tests/mesh.c | 352
1 files changed, 352 insertions(+), 0 deletions(-)
diff --git a/dlls/d3dx9_36/tests/mesh.c b/dlls/d3dx9_36/tests/mesh.c
index 668097f..ae4dd26 100644
--- a/dlls/d3dx9_36/tests/mesh.c
+++ b/dlls/d3dx9_36/tests/mesh.c
@@ -2,6 +2,7 @@
* Copyright 2008 David Adam
* Copyright 2008 Luis Busquets
* Copyright 2009 Henri Verbeet for CodeWeavers
+ * Copyright 2011 Michael Mc Donnell
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -89,6 +90,93 @@ static BOOL compare_face(face a, face b)
return (a[0]==b[0] && a[1] == b[1] && a[2] == b[2]);
}
+struct test_context
+{
+HWND hwnd;
+IDirect3D9 *d3d;
+IDirect3DDevice9 *device;
+};
+
+/* Initializes a test context struct. Use it to initialize DirectX.
+ *
+ * Returns NULL if an error occured.
+ */
+static struct test_context *new_test_context(void)
+{
+HRESULT hr;
+HWND hwnd = NULL;
+IDirect3D9 *d3d = NULL;
+IDirect3DDevice9 *device = NULL;
+D3DPRESENT_PARAMETERS d3dpp = {0};
+struct test_context *test_context;
+
+hwnd = CreateWindow("static", "d3dx9_test", 0, 0, 0, 0, 0, NULL, NULL, NULL, NULL);
+if (!hwnd)
+{
+skip("Couldn't create application window\n");
+goto error;
+}
+
+d3d = Direct3DCreate9(D3D_SDK_VERSION);
+if (!d3d)
+{
+skip("Couldn't create IDirect3D9 object\n");
+goto error;
+}
+
+memset(&d3dpp, 0, sizeof(d3dpp));
+d3dpp.Windowed = TRUE;
+d3dpp.SwapEffect = D3DSWAPEFFECT_DISCARD;
+hr = IDirect3D9_CreateDevice(d3d, D3DADAPTER_DEFAULT, D3DDEVTYPE_HAL, hwnd,
+ D3DCREATE_MIXED_VERTEXPROCESSING, &d3dpp, &device);
+if (FAILED(hr))
+{
+sk
Re: [PATCH 1/5] shell32: Add IKnownFolderManager::RegisterFolder implementation (resend)
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=11886 Your paranoid android. === WVISTAADM (32 bit shellpath) === Failure running script in VM: The specified guest user must be logged in interactively to perform this operation
Re: [PATCH 4/6] msvcp90: Added locale class stub
Piotr Caban writes:
> +/*
> ?name@locale@std@@QBE?AV?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@2@XZ
> */
> +/*
> ?name@locale@std@@QEBA?AV?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@2@XZ
> */
> +DEFINE_THISCALL_WRAPPER(locale_name, 8)
> +void __thiscall locale_name(THIS_AND_RET(const locale *this,
> basic_string_char *ret))
> +{
> +FIXME("(%p %p) stub\n", this, ret);
> +}
You should declare it correctly for the non-i386 case, the return
pointer is a calling convention detail that the compiler should take
care of.
--
Alexandre Julliard
julli...@winehq.org
[1/2] d3dx9/test: Add UpdateSemantics test. (try 3)
Hi Alexandre My UpdateSemantics patches haven't been applied yet. Stefan Dösinger and Dylan Smith think they look ok. Is there something that looks wrong with them to you, or have you just not have the time to look at them yet? Thanks, Michael Mc Donnell
