Ian Romanick wrote:
> -----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.
>
Currently all NULL cases are blocked in the initial error check in the
glx code.
These checks are for future use if the spec is changed to allow
MakeCurrent(ctx, NULL, NULL),
for example for fbo only rendering.
>
>>
>> /*
>> @@ -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.
>
The previous code read the stamps outside of the lock on each
makeCurrent and went ahead updating the drawables. Their info
immediately became stale so the driver had to do it again anyway.
So this code removes that check and only updates the drawables if they
aren't initialized.
>
>> }
>>
>> /* 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.
>
Sure. I'll split it out.
/Thomas
> 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
