[Mesa-dev] [Mesa-stable] [PATCH] egl: support EGL_LARGEST_PBUFFER in eglCreatePbufferSurface(...)

[Emil Velikov]

On 19 February 2016 at 05:34, Tapani Pälli <tapani.palli at intel.com> wrote:
>
> On 02/18/2016 08:19 PM, Emil Velikov wrote:
>>
>> Hi guys,
>>
>> On 15 February 2016 at 10:54, Tapani Pälli <tapani.palli at intel.com> wrote:
>>>
>>> From: Daniel Czarnowski <daniel.czarnowski at intel.com>
>>>
>>> Patch provides a 'sane default' for a set pbuffer surface size when
>>
>> Perhaps too cheeky of a question but "who is to determine these 'sane
>> defaults'" ?
>
>
> I agree this is not ideal but it's a lot better than complete failure? The
> default set in the patch is quite conservative. We could have a driver hook
> but I'm not convinced it's worth it in this particular case, again this is
> really a case for robustness testing, not for a real life application.
>
>> If those are OK for one hardware/generation they won't be for another.
>> I believe the whole problem boils down to the fact that we don't do
>> anything in case of EGL_LARGEST_PBUFFER. Looking at GLX -
>> GLX_LARGEST_PBUFFER seems to be in the same boat :-\
>>
>> Imho one should honour it while creating the surface. Afaics the spec
>> isn't clear if height/width should be ignored when the bool is set (or
>> I missed that hunk), so I'm leaning towards "yes".
>
>
> The spec is "if we can't allocate the size what is requested we allocate max
> possible size":
>
> "Use EGL_LARGEST_PBUFFER to get the largest available pbuffer when the
> allocation of the pbuffer would otherwise fail."
>
> So we would try to allocate what is requested but we have a limit that says
> it will fail so let's not. IIRC X driver has 32k for max pixmap size but as
> they are textures nowadays in practice this limit will be much smaller.
>
>>> EGL_LARGEST_PBUFFER is used by the client. MIN2 macro is moved to
>>> egldefines so that it can be shared.
>>>
>>> Fixes following Piglit test:
>>>     egl-create-largest-pbuffer-surface
>>>
>> With the above said, I'm not sure how much sense it makes to have
>> explicit EGL_HEIGHT/WIDTH in the test, alongside EGL_LARGEST_PBUFFER
>> in the mentioned test.
>>
>> In all fairness, fixing this properly might result in a serious
>> (series of) patch(es). Which might be a bit too much for stable ?
>> Better to cross that road as we reach it :-)
>
>
> I did some investigation and it seems currently there is no way to query
> maximum size of a pixmap using xlib or xcb, only way would be to try this in
> a loop. Xcb uses uint16_t type so you could determine it from that but with
> most (all?) drivers it would fail with max.
>
Yay fun times ! I wonder if the binary drivers handle this in the same
way ... or perhaps they also crash ?

Regardless, let's go ahead with this patch, but please add some of the
above discussion/justification in the commit message and code. Adding
workarounds without comprehensive description will come to bite us
sooner or later.

Thanks
Emil