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

[Thomas Guillem]

On Thu, Nov 17, 2016, at 18:03, Steve Lhomme wrote:
> 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.

>From my tests, with a fix sar, with a fov a 75, zoom_min is around
-1.16. I got black borders if I zoom just 0.1 lower (-1.17). With your
patch, you won't zoom out close to the border of the sphere, since your
minimum is -1 (and you could go to -1.16).

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