[Mesa-dev] [PATCH 7/8] glx: fix the refcounting of dri drawables

[Miklós Máté]

On 04/07/2016 07:57 PM, Stéphane Marchesin wrote:
> On Thu, Apr 7, 2016 at 10:44 AM, Miklós Máté <mtmkls at gmail.com> wrote:
>> On 04/07/2016 12:40 PM, Stéphane Marchesin wrote:
>>> On Thu, Apr 7, 2016 at 3:37 AM, Miklós Máté <mtmkls at gmail.com> wrote:
>>>> On 04/07/2016 12:25 PM, Stéphane Marchesin wrote:
>>>>
>>>>
>>>> On Apr 7, 2016 02:27, "Michel Dänzer" <michel at daenzer.net> wrote:
>>>>> On 07.04.2016 18:01, Marek Olšák wrote:
>>>>>> On Thu, Apr 7, 2016 at 7:32 AM, Ian Romanick <idr at freedesktop.org>
>>>>>> wrote:
>>>>>>> Why would you do that?  I've NAKed this patch several times.
>>>>>> You NAKed the previous version of the patch, not this one. I guess
>>>>>> that means NAK for this one too.
>>>>> I'd be interested in a specific justification for NAKing this version.
>>>>> Seems to me like this might be the best that can be done.
>>>> We asked ourselves the same question.
>>>>
>>>> It is the best that can be done to fix glx 1.2 for sure. But it will
>>>> break
>>>> glx 1.3, so for example it will break chrome (I have a vested interest
>>>> here
>>>> :).
>>>>
>>>> So what we figured out is, if we have to choose between the old and new
>>>> api,
>>>> why break the newer (and arguably better) one? Instead we should consider
>>>> the old one deprecated/legacy and move on.
>>>>
>>>> Stéphane
>>>>
>>>> How does this break glX 1.3, and how does this break Chrome?
>>> I don't have the source here, so it's off the top of my head. But
>>> isn't this going to start leaking dri drawables if you constantly
>>> makecurrent?
>> No, it doesn't leak anything in makecurrent.
>>
>>>    By itself this is ok (because they all point to the same
>>> buffer) but there are pieces of glx 1.2 (like swapbuffers) that
>>> iterate over all the dri drawables and then they become really slow.
>>>
>>> Stéphane
>>>
>> The problem with this patch, as I was informed, is that people don't call
>> glXDestroyPixmap (or glXDestroyGLXPixmap) before calling XFreePixmap, and
> Yes, because those don't exist in glx 1.2.
glXDestroyGLXPixmap is in glx 1.0.
>
>> thus the resources allocated by glx are leaked (until the process exits). In
>> current Mesa, the dri drawable associated with the glx object only lives
>> while the glx object is current to a context, which prevents the above
>> problem in a crude way.
> Yes, and you end up accumulating lots of dri drawables which are all
> iterated at swapbuffers time. That makes swapbuffers really slow
> eventually.
>
> Stéphane
Only one dri drawable is allocated for a glx object. I don't see how 
they could accumulate.

MM