Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrong view projection matrices #517

Open
Azkellas opened this issue Dec 11, 2023 · 4 comments
Open

Wrong view projection matrices #517

Azkellas opened this issue Dec 11, 2023 · 4 comments

Comments

@Azkellas
Copy link

Azkellas commented Dec 11, 2023

The view proj matrices in the course don't provide far clipping. Projecting a point at dist = far_plane returns 0.33.

After comparing with wgpu examples, for the same parameters, the computed matrices are indeed different, and their matrices return the correct depth. This also affects xy that are 33% smaller in learn-wgpu, resulting in a smaller fov.

#478 transposed OPENGL_TO_WGPU_MATRIX but reverting this change fixes the discrepancy between the two libraries, both for perspective and orthogonal matrices, fixing the issue.

I lack insight to know what was the issue in #478, but without more analysis, I think it's better to revert the change to be aligned with wgpu examples and get a correct depth/fov back.

On a side note, wgpu examples use glam, a "wgpu-ready" linalg crate with bytemuck integration and only has specific functions for gl matrices, wgpu being the default. I think it is a good alternative to consider to be more pure wgpu. I'm willing to look into it if you agree with my sentiment.

I'm copying the comparison code here for reference, and the archive in a zip file:

Comparison code
use cgmath::prelude::*;

#[rustfmt::skip]
const CURRENT_OPENGL_TO_WGPU_MATRIX: cgmath::Matrix4<f32> = cgmath::Matrix4::new(
  1.0, 0.0, 0.0, 0.0,
  0.0, 1.0, 0.0, 0.0,
  0.0, 0.0, 0.5, 0.5,
  0.0, 0.0, 0.0, 1.0,
);

#[rustfmt::skip]
const FIXED_OPENGL_TO_WGPU_MATRIX: cgmath::Matrix4<f32> = cgmath::Matrix4::new(
  1.0, 0.0, 0.0, 0.0,
  0.0, 1.0, 0.0, 0.0,
  0.0, 0.0, 0.5, 0.0,
  0.0, 0.0, 0.5, 1.0,
);

struct CameraData {
  eye: (f32, f32, f32),
  target: (f32, f32, f32),
  up: (f32, f32, f32),
  fovy: f32,
  aspect: f32,
  znear: f32,
  zfar: f32,
}

fn build_cgmath_view_projection_matrix(camera_data: &CameraData) -> cgmath::Matrix4<f32> {
  let view = cgmath::Matrix4::look_at_rh(
      camera_data.eye.into(),
      camera_data.target.into(),
      camera_data.up.into(),
  );
  let proj = cgmath::perspective(
      cgmath::Deg(camera_data.fovy),
      camera_data.aspect,
      camera_data.znear,
      camera_data.zfar,
  );
  proj * view
}

fn build_glam_view_projection_matrix(camera_data: &CameraData) -> glam::Mat4 {
  let view = glam::Mat4::look_at_rh(
      camera_data.eye.into(),
      camera_data.target.into(),
      camera_data.up.into(),
  );
  let proj = glam::Mat4::perspective_rh(
      camera_data.fovy.to_radians(),
      camera_data.aspect,
      camera_data.znear,
      camera_data.zfar,
  );
  proj * view
}

fn main() {
  let camera_data = CameraData {
      eye: (0.0, 0.0, 0.0),
      target: (0.0, 0.0, 1.0),
      up: (0.0, 1.0, 0.0),
      fovy: 45.0,
      aspect: 1.0,
      znear: 0.1,
      zfar: 100.0,
  };

  let cgmath_view_projection_matrix = build_cgmath_view_projection_matrix(&camera_data);

  let current_cgmath = CURRENT_OPENGL_TO_WGPU_MATRIX * cgmath_view_projection_matrix;
  let fixed_cgmath = FIXED_OPENGL_TO_WGPU_MATRIX * cgmath_view_projection_matrix;
  println!("{:?}", current_cgmath);
  // [[-2.4142134, 0.0, 0.0, 0.0], [0.0, 2.4142134, 0.0, 0.0], [0.0, 0.0, 0.501001, 1.501001], [0.0, 0.0, -0.1001001, -0.1001001]]
  println!("{:?}", fixed_cgmath);
  // [[-2.4142134, 0.0, 0.0, 0.0], [0.0, 2.4142134, 0.0, 0.0], [0.0, 0.0, 1.001001, 1.0], [0.0, 0.0, -0.1001001, 0.0]]

  let glam_view_projection_matrix = build_glam_view_projection_matrix(&camera_data);
  println!("{}", glam_view_projection_matrix);
  // [[-2.4142134, 0.0, 0.0, 0.0], [0.0, 2.4142134, 0.0, 0.0], [0.0, 0.0, 1.001001, 1.0], [0.0, 0.0, -0.1001001, 0.0]]

  // target on far plane
  let target = (20.0, -10.0, camera_data.zfar);

  let current_cgmath_projected = current_cgmath.transform_point(target.into());
  let fixed_cgmath_projected = fixed_cgmath.transform_point(target.into());
  let mut glam_projected =
      glam_view_projection_matrix * glam::Vec4::new(target.0, target.1, target.2, 1.0);
  glam_projected /= glam_projected.w;

  println!("{:?}", current_cgmath_projected);
  // Point3 [-0.32189512, -0.16094756, 0.33333334]
  println!("{:?}", fixed_cgmath_projected);
  // Point3 [-0.48284265, -0.24142133, 1.0]
  println!("{:?}", glam_projected);
  // Vec4(-0.48284268, -0.24142134, 1.0, 1.0)
}

learn_wgpu_matrices.zip

@shanebenlolo
Copy link

when I run the depth buffer locally I do not see anything but a black screen. When I run in the browser it works just fine. Do you have any insight into how I might fix that?

I just had to struggle for two days trying to figure out why my ray-sphere intersection was not working only to find out I needed to remove the OPENGL_TO_WGPU_MATRIX to get it working natively, and I still haven't figure out how to get it working in the browser.

@sotrh
Copy link
Owner

sotrh commented Dec 16, 2023

I'm considering removing OPENGL_TO_WGPU_MATRIX entirely. I've already removed it in the HDR tutorial as it messes with the math for drawing the skybox.

@shanebenlolo
Copy link

I'm considering removing OPENGL_TO_WGPU_MATRIX entirely. I've already removed it in the HDR tutorial as it messes with the math for drawing the skybox.

How would you go about removing it? I assumed it was necessary for normalizing the z axis properly.

@sotrh
Copy link
Owner

sotrh commented Dec 16, 2023

I believe it's meant to move the projection forward and scale it in the z by 2. Opengl has the z buffer between -1 and 1 while wgpu has it between 0 and 1. To be fair I'm not sure about the specifics, but removing it from the camera calculation made the skybox code work. I could have had add an extra matrix to the uniforms or something similar, but removing it worked fine.

I wouldn't be opposed to switching to glam as I use that in any new projects I make, though it would be a big refactor.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants