On 19 February 2016 at 05:34, Tapani Pälli <tapani.pa...@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.pa...@intel.com> wrote: >>> >>> From: Daniel Czarnowski <daniel.czarnow...@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 _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
