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

[Steve Lhomme]

On Thu, Nov 17, 2016 at 6:17 PM, Thomas Guillem <thomas at gllm.fr> wrote:
>
>
> 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).

I don't see how this is possible. The zoom matrix is a translation on
the Z axis as described here:
https://www.opengl.org/sdk/docs/man2/xhtml/glTranslate.xml

Outside of [-1,1[ you're outside of the radius and thus outside of the
sphere. And that's exactly what I get without the formula. Even too
close to 1.0 it doesn't work anymore. For the negative translation -1
works.

Here is what I get with -1.1 and a fov of 130°.
https://goo.gl/photos/BnF5a4yec3zTYK2S6

I can get to a fov of 161° when I'm in the [-1,1[ boundary.

>>
>> > 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
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel