MirMotionEvent.action needs stronger typing (to MirMotionAction etc)

[Daniel d'Andrada]

MirMotionEvent.action is currently an int for legacy android compatibility reasons. We need to clean this up and use MirMotionAction properly, unlike in examples...

MirMotionAction action = static_cast<MirMotionAction>(event.motion.action & ~0xff00);

OLD DESCRIPTION:
MirEvent::action is currently effectively an opaque value, as Mir headers do not specify its meaning.

So event.h must either define counterparts to the android AMOTION_EVENT_ACTION_* values below or split up the action and pointer index into separate variables.

"""
/* Bit shift for the action bits holding the pointer index as
 * defined by AMOTION_EVENT_ACTION_POINTER_INDEX_MASK.
 */
#define AMOTION_EVENT_ACTION_POINTER_INDEX_SHIFT 8

enum {
    /* Bit mask of the parts of the action code that are the action itself.
     */
    AMOTION_EVENT_ACTION_MASK = 0xff,

    /* Bits in the action code that represent a pointer index, used with
     * AMOTION_EVENT_ACTION_POINTER_DOWN and AMOTION_EVENT_ACTION_POINTER_UP. Shifting
     * down by AMOTION_EVENT_ACTION_POINTER_INDEX_SHIFT provides the actual pointer
     * index where the data for the pointer going up or down can be found.
     */
    AMOTION_EVENT_ACTION_POINTER_INDEX_MASK = 0xff00,
"""

Right now in the "Qt compositor" code I had to resort to locally defining those values, which is a hack:

""
// from android-input AMOTION_EVENT_ACTION_*, hidden inside mir bowels
// mir headers should define them
const int QtEventFeeder::MirEventActionMask = 0xff;
const int QtEventFeeder::MirEventActionPointerIndexMask = 0xff00;
const int QtEventFeeder::MirEventActionPointerIndexShift = 8;
""

[Brandon Schaefer (brandontschaefer)]

Just a comment in mir_toolkit/event.h

    /*
     * TODO(racarr): We would like to store this as a MirMotionAction but the android input stack
     * encodes some non enumerable values in it. It's convenient to keep things
     * this way for now until we can drop SF/Hybris support in QtUbuntu.
     */
    int action;

[Daniel van Vugt (vanvugt)]

Part of the problem description is incorrect. It does have a local definition: MirMotionAction

And a while ago "action" was defined as a MirMotionAction. However for compatibility reasons only racarr changed it to an int. You can still use MirMotionAction so long as you remember the compatibility hack like in examples/demo-shell/window_manager.cpp:

         // FIXME: https://bugs.launchpad.net/mir/+bug/1197108
        MirMotionAction action = static_cast<MirMotionAction>(event.motion.action & ~0xff00);

But obviously we need to retire such hacks. So it's a bug.

[Sylvain Becker (sylvain-becker)]

FYI

http://developer.android.com/reference/android/view/MotionEvent.html

you should handle
ACTION_CANCEL like ACTION_UP for all FingerId
ACTION_UP, ACTION_DOWN
POINTER_ACTION_UP and POINTER_ACTION_DOWN requires the use of getActionIndex()

mir/3rd_party/android-input/android/frameworks/base/include/androidfw/Input.h :
324 inline int32_t getActionMasked() const { return mAction & AMOTION_EVENT_ACTION_MASK; }
325
326 inline int32_t getActionIndex() const {
327 return (mAction & AMOTION_EVENT_ACTION_POINTER_INDEX_MASK)
328 >> AMOTION_EVENT_ACTION_POINTER_INDEX_SHIFT;
329 }

[Daniel van Vugt (vanvugt)]

Yes, the Android approach obviously works. Although the Mir API is free to be different and I think we would like to avoid masking in Mirs APIs.

[Robert Carr (robertcarr)]

In progress as part of MirEvent 2.0 (a.k.a. MirInputEvent)

[Daniel van Vugt (vanvugt)]

mir (0.10.0+15.04.20150107.2-0ubuntu1) vivid; urgency=medium