[vlc-devel] [PATCH 14/20] opengl: flip the alignment once on Open
Alexandre Janniaux
ajanni at videolabs.io
Tue Aug 25 11:36:13 CEST 2020
Hi,
On Tue, Aug 25, 2020 at 11:16:13AM +0200, Steve Lhomme wrote:
> On 2020-08-25 10:39, Alexandre Janniaux wrote:
> > Hi,
> >
> > This seems wrong. Alignment is reversed in OpenGL because
> > coordinate system is reversed too, so ppicture_place data
> > need to be reversed too.
> >
> > We don't want to report anything to the core and we want
> > the correct behaviour when doing control. This breaks both
> > points.
> >
> > I'm not sure why you remove the FlipVerticalAlign too.
>
> I forgot to add more comments to the commit.
>
> Right now the align.vertical is set when enabling the window. After that
> it's never changed after that (in the core). It's only used inside
> vout_display_PlacePicture() and nowhere else.
In the future, it could also set by the interface. I don't
really know why it's not bindable to hotkeys but it makes
as much sense as other keys like `o` imho.
>
> Moving this hack to where the actual vout_display_PlacePicture() calls are
> done (in the core after this patchset) seem logical.
> As explained in the rational for this patchset, the goal is to have the
> mouse translation using the proper video placement. It's possible that
> OpenGL is doing things backward.
It's not that OpenGL is doing it backward, it's that OpenGL
is doing it with a different coordinate system, thus you
have to adapt what the core request into how OpenGL
understand it, meaning reverse the y axis.
To be clear, VLC is using usual window coordinates,
while OpenGL has origin in bottom left corner
VLC OpenGL
+----> A
| |
| |
V +------>
Thus, if you don't inverse alignment before placing, the core
will place the image at the top when it should have been the
bottom or the opposite.
Regards,
--
Alexandre Janniaux
Videolabs
>
> > Regards,
> > --
> > Alexandre Janniaux
> > Videolabs
> >
> > On Tue, Aug 25, 2020 at 09:30:02AM +0200, Steve Lhomme wrote:
> > > ---
> > > modules/video_output/caopengllayer.m | 12 +++++------
> > > modules/video_output/macosx.m | 18 +++++++----------
> > > modules/video_output/opengl/display.c | 29 ++++++++-------------------
> > > 3 files changed, 21 insertions(+), 38 deletions(-)
> > >
> > > diff --git a/modules/video_output/caopengllayer.m b/modules/video_output/caopengllayer.m
> > > index e81f8210117..fbdbc8b59dc 100644
> > > --- a/modules/video_output/caopengllayer.m
> > > +++ b/modules/video_output/caopengllayer.m
> > > @@ -213,6 +213,12 @@ static int Open (vout_display_t *vd, vout_display_cfg_t *cfg,
> > > outputSize = [sys->container visibleRect].size;
> > > vout_window_ReportSize(sys->embed, (int)outputSize.width, (int)outputSize.height);
> > >
> > > + /* 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;
> > > +
> > > return VLC_SUCCESS;
> > >
> > > bailout:
> > > @@ -317,12 +323,6 @@ static int Control (vout_display_t *vd, int query, va_list ap)
> > > cfg_tmp.display.width = bounds.size.width;
> > > cfg_tmp.display.height = bounds.size.height;
> > >
> > > - /* Reverse vertical alignment as the GL tex are Y inverted */
> > > - if (cfg_tmp.align.vertical == VLC_VIDEO_ALIGN_TOP)
> > > - cfg_tmp.align.vertical = VLC_VIDEO_ALIGN_BOTTOM;
> > > - else if (cfg_tmp.align.vertical == VLC_VIDEO_ALIGN_BOTTOM)
> > > - cfg_tmp.align.vertical = VLC_VIDEO_ALIGN_TOP;
> > > -
> > > vout_display_place_t place;
> > > vout_display_PlacePicture(&place, &vd->source, &cfg_tmp);
> > > if (unlikely(OpenglLock(sys->gl)))
> > > diff --git a/modules/video_output/macosx.m b/modules/video_output/macosx.m
> > > index 80247f1f29a..ffa6b83ee0f 100644
> > > --- a/modules/video_output/macosx.m
> > > +++ b/modules/video_output/macosx.m
> > > @@ -241,6 +241,12 @@ static int Open (vout_display_t *vd, vout_display_cfg_t *cfg,
> > > vd->control = Control;
> > > vd->close = Close;
> > >
> > > + /* 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;
> > > +
> > > /* */
> > > // FIXME: this call leads to a fatal mutex locking error in vout_ChangeDisplaySize()
> > > // vout_window_ReportSize(sys->embed, fmt->i_visible_width, fmt->i_visible_height);
> > > @@ -350,18 +356,8 @@ static int Control (vout_display_t *vd, int query, va_list ap)
> > > case VOUT_DISPLAY_CHANGE_SOURCE_CROP:
> > > case VOUT_DISPLAY_CHANGE_DISPLAY_SIZE:
> > > {
> > > - /* we always use our current frame here, because we have some size constraints
> > > - in the ui vout provider */
> > > - vout_display_cfg_t cfg_tmp = *vd->cfg;
> > > -
> > > - /* Reverse vertical alignment as the GL tex are Y inverted */
> > > - if (cfg_tmp.align.vertical == VLC_VIDEO_ALIGN_TOP)
> > > - cfg_tmp.align.vertical = VLC_VIDEO_ALIGN_BOTTOM;
> > > - else if (cfg_tmp.align.vertical == VLC_VIDEO_ALIGN_BOTTOM)
> > > - cfg_tmp.align.vertical = VLC_VIDEO_ALIGN_TOP;
> > > -
> > > vout_display_place_t place;
> > > - vout_display_PlacePicture(&place, &vd->source, &cfg_tmp);
> > > + vout_display_PlacePicture(&place, &vd->source, vd->cfg);
> > > @synchronized (sys->glView) {
> > > sys->cfg = *vd->cfg;
> > > }
> > > diff --git a/modules/video_output/opengl/display.c b/modules/video_output/opengl/display.c
> > > index 2ef8df9910b..a5738310531 100644
> > > --- a/modules/video_output/opengl/display.c
> > > +++ b/modules/video_output/opengl/display.c
> > > @@ -141,6 +141,12 @@ static int Open(vout_display_t *vd, vout_display_cfg_t *cfg,
> > > if (sys->vgl == NULL)
> > > goto error;
> > >
> > > + /* 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;
> > > +
> > > vd->sys = sys;
> > > vd->info.subpicture_chromas = spu_chromas;
> > > vd->prepare = PictureRender;
> > > @@ -206,16 +212,6 @@ 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;
> > > @@ -231,12 +227,7 @@ 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 cfg = *vd->cfg;
> > > - const video_format_t *src = &vd->source;
> > > -
> > > - FlipVerticalAlign(&cfg);
> > > -
> > > - vout_display_PlacePicture(&sys->place, src, &cfg);
> > > + vout_display_PlacePicture(&sys->place, &vd->source, vd->cfg);
> > > sys->place_changed = true;
> > > vlc_gl_Resize (sys->gl, vd->cfg->display.width, vd->cfg->display.height);
> > > return VLC_SUCCESS;
> > > @@ -245,11 +236,7 @@ static int Control (vout_display_t *vd, int query, va_list ap)
> > > case VOUT_DISPLAY_CHANGE_SOURCE_ASPECT:
> > > case VOUT_DISPLAY_CHANGE_SOURCE_CROP:
> > > {
> > > - vout_display_cfg_t cfg = *vd->cfg;
> > > -
> > > - FlipVerticalAlign(&cfg);
> > > -
> > > - vout_display_PlacePicture(&sys->place, &vd->source, &cfg);
> > > + vout_display_PlacePicture(&sys->place, &vd->source, vd->cfg);
> > > sys->place_changed = true;
> > > return VLC_SUCCESS;
> > > }
> > > --
> > > 2.26.2
> > >
> > > _______________________________________________
> > > 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