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

glTF2 Importer uses incorrect code to calculate horizontal FOV #4435

Closed
jamesn031199 opened this issue Mar 9, 2022 · 1 comment
Closed

Comments

@jamesn031199
Copy link

jamesn031199 commented Mar 9, 2022

The importer code for glTF2.0 assumes multiplying the vertical FOV by the aspect ratio is how horizontal FOV is calculated.
However, this is not correct and the correct way to convert between the two is by this method referenced in this Reddit post.

shimaowo added a commit to shimaowo/assimp that referenced this issue Jan 16, 2023
kimkulling added a commit that referenced this issue Jan 23, 2023
Fix: fix incorrect math for calculating the horizontal FOV of a perspective camera in gltf2 import #4435
@kvnnap
Copy link

kvnnap commented Nov 17, 2023

This code does not respect the aiCamera's interface. It computes the correct horizontal FOV, however, the camera struct aiCamera mHorizontalFOV is defined as being half the horizontal field of view according to the documentation. I have noticed this issue since when I import FBX files the cameras load properly, however with gltf2 the FOV is always wrong. I tested a scene with cameras in this online gltf viewer, and indeed they load fine here. Additionally, I checked the FBX importer code here. This code computes half the horizontal FOV as seen below:

// FBX fov is full-view degrees. We want half-view radians.
out_camera->mHorizontalFOV = AI_DEG_TO_RAD(fov_deg) * 0.5f;

The code here should therefore change from:

aicam->mHorizontalFOV = 2.0f * std::atan(std::tan(cam.cameraProperties.perspective.yfov * 0.5f) * ((aicam->mAspect == 0.f) ? 1.f : aicam->mAspect));

to

aicam->mHorizontalFOV = std::atan(std::tan(cam.cameraProperties.perspective.yfov * 0.5f) * ((aicam->mAspect == 0.f) ? 1.f : aicam->mAspect));

I will gladly provide a pull request if this is agreed to be correct.

Update:

I noticed that the semantics of mHorizontalFOV may be changing from half to full FOV. In which case, this would make the FBX imported not compliant. Not sure what to do.

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

No branches or pull requests

2 participants