[prev in list] [next in list] [prev in thread] [next in thread]
List: wine-devel
Subject: Re: d3dx9_36 [patch 2/2]: Add tests for D3DXCreateBox
From: David Adam <david.adam.cnrs () gmail ! com>
Date: 2011-03-31 18:19:57
Message-ID: AANLkTi=LTcUh9gfucDoOOdn_f-XhWZ-ORcZLoDSAnLeD () mail ! gmail ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
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
> >
>
[Attachment #5 (text/html)]
<br><br><div class="gmail_quote">2011/3/31 Matteo Bruni <span dir="ltr"><<a \
href="mailto:matteo.mystral@gmail.com">matteo.mystral@gmail.com</a>></span><br><blockquote \
class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, \
204, 204); padding-left: 1ex;"> <div class="im">2011/3/31 David Adam <<a \
href="mailto:david.adam.cnrs@gmail.com">david.adam.cnrs@gmail.com</a>>:<br> \
><br> ><br>
> 2011/3/31 Matteo Bruni <<a \
href="mailto:matteo.mystral@gmail.com">matteo.mystral@gmail.com</a>><br> \
>><br> >> 2011/3/31 David Adam <<a \
href="mailto:david.adam.cnrs@gmail.com">david.adam.cnrs@gmail.com</a>>:<br> \
>> ><br> >> > + /* Check the width */<br>
>> > + v1 = *((D3DXVECTOR3*)(data+19*num_bytes_per_vertex));<br>
>> > + v2 = *((D3DXVECTOR3*)(data+18*num_bytes_per_vertex));<br>
>> > + length = \
D3DXVec3Length(D3DXVec3Subtract(&v2,&v1,&v2));<br> >> > + \
ok(fabs(length-10.0f)<admitted_error, "Width expected= 10.0,<br> >> \
> received %f\n", length);<br> >><br>
>> This is not really different from the previous version: you are still<br>
>> making assumptions about the vertex ordering (in this specific case,<br>
>> you're assuming that vertices 18 and 19 have different X coordinate<br>
>> but the same Y and Z coordinate values).<br>
><br>
> I have an application (Geoprofs) qui use the specific ordering of the<br>
> vertex. So if we implement randomly the vertex, it will not work.<br>
<br>
</div>Really? Well, that makes my original point mostly moot.<br>
Still, I don't think the current tests are very meaningful. Do you<br>
know exactly what that application needs? You could test for that<br>
specific thing, so if, for example, the application requires the left<br>
face to be the first one, you could test just for that.<br>
<div class="im"><br></div></blockquote><div><br>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<br> Fetching left and right \
faces is doing by fetching vertex .<br><br><br> </div><blockquote class="gmail_quote" \
style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); \
padding-left: 1ex;"><div class="im">
>><br>
>> What I was proposing was something like this:<br>
>><br>
>> for (i = 0; i < num_vertices; i++)<br>
>> {<br>
>> float *vertex_data = data + i * num_bytes_per_vertex;<br>
>> if (vertex_data[0] < xmin) xmin = vertex_data[0];<br>
>> if (vertex_data[0] > xmax) xmax = vertex_data[0];<br>
>> if (vertex_data[1] < ymin) ymin = vertex_data[1];<br>
>> if (vertex_data[1] > ymax) ymax = vertex_data[1];<br>
>> if (vertex_data[2] < zmin) zmin = vertex_data[2];<br>
>> if (vertex_data[2] > zmax) zmax = vertex_data[2];<br>
>> }<br>
>><br>
>> and then proceed to check whether the minimum and maximum values you<br>
>> got match the expected ones. This code snippet can certainly be<br>
>> improved, it is only to show you what I meant.<br>
>><br>
>> Also, again, please try to fix the whitespaces...<br>
><br>
> People changed the original style of the file. So, Which one must I keep ?<br>
><br>
<br>
</div>Arguably it is better to adopt the style used for most recent code in<br>
wined3d or d3dcompiler_43. But, regardless of that, you should at<br>
least be consistent in the same function. You are mixing 0 or 1 spaces<br>
after commas, spaces or no spaces around operators and stuff like<br>
that.<br>
<div><div></div><div class="h5"><br></div></div></blockquote><div><br>That looks \
reasonnable :D<br><br>Thanks again.<br><br>A+<br><br>David<br> </div><blockquote \
class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, \
204, 204); padding-left: 1ex;"> <div><div class="h5">
> Thanks for your advices ;)<br>
><br>
> A+<br>
><br>
> David<br>
><br>
</div></div></blockquote></div><br>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic