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

2017-09-27 Thread Tomasz Figa 
On Wed, Sep 27, 2017 at 11:06 PM, Emil Velikov  wrote:
> On 25 September 2017 at 08:25, Tomasz Figa  wrote:
>
>> Gentle ping. :)
>>
>
> I forgot that you don't have commit access. Can you please apply for one?

Yep, I have it on my list, sorry for being slow. Thanks for useful
links. (Need to dig out my PGP key.)

Best regards,
Tomasz

>
> Thanks
> Emil
>
> [1] https://www.freedesktop.org/wiki/AccountRequests/
> [2] https://bugs.freedesktop.org/show_bug.cgi?id=99601
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2017-09-27 Thread Emil Velikov 
On 25 September 2017 at 08:25, Tomasz Figa  wrote:

> Gentle ping. :)
>

I forgot that you don't have commit access. Can you please apply for one?

Thanks
Emil

[1] https://www.freedesktop.org/wiki/AccountRequests/
[2] https://bugs.freedesktop.org/show_bug.cgi?id=99601
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2017-09-25 Thread Tomasz Figa 
On Fri, Aug 11, 2017 at 1:31 PM, Tomasz Figa  wrote:
> On Fri, Aug 11, 2017 at 2:29 AM, Emil Velikov  
> wrote:
>> On 10 August 2017 at 14:59, Tomasz Figa  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 
>>
>> 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. :)

Thanks,
Tomasz
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2017-08-10 Thread Tomasz Figa 
On Fri, Aug 11, 2017 at 2:29 AM, Emil Velikov  wrote:
> On 10 August 2017 at 14:59, Tomasz Figa  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 
>
> 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
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2017-08-10 Thread Tapani Pälli 

Reviewed-by: Tapani Pälli 

On 08/10/2017 04:59 PM, Tomasz Figa 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.

Signed-off-by: Tomasz Figa 
---
  src/egl/drivers/dri2/egl_dri2.c | 12 
  src/egl/drivers/dri2/egl_dri2_fallbacks.h   |  7 ---
  src/egl/drivers/dri2/platform_android.c |  1 -
  src/egl/drivers/dri2/platform_drm.c |  1 -
  src/egl/drivers/dri2/platform_surfaceless.c |  1 -
  src/egl/drivers/dri2/platform_x11.c |  2 +-
  6 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
index f0d1ded408..686dd68d29 100644
--- a/src/egl/drivers/dri2/egl_dri2.c
+++ b/src/egl/drivers/dri2/egl_dri2.c
@@ -630,6 +630,18 @@ dri2_setup_screen(_EGLDisplay *disp)
 struct dri2_egl_display *dri2_dpy = dri2_egl_display(disp);
 unsigned int api_mask;
  
