[vlc-devel] [PATCH 14/20] opengl: flip the alignment once on Open
Steve Lhomme
robux4 at ycbcr.xyz
Tue Aug 25 11:16:13 CEST 2020
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.
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.
> 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
>
More information about the vlc-devel
mailing list