Replaced SDL_HAPTIC_SQUARE with SDL_HAPTIC_LEFTRIGHT.

We needed a bit, so we're hoping no one needs this effect, especially when
it's fairly close to SDL_HAPTIC_SINE, we hope.

SDL_HAPTIC_LEFTRIGHT maps to XInput's functionality, so this removes the SINE
code for the XInput driver to keep things clean.

This also makes the simplified Rumble API use SDL_HAPTIC_LEFTRIGHT if
SDL_HAPTIC_SINE isn't available, to keep XInput working.

When we break the ABI, and can extend the supported capabilities field from
a Uint16, we'll add SDL_HAPTIC_SQUARE back in.

This patch is based on work by Ethan Lee.

include/SDL_haptic.h

@@ -166,13 +166,18 @@ typedef struct _SDL_Haptic SDL_Haptic;
 #define SDL_HAPTIC_SINE       (1<<1)
 
 /**
- *  \brief Square wave effect supported.
+ *  \brief Left/Right effect supported.
  *
- *  Periodic haptic effect that simulates square waves.
+ *  Haptic effect for direct control over high/low frequency motors.
  *
- *  \sa SDL_HapticPeriodic
+ *  \sa SDL_HapticLeftRight
+ * \warning this value was SDL_HAPTIC_SQUARE right before 2.0.0 shipped. Sorry,
+ *          we ran out of bits, and this is important for XInput devices.
  */
-#define SDL_HAPTIC_SQUARE     (1<<2)
+#define SDL_HAPTIC_LEFTRIGHT     (1<<2)
+
+/* !!! FIXME: put this back when we have more bits in 2.1 */
+/*#define SDL_HAPTIC_SQUARE     (1<<2)*/
 
 /**
  *  \brief Triangle wave effect supported.
@@ -645,6 +650,31 @@ typedef struct SDL_HapticRamp
     Uint16 fade_level;      /**< Level at the end of the fade. */
 } SDL_HapticRamp;
 
+/**
+ * \brief A structure containing a template for a Left/Right effect.
+ *
+ * This struct is exclusively for the ::SDL_HAPTIC_LEFTRIGHT effect.
+ *
+ * The Left/Right effect is used to explicitly control the large and small
+ * motors, commonly found in modern game controllers. One motor is high
+ * frequency, the other is low frequency.
+ *
+ * \sa SDL_HAPTIC_LEFTRIGHT
+ * \sa SDL_HapticEffect
+ */
+typedef struct SDL_HapticLeftRight
+{
+    /* Header */
+    Uint16 type;            /**< ::SDL_HAPTIC_LEFTRIGHT */
+
+    /* Replay */
+    Uint32 length;          /**< Duration of the effect. */
+
+    /* Rumble */
+    Uint16 large_magnitude; /**< Control of the large controller motor. */
+    Uint16 small_magnitude; /**< Control of the small controller motor. */
+} SDL_HapticLeftRight;
+
 /**
  *  \brief A structure containing a template for the ::SDL_HAPTIC_CUSTOM effect.
  *
@@ -751,6 +781,7 @@ typedef struct SDL_HapticCustom
  *  \sa SDL_HapticPeriodic
  *  \sa SDL_HapticCondition
  *  \sa SDL_HapticRamp
+ *  \sa SDL_HapticLeftRight
  *  \sa SDL_HapticCustom
  */
 typedef union SDL_HapticEffect
@@ -761,6 +792,7 @@ typedef union SDL_HapticEffect
     SDL_HapticPeriodic periodic;    /**< Periodic effect. */
     SDL_HapticCondition condition;  /**< Condition effect. */
     SDL_HapticRamp ramp;            /**< Ramp effect. */
+    SDL_HapticLeftRight leftright;  /**< Left/Right effect. */
     SDL_HapticCustom custom;        /**< Custom effect. */
 } SDL_HapticEffect;
 
