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

[Marvin Scholz]

On 12 Aug 2019, at 9:57, Thomas Guillem wrote:

> 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.
>

I just tried it again and it turns out this patch unfortunately does not 
fix all issues.
A debug logging output apparently changed timings in a way that it would 
luckily work…
So disregard this patch for now.

>>
>>>>  }
>>>>
>>>> @@ -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
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel