[Mesa-dev] [PATCH 4/5] glx/dri3: Don't fail on glXSwapBuffersMscOML(dpy, window, 0, 0, 0)

[Eric Anholt]

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

> glXSwapBuffersMscOML() with target_msc=divisor=remainder=0 gets
> translated into target_msc=divisor=0 but remainder=1 by the mesa
> api. This is done for server DRI2 where there needs to be a way
> to tell the server-side DRI2ScheduleSwap implementation if a call
> to glXSwapBuffers() or glXSwapBuffersMscOML(dpy,window,0,0,0) was
> done. remainder = 1 was (ab)used as a flag to tell the server to
> select proper semantic. The DRI3/Present backend ignored this
> signalling, treated any target_msc=0 as glXSwapBuffers() request,
> and called xcb_present_pixmap with invalid divisor=0, remainder=1
> combo. The present extension responded kindly to this with a
> BadValue error and dropped the request, but mesa's DRI3/Present
> backend doesn't check for error codes. From there on stuff went
> downhill quickly for the calling OpenGL client...
>
> 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 | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/src/glx/dri3_glx.c b/src/glx/dri3_glx.c
> index c53be1b..efff907 100644
> --- a/src/glx/dri3_glx.c
> +++ b/src/glx/dri3_glx.c
> @@ -1552,11 +1552,21 @@ dri3_swap_buffers(__GLXDRIdrawable *pdraw, int64_t target_msc, int64_t divisor,
>        dri3_fence_reset(c, back);
>  
>        /* Compute when we want the frame shown by taking the last known successful
> -       * MSC and adding in a swap interval for each outstanding swap request
> +       * MSC and adding in a swap interval for each outstanding swap request.
> +       * target_msc=divisor=remainder=0 means "Use glXSwapBuffers() semantic"
>         */
>        ++priv->send_sbc;
> -      if (target_msc == 0)
> +      if (target_msc == 0 && divisor == 0 && remainder == 0)
>           target_msc = priv->msc + priv->swap_interval * (priv->send_sbc - priv->recv_sbc);
> +      else if (divisor == 0 && remainder > 0) {
> +         /* Special case signalling: This means "glXSwapBuffersMscOML() called with
> +          * target_msc=divisor=remainder=0. Needed to distinguish from glXSwapBuffers
> +          * case above. Reset remainder to zero, so PresentPixmap won't bail on us.
> +          * GLX_OML_sync_control says for divisor == 0 any remainder >= 0 is fine,
> +          * whereas Present extension wants remainder == 0 for divisor == 0.
> +          */
> +         remainder = 0;
> +      }

There's at least a mismatched '"' in the second comment, and I'd have
thought the msc/divisor/msc==0 would be the "special case signalling"
case.  I'd replace the comment with:

/* From the GLX_OML_sync_control spec:
 *
 *     "If <divisor> = 0, the swap will occur when MSC becomes
 *       greater than or equal to <target_msc>."
 *
 * Note that there's no mention of the remainder.  The Present extension
 * throws BadValue for remainder != 0 with divisor == 0, so just drop
 * the passed in value.
 */

With that,

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-dev/attachments/20141204/dc9abc2e/attachment.sig>