On Wed, 2017-09-27 at 09:09 +0900, Tomasz Figa wrote:
> On Wed, Sep 27, 2017 at 2:32 AM, Juan A. Suarez Romero
> <[email protected]> wrote:
> > On Mon, 2017-09-25 at 16:25 +0900, Tomasz Figa wrote:
> > > On Fri, Aug 11, 2017 at 1:31 PM, Tomasz Figa <[email protected]> wrote:
> > > > On Fri, Aug 11, 2017 at 2:29 AM, Emil Velikov
> > > > <[email protected]> wrote:
> > > > > On 10 August 2017 at 14:59, Tomasz Figa <[email protected]> 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 <[email protected]>
> > > > >
> > > > > 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.
> > >
> > > Gentle ping. :)
> > >
> >
> > Tomasz, didn't Chad and Tapani granted a R-b?
>
> That's right. I'd appreciate if someone committed it to the tree,
> though. (Sorry, I haven't managed to get through the commit access
> application yet.)
>
I'll push it. Thanks!
J.A.
> Best regards,
> Tomasz
> _______________________________________________
> mesa-stable mailing list
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/mesa-stable
_______________________________________________
mesa-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/mesa-dev