[Mesa-dev] EGL/android: pbuffer implementation.

[Tomasz Figa]

On Mon, Nov 21, 2016 at 11:15 PM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
> On 21 November 2016 at 07:23, Tomasz Figa <tfiga at chromium.org> wrote:
>> Hi,
>>
>> On Wed, Nov 16, 2016 at 11:11 AM, Liu Zhiquan <zhiquan.liu at intel.com> wrote:
>>> mesa android path didn't support pbuffer, so add pbuffer support to
>>> fix most deqp and cts pbuffer test cases fail;
>>> add single buffer config to support pbuffer, and create image for
>>> pbuffer when pbuffer type is front surface.
>>> The EGL 1.5 spec states that pbuffers have a back buffer but no front
>>> buffer, single-buffered surfaces with no front buffer confuse Mesa;
>>> so we deviate from the spec, following the precedent of Mesa's
>>> EGL X11 platform.
>>>
>>> Test status: android CTS EGL pbuffer test can run without native crash.
>>> test:[DEQP,EGL]all deqp pbuffer case passed.
>>>
>>> V3: update commit message and code review changes.
>>>
>>> Signed-off-by: Liu Zhiquan <zhiquan.liu at intel.com>
>>> Signed-off-by: Kalyan Kondapally <kalyan.kondapally at intel.com>
>>> ---
>>>  src/egl/drivers/dri2/egl_dri2.h         |  3 +-
>>>  src/egl/drivers/dri2/platform_android.c | 98 +++++++++++++++++++++++++--------
>>>  2 files changed, 78 insertions(+), 23 deletions(-)
>>>
>>
>> Looks like this patch has already landed, but please let me try to
>> confirm some things here anyway. Would you mind keeping me on CC for
>> any future patches for the EGL/Android module? (
>
>
>> I believe
>> scripts/get_reviewer.pl should already include me on the list of
>> suggested reviewers, similarly for Rob Herring, who did a great job
>> before helping us with testing on platforms other than i915.)
>>
> I'll add you and update the documentation to reference the script.
>
> Rob H let me know if you'd like to be in there as well which files
> (platform_egl.c, Android build and/or other) and which email you'd
> prefer.

Thanks!

>>>
>>>     if (update_buffers(dri2_surf) < 0)
>>>        return 0;
>>>
>>>     if (buffer_mask & __DRI_IMAGE_BUFFER_FRONT) {
>>> -      /*
>>> -       * We don't support front buffers and GLES doesn't require them for
>>> -       * window surfaces, but some DRI drivers will request them anyway.
>>> -       * We just ignore such request as other platforms backends do.
>>> +      if (dri2_surf->base.Type == EGL_WINDOW_BIT) {
>>> +         /* According current EGL spec,
>>> +          * front buffer rendering for window surface is not supported now */
>>> +         _eglLog(_EGL_WARNING,
>>> +               "%s:%d front buffer rendering for window surface is not supported!\n",
>>> +               __func__, __LINE__);
>>> +         return 0;
>>
>> This is a semantic change and according to the old comment it might
>> break some drivers ("We don't support front buffers and GLES doesn't
>> require them for window surfaces, but some DRI drivers will request
>> them anyway.").
>>
> Fwiw -

Hmm, not sure I'm getting your point here correctly (it's quite late
so my parser might be half asleep already), but comment below is about
pbuffers only. The comment above apparently says that EGL doesn't
support front buffer rendering, but my comment here is more about the
DRI extension. The semantics of the image loader extension seemed to
allow this kind of "aggresive" probing, when the driver sets bits for
any kinds of buffers it's interested in and it's not fatal if some of
them are not available. However this was inferred from looking at
driver code, as I couldn't really find any precise spec.

Best regards,
Tomasz