+   /*

+* EGL 1.5 specification defines the default value to 1. Moreover,
+* eglSwapInterval() is required to clamp requested value to the supported
+* range. Since the default value is implicitly assumed to be supported,
+* use it as both minimum and maximum for the platforms that do not allow
+* changing the interval. Platforms, which allow it (e.g. x11, wayland)
+* override these values already.
+*/
+   dri2_dpy->min_swap_interval = 1;
+   dri2_dpy->max_swap_interval = 1;
+   dri2_dpy->default_swap_interval = 1;
+
 if (dri2_dpy->image_driver) {
api_mask = dri2_dpy->image_driver->getAPIMask(dri2_dpy->dri_screen);
 } else if (dri2_dpy->dri2) {
diff --git a/src/egl/drivers/dri2/egl_dri2_fallbacks.h 
b/src/egl/drivers/dri2/egl_dri2_fallbacks.h
index 604db881a8..a664677572 100644
--- a/src/egl/drivers/dri2/egl_dri2_fallbacks.h
+++ b/src/egl/drivers/dri2/egl_dri2_fallbacks.h
@@ -55,13 +55,6 @@ dri2_fallback_create_image_khr(_EGLDriver *drv, _EGLDisplay 
*disp,
 return NULL;
  }
  
-static inline EGLBoolean

-dri2_fallback_swap_interval(_EGLDriver *drv, _EGLDisplay *dpy,
-_EGLSurface *surf, EGLint interval)
-{
-   return EGL_FALSE;
-}
-
  static inline EGLBoolean
  dri2_fallback_swap_buffers_with_damage(_EGLDriver *drv, _EGLDisplay *dpy,
_EGLSurface *surf,
diff --git a/src/egl/drivers/dri2/platform_android.c 
b/src/egl/drivers/dri2/platform_android.c
index 50a8248695..c1bae34f2d 100644
--- a/src/egl/drivers/dri2/platform_android.c
+++ b/src/egl/drivers/dri2/platform_android.c
@@ -1118,7 +1118,6 @@ static const struct dri2_egl_display_vtbl 
droid_display_vtbl = {

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

2017-08-10 Thread Chad Versace 
On Thu 10 Aug 2017, Tomasz Figa 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:

...

> Signed-off-by: Tomasz Figa 
> ---
>  src/egl/drivers/dri2/egl_dri2.c | 12 
>  src/egl/drivers/dri2/egl_dri2_fallbacks.h   |  7 ---
>  src/egl/drivers/dri2/platform_android.c |  1 -
>  src/egl/drivers/dri2/platform_drm.c |  1 -
>  src/egl/drivers/dri2/platform_surfaceless.c |  1 -
>  src/egl/drivers/dri2/platform_x11.c |  2 +-
>  6 files changed, 13 insertions(+), 11 deletions(-)

Reviewed-by: Chad Versace 

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2017-08-10 Thread Emil Velikov 
On 10 August 2017 at 14:59, Tomasz Figa  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 

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?

-Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2017-08-10 Thread Tomasz Figa 
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.

Signed-off-by: Tomasz Figa 
---
 src/egl/drivers/dri2/egl_dri2.c | 12 
 src/egl/drivers/dri2/egl_dri2_fallbacks.h   |  7 ---
 src/egl/drivers/dri2/platform_android.c |  1 -
 src/egl/drivers/dri2/platform_drm.c |  1 -
 src/egl/drivers/dri2/platform_surfaceless.c |  1 -
 src/egl/drivers/dri2/platform_x11.c |  2 +-
 6 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
index f0d1ded408..686dd68d29 100644
--- a/src/egl/drivers/dri2/egl_dri2.c
+++ b/src/egl/drivers/dri2/egl_dri2.c
@@ -630,6 +630,18 @@ dri2_setup_screen(_EGLDisplay *disp)
struct dri2_egl_display *dri2_dpy = dri2_egl_display(disp);
unsigned int api_mask;
 
+   /*
+* EGL 1.5 specification defines the default value to 1. Moreover,
+* eglSwapInterval() is required to clamp requested value to the supported
+* range. Since the default value is implicitly assumed to be supported,
+* use it as both minimum and maximum for the platforms that do not allow
+* changing the interval. Platforms, which allow it (e.g. x11, wayland)
+* override these values already.
+*/
+   dri2_dpy->min_swap_interval = 1;
+   dri2_dpy->max_swap_interval = 1;
+   dri2_dpy->default_swap_interval = 1;
+
if (dri2_dpy->image_driver) {
   api_mask = dri2_dpy->image_driver->getAPIMask(dri2_dpy->dri_screen);
} else if (dri2_dpy->dri2) {
diff --git a/src/egl/drivers/dri2/egl_dri2_fallbacks.h 
b/src/egl/drivers/dri2/egl_dri2_fallbacks.h
index 604db881a8..a664677572 100644
--- a/src/egl/drivers/dri2/egl_dri2_fallbacks.h
+++ b/src/egl/drivers/dri2/egl_dri2_fallbacks.h
@@ -55,13 +55,6 @@ dri2_fallback_create_image_khr(_EGLDriver *drv, _EGLDisplay 
*disp,
return NULL;
 }
 
-static inline EGLBoolean
-dri2_fallback_swap_interval(_EGLDriver *drv, _EGLDisplay *dpy,
-_EGLSurface *surf, EGLint interval)
-{
-   return EGL_FALSE;
-}
-
 static inline EGLBoolean
 dri2_fallback_swap_buffers_with_damage(_EGLDriver *drv, _EGLDisplay *dpy,
   _EGLSurface *surf,
diff --git a/src/egl/drivers/dri2/platform_android.c 
b/src/egl/drivers/dri2/platform_android.c
index 50a8248695..c1bae34f2d 100644
--- a/src/egl/drivers/dri2/platform_android.c
+++ b/src/egl/drivers/dri2/platform_android.c
@@ -1118,7 +1118,6 @@ static const struct dri2_egl_display_vtbl 
droid_display_vtbl = {
.create_pbuffer_surface = droid_create_pbuffer_surface,
.destroy_surface = droid_destroy_surface,
.create_image =