sync: always set the brackets (#59644)

The current code sets bracket_greater to the first trigger after the current
value, and bracket_less to the last trigger before the current value.

For example, the idle timer with three negative and three positive transitions
would set this:

         nt1       nt2                 nt3
|--------|------|--|------- idle --|---|--|-----> t
               pt1                pt2    pt3
bracket_less == nt2
bracket_greater == pt2

This is an optimization so we can skip code paths in the block/wakeup handlers
if the current value doesn't meet any of the trigger requirements. Those
handlers largely do a
   if (bracket_less is less than current value &&
       bracket_greater is greater than current value)
        return, nothing to do

However, unless the bracket values are updated at the correct time, the
following may happen:

                                      nt
|--------------|---------- idle ------|--------> t
               pt

In this case, neither bracket is set, we're past the pos transition and not
yet at the neg transition. idle may now go past nt, but the brackets are not
updated. If idle is then reset to 0, no alarm is triggered for nt. Likewise,
idle may now go past pt and no alarm is triggered.

Changing an alarm or triggering an alarm will re-calculate the brackets, so
this bug is somewhat random. If any other client triggers an alarm when the
brackets are wrongly NULL, the recalculation will set them this bug may not
appear.

This patch changes the behavior, so that the brackets are always the nearest
positive or negative transitions to the current counter value. In the example
above, nt will trigger a wakeup and a re-calculation of the brackets, so that
going past it in the negative direction will then cause the proper alarm
triggers.

Or, in Keith's words:

  Timer currently past a positive trigger
      No bracket values, because no trigger in range

  Timer moves backwards before the positive trigger
      Brackets not reset, even though there is now a trigger in range

  Timer moves forward past the positive trigger
      Trigger doesn't fire because brackets not set

Setting the LT bracket in this case will cause everything to get
re-evaluated when the sync value moves backwards before the trigger
value.

X.Org Bug 59644 <http://bugs.freedesktop.org/show_bug.cgi?id=59644>

Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Adam Jackson <ajax@redhat.com>
Reviewed-by: Keith Packard <keithp@keithp.com>

1 files changed, 20 insertions, 0 deletions

diff --git a/Xext/sync.c b/Xext/sync.c
index ab46d8972..ad3dec252 100644
--- a/Xext/sync.c
+++ b/Xext/sync.c
@@ -1019,6 +1019,11 @@ SyncComputeBracketValues(SyncCounter * pCounter)
                 psci->bracket_greater = pTrigger->test_value;
                 pnewgtval = &psci->bracket_greater;
             }
+            else if (XSyncValueGreaterThan(pCounter->value, pTrigger->test_value) &&
+                     XSyncValueGreaterThan(pTrigger->test_value, psci->bracket_less)) {
+                    psci->bracket_less = pTrigger->test_value;
+                    pnewltval = &psci->bracket_less;
+            }
         }
         else if (pTrigger->test_type == XSyncNegativeComparison &&
                  ct != XSyncCounterNeverDecreases) {
@@ -1028,6 +1033,11 @@ SyncComputeBracketValues(SyncCounter * pCounter)
                 psci->bracket_less = pTrigger->test_value;
                 pnewltval = &psci->bracket_less;
             }
+            else if (XSyncValueLessThan(pCounter->value, pTrigger->test_value) &&
+                     XSyncValueLessThan(pTrigger->test_value, psci->bracket_greater)) {
+                    psci->bracket_greater = pTrigger->test_value;
+                    pnewgtval = &psci->bracket_greater;
+            }
         }
         else if (pTrigger->test_type == XSyncNegativeTransition &&
                  ct != XSyncCounterNeverIncreases) {
@@ -1041,6 +1051,11 @@ SyncComputeBracketValues(SyncCounter * pCounter)
                     psci->bracket_less = pTrigger->test_value;
                     pnewltval = &psci->bracket_less;
             }
+            else if (XSyncValueLessThan(pCounter->value, pTrigger->test_value) &&
+                     XSyncValueLessThan(pTrigger->test_value, psci->bracket_greater)) {
+                    psci->bracket_greater = pTrigger->test_value;
+                    pnewgtval = &psci->bracket_greater;
+            }
         }
         else if (pTrigger->test_type == XSyncPositiveTransition &&
                  ct != XSyncCounterNeverDecreases) {
@@ -1055,6 +1070,11 @@ SyncComputeBracketValues(SyncCounter * pCounter)
                     psci->bracket_greater = pTrigger->test_value;
                     pnewgtval = &psci->bracket_greater;
             }
+            else if (XSyncValueGreaterThan(pCounter->value, pTrigger->test_value) &&
+                     XSyncValueGreaterThan(pTrigger->test_value, psci->bracket_less)) {
+                    psci->bracket_less = pTrigger->test_value;
+                    pnewltval = &psci->bracket_less;
+            }
         }
     }                           /* end for each trigger */