[Mesa-dev] [PATCH] egl/dri2: Implement swapInterval fallback in a conformant way (v2)

[Tomasz Figa]

On Fri, Aug 11, 2017 at 2:29 AM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
> On 10 August 2017 at 14:59, Tomasz Figa <tfiga at chromium.org> wrote:
>> dri2_fallback_swap_interval() currently used to stub out swap interval
>> support in Android backend does nothing besides returning EGL_FALSE.
>> This causes at least one known application (Android Snapchat) to fail
>> due to an unexpected error and my loose interpretation of the EGL 1.5
>> specification justifies it. Relevant quote below:
>>
>>     The function
>>
>>         EGLBoolean eglSwapInterval(EGLDisplay dpy, EGLint interval);
>>
>>     specifies the minimum number of video frame periods per buffer swap
>>     for the draw surface of the current context, for the current rendering
>>     API. [...]
>>
>>     The parameter interval specifies the minimum number of video frames
>>     that are displayed before a buffer swap will occur. The interval
>>     specified by the function applies to the draw surface bound to the
>>     context that is current on the calling thread. [...] interval is
>>     silently clamped to minimum and maximum implementation dependent
>>     values before being stored; these values are defined by EGLConfig
>>     attributes EGL_MIN_SWAP_INTERVAL and EGL_MAX_SWAP_INTERVAL
>>     respectively.
>>
>>     The default swap interval is 1.
>>
>> Even though it does not specify the exact behavior if the platform does
>> not support changing the swap interval, the default assumed state is the
>> swap interval of 1, which I interpret as a value that eglSwapInterval()
>> should succeed if called with, even if there is no ability to change the
>> interval (but there is no change requested). Moreover, since the
>> behavior is defined to clamp the requested value to minimum and maximum
>> and at least the default value of 1 must be present in the range, the
>> implementation might be expected to have a valid range, which in case of
>> the feature being unsupported, would correspond to {1} and any request
>> might be expected to be clamped to this value.
>>
>> Fix this by defaulting dri2_dpy's min_swap_interval, max_swap_interval
>> and default_swap_interval to 1 in dri2_setup_screen() and let platforms,
>> which support this functionality set their own values after this
>> function returns. Thanks to patches merged earlier, we can also remove
>> the dri2_fallback_swap_interval() completely, as with a singular range
>> it would not be called anyway.
>>
>> v2: Remove dri2_fallback_swap_interval() completely thanks to higher
>>     layer already clamping the requested interval and not calling the
>>     driver layer if the clamped value is the same as current.
>>
> Not every driver is EGL 1.5, although the 1.4 spec has exact same text.
>
> Reviewed-by: Emil Velikov <emil.velikov at collabora.com>
>
> We could have this in stable, but will need a clamp like v1 since
> Eric's rework did not land there.
> Gents, how do you feel on the topic?

FWIW, Eric's patches applied cleanly onto our chromium branch, which
was based on something not very far away from what 17.2 branched at.
If for some reasons we prefer to avoid further cherry picks, we can go
with my v1 for stable, which doesn't depend on them.

Best regards,
Tomasz