john-chapman / im3d

Immediate mode rendering and 3d gizmos.
MIT License
1.21k stars 63 forks source link

im3d default Camera model does not suppot aspect ratio on height #54

Closed ChemistAion closed 2 years ago

ChemistAion commented 2 years ago

Hi!

The default Im3d camera model does not support "height" (aspect ratio) influence onto its projection matrix. If you change view width the pixel size (as a square) is kept, when height changes projection matrix "scales" the output.

Here is the snippet for camera projection matrix that handles that:

    float a = viewport_res_x / viewport_res_y;
    fov_rad_ = Im3d::Radians(fov_deg_);

    const float n = 0.1f;
    const float f = 500.0f;

    float scale = std::tanf(fov_rad_ * 0.5f) * n;   

    float r = scale; //no aspect correction here
    float l = -scale; //no aspect correction here

    float t = scale;
    float b = -scale;

    //-----
    if (a > 1.0f)
    {
        b /= a;
        t /= a;
    }
    else
    {
        l *= a;
        r *= a;
    }
    //-----

    const float viewZ = -1.0f;

    proj_ = Im3d::Mat4
    (
        2.0f * n / (r - l),
        0.0f,
        -viewZ * (r + l) / (r - l), //unnecessary for infinite cam (lim -> 0.0f)
        0.0f,

        0.0f,
        2.0f * n / (t - b),
        -viewZ * (t + b) / (t - b), //unnecessary for infinite cam (lim -> 0.0f)
        0.0f,

        0.0f,
        0.0f,
        viewZ,
        -n,

        0.0f,
        0.0f,
        viewZ,
        0.0f
    ); 
john-chapman commented 2 years ago

Hi! I don't think I quite understand the issue, can you provide a screenshot of the incorrect behavior?

Internally, Im3d doesn't use a projection matrix at all. The m_projScaleY and m_viewportSize members on AppData are used to do world -> pixel scale conversions, and as far as I can see this is working correctly. Are you sending those values to Im3d correctly every frame?

ChemistAion commented 2 years ago

Just squeeze Im3d Example window that its width is (let's say) 3-4x longer than the height. image image

For tall window (height is much longer than width) cam projection matrix is OK.

john-chapman commented 2 years ago

Actually this behavior is correct; it's preserving the vertical FOV of the camera as the viewport aspect ratio changes by modifying the horizontal FOV. This is an application-level choice, you could very well do the opposite and preserve horizontal FOV instead. The Im3d example app parameterizes the camera projection using the vertical FOV so it makes sense to keep that fixed when accounting for aspect ratio.

None of this should affect Im3d itself - as mentioned previously there is no projection matrix internally; as long as m_projScaleY and m_viewportSize are set correctly everything should work as expected.

ChemistAion commented 2 years ago

Indeed, this not related to Im3d directly - I am closing this. What I was trying to point out here is that example could be easily improved by automatically detection which of h/v FOV should be preserved - based on viewport aspect ratio. In my case, I have ImGui windows (kind of 3D views with Im3d stuff) and you could easily spot this behavior (only vFOV) even for slightly no squared window.