Ray3d::from_screenspace orthographic improvements

[dmlary]

Ray3d::from_screenspace() previously was not taking into account the near field for orthographic projections. When near was set to a negative value, bevy_mod_raycast would discard the aabb intersection of some items on screen as they were behind the camera's location.

In this change I extract the near & far values from the projection matrix and use those for origin calculations. This moves the origin of the Ray3d to be at the near location, ensuring everything visibile on the screen is in front of the raycast origin.

Support for perspective projections is maintained by the fact that one of the fields used for calculating near & far, projection.z_axis.z is set to zero for perspective projection. In that case we use the old values of (-1, 1) for the near & far values.

Original research & discussion for this research can be found here: https://github.com/aevyrie/bevy_mod_picking/issues/209

[dmlary]

I do have a testcase for this, but it is awful. I had to do some ugly things (std::mem::transmute) to manually construct a Camera instance with the needed ComputedCameraValues (all fields are private 😑). I don't feel comfortable submitting it as a real test because upstream changes to private fields will break it.

Would you like me to throw together an orthographic example with objects behind the camera?

[aevyrie]

I added a test case, though it does not seem to be working as expected. The ray origin is still at 0.0, even if the near plane is made negative.

[dmlary]

I was testing by putting a plane behind the camera and setting near negative, then verifying the hit depth was >= 0. I’ll take a look at your test tomorrow and try to sort out what’s up.

[dmlary]

I believe this is the problem:

RaycastSource::<MyRaycastSet>::new_transform_empty(),

This is usingRaycastMethod::Transform as the raycast method, but this fix is only for RaycastMethod::Screenspace, with the code being in Ray3d::from_screenspace().

I don't think this change makes sense for the Transform method.

[dmlary]

This is the testing method I used originally. It has to do some ugly things to get a valid Camera instance to use with from_screenspace. It shows origin being correctly updated for this case.

---- primitives::tests::orthographic_projection_ray_test stdout ----
cursor pos Vec2(640.0, 360.0)
camera pos Vec3(0.0, 3.0, 5.0)
camera direction Vec3(-0.0, -0.44721362, -0.8944272)
projection near -20, far 10
ray Ray3d {
    origin: Vec3A(
        0.0,
        11.944271,
        22.888542,  // note: camera at z = 5, near is -20, so we expect 25, but with Y = 3, we end up at ~23
    ),
    direction: Vec3A(
        0.0,
        -0.4472136,
        -0.8944272,
    ),
}

successes:
    primitives::tests::orthographic_projection_ray_test
    raycast::tests::raycast_triangle_mt
    raycast::tests::raycast_triangle_mt_culling

#[cfg(test)]
mod tests {
    use super::*;
    use bevy::render::{
        camera::{
            CameraProjection, ComputedCameraValues, OrthographicProjection, RenderTargetInfo,
        },
        primitives::Aabb,
    };

    // -__- ComputedCameraValues has all private members, but we need to set
    // the projection matrix for testing.  We're going to duplicate that
    // structure here, then `mem::transmute()` to the real thing.
    //
    // XXX This will break when `ComputedCameraValues` changes.
    #[allow(dead_code)]
    struct ComputedCameraValuesPub {
        projection_matrix: Mat4,
        target_info: Option<bevy::render::camera::RenderTargetInfo>,
        old_viewport_size: Option<UVec2>,
    }

    #[test]
    fn orthographic_projection_ray_test() {
        // orthographic projection for 1280x720 region
        let mut projection = OrthographicProjection {
            near: -20.0, // ensure near extends past the plane behind us
            far: 10.0,
            ..default()
        };
        projection.update(1280.0, 720.0);

        // build a camera positioned at the origin, looking at Z < 0
        //
        // ugly workaround to get past `ComputedCameraValues` having all private
        // fields.
        let computed_camera_values: ComputedCameraValues = unsafe {
            let values = ComputedCameraValuesPub {
                projection_matrix: projection.get_projection_matrix(),
                target_info: Some(RenderTargetInfo {
                    physical_size: UVec2::new(1280, 720),
                    scale_factor: 1.0,
                }),
                old_viewport_size: None,
            };
            std::mem::transmute(values)
        };
        let camera = Camera {
            viewport: Some(bevy::render::camera::Viewport {
                physical_position: UVec2::new(0, 0),
                physical_size: UVec2::new(1280, 720),
                ..default()
            }),
            computed: computed_camera_values,
            ..default()
        };
        let camera_transform =
            Transform::from_translation(Vec3::new(0.0, 3.0, 5.0)).looking_at(Vec3::NEG_Z, Vec3::Y);
        let global_transform = camera_transform.into();

        let cursor_pos = Vec2::new(1280.0 * 0.5, 720.0 * 0.5);

        let ray = Ray3d::from_screenspace(cursor_pos, &camera, &global_transform).unwrap();
        println!("cursor pos {:?}", cursor_pos);
        println!("camera pos {:?}", camera_transform.translation);
        println!("camera direction {:?}", global_transform.forward());
        println!(
            "projection near {}, far {}",
            projection.near, projection.far
        );
        println!("ray {:#?}", ray);
        assert!(
            ray.origin().z > 19.9,
            "ray.origin().z should be past the plane at Z = 10; {:#?}",
            ray
        );
    }
}

[dmlary]

Ah, I finally saw the merge conflict on this with the new implementation of from_screenspace:

https://github.com/aevyrie/bevy_mod_raycast/blob/2410720e81ed9cb745fab464e82f56dbbd8f9dd8/src/primitives.rs#L161-L169

I adapted the example for bevy 0.11 and confirmed it correctly accounts for camera near being negative.

Closing this PR and associated issue as resolved.