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

[Stéphane Marchesin]

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.

> 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