Repository: bevyengine/bevy
File: crates/bevy_camera/src/camera.rs (Rust)

Code before:
    fn world_to_viewport_core(
        &self,
        camera_transform: &GlobalTransform,
        world_position: Vec3,
    ) -> Result<(Vec2, f32), ViewportConversionError> {
        let target_rect = self
            .logical_viewport_rect()
            .ok_or(ViewportConversionError::NoViewportSize)?;
        let mut ndc_space_coords = self
            .world_to_ndc(camera_transform, world_position)
            .ok_or(ViewportConversionError::InvalidData)?;
        // NDC z-values outside of 0 < z < 1 are outside the (implicit) camera frustum and are thus not in viewport-space
        if ndc_space_coords.z < 0.0 {
            return Err(ViewportConversionError::PastFarPlane);
        }
        if ndc_space_coords.z > 1.0 {
            return Err(ViewportConversionError::PastNearPlane);
        }

        // Flip the Y co-ordinate origin from the bottom to the top.
        ndc_space_coords.y = -ndc_space_coords.y;

        // Once in NDC space, we can discard the z element and map x/y to the viewport rect
        let viewport_position =
            (ndc_space_coords.truncate() + Vec2::ONE) / 2.0 * target_rect.size() + target_rect.min;
        Ok((viewport_position, ndc_space_coords.z))
    }

    /// Given a position in world space, use the camera to compute the viewport-space coordinates.
    ///
    /// To get the coordinates in Normalized Device Coordinates, you should use
    /// [`world_to_ndc`](Self::world_to_ndc).
    ///
    /// # Panics
    ///
    /// Will panic if `glam_assert` is enabled and the `camera_transform` contains `NAN`
    /// (see [`world_to_ndc`][Self::world_to_ndc]).
    #[doc(alias = "world_to_screen")]
    pub fn world_to_viewport(
        &self,
        camera_transform: &GlobalTransform,
        world_position: Vec3,
    ) -> Result<Vec2, ViewportConversionError> {
        Ok(self
            .world_to_viewport_core(camera_transform, world_position)?
            .0)
    }

    /// Given a position in world space, use the camera to compute the viewport-space coordinates and depth.
    ///
    /// To get the coordinates in Normalized Device Coordinates, you should use

Review comment (suggestion):
Does `truncate()` when creating `viewport_position`  mutate `ndc_space_coords` at all, specifically the z-component? if it does, the value for z might not be what is expected here, and it might be safer for this function to return `depth` (since `depth` in the previous logic seemed to be created before `viewport_position`)

(New here so take this with a grain of salt!)

Code after:
    fn world_to_viewport_core(
        &self,
        camera_transform: &GlobalTransform,
        world_position: Vec3,
    ) -> Result<(Vec2, f32), ViewportConversionError> {
        let target_rect = self
            .logical_viewport_rect()
            .ok_or(ViewportConversionError::NoViewportSize)?;
        let mut ndc_space_coords = self
            .world_to_ndc(camera_transform, world_position)
            .ok_or(ViewportConversionError::InvalidData)?;
        // NDC z-values outside of 0 < z < 1 are outside the (implicit) camera frustum and are thus not in viewport-space
        if ndc_space_coords.z < 0.0 {
            return Err(ViewportConversionError::PastFarPlane);
        }
        if ndc_space_coords.z > 1.0 {
            return Err(ViewportConversionError::PastNearPlane);
        }

        let depth = ndc_space_coords.z;

        // Flip the Y co-ordinate origin from the bottom to the top.
        ndc_space_coords.y = -ndc_space_coords.y;

        // Once in NDC space, we can discard the z element and map x/y to the viewport rect
        let viewport_position =
            (ndc_space_coords.truncate() + Vec2::ONE) / 2.0 * target_rect.size() + target_rect.min;
        Ok((viewport_position, depth))
    }

    /// Given a position in world space, use the camera to compute the viewport-space coordinates.
    ///
    /// To get the coordinates in Normalized Device Coordinates, you should use
    /// [`world_to_ndc`](Self::world_to_ndc).
    ///
    /// # Panics
    ///
    /// Will panic if `glam_assert` is enabled and the `camera_transform` contains `NAN`
    /// (see [`world_to_ndc`][Self::world_to_ndc]).
    #[doc(alias = "world_to_screen")]
    pub fn world_to_viewport(
        &self,
        camera_transform: &GlobalTransform,
        world_position: Vec3,
    ) -> Result<Vec2, ViewportConversionError> {
        Ok(self
            .world_to_viewport_core(camera_transform, world_position)?
            .0)
    }

    /// Given a position in world space, use the camera to compute the viewport-space coordinates and depth.