[PATCH] winex11: Don't create a temporary context each time X11DRV_wglBindTexImageARB is called.
Matteo Bruni
matteo.mystral at gmail.com
Tue Mar 1 07:19:22 CST 2016
2016-02-29 23:20 GMT+01:00 Miklós Máté <mtmkls at gmail.com>:
> On 02/29/2016 06:10 PM, Matteo Bruni wrote:
>>
>> 2016-02-29 3:59 GMT+01:00 Ken Thomases <ken at codeweavers.com>:
>>>
>>> On Feb 26, 2016, at 5:29 PM, Miklós Máté <mtmkls at gmail.com> wrote:
>>>
>>>> @@ -2320,6 +2322,7 @@ static BOOL X11DRV_wglDestroyPbufferARB( struct
>>>> wgl_pbuffer *object )
>>>> TRACE("(%p)\n", object);
>>>>
>>>> pglXDestroyPbuffer(gdi_display, object->drawable);
>>>> + pglXDestroyContext(gdi_display, object->tmp_context);
>>>
>>> I think you need to check if object->tmp_context is non-zero before
>>> calling glXDestroyContext() to avoid a GLXBadContext error.
>
> Mesa handles ctx=0 by doing nothing. I guess the blob drivers do the same.
Probably, but AFAIU the spec is vague enough to allow the error.
>>>
>>>
>>>> + if (!object->tmp_context) {
>>>> + object->tmp_context = pglXCreateNewContext(gdi_display,
>>>> object->fmt->fbconfig, object->fmt->render_type, prev_context, True);
>>>> + object->prev_context = prev_context;
>>>> + } else if (object->prev_context != prev_context) {
>>>> + pglXDestroyContext(gdi_display, object->tmp_context);
>>>> + object->tmp_context = pglXCreateNewContext(gdi_display,
>>>> object->fmt->fbconfig, object->fmt->render_type, prev_context, True);
>>>> + object->prev_context = prev_context;
>>>> + }
>>>
>>> Those two branches are mostly the same. This could be reduced to:
>>>
>>> if (!object->tmp_context || object->prev_context !=
>>> prev_context) {
>>> if (object->tmp_context)
>>> pglXDestroyContext(gdi_display, object->tmp_context);
>>> object->tmp_context = pglXCreateNewContext(gdi_display,
>>> object->fmt->fbconfig, object->fmt->render_type, prev_context, True);
>>> object->prev_context = prev_context;
>>> }
>
> I don't think this improves readability, but whatever.
I prefer Ken's version but it's certainly fine if you don't take this
suggestion.
>
>>>
>>>
>>> Is it necessary, when a context is destroyed, to go through all
>>> wgl_pbuffer structs looking for any whose prev_context matches the one being
>>> destroyed and clear it? Otherwise, you risk having a stale reference that
>>> could be reused for a different context and not realize you need to recreate
>>> tmp_context. Of course, enumerating all of them in a thread-safe manner is
>>> going to require a lot of added infrastructure and complexity.
>>
>> Good point, that's necessary in principle and it shouldn't be too hard
>> to do by keeping a wgl_pbuffers list (similarly to context_list for
>> wgl_context, you should probably reuse the same context_section
>> critical section for synchronization).
>>
>> The other points brought up by Ken are valid too of course.
>
> If a context is destroyed, and a new context is created, what are the
> chances that the two pointers end up being the same?
Definitely not impossible, when you take into account how memory
allocation usually works. Is keeping track of the wgl_pbuffer structs
more complicated than it looks?
More information about the wine-devel
mailing list