[vlc-devel] [PATCH] vout/opengl: Store and restore last viewport

[Thomas Guillem]

On Mon, Aug 12, 2019, at 09:42, Marvin Scholz wrote:
> 
> 
> On 12 Aug 2019, at 9:37, Thomas Guillem wrote:
> 
> > On Sat, Aug 10, 2019, at 19:56, Marvin Scholz wrote:
> >> Between the call to vout_display_opengl_Viewport and the call to
> >> vout_display_opengl_Display the viewport can be changed by the OS
> >> causing the picture to render incorrectly.
> >>
> >> To prevent that, store the last set viewport and restore it in the
> >> display function.
> >>
> >> Fix #21357
> >> Fix #22209
> >> ---
> >>  modules/video_output/opengl/vout_helper.c | 17 +++++++++++++++++
> >>  1 file changed, 17 insertions(+)
> >>
> >> diff --git a/modules/video_output/opengl/vout_helper.c
> >> b/modules/video_output/opengl/vout_helper.c
> >> index b37b653257..2a8ec9cc12 100644
> >> --- a/modules/video_output/opengl/vout_helper.c
> >> +++ b/modules/video_output/opengl/vout_helper.c
> >> @@ -166,6 +166,13 @@ struct vout_display_opengl_t {
> >>      float f_fovy; /* to avoid recalculating them when needed.      
> >> */
> >>      float f_z;    /* Position of the camera on the shpere radius
> >> vector */
> >>      float f_sar;
> >> +
> >> +    struct {
> >> +        GLint i_x;
> >> +        GLint i_y;
> >> +        GLint i_width;
> >> +        GLint i_height;
> >> +    } last_viewport;
> >>  };
> >>
> >>  static const vlc_fourcc_t gl_subpicture_chromas[] = {
> >> @@ -1021,6 +1028,10 @@ void
> >> vout_display_opengl_SetWindowAspectRatio(vout_display_opengl_t *vgl,
> >>  void vout_display_opengl_Viewport(vout_display_opengl_t *vgl, int x,
> >> int y,
> >>                                    unsigned width, unsigned height)
> >>  {
> >> +    vgl->last_viewport.i_x = x;
> >> +    vgl->last_viewport.i_y = y;
> >> +    vgl->last_viewport.i_width = width;
> >> +    vgl->last_viewport.i_height = height;
> >>      vgl->vt.Viewport(x, y, width, height);
> >
> > So, why not removing this vgl->vt.Viewport(x, y, width, height) call ?
> >
> 
> The reason for this was that I wasn't sure if something would call
> vout_display_opengl_Viewport with the intention to set the viewport
> immediately and not call vout_display_opengl_Display later.
> After checking the uses of vout_display_opengl_Viewport again it seems
> thats nowhere the case right now though so I could indeed remove that.

Maybe it is possible to call Display() with viewport arguments. That is one less call/lock.

> 
> >>  }
> >>
> >> @@ -1564,6 +1575,12 @@ int
> >> vout_display_opengl_Display(vout_display_opengl_t *vgl,
> >>  {
> >>      GL_ASSERT_NOERROR();
> >>
> >> +    // Restore Viewport
> >> +    vgl->vt.Viewport(vgl->last_viewport.i_x,
> >> +                     vgl->last_viewport.i_y,
> >> +                     vgl->last_viewport.i_width,
> >> +                     vgl->last_viewport.i_height);
> >
> >
> > And set the viewport only here.
> >
> >> +
> >>      /* Why drawing here and not in Render()? Because this way, the
> >>         OpenGL providers can call vout_display_opengl_Display to 
> >> force redraw.
> >>         Currently, the OS X provider uses it to get a smooth window 
> >> resizing */
> >> -- 
> >> 2.19.1
> >>
> >> _______________________________________________
> >> 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