Re: [PATCH 1/1] d3dx9: Complete test for D3DXCreateSphere. (try 5)
Ok I am very very sorry email is clearly not my forte. Linking same file in test and main dll: I am not aware of a precedent for this but if there is one of course I would be glad to be shown otherwise. Second comment: please pardon if I am misunderstanding but how is what you suggest not simply moving the lookup table calculation from the helper function which we will also use for, at the very least, the cylinder test, inside the actual compute_sphere function itself? Thank you Misha On Jul 26, 2010 8:40 PM, "Octavian Voicu" <octavian.voicu(a)gmail.com> wrote: On Tue, Jul 27, 2010 at 4:15 AM, Misha Koshelev <misha680(a)gmail.com> wrote:
Why do you ask?
Ar... That's why I was asking, as I suppose you can't use functions from the test in the implementation, nor have internal functions used in the tests. Only way I see it is to have something like mesh_utils.[ch] which is linked with both the dll and the test, but I'm not sure if that's acceptable. Maybe someone else can answer that.
A little observation is that you don't really need to precompute both tables, just the one for phi (if you compute sin and cos of current theta outside the inner loop). Octavian
On Tue, Jul 27, 2010 at 5:04 AM, Misha Koshelev <misha680(a)gmail.com> wrote:
Linking same file in test and main dll: I am not aware of a precedent for this but if there is one of course I would be glad to be shown otherwise.
On second thought, not sure if it is worth it anyway, there is probably not that much to share.
Second comment: please pardon if I am misunderstanding but how is what you suggest not simply moving the lookup table calculation from the helper function which we will also use for, at the very least, the cylinder test, inside the actual compute_sphere function itself?
My observation is that you don't need to precompute *both* tables for compute_sphere (you still need to compute the phi table) -- so no, I'm not suggesting you move the lookup table calculation from the helper. As for the theta table, you only access it by theta.sin[stack] and theta.cos[stack] from the inner loop, so you could do something like this: /* middle */ theta = theta_start; for (stack = 0; stack < stacks - 1; stack++) { sin_theta = sin(theta); cos_theta = cos(theta); for (slice = 0; slice < slices; slice++) { mesh->vertices[vertex].normal.x = sin_theta * phi.cos[slice]; mesh->vertices[vertex].normal.y = sin_theta * phi.sin[slice]; mesh->vertices[vertex].normal.z = cos_theta; mesh->vertices[vertex].position.x = radius * sin_theta * phi.cos[slice]; mesh->vertices[vertex].position.y = radius * sin_theta * phi.sin[slice]; mesh->vertices[vertex].position.z = radius * cos_theta; vertex++; /* ... */ } /* ... */ theta += theta_step; } Anyway, this is a minor issue. Although the above is a bit faster (because you don't need to allocate an extra table and then access it in memory), it's not very relevant for the tests. I only mentioned it because I noticed you didn't update the D3DXCreateSphere implementation to use tables yet, so maybe you use this when you do. If some app would call D3DXCreateSphere very often, then it might be worth it. Keep up the good work! Octavian
On Tue, 2010-07-27 at 05:56 +0300, Octavian Voicu wrote:
On Tue, Jul 27, 2010 at 5:04 AM, Misha Koshelev <misha680(a)gmail.com> wrote:
Linking same file in test and main dll: I am not aware of a precedent for this but if there is one of course I would be glad to be shown otherwise.
On second thought, not sure if it is worth it anyway, there is probably not that much to share.
Second comment: please pardon if I am misunderstanding but how is what you suggest not simply moving the lookup table calculation from the helper function which we will also use for, at the very least, the cylinder test, inside the actual compute_sphere function itself?
My observation is that you don't need to precompute *both* tables for compute_sphere (you still need to compute the phi table) -- so no, I'm not suggesting you move the lookup table calculation from the helper.
As for the theta table, you only access it by theta.sin[stack] and theta.cos[stack] from the inner loop, so you could do something like this:
/* middle */ theta = theta_start; for (stack = 0; stack < stacks - 1; stack++) { sin_theta = sin(theta); cos_theta = cos(theta); for (slice = 0; slice < slices; slice++) { mesh->vertices[vertex].normal.x = sin_theta * phi.cos[slice]; mesh->vertices[vertex].normal.y = sin_theta * phi.sin[slice]; mesh->vertices[vertex].normal.z = cos_theta; mesh->vertices[vertex].position.x = radius * sin_theta * phi.cos[slice]; mesh->vertices[vertex].position.y = radius * sin_theta * phi.sin[slice]; mesh->vertices[vertex].position.z = radius * cos_theta; vertex++; /* ... */ } /* ... */ theta += theta_step; }
Anyway, this is a minor issue. Although the above is a bit faster (because you don't need to allocate an extra table and then access it in memory), it's not very relevant for the tests.
I only mentioned it because I noticed you didn't update the D3DXCreateSphere implementation to use tables yet, so maybe you use this when you do. If some app would call D3DXCreateSphere very often, then it might be worth it. No you're right. Thank you. This would result in a speed increase and I will definitely consider adding it into the D3DXCreateSphere implementation.
As you mention yourself, its not really an issue for the tests though.
Keep up the good work!
Thanks :) I still wish I could figure out that teapot... oh well maybe in some time. Misha
Octavian
participants (2)
-
Misha Koshelev -
Octavian Voicu