TheAssemblyArmada / Thyme

An open source re-implementation of Generals : Zero Hour written in C++.
GNU General Public License v2.0
585 stars 59 forks source link

reinterpret_cast in W3DVolumetricShadow::Update_Mesh_Volume #1048

Open xezon opened 10 months ago

xezon commented 10 months ago
    if (GameMath::Fabs(reinterpret_cast<Vector3 &>(m_objectXformHistory[light_index][mesh_index][0])
            * reinterpret_cast<Vector3 &>(object_to_world[0]))
        >= s_cosAngleToCare) {
        if (GameMath::Fabs(reinterpret_cast<Vector3 &>(m_objectXformHistory[light_index][mesh_index][1])
                * reinterpret_cast<Vector3 &>(object_to_world[1]))
            >= s_cosAngleToCare) {
            if (GameMath::Fabs(reinterpret_cast<Vector3 &>(m_objectXformHistory[light_index][mesh_index][2])
                    * reinterpret_cast<Vector3 &>(object_to_world[2]))
                < s_cosAngleToCare) {
                is_mesh_rotating = true;
            }
        } else {
            is_mesh_rotating = true;
        }
    } else {
        is_mesh_rotating = true;
    }

Better be written as

    if (GameMath::Fabs(Thyme::To_Vector3(m_objectXformHistory[light_index][mesh_index][0])
            * Thyme::To_Vector3(object_to_world[0]))
        >= s_cosAngleToCare) {
        if (GameMath::Fabs(Thyme::To_Vector3(m_objectXformHistory[light_index][mesh_index][1])
                * Thyme::To_Vector3(object_to_world[1]))
            >= s_cosAngleToCare) {
            if (GameMath::Fabs(Thyme::To_Vector3(m_objectXformHistory[light_index][mesh_index][2])
                    * Thyme::To_Vector3(object_to_world[2]))
                < s_cosAngleToCare) {
                is_mesh_rotating = true;
            }
        } else {
            is_mesh_rotating = true;
        }
    } else {
        is_mesh_rotating = true;
    }

Needs new functions in Vector4 class file:

inline const Vector3 &To_Vector3(const Vector4 &vector);
inline Vector3 &To_Vector3(Vector4 &vector);

Reason being:

reinterpret_cast requires strong contract that Vector3 and Vector4 are castable this way. Only Vector4 provided functions should do that.

xezon commented 10 months ago

Another in same function:

Matrix4 world_to_object;
Matrix4 object_to_world(*mesh_xform);
...
D3DXMatrixInverse(
            reinterpret_cast<D3DXMATRIX *>(&world_to_object), &det, reinterpret_cast<D3DXMATRIX *>(&object_to_world));

Better:

Matrix4 world_to_object;
Matrix4 object_to_world(*mesh_xform);
...
world_to_object = object_to_world.Inverse();