[vlc-devel] [PATCH 5/5] opengl: limit the fov and zoom to sane values

[Steve Lhomme]

On Thu, Nov 17, 2016 at 5:32 PM, Adrien Maglo <magsoft at videolan.org> wrote:
> Le 17/11/2016 à 14:06, Steve Lhomme a écrit :
>>
>> the zoom translation needs to remain within the sphere
>> the field of view should be within 20° and 161°
>>
>> --
>> applies after the previous OpenGL cleaning patches
>> ---
>>  modules/video_output/opengl.c | 27 +++------------------------
>>  1 file changed, 3 insertions(+), 24 deletions(-)
>>
>> diff --git a/modules/video_output/opengl.c b/modules/video_output/opengl.c
>> index 7c0ccb6..5be8449 100644
>> --- a/modules/video_output/opengl.c
>> +++ b/modules/video_output/opengl.c
>> @@ -31,6 +31,7 @@
>>
>>  #include <assert.h>
>>  #include <math.h>
>> +#include <float.h>
>>
>>  #include <vlc_common.h>
>>  #include <vlc_picture_pool.h>
>> @@ -211,7 +212,6 @@ struct vout_display_opengl_t {
>>      float f_roll;
>>      float f_fov;
>>      float f_zoom;
>> -    float f_zoom_min;
>>  };
>>
>>  static inline int GetAlignedSize(unsigned size)
>> @@ -449,8 +449,6 @@ vout_display_opengl_t
>> *vout_display_opengl_New(video_format_t *fmt,
>>          return NULL;
>>      }
>>
>> -    vgl->f_fov = -1.f; /* In order to init vgl->f_zoom_min */
>> -
>>      const char *extensions = (const char *)glGetString(GL_EXTENSIONS);
>>  #if !USE_OPENGL_ES
>>      const unsigned char *ogl_version = glGetString(GL_VERSION);
>> @@ -792,30 +790,11 @@ int
>> vout_display_opengl_SetViewpoint(vout_display_opengl_t *vgl,
>>                                       const vlc_viewpoint_t *p_vp)
>>  {
>>  #define RAD(d) ((float) ((d) * M_PI / 180.f))
>> -    float f_fov = RAD(p_vp->fov);
>> -    if (f_fov > (float) M_PI -0.001f || f_fov < 0.001f)
>> -        return VLC_EBADVAR;
>> -    if (p_vp->zoom > 1.f || p_vp->zoom < -1.f)
>> -        return VLC_EBADVAR;
>>      vgl->f_teta = RAD(p_vp->yaw) - (float) M_PI_2;
>>      vgl->f_phi  = RAD(p_vp->pitch);
>>      vgl->f_roll = RAD(p_vp->roll);
>> -
>> -    if (fabsf(f_fov - vgl->f_fov) >= 0.001f)
>> -    {
>> -        /* The fov changed, do trigonometry to calculate the minimal zoom
>> value
>> -         * that will allow us to zoom out without seeing the outside of
>> the
>> -         * sphere (black borders). */
>> -        float sar = (float) vgl->fmt.i_visible_width /
>> vgl->fmt.i_visible_height;
>> -        float fovx = 2 * atanf(tanf(f_fov / 2) * sar);
>> -        float tan_fovx_2 = tanf(fovx / 2);
>> -        float tan_fovy_2 = tanf(f_fov / 2 );
>> -        vgl->f_zoom_min = SPHERE_RADIUS / sinf(atanf(sqrtf(
>> -                          tan_fovx_2 * tan_fovx_2 + tan_fovy_2 *
>> tan_fovy_2)));
>> -    }
>> -
>> -    vgl->f_fov  = f_fov;
>> -    vgl->f_zoom = p_vp->zoom * (p_vp->zoom >= 0 ? 0.5 : vgl->f_zoom_min)
>> * SPHERE_RADIUS;
>> +    vgl->f_fov  = RAD( VLC_CLIP(p_vp->fov, 20.f, 161.f) );
>> +    vgl->f_zoom = VLC_CLIP(p_vp->zoom, -1.f+FLT_EPSILON, 0.98f) *
>> SPHERE_RADIUS;
>>
>>      return VLC_SUCCESS;
>>  #undef RAD
>
>
> Why removing the trigonometry calculation? It was giving the exact minimal
> value for the zoom for any SAR and FOVy value.

First because I don't think it's right to modify the zoom value set by
the caller. If he has done his own calculation why do we change it to
some obscure formula ?

Second because I don't see how the zoom can be related to the aspect
ratio. This is just a translation from the center to the border of the
sphere. Nothing will change if the source aspect ratio changes.

Third because the formula was apparently never going close to the
border of the sphere, and thus never having a proper "little planet"
aspect from there.

> Best regards,
>
>
> --
> MagSoft
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel