2011/3/31 David Adam david.adam.cnrs@gmail.com:
/* Check the width */v1 = *((D3DXVECTOR3*)(data+19*num_bytes_per_vertex));v2 = *((D3DXVECTOR3*)(data+18*num_bytes_per_vertex));length = D3DXVec3Length(D3DXVec3Subtract(&v2,&v1,&v2));ok(fabs(length-10.0f)<admitted_error, "Width expected= 10.0, received %f\n", length);
This is not really different from the previous version: you are still making assumptions about the vertex ordering (in this specific case, you're assuming that vertices 18 and 19 have different X coordinate but the same Y and Z coordinate values).
What I was proposing was something like this:
for (i = 0; i < num_vertices; i++) { float *vertex_data = data + i * num_bytes_per_vertex; if (vertex_data[0] < xmin) xmin = vertex_data[0]; if (vertex_data[0] > xmax) xmax = vertex_data[0]; if (vertex_data[1] < ymin) ymin = vertex_data[1]; if (vertex_data[1] > ymax) ymax = vertex_data[1]; if (vertex_data[2] < zmin) zmin = vertex_data[2]; if (vertex_data[2] > zmax) zmax = vertex_data[2]; }
and then proceed to check whether the minimum and maximum values you got match the expected ones. This code snippet can certainly be improved, it is only to show you what I meant.
Also, again, please try to fix the whitespaces...
2011/3/31 Matteo Bruni matteo.mystral@gmail.com
2011/3/31 David Adam david.adam.cnrs@gmail.com:
/* Check the width */v1 = *((D3DXVECTOR3*)(data+19*num_bytes_per_vertex));v2 = *((D3DXVECTOR3*)(data+18*num_bytes_per_vertex));length = D3DXVec3Length(D3DXVec3Subtract(&v2,&v1,&v2));ok(fabs(length-10.0f)<admitted_error, "Width expected= 10.0,received %f\n", length);
This is not really different from the previous version: you are still making assumptions about the vertex ordering (in this specific case, you're assuming that vertices 18 and 19 have different X coordinate but the same Y and Z coordinate values).
I have an application (Geoprofs) qui use the specific ordering of the vertex. So if we implement randomly the vertex, it will not work.
What I was proposing was something like this:
for (i = 0; i < num_vertices; i++) { float *vertex_data = data + i * num_bytes_per_vertex; if (vertex_data[0] < xmin) xmin = vertex_data[0]; if (vertex_data[0] > xmax) xmax = vertex_data[0]; if (vertex_data[1] < ymin) ymin = vertex_data[1]; if (vertex_data[1] > ymax) ymax = vertex_data[1]; if (vertex_data[2] < zmin) zmin = vertex_data[2]; if (vertex_data[2] > zmax) zmax = vertex_data[2]; }
and then proceed to check whether the minimum and maximum values you got match the expected ones. This code snippet can certainly be improved, it is only to show you what I meant.
Also, again, please try to fix the whitespaces...
People changed the original style of the file. So, Which one must I keep ?
Thanks for your advices ;)
A+
David
2011/3/31 David Adam david.adam.cnrs@gmail.com:
2011/3/31 Matteo Bruni matteo.mystral@gmail.com
2011/3/31 David Adam david.adam.cnrs@gmail.com:
- /* Check the width */
- v1 = *((D3DXVECTOR3*)(data+19*num_bytes_per_vertex));
- v2 = *((D3DXVECTOR3*)(data+18*num_bytes_per_vertex));
- length = D3DXVec3Length(D3DXVec3Subtract(&v2,&v1,&v2));
- ok(fabs(length-10.0f)<admitted_error, "Width expected= 10.0,
received %f\n", length);
This is not really different from the previous version: you are still making assumptions about the vertex ordering (in this specific case, you're assuming that vertices 18 and 19 have different X coordinate but the same Y and Z coordinate values).
I have an application (Geoprofs) qui use the specific ordering of the vertex. So if we implement randomly the vertex, it will not work.
Really? Well, that makes my original point mostly moot. Still, I don't think the current tests are very meaningful. Do you know exactly what that application needs? You could test for that specific thing, so if, for example, the application requires the left face to be the first one, you could test just for that.
What I was proposing was something like this:
for (i = 0; i < num_vertices; i++) { float *vertex_data = data + i * num_bytes_per_vertex; if (vertex_data[0] < xmin) xmin = vertex_data[0]; if (vertex_data[0] > xmax) xmax = vertex_data[0]; if (vertex_data[1] < ymin) ymin = vertex_data[1]; if (vertex_data[1] > ymax) ymax = vertex_data[1]; if (vertex_data[2] < zmin) zmin = vertex_data[2]; if (vertex_data[2] > zmax) zmax = vertex_data[2]; }
and then proceed to check whether the minimum and maximum values you got match the expected ones. This code snippet can certainly be improved, it is only to show you what I meant.
Also, again, please try to fix the whitespaces...
People changed the original style of the file. So, Which one must I keep ?
Arguably it is better to adopt the style used for most recent code in wined3d or d3dcompiler_43. But, regardless of that, you should at least be consistent in the same function. You are mixing 0 or 1 spaces after commas, spaces or no spaces around operators and stuff like that.
Thanks for your advices ;)
A+
David
2011/3/31 Matteo Bruni matteo.mystral@gmail.com
2011/3/31 David Adam david.adam.cnrs@gmail.com:
2011/3/31 Matteo Bruni matteo.mystral@gmail.com
2011/3/31 David Adam david.adam.cnrs@gmail.com:
/* Check the width */v1 = *((D3DXVECTOR3*)(data+19*num_bytes_per_vertex));v2 = *((D3DXVECTOR3*)(data+18*num_bytes_per_vertex));length = D3DXVec3Length(D3DXVec3Subtract(&v2,&v1,&v2));ok(fabs(length-10.0f)<admitted_error, "Width expected= 10.0,received %f\n", length);
This is not really different from the previous version: you are still making assumptions about the vertex ordering (in this specific case, you're assuming that vertices 18 and 19 have different X coordinate but the same Y and Z coordinate values).
I have an application (Geoprofs) qui use the specific ordering of the vertex. So if we implement randomly the vertex, it will not work.
Really? Well, that makes my original point mostly moot. Still, I don't think the current tests are very meaningful. Do you know exactly what that application needs? You could test for that specific thing, so if, for example, the application requires the left face to be the first one, you could test just for that.
The application colors the left face and the right face and applies geometric tranformations, so the student can see where arrives the faces in comparison with the initial box Fetching left and right faces is doing by fetching vertex .
What I was proposing was something like this:
for (i = 0; i < num_vertices; i++) { float *vertex_data = data + i * num_bytes_per_vertex; if (vertex_data[0] < xmin) xmin = vertex_data[0]; if (vertex_data[0] > xmax) xmax = vertex_data[0]; if (vertex_data[1] < ymin) ymin = vertex_data[1]; if (vertex_data[1] > ymax) ymax = vertex_data[1]; if (vertex_data[2] < zmin) zmin = vertex_data[2]; if (vertex_data[2] > zmax) zmax = vertex_data[2]; }
and then proceed to check whether the minimum and maximum values you got match the expected ones. This code snippet can certainly be improved, it is only to show you what I meant.
Also, again, please try to fix the whitespaces...
People changed the original style of the file. So, Which one must I keep
?
Arguably it is better to adopt the style used for most recent code in wined3d or d3dcompiler_43. But, regardless of that, you should at least be consistent in the same function. You are mixing 0 or 1 spaces after commas, spaces or no spaces around operators and stuff like that.
That looks reasonnable :D
Thanks again.
A+
David
Thanks for your advices ;)
A+
David
-
David Adam -
Matteo Bruni