patriciogonzalezvivo / lygia

LYGIA, it's a granular and multi-language (GLSL, HLSL, WGSL, MSL and CUDA) shader library designed for performance and flexibility
https://lygia.xyz
Other
2.61k stars 169 forks source link

Changes to raymarching camera FOV #181

Closed shadielhajj closed 3 months ago

shadielhajj commented 3 months ago

This one is a bit tricky as it will affect existing scenes, but I think it’s something we’ll have to grapple with sooner or later.

There are 3 issues:

  1. In lighting/raymarch, the camera position is being multiplied by a factor named RAYMARCH_CAMERA_SCALE which defaults to an apparently arbitrary 0.11. Unless I’m missing something, I can’t see the rationale or the usefulness of this. But more importantly, it leads to the scene and the camera having different coordinate systems. It’s as if the scene is specified in meters and the camera in cms (except the factor is 0.11, not 0.1). For a 1m quad to cover a 60 degrees camera viewport, the camera currently needs to be positioned 7.89m away which is absurd and counter-intuitive (some basic trigonometry shows it should be just 0.86m)
  2. RAYMARCH_CAMERA_FOVis currently specified as the inverse of the tangent of the half FOV angle. This is not intuitive, especially since decreasing this value leads to a higher FOV.
  3. The x-axis is flipped. Moving an object along the positive x-axis visually displaces it to the left. Again, not very intuitive.

Proposed changes:

  1. Get rid of RAYMARCH_CAMERA_SCALE altogether. I don’t see any case where we would want to have anything but 1 here. Or at least, set the default to one.
  2. Specify RAYMARCH_CAMERA_FOVin degrees (vertical FOV), and default to 60.
  3. Flip the x-axis, so that the positive x-axis points to the right, as it’s usual with cartesian coordinate systems.

I've applied these changes to both GLSL and HLSL versions. I’ve also back-to-backed these changes against the Unity camera to make sure it's behaving consistently and in a physically correct manner.

Keen to hear your thoughts on this one @patriciogonzalezvivo.

patriciogonzalezvivo commented 3 months ago

This is great. I'm not really a RayMarching guy. So If you feel confident with this direction I will let you steer the ship. Because this PR will break things for folks. I'm going to make a new LYGIA version for it, so it's clear that version contain breaking changes.

I really appreciate all your time and expertise in this matter.

shadielhajj commented 3 months ago

Sounds good @patriciogonzalezvivo!

Yes, I believe these changes are important, so let's get them out of the way as soon as possible. I'm still testing and assessing the Raymarching subsystem in Lygia, but this is the biggest issue I've found so far. I'd also like to add support for multiple materials in a single scene, but I don't have an elegant solution for that yet.

In any case, I think creating a new version branch is a great idea.

Very excited about the potential of Lygia as a Raymarching engine!

patriciogonzalezvivo commented 3 months ago

Awesome! Let's do it!

patriciogonzalezvivo commented 3 months ago

Let's go! <3 <3 <3