[vlc-devel] [PATCH] opengl: fix vertical alignment on control events

[Alexandre Janniaux]

Hi,

Perfect, can you send it to me?

Regards,
--
Alexandre Janniaux
Videolabs

On Thu, Jun 18, 2020 at 10:59:59AM +0200, Romain Vimont wrote:
> On Thu, Jun 18, 2020 at 10:18:45AM +0200, Alexandre Janniaux wrote:
> > Hi,
> >
> > LGTM, but the commit message is not 100% correct.
> > The issue here is not that the textures are potentially
> > vertically flipped, but that default opengl convention
> > orients the Y axis upwards while VLC convention orients
> > it downwards.
>
> Yes, but as a result, the texture content is vertically flipped (because
> it is initialized from a picture stored in the VLC convention).
>
> But ok, if you think it's more clear, I changed the text to:
>
> > In OpenGL, the vertical alignment is reversed, because the default
> > OpenGL convention orients the Y axis upwards while VLC convention
> > orients it downwards.
>
> >
> > Regards,
> > --
> > Alexandre Janniaux
> > Videolabs
> >
> > On Wed, Jun 17, 2020 at 09:31:04PM +0200, Romain Vimont wrote:
> > > The video can be top-aligned (--align=4) or bottom-aligned (--align=8)
> > > in the window.
> > >
> > > In OpenGL, the vertical alignment is reversed, because the textures are
> > > vertically flipped.
> > >
> > > It was correctly handled for some control events (e.g.
> > > CHANGE_DISPLAY_SIZE), but not for others (e.g. CHANGE_SOURCE_CROP).
> > > ---
> > >  modules/video_output/opengl/display.c | 28 +++++++++++++++++----------
> > >  1 file changed, 18 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/modules/video_output/opengl/display.c b/modules/video_output/opengl/display.c
> > > index ebb313926a80..f4e46f6f33d2 100644
> > > --- a/modules/video_output/opengl/display.c
> > > +++ b/modules/video_output/opengl/display.c
> > > @@ -206,6 +206,16 @@ static void PictureDisplay (vout_display_t *vd, picture_t *pic)
> > >      }
> > >  }
> > >
> > > +static void
> > > +FlipVerticalAlign(vout_display_cfg_t *cfg)
> > > +{
> > > +    /* Reverse vertical alignment as the GL tex are Y inverted */
> > > +    if (cfg->align.vertical == VLC_VIDEO_ALIGN_TOP)
> > > +        cfg->align.vertical = VLC_VIDEO_ALIGN_BOTTOM;
> > > +    else if (cfg->align.vertical == VLC_VIDEO_ALIGN_BOTTOM)
> > > +        cfg->align.vertical = VLC_VIDEO_ALIGN_TOP;
> > > +}
> > > +
> > >  static int Control (vout_display_t *vd, int query, va_list ap)
> > >  {
> > >      vout_display_sys_t *sys = vd->sys;
> > > @@ -221,27 +231,25 @@ static int Control (vout_display_t *vd, int query, va_list ap)
> > >        case VOUT_DISPLAY_CHANGE_DISPLAY_FILLED:
> > >        case VOUT_DISPLAY_CHANGE_ZOOM:
> > >        {
> > > -        vout_display_cfg_t c = *va_arg (ap, const vout_display_cfg_t *);
> > > +        vout_display_cfg_t cfg = *va_arg(ap, const vout_display_cfg_t *);
> > >          const video_format_t *src = &vd->source;
> > >
> > > -        /* Reverse vertical alignment as the GL tex are Y inverted */
> > > -        if (c.align.vertical == VLC_VIDEO_ALIGN_TOP)
> > > -            c.align.vertical = VLC_VIDEO_ALIGN_BOTTOM;
> > > -        else if (c.align.vertical == VLC_VIDEO_ALIGN_BOTTOM)
> > > -            c.align.vertical = VLC_VIDEO_ALIGN_TOP;
> > > +        FlipVerticalAlign(&cfg);
> > >
> > > -        vout_display_PlacePicture(&sys->place, src, &c);
> > > +        vout_display_PlacePicture(&sys->place, src, &cfg);
> > >          sys->place_changed = true;
> > > -        vlc_gl_Resize (sys->gl, c.display.width, c.display.height);
> > > +        vlc_gl_Resize (sys->gl, cfg.display.width, cfg.display.height);
> > >          return VLC_SUCCESS;
> > >        }
> > >
> > >        case VOUT_DISPLAY_CHANGE_SOURCE_ASPECT:
> > >        case VOUT_DISPLAY_CHANGE_SOURCE_CROP:
> > >        {
> > > -        const vout_display_cfg_t *cfg = va_arg (ap, const vout_display_cfg_t *);
> > > +        vout_display_cfg_t cfg = *va_arg(ap, const vout_display_cfg_t *);
> > > +
> > > +        FlipVerticalAlign(&cfg);
> > >
> > > -        vout_display_PlacePicture(&sys->place, &vd->source, cfg);
> > > +        vout_display_PlacePicture(&sys->place, &vd->source, &cfg);
> > >          sys->place_changed = true;
> > >          return VLC_SUCCESS;
> > >        }
> > > --
> > > 2.27.0
> > >
> > > _______________________________________________
> > > 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