[vlc-devel] [PATCH 5/5] opengl: limit the fov and zoom to sane values
Thomas Guillem
thomas at gllm.fr
Fri Nov 18 09:08:20 CET 2016
On Thu, Nov 17, 2016, at 22:24, Steve Lhomme wrote:
> 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
Yes, that is normal, the zoom_min calculation with a fov a 130 doesn't
give -1.1, more something around -1.0x (don't want to redo the math).
>
> 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
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
More information about the vlc-devel
mailing list