On Thu, Aug 25, 2016 at 05:18:32PM +0100, Emil Velikov wrote: > From: Emil Velikov <emil.veli...@collabora.com> > > Duplicate a few lines at the expense of making the code-flow clearer > while adding a few extra comments ;-) > > Signed-off-by: Emil Velikov <emil.veli...@collabora.com> > --- > > It makes things clearer the way on this end, but if people have better > suggestions I would love to hear them. > > Emil > --- > src/egl/drivers/dri2/egl_dri2.c | 43 > ++++++++++++++++++++++------------------- > 1 file changed, 23 insertions(+), 20 deletions(-) > > diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c > index a481236..d53c9b9 100644 > --- a/src/egl/drivers/dri2/egl_dri2.c > +++ b/src/egl/drivers/dri2/egl_dri2.c > @@ -1248,11 +1248,12 @@ dri2_make_current(_EGLDriver *drv, _EGLDisplay *disp, > _EGLSurface *dsurf, > struct dri2_egl_display *dri2_dpy = dri2_egl_display(disp); > struct dri2_egl_context *dri2_ctx = dri2_egl_context(ctx); > _EGLContext *old_ctx; > + _EGLDisplay *old_disp = NULL; > _EGLSurface *old_dsurf, *old_rsurf; > _EGLSurface *tmp_dsurf, *tmp_rsurf; > - __DRIdrawable *ddraw, *rdraw; > - __DRIcontext *cctx; > - EGLBoolean unbind; > + __DRIdrawable *ddraw = (dsurf) ? dri2_dpy->vtbl->get_dri_drawable(dsurf) > : NULL; > + __DRIdrawable *rdraw = (rsurf) ? dri2_dpy->vtbl->get_dri_drawable(rsurf) > : NULL; > + __DRIcontext *cctx = (dri2_ctx) ? dri2_ctx->dri_context : NULL; > > if (!dri2_dpy) > return _eglError(EGL_NOT_INITIALIZED, "eglMakeCurrent"); > @@ -1263,32 +1264,34 @@ dri2_make_current(_EGLDriver *drv, _EGLDisplay *disp, > _EGLSurface *dsurf, > return EGL_FALSE; > } > > - /* flush before context switch */ > - if (old_ctx) > - dri2_drv->glFlush(); > - > - ddraw = (dsurf) ? dri2_dpy->vtbl->get_dri_drawable(dsurf) : NULL; > - rdraw = (rsurf) ? dri2_dpy->vtbl->get_dri_drawable(rsurf) : NULL; > - cctx = (dri2_ctx) ? dri2_ctx->dri_context : NULL; > - > if (old_ctx) { > __DRIcontext *old_cctx = dri2_egl_context(old_ctx)->dri_context; > + > + /* flush before context switch */ > + dri2_drv->glFlush(); > dri2_dpy->core->unbindContext(old_cctx); > + > + /* Keep track of the old dpy as we'll need it for resource cleanup */ > + old_disp = old_ctx->Resource.Display; > } > > - unbind = (cctx == NULL && ddraw == NULL && rdraw == NULL); > + /* There's no new ctx to bind, so just destroy the old resources */ > + if (cctx == NULL) { > + dri2_destroy_surface(drv, disp, old_dsurf); > + dri2_destroy_surface(drv, disp, old_rsurf); > + dri2_destroy_context(drv, disp, old_ctx); > + dri2_display_release(old_disp); > > - if (unbind || dri2_dpy->core->bindContext(cctx, ddraw, rdraw)) { > + return EGL_TRUE; > + } > + if (dri2_dpy->core->bindContext(cctx, ddraw, rdraw)) {
I'm pretty sure dropping `unbind` changes the behaviour, which means this isn't just a refactor. (Also, nit: the grouping on that diff would have been better if you had left an empty line between the two `if` :) > dri2_destroy_surface(drv, disp, old_dsurf); > dri2_destroy_surface(drv, disp, old_rsurf); > + dri2_destroy_context(drv, disp, old_ctx); > + dri2_display_release(old_disp); I think `dri2_display_release()` needs to be in an `if (old_disp)` (or `if (old_ctx)`, they're equivalent at this point), as it will dereference old_disp. Cheers, Eric > > - if (!unbind) > - dri2_dpy->ref_count++; > - if (old_ctx) { > - EGLDisplay old_disp = > _eglGetDisplayHandle(old_ctx->Resource.Display); > - dri2_destroy_context(drv, disp, old_ctx); > - dri2_display_release(old_disp); > - } > + /* Refcount the new dpy */ > + dri2_dpy->ref_count++; > > return EGL_TRUE; > } else { > -- > 2.9.0 _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev