0
\$\begingroup\$

and can I make it more efficient? It's to test the collision between a ray and a sphere.

float BoundingSphere::Intersects(Ray ray)
{
D3DXVECTOR3 direction = ray.Direction();
D3DXVECTOR3 vec = D3DXVECTOR3();
D3DXVECTOR3 cToO = ray.Origin() - centre;
float rSquared = radius * radius;
float vSquared = 0;

if(D3DXVec3Dot(&cToO, &cToO) <= rSquared)
{
    return 0;
}
else if(D3DXVec3Dot(&cToO, &direction) <= 0)
{
    vec = cToO - direction * D3DXVec3Dot(&cToO, &direction);
    vSquared = D3DXVec3Dot(&vec, &vec);

    if(vSquared <= rSquared)
    {
        D3DXVECTOR3 normDir = *D3DXVec3Normalize(&direction, &direction);
        float mul = sqrt(rSquared - vSquared);

        return D3DXVec3Length(
            &(ray.Origin() - 
            (centre + vec - D3DXVECTOR3(normDir.x *= mul, normDir.y *= mul, normDir.z *= mul))));
    }
    else
    {
        return -1;
    }
}
else
{
    return -1;
}
}

it should return -1 if there is no collision, 0 if the origin of the ray is inside the sphere. Otherwise it should return a float of the distance from the ray's origin to the first point of collision.

\$\endgroup\$

closed as off-topic by Jamal May 3 '14 at 1:35

This question appears to be off-topic. The users who voted to close gave this specific reason:

  • "Questions containing broken code or asking for advice about code not yet written are off-topic, as the code is not ready for review. Such questions may be suitable for Stack Overflow or Programmers. After the question has been edited to contain working code, we will consider reopening it." – Jamal
If this question can be reworded to fit the rules in the help center, please edit the question.

  • 3
    \$\begingroup\$ Did you test it? Any uni tests? \$\endgroup\$ – Martin York Dec 12 '11 at 17:12
  • \$\begingroup\$ it's basically an attempt to port someone elses suedocode. I have tested and it's not working properly. \$\endgroup\$ – SirYakalot Dec 12 '11 at 17:49
2
\$\begingroup\$

Since there's no answer yet, I'll take a crack at it.

My first impression is that the nesting is somewhat deep and some of the variables could be moved closer to its site of usage. Here's one possible refactor:

float BoundingSphere::Intersects(Ray ray)
{
    D3DXVECTOR3 cToO = ray.Origin() - centre;
    float rSquared = radius * radius;
    if(D3DXVec3Dot(&cToO, &cToO) <= rSquared) return 0;

    D3DXVECTOR3 direction = ray.Direction();
    if(D3DXVec3Dot(&cToO, &direction) > 0) return -1;

    D3DXVECTOR3 vec = cToO - direction * D3DXVec3Dot(&cToO, &direction);
    float vSquared = D3DXVec3Dot(&vec, &vec);
    if(vSquared > rSquared) return -1;
    // ...
}

There should be someway to multiple a D3DXVECTOR3 with a scalar value at once. Even if that's not directly supported it's trivial to overload the operator *= yourself. The return is also getting rather long. I broken that long statement into more digestible pieces:

    float mul = sqrt(rSquared - vSquared);
    D3DXVECTOR3 normDir = *D3DXVec3Normalize(&direction, &direction);
    normDir *= mul;

    return D3DXVec3Length(&(ray.Origin() - centre - vec + normDir));
}

I haven't looked at the collision in too much detail, it seems ok from a glance.

it should return -1 if there is no collision, 0 if the origin of the ray is inside the sphere...

It sounds like you have a clear idea of how this method is suppose to behave. Why not grab a unit testing framework, like google test, and write a unit test for each of those cases? Testing BoundingSphere::Intersects can be as simple as this:

TEST(BoundingSphereTest, testRayInsideSphere)
{
    BoundingSphere testSphere(D3DXVECTOR3(0, 0, 0), 42);
    Ray testRay(0, 0);

    EXPECT_EQ(-1, testSphere.Intersects(testRay));
}

Note, I'm just guessing on how those objects are constructed but this should give you the main idea. Fix them up as appropriate.

\$\endgroup\$

Not the answer you're looking for? Browse other questions tagged or ask your own question.