@@ -1182,8 +1214,6 @@ extern DECLSPEC int SDLCALL SDL_HapticRumblePlay(SDL_Haptic * haptic, float stre
  */
 extern DECLSPEC int SDLCALL SDL_HapticRumbleStop(SDL_Haptic * haptic);
 
-
-
 /* Ends C function definitions when using C++ */
 #ifdef __cplusplus
 }

src/haptic/SDL_haptic.c

@@ -23,7 +23,7 @@
 #include "SDL_syshaptic.h"
 #include "SDL_haptic_c.h"
 #include "../joystick/SDL_joystick_c.h" /* For SDL_PrivateJoystickValid */
-
+#include "SDL_assert.h"
 
 Uint8 SDL_numhaptics = 0;
 SDL_Haptic **SDL_haptics = NULL;
@@ -723,32 +723,18 @@ SDL_HapticStopAll(SDL_Haptic * haptic)
     return SDL_SYS_HapticStopAll(haptic);
 }
 
-static void
-SDL_HapticRumbleCreate(SDL_HapticEffect * efx)
-{
-   SDL_memset(efx, 0, sizeof(SDL_HapticEffect));
-   efx->type = SDL_HAPTIC_SINE;
-   efx->periodic.period = 1000;
-   efx->periodic.magnitude = 0x4000;
-   efx->periodic.length = 5000;
-   efx->periodic.attack_length = 0;
-   efx->periodic.fade_length = 0;
-}
-
 /*
  * Checks to see if rumble is supported.
  */
 int
 SDL_HapticRumbleSupported(SDL_Haptic * haptic)
 {
-    SDL_HapticEffect efx;
-
     if (!ValidHaptic(haptic)) {
         return -1;
     }
 
-    SDL_HapticRumbleCreate(&efx);
+    /* Most things can use SINE, but XInput only has LEFTRIGHT. */
-    return SDL_HapticEffectSupported(haptic, &efx);
+    return ((haptic->supported & (SDL_HAPTIC_SINE|SDL_HAPTIC_LEFTRIGHT)) != 0);
 }
 
 /*
@@ -757,6 +743,8 @@ SDL_HapticRumbleSupported(SDL_Haptic * haptic)
 int
 SDL_HapticRumbleInit(SDL_Haptic * haptic)
 {
+    SDL_HapticEffect *efx = &haptic->rumble_effect;
+
     if (!ValidHaptic(haptic)) {
         return -1;
     }
@@ -766,8 +754,23 @@ SDL_HapticRumbleInit(SDL_Haptic * haptic)
         return 0;
     }
 
-    /* Copy over. */
+    SDL_zerop(efx);
-    SDL_HapticRumbleCreate(&haptic->rumble_effect);
+    if (haptic->supported & SDL_HAPTIC_SINE) {
+        efx->type = SDL_HAPTIC_SINE;
+        efx->periodic.period = 1000;
+        efx->periodic.magnitude = 0x4000;
+        efx->periodic.length = 5000;
+        efx->periodic.attack_length = 0;
+        efx->periodic.fade_length = 0;
+    } else if (haptic->supported & SDL_HAPTIC_LEFTRIGHT) {  /* XInput? */
+        efx->type = SDL_HAPTIC_LEFTRIGHT;
+        efx->leftright.length = 5000;
+        efx->leftright.large_magnitude = 0x4000;
+        efx->leftright.small_magnitude = 0x4000;
+    } else {
+        return SDL_SetError("Device doesn't support rumble");
+    }
+
     haptic->rumble_id = SDL_HapticNewEffect(haptic, &haptic->rumble_effect);
     if (haptic->rumble_id >= 0) {
         return 0;
@@ -781,7 +784,8 @@ SDL_HapticRumbleInit(SDL_Haptic * haptic)
 int
 SDL_HapticRumblePlay(SDL_Haptic * haptic, float strength, Uint32 length)
 {
-    SDL_HapticPeriodic *efx;
+    SDL_HapticEffect *efx;
+    Sint16 magnitude;
 
     if (!ValidHaptic(haptic)) {
         return -1;
@@ -794,15 +798,22 @@ SDL_HapticRumblePlay(SDL_Haptic * haptic, float strength, Uint32 length)
     /* Clamp strength. */
     if (strength > 1.0f) {
         strength = 1.0f;
-    }
+    } else if (strength < 0.0f) {
-    else if (strength < 0.0f) {
         strength = 0.0f;
     }
+    magnitude = (Sint16)(32767.0f*strength);
+
+    efx = &haptic->rumble_effect;
+    if (efx->type == SDL_HAPTIC_SINE) {
+        efx->periodic.magnitude = magnitude;
+        efx->periodic.length = length;
+    } else if (efx->type == SDL_HAPTIC_LEFTRIGHT) {
+        efx->leftright.small_magnitude = efx->leftright.large_magnitude = magnitude;
+        efx->leftright.length = length;
+    } else {
+        SDL_assert(0 && "This should have been caught elsewhere");
+    }
 
-    /* New effect. */
-    efx = &haptic->rumble_effect.periodic;
-    efx->magnitude = (Sint16)(32767.0f*strength);
-    efx->length = length;
     if (SDL_HapticUpdateEffect(haptic, haptic->rumble_id, &haptic->rumble_effect) < 0) {
         return -1;
     }
@@ -827,4 +838,3 @@ SDL_HapticRumbleStop(SDL_Haptic * haptic)
     return SDL_HapticStopEffect(haptic, haptic->rumble_id);
 }
 
-

src/haptic/darwin/SDL_syshaptic.c

@@ -342,7 +342,8 @@ GetSupportedFeatures(SDL_Haptic * haptic)
     /* Test for effects. */
     FF_TEST(FFCAP_ET_CONSTANTFORCE, SDL_HAPTIC_CONSTANT);
     FF_TEST(FFCAP_ET_RAMPFORCE, SDL_HAPTIC_RAMP);
-    FF_TEST(FFCAP_ET_SQUARE, SDL_HAPTIC_SQUARE);
+    /* !!! FIXME: put this back when we have more bits in 2.1 */
+    /*FF_TEST(FFCAP_ET_SQUARE, SDL_HAPTIC_SQUARE);*/
     FF_TEST(FFCAP_ET_SINE, SDL_HAPTIC_SINE);
     FF_TEST(FFCAP_ET_TRIANGLE, SDL_HAPTIC_TRIANGLE);
     FF_TEST(FFCAP_ET_SAWTOOTHUP, SDL_HAPTIC_SAWTOOTHUP);
@@ -750,7 +751,8 @@ SDL_SYS_ToFFEFFECT(SDL_Haptic * haptic, FFEFFECT * dest,
         break;
 
     case SDL_HAPTIC_SINE:
-    case SDL_HAPTIC_SQUARE:
+    /* !!! FIXME: put this back when we have more bits in 2.1 */
+    /*case SDL_HAPTIC_SQUARE:*/
     case SDL_HAPTIC_TRIANGLE:
     case SDL_HAPTIC_SAWTOOTHUP:
     case SDL_HAPTIC_SAWTOOTHDOWN:
@@ -978,8 +980,9 @@ SDL_SYS_HapticEffectType(Uint16 type)
     case SDL_HAPTIC_RAMP:
         return kFFEffectType_RampForce_ID;
 
-    case SDL_HAPTIC_SQUARE:
+    /* !!! FIXME: put this back when we have more bits in 2.1 */
-        return kFFEffectType_Square_ID;
+    /*case SDL_HAPTIC_SQUARE:
+        return kFFEffectType_Square_ID;*/
 
     case SDL_HAPTIC_SINE:
         return kFFEffectType_Sine_ID;

src/haptic/linux/SDL_syshaptic.c

@@ -99,7 +99,8 @@ EV_IsHaptic(int fd)
     /* Convert supported features to SDL_HAPTIC platform-neutral features. */
     EV_TEST(FF_CONSTANT, SDL_HAPTIC_CONSTANT);
     EV_TEST(FF_SINE, SDL_HAPTIC_SINE);
-    EV_TEST(FF_SQUARE, SDL_HAPTIC_SQUARE);
+    /* !!! FIXME: put this back when we have more bits in 2.1 */
+    /*EV_TEST(FF_SQUARE, SDL_HAPTIC_SQUARE);*/
     EV_TEST(FF_TRIANGLE, SDL_HAPTIC_TRIANGLE);
     EV_TEST(FF_SAW_UP, SDL_HAPTIC_SAWTOOTHUP);
     EV_TEST(FF_SAW_DOWN, SDL_HAPTIC_SAWTOOTHDOWN);
@@ -111,6 +112,7 @@ EV_IsHaptic(int fd)
     EV_TEST(FF_CUSTOM, SDL_HAPTIC_CUSTOM);
     EV_TEST(FF_GAIN, SDL_HAPTIC_GAIN);
     EV_TEST(FF_AUTOCENTER, SDL_HAPTIC_AUTOCENTER);
+    EV_TEST(FF_RUMBLE, SDL_HAPTIC_LEFTRIGHT);
 
     /* Return what it supports. */
     return ret;
@@ -559,6 +561,7 @@ SDL_SYS_ToFFEffect(struct ff_effect *dest, SDL_HapticEffect * src)
     SDL_HapticPeriodic *periodic;
     SDL_HapticCondition *condition;
     SDL_HapticRamp *ramp;
+    SDL_HapticLeftRight *leftright;
 
     /* Clear up */
     SDL_memset(dest, 0, sizeof(struct ff_effect));
@@ -596,7 +599,8 @@ SDL_SYS_ToFFEffect(struct ff_effect *dest, SDL_HapticEffect * src)
         break;
 
     case SDL_HAPTIC_SINE:
-    case SDL_HAPTIC_SQUARE:
+    /* !!! FIXME: put this back when we have more bits in 2.1 */
+    /*case SDL_HAPTIC_SQUARE:*/
     case SDL_HAPTIC_TRIANGLE:
     case SDL_HAPTIC_SAWTOOTHUP:
     case SDL_HAPTIC_SAWTOOTHDOWN:
@@ -620,8 +624,9 @@ SDL_SYS_ToFFEffect(struct ff_effect *dest, SDL_HapticEffect * src)
         /* Periodic */
         if (periodic->type == SDL_HAPTIC_SINE)
             dest->u.periodic.waveform = FF_SINE;
-        else if (periodic->type == SDL_HAPTIC_SQUARE)
+        /* !!! FIXME: put this back when we have more bits in 2.1 */
-            dest->u.periodic.waveform = FF_SQUARE;
+        /*else if (periodic->type == SDL_HAPTIC_SQUARE)
+            dest->u.periodic.waveform = FF_SQUARE;*/
         else if (periodic->type == SDL_HAPTIC_TRIANGLE)
             dest->u.periodic.waveform = FF_TRIANGLE;
         else if (periodic->type == SDL_HAPTIC_SAWTOOTHUP)
@@ -725,6 +730,27 @@ SDL_SYS_ToFFEffect(struct ff_effect *dest, SDL_HapticEffect * src)
 
         break;
 
+    case SDL_HAPTIC_LEFTRIGHT:
+        leftright = &src->leftright;
+
+        /* Header */
+        dest->type = FF_RUMBLE;
+        dest->direction = 0;
+
+        /* Replay */
+        dest->replay.length = (leftright->length == SDL_HAPTIC_INFINITY) ?
+            0 : CLAMP(leftright->length);
+
+        /* Trigger */
+        dest->trigger.button = 0;
+        dest->trigger.interval = 0;
+
+        /* Rumble */
+        dest->u.rumble.strong_magnitude = leftright->large_magnitude;
+        dest->u.rumble.weak_magnitude = leftright->small_magnitude;
+
+	break;
+
 
     default:
         return SDL_SetError("Haptic: Unknown effect type.");

src/haptic/windows/SDL_syshaptic.c

@@ -303,7 +303,8 @@ DI_EffectCallback(LPCDIEFFECTINFO pei, LPVOID pv)
     EFFECT_TEST(GUID_ConstantForce, SDL_HAPTIC_CONSTANT);
     EFFECT_TEST(GUID_CustomForce, SDL_HAPTIC_CUSTOM);
     EFFECT_TEST(GUID_Sine, SDL_HAPTIC_SINE);
-    EFFECT_TEST(GUID_Square, SDL_HAPTIC_SQUARE);
+    /* !!! FIXME: put this back when we have more bits in 2.1 */
+    /*EFFECT_TEST(GUID_Square, SDL_HAPTIC_SQUARE);*/
     EFFECT_TEST(GUID_Triangle, SDL_HAPTIC_TRIANGLE);
     EFFECT_TEST(GUID_SawtoothUp, SDL_HAPTIC_SAWTOOTHUP);
     EFFECT_TEST(GUID_SawtoothDown, SDL_HAPTIC_SAWTOOTHDOWN);
@@ -389,8 +390,7 @@ SDL_SYS_HapticOpenFromXInput(SDL_Haptic * haptic, Uint8 userid)
     XINPUT_VIBRATION vibration = { 0, 0 };  /* stop any current vibration */
     XINPUTSETSTATE(userid, &vibration);
 
-    /* !!! FIXME: we can probably do more than SINE if we figure out how to set up the left and right motors properly. */
+    haptic->supported = SDL_HAPTIC_LEFTRIGHT;
-    haptic->supported = SDL_HAPTIC_SINE;
 
     haptic->neffects = 1;
     haptic->nplaying = 1;
@@ -935,7 +935,8 @@ SDL_SYS_ToDIEFFECT(SDL_Haptic * haptic, DIEFFECT * dest,
         break;
 
     case SDL_HAPTIC_SINE:
-    case SDL_HAPTIC_SQUARE:
+    /* !!! FIXME: put this back when we have more bits in 2.1 */
+    /*case SDL_HAPTIC_SQUARE:*/
     case SDL_HAPTIC_TRIANGLE:
     case SDL_HAPTIC_SAWTOOTHUP:
     case SDL_HAPTIC_SAWTOOTHDOWN:
@@ -1163,8 +1164,9 @@ SDL_SYS_HapticEffectType(SDL_HapticEffect * effect)
     case SDL_HAPTIC_RAMP:
         return &GUID_RampForce;
 
-    case SDL_HAPTIC_SQUARE:
+    /* !!! FIXME: put this back when we have more bits in 2.1 */
-        return &GUID_Square;
+    /*case SDL_HAPTIC_SQUARE:
+        return &GUID_Square;*/
 
     case SDL_HAPTIC_SINE:
         return &GUID_Sine;
@@ -1210,7 +1212,7 @@ SDL_SYS_HapticNewEffect(SDL_Haptic * haptic, struct haptic_effect *effect,
     HRESULT ret;
     REFGUID type = SDL_SYS_HapticEffectType(base);
 
-    if (type == NULL) {
+    if ((type == NULL) && (!haptic->hwdata->bXInputHaptic)) {
         goto err_hweffect;
     }
 
@@ -1225,7 +1227,7 @@ SDL_SYS_HapticNewEffect(SDL_Haptic * haptic, struct haptic_effect *effect,
     SDL_zerop(effect->hweffect);
 
     if (haptic->hwdata->bXInputHaptic) {
-        SDL_assert(base->type == SDL_HAPTIC_SINE);  /* should catch this at higher level */
+        SDL_assert(base->type == SDL_HAPTIC_LEFTRIGHT);  /* should catch this at higher level */
         return SDL_SYS_HapticUpdateEffect(haptic, effect, base);
     }
 
@@ -1269,20 +1271,14 @@ SDL_SYS_HapticUpdateEffect(SDL_Haptic * haptic,
     DIEFFECT temp;
 
     if (haptic->hwdata->bXInputHaptic) {
-        /* !!! FIXME: this isn't close to right. We only support "sine" effects,
-         * !!! FIXME:  we ignore most of the parameters, and we probably get
-         * !!! FIXME:  the ones we don't ignore wrong, too.
-         * !!! FIXME: if I had a better understanding of how the two motors
-         * !!! FIXME:  could be used in unison, perhaps I could implement other
-         * !!! FIXME:  effect types?
-         */
         /* From MSDN:
             "Note that the right motor is the high-frequency motor, the left
              motor is the low-frequency motor. They do not always need to be
              set to the same amount, as they provide different effects." */
         XINPUT_VIBRATION *vib = &effect->hweffect->vibration;
-        SDL_assert(data->type == SDL_HAPTIC_SINE);
+        SDL_assert(data->type == SDL_HAPTIC_LEFTRIGHT);
-        vib->wLeftMotorSpeed = vib->wRightMotorSpeed = data->periodic.magnitude * 2;
+        vib->wLeftMotorSpeed = data->leftright.large_magnitude;
+        vib->wRightMotorSpeed = data->leftright.small_magnitude;
         return 0;
     }
 
@@ -1333,9 +1329,9 @@ SDL_SYS_HapticRunEffect(SDL_Haptic * haptic, struct haptic_effect *effect,
 
     if (haptic->hwdata->bXInputHaptic) {
         XINPUT_VIBRATION *vib = &effect->hweffect->vibration;
-        SDL_assert(effect->effect.type == SDL_HAPTIC_SINE);  /* should catch this at higher level */
+        SDL_assert(effect->effect.type == SDL_HAPTIC_LEFTRIGHT);  /* should catch this at higher level */
         SDL_LockMutex(haptic->hwdata->mutex);
-        haptic->hwdata->stopTicks = SDL_GetTicks() + (effect->effect.periodic.length * iterations);
+        haptic->hwdata->stopTicks = SDL_GetTicks() + (effect->effect.leftright.length * iterations);
         SDL_UnlockMutex(haptic->hwdata->mutex);
         return (XINPUTSETSTATE(haptic->hwdata->userid, vib) == ERROR_SUCCESS) ? 0 : -1;
     }

test/testhaptic.c

@@ -213,6 +213,22 @@ main(int argc, char **argv)
         nefx++;
     }
 
+    /* First we'll try a SINE effect. */
+    if (supported & SDL_HAPTIC_LEFTRIGHT) {
+        printf("   effect %d: Left/Right\n", nefx);
+        efx[nefx].type = SDL_HAPTIC_LEFTRIGHT;
+        efx[nefx].leftright.length = 5000;
+        efx[nefx].leftright.large_magnitude = 0x3000;
+        efx[nefx].leftright.small_magnitude = 0xFFFF;
+        id[nefx] = SDL_HapticNewEffect(haptic, &efx[nefx]);
+        if (id[nefx] < 0) {
+            printf("UPLOADING EFFECT ERROR: %s\n", SDL_GetError());
+            abort_execution();
+        }
+        nefx++;
+    }
+
+
     printf
         ("\nNow playing effects for 5 seconds each with 1 second delay between\n");
     for (i = 0; i < nefx; i++) {
@@ -260,8 +276,9 @@ HapticPrintSupported(SDL_Haptic * haptic)
         printf("      constant\n");
     if (supported & SDL_HAPTIC_SINE)
         printf("      sine\n");
-    if (supported & SDL_HAPTIC_SQUARE)
+    /* !!! FIXME: put this back when we have more bits in 2.1 */
-        printf("      square\n");
+    /*if (supported & SDL_HAPTIC_SQUARE)
+        printf("      square\n");*/
     if (supported & SDL_HAPTIC_TRIANGLE)
         printf("      triangle\n");
     if (supported & SDL_HAPTIC_SAWTOOTHUP)
@@ -280,6 +297,8 @@ HapticPrintSupported(SDL_Haptic * haptic)
         printf("      intertia\n");
     if (supported & SDL_HAPTIC_CUSTOM)
         printf("      custom\n");
+    if (supported & SDL_HAPTIC_LEFTRIGHT)
+        printf("      left/right\n");
     printf("   Supported capabilities:\n");
     if (supported & SDL_HAPTIC_GAIN)
         printf("      gain\n");