-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
[email protected] wrote:
> From: Thomas Hellstrom <thellstrom-at-vmware-dot-com>
>
> 1) Don't error-check here. It's done in makeCurrent.
> 2) Don't segfault if ctx happens to be 0.
> 3) Don't update drawables if the stamps aren't matching. We're outside of
> a locked region and the driver would do it anyway.
> 4) Propagate errors from driver makeCurrent.
> Such errors could for example be if the drawable bound to the old
> context is invalid.
>
> Signed-off-by: Thomas Hellstrom <thellstrom-at-vmware-dot-com>
> ---
> src/mesa/drivers/dri/common/dri_util.c | 36 +++++++++++++------------------
> 1 files changed, 15 insertions(+), 21 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/common/dri_util.c
> b/src/mesa/drivers/dri/common/dri_util.c
> index ae79055..52e123b 100644
> --- a/src/mesa/drivers/dri/common/dri_util.c
> +++ b/src/mesa/drivers/dri/common/dri_util.c
> @@ -163,21 +163,18 @@ static int driBindContext(__DRIcontext *pcp,
> {
> __DRIscreenPrivate *psp = pcp->driScreenPriv;
>
> - /*
> - ** Assume error checking is done properly in glXMakeCurrent before
> - ** calling driBindContext.
> - */
> -
> - if (pcp == NULL || pdp == None || prp == None)
> - return GL_FALSE;
> -
> /* Bind the drawable to the context */
> - pcp->driDrawablePriv = pdp;
> - pcp->driReadablePriv = prp;
> - pdp->driContextPriv = pcp;
> - pdp->refcount++;
> - if ( pdp != prp ) {
> - prp->refcount++;
> +
> + if (pcp) {
> + pcp->driDrawablePriv = pdp;
> + pcp->driReadablePriv = prp;
> + if (pdp) {
> + pdp->driContextPriv = pcp;
> + pdp->refcount++;
> + }
> + if ( prp && pdp != prp ) {
> + prp->refcount++;
> + }
> }
Is there a valid case where pdp could be NULL? The old code would just
bail in this case. This patch changes it to do the BindContext, but is
that valid? Likewise of pdp and prp. If the only case is when they're
all NULL, we should have a comment and possibly an assertion to that effect.
>
> /*
> @@ -186,23 +183,20 @@ static int driBindContext(__DRIcontext *pcp,
> */
>
> if (!psp->dri2.enabled) {
> - if (!pdp->pStamp || *pdp->pStamp != pdp->lastStamp) {
> + if (pdp && !pdp->pStamp) {
> DRM_SPINLOCK(&psp->pSAREA->drawable_lock, psp->drawLockID);
> __driUtilUpdateDrawableInfo(pdp);
> DRM_SPINUNLOCK(&psp->pSAREA->drawable_lock, psp->drawLockID);
> }
> -
> - if ((pdp != prp) && (!prp->pStamp || *prp->pStamp != prp->lastStamp)) {
> + if (prp && pdp != prp && !prp->pStamp) {
> DRM_SPINLOCK(&psp->pSAREA->drawable_lock, psp->drawLockID);
> __driUtilUpdateDrawableInfo(prp);
> DRM_SPINUNLOCK(&psp->pSAREA->drawable_lock, psp->drawLockID);
> - }
> + }
These changes seem to do the opposite of what the commit message says.
"Don't update drawables if the stamps aren't matching," but this code
removes the check.
> }
>
> /* Call device-specific MakeCurrent */
> - (*psp->DriverAPI.MakeCurrent)(pcp, pdp, prp);
> -
> - return GL_TRUE;
> + return (*psp->DriverAPI.MakeCurrent)(pcp, pdp, prp);
> }
I'd suggest committing this on its own. It's obviously correct, but I
want to understand the other changes better.
Reviewed-by: Ian Romanick <[email protected]>
>
> /*...@}*/
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
iEYEARECAAYFAknSkwMACgkQX1gOwKyEAw+QHwCcDM8wK5BGniqF+KLlBodfPKK5
gecAniFPiD2OeKGGxawP0tbtk5+Xf2C2
=cfLj
-----END PGP SIGNATURE-----
------------------------------------------------------------------------------
_______________________________________________
Mesa3d-dev mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev
