[prev in list] [next in list] [prev in thread] [next in thread] 

List:       wine-devel
Subject:    Re: [PATCH] winex11: Don't create a temporary context each time X11DRV_wglBindTexImageARB is called.
From:       Miklós Máté <mtmkls () gmail ! com>
Date:       2016-02-29 22:20:50
Message-ID: 56D4C442.1070606 () gmail ! com
[Download RAW message or body]

On 02/29/2016 06:10 PM, Matteo Bruni wrote:
> 2016-02-29 3:59 GMT+01:00 Ken Thomases <ken@codeweavers.com>:
> > On Feb 26, 2016, at 5:29 PM, Miklós Máté <mtmkls@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.

> > 
> > 
> > > +        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.

> > 
> > 
> > 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?

MM


[prev in list] [next in list] [prev in thread] [next in thread]