[RFC PATCH] sync: Fix logic error from b55bf248581dc66321b24b29f199f6dc8d02db1b

[Peter Hutterer]

On Mon, Jul 23, 2012 at 04:34:28PM -0400, Adam Jackson wrote:
> That commit adds two hunks, and I _think_ they're backwards.  It adds
> code to modify bracket_greater on NegativeTransition triggers, and
> bracket_less on PositiveTransition triggers.  That breaks symmetry with
> the surrounding code; the code as of this commit could probably be
> simplified further.
> 
> I can't keep the sync trigger rules in my head for more than about five
> minutes at a time, so I'm sending this on for more eyes.  RHEL 6.3's
> xserver is shipping with b55bf248 reverted:
> 
>     https://bugzilla.redhat.com/show_bug.cgi?id=748704#c33
> 
> And there appear to be some upstream reports of the same issue:
> 
>     https://bugzilla.gnome.org/show_bug.cgi?id=658955
> 
> So I'd like to get this sorted out.
> 
> Signed-off-by: Adam Jackson <ajax at redhat.com>

sounds right.
Reviewed-by: Peter Hutterer <peter.hutterer at who-t.net>

the optimised condition would be
    if ((XSyncValueGreaterOrEqual(pCounter->value, pTrigger->test_value) &&
        XSyncValueGreaterThan(pTrigger->test_value, psci->bracket_less))
        ... adjust lower bracket

which still looks correct behaviour.

Cheers,
  Peter

> ---
>  Xext/sync.c |   24 ++++++++++++------------
>  1 files changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/Xext/sync.c b/Xext/sync.c
> index b8f094d..4d11992 100644
> --- a/Xext/sync.c
> +++ b/Xext/sync.c
> @@ -1038,15 +1038,15 @@ SyncComputeBracketValues(SyncCounter * pCounter)
>                  pnewltval = &psci->bracket_less;
>              }
>              else if (XSyncValueEqual(pCounter->value, pTrigger->test_value) &&
> -                     XSyncValueLessThan(pTrigger->test_value,
> -                                        psci->bracket_greater)) {
> +                     XSyncValueGreaterThan(pTrigger->test_value,
> +                                           psci->bracket_less)) {
>                  /*
>                   * The value is exactly equal to our threshold.  We want one
> -                 * more event in the positive direction to ensure we pick up
> -                 * when the value *exceeds* this threshold.
> +                 * more event in the negative direction to ensure we pick up
> +                 * when the value is less than this threshold.
>                   */
> -                psci->bracket_greater = pTrigger->test_value;
> -                pnewgtval = &psci->bracket_greater;
> +                psci->bracket_less = pTrigger->test_value;
> +                pnewltval = &psci->bracket_less;
>              }
>          }
>          else if (pTrigger->test_type == XSyncPositiveTransition &&
> @@ -1058,15 +1058,15 @@ SyncComputeBracketValues(SyncCounter * pCounter)
>                  pnewgtval = &psci->bracket_greater;
>              }
>              else if (XSyncValueEqual(pCounter->value, pTrigger->test_value) &&
> -                     XSyncValueGreaterThan(pTrigger->test_value,
> -                                           psci->bracket_less)) {
> +                     XSyncValueLessThan(pTrigger->test_value,
> +                                        psci->bracket_greater)) {
>                  /*
>                   * The value is exactly equal to our threshold.  We want one
> -                 * more event in the negative direction to ensure we pick up
> -                 * when the value is less than this threshold.
> +                 * more event in the positive direction to ensure we pick up
> +                 * when the value *exceeds* this threshold.
>                   */
> -                psci->bracket_less = pTrigger->test_value;
> -                pnewltval = &psci->bracket_less;
> +                psci->bracket_greater = pTrigger->test_value;
> +                pnewgtval = &psci->bracket_greater;
>              }
>          }
>      }                           /* end for each trigger */
> -- 
> 1.7.7.6
> 
> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
>