[Mesa-stable] [Mesa-dev] [PATCH 1/5] glx/dri3: Fix glXWaitForSbcOML() to handle targetSBC==0 correctly.

[Eric Anholt]

Mario Kleiner <mario.kleiner.de at gmail.com> writes:

> On 12/04/2014 11:48 AM, Axel Davy wrote:
>> Le 04/12/2014 11:44, Axel Davy a écrit :
>>> On 02/12/2014 20:53, Mario Kleiner wrote :
>>>> targetSBC == 0 is a special case, which asks the function
>>>> to block until all pending OpenGL bufferswap requests have
>>>> completed.
>>>>
>>>> Currently the function just falls through for targetSBC == 0,
>>>> returning bogus results.
>>>>
>>>> This breaks applications originally written and tested against
>>>> DRI2 which also rely on this not regressing under DRI3/Present,
>>>> e.g., Neuro-Science software like Psychtoolbox-3.
>>>>
>>>> This patch fixes the problem.
>>>>
>>>> Cc: "10.3 10.4" <mesa-stable at lists.freedesktop.org>
>>>> Signed-off-by: Mario Kleiner <mario.kleiner.de at gmail.com>
>>>> ---
>>>>   src/glx/dri3_glx.c | 3 ++-
>>>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/src/glx/dri3_glx.c b/src/glx/dri3_glx.c
>>>> index a9ff73b..b4ac278 100644
>>>> --- a/src/glx/dri3_glx.c
>>>> +++ b/src/glx/dri3_glx.c
>>>> @@ -529,7 +529,8 @@ dri3_wait_for_sbc(__GLXDRIdrawable *pdraw, 
>>>> int64_t target_sbc, int64_t *ust,
>>>>   {
>>>>      struct dri3_drawable *priv = (struct dri3_drawable *) pdraw;
>>>>   -   while (priv->recv_sbc < target_sbc) {
>>>> +   while ((target_sbc != 0 && priv->recv_sbc < target_sbc) ||
>>>> +          (target_sbc == 0 && priv->recv_sbc < priv->send_sbc)) {
>>>>         if (!dri3_wait_for_event(pdraw))
>>>>            return 0;
>>>>      }
>>> Looks good to me.
>>>
>>> Reviewed-by: Axel Davy <axel.davy at ens.fr>
>>> _______________________________________________
>>>
>> Just an additional thought:
>>
>> it would probably better to do instead:
>>
>> if (!target_sbc)
>>     target_sbc = priv->send_sbc;
>
> Yep. Will change it. Will also make the other suggested changes for the 
> other patches and then send out a new series.

It can be nice to add spec citations so the next reader knows why odd
things are there, like:

/* From the GLX_OML_sync_control spec:
 *
 *     "If <target_sbc> = 0, the function will block until all previous
 *      swaps requested with glXSwapBuffersMscOML for that window have
 *      completed."
 */

Bonus points if you add it, but either way the simplified lines here
get:

Reviewed-by: Eric Anholt <eric at anholt.net>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 818 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-stable/attachments/20141204/781dbdc2/attachment.sig>