Merge lp://staging/~mir-team/mir/introduce-pointer-event into lp://staging/mir

FAILED: Continuous integration, rev:2194
http://jenkins.qa.ubuntu.com/job/mir-ci/2574/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/739
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/739/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/704/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/571/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/704/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/2574/rebuild

FAILED: Continuous integration, rev:2196
http://jenkins.qa.ubuntu.com/job/mir-ci/2576/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/742
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/742
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/706
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/573/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/706
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/706/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/3837
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/16847

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/2576/rebuild

PASSED: Continuous integration, rev:2197
http://jenkins.qa.ubuntu.com/job/mir-ci/2579/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/745
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/745
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/709
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/576
        deb: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/576/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/709
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/709/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/3840
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/16853

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/2579/rebuild

needs fixing:
Shouldn't we have the new symbol be in its own stanza in the symbols.map for the client library?

needs info:
With MirPointerInputEventButtons, should we provide a flexible number of buttons?

>> Shouldn't we have the new symbol be in its own stanza in the symbols.map for the client library?

Ah...I didn't know about that. How does this work with the global: * catch in the MIR_CLIENT_8 stanza?

Updated anyway :)

>> With MirPointerInputEventButtons, should we provide a flexible number of buttons?

I've been thinking that once the device introspection API is available it will also be possible to obtain the identifiers for more buttons via that, then you can use the introspection API to query their names etc.

MirPointerInputEventButtons is a bit inflexible though so I've changed

MirPointerInputEventButtons mir_pointer_input_event_get_button_state(MirPointerEvent)

To

bool mir_pointer_input_event_get_button_state(MirPointerEvent, MirPointerInputEventButton)

This way the button enum is not using flags and it will be easier to expand/use other values in the future.

This way the pointer button enum doesnt have to be flags and wi to expand as time goes on.

FAILED: Continuous integration, rev:2199
http://jenkins.qa.ubuntu.com/job/mir-ci/2590/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/762
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/762/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/724
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/587
        deb: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/587/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/724
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/724/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/3854
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/16897

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/2590/rebuild

lgtm then

PASSED: Continuous integration, rev:2200
http://jenkins.qa.ubuntu.com/job/mir-ci/2602/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/778
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/778
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/740
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/599
        deb: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/599/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/740
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/740/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/3869
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/16949

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/2602/rebuild

This distinction is confusing:

306 +// Differentiate between MirTouchInputEvents and MirPointerInputEvents based on old device class
307 +MirInputEventType type_from_device_class(int32_t source_class)
308 +{
309 + switch (source_class)
310 + {
311 + case AINPUT_SOURCE_MOUSE:
312 + case AINPUT_SOURCE_TRACKBALL:
313 + case AINPUT_SOURCE_TOUCHPAD:
314 + return mir_input_event_type_pointer;
315 + // Realistically touch events should only come from Stylus and Touchscreen
316 + // device classes...practically its not clear this is a safe assumption.
317 + default:
318 + return mir_input_event_type_touch;
319 + }
320 +}

If "pointer" events don't include the pointiest kind of input (stylus, finger) then we need to clarify the terminology. Perhaps:
   MirAbsoluteInputEvent (stylus, finger)
   MirRelativeInputEvent (mouse, trackball, touchpad)
   MirFocussedInputEvent (keyboard and other buttons)

>> If "pointer" events don't include the pointiest kind of input (stylus, finger)
>> then we need to clarify the terminology

Pointer refers to the system pointer/cursor rather than the "pointiness of the input".

This MP does leave some open questions....e.g. what becomes of joystick events, or how would you get access to raw relative events from a mouse?

This is to be addressed in future MPs through the distinction of raw and synthetic events. For mice, joysticks, etc, the raw event would feature the relative axis. In the case that the relative axis device is controlling the system pointer, a Pointer event will also be generated with a "synthesized_from" member referring to the raw event.

Ah OK. That does kind of clarify another question I had --
Do we mean something that's a physical pointer or something that results in a mouse arrow cursor on screen? I think "pointer" means the latter here.

It sounds like MirRelativeInputEvent might still be more appropriate. At the raw level, they are indeed relative input devices. Only when "cooked" do such devices gain a logical pointing location on screen.

Maybe to avoid confusion over different meanings of pointer, we should say the source of the input is a MirRelativeInputEvent, and that /moves/ a cursor. The ambiguous word "pointer" doesn't need to come into it.

Similarly, a MirAbsoluteInputEvent (digitizer pen) might also move a cursor, in absolute terms (LP: #1396421)

133 +/* Relative axis containing the speed of the vertical scroll wheel */
134 + mir_pointer_input_axis_vscroll = 2,
135 +/* Relative axis containing the speed of the horizontal scroll wheel */

speed -> change?

looks good to me otherwise

>> speed -> change?

Hey! You're back. Happy new years...we have some fun to have on evdev-input-platform ;)

Hmm. I agree its kind of confusing.

I think speed may be correct because change makes it seem like you are talking about change in the axis state over time. Imagine you are scrolling up and receive a +7...then you reverse the direction of your scroll...at one point you will hit +0 which has to be interpreted as a current scroll speed of zero, not a change in the scroll axis state over time (because the change would be zero). Likewise if this entire reveral of speed (say from +7 to -3) happens in a single input frame, -3 is not your change (which would be 10) but rather just your new negative speed.

I have this sinking feeling that I am turning things over wrong in my head though so will continue to think some today.

>>
>> It sounds like MirRelativeInputEvent might still be more appropriate. At the raw level, they are
>> indeed relative input devices. Only when "cooked" do such devices gain a logical pointing location
>> on screen.

Yes. Though I am imagining there will only be one MirRawInputEvent type, and the user can introspect whether a given axis is relative or absolute. It seems to be a good strategy to report input in frames which contain snapshots of device state...so it should be easy to mix relative and absolute axis on a single event (e.g. input frame).

>> Maybe to avoid confusion over different meanings of pointer, we should say the source of the input >> is a MirRelativeInputEvent, and that /moves/ a cursor. The ambiguous word "pointer" doesn't need to
>> come into it.

We are close to agreement :). In this model the source is a raw input event with relative axis, and a pointer event is synthesized from it. I don't think pointer is particularly ambiguous and there is a long history of using "pointer" or "cursor" to refer to the onscreen system....pointer/cursor. I don't think this meaning is diluted by the fact that other things have points.

>> Similarly, a MirAbsoluteInputEvent (digitizer pen) might also move a cursor, in absolute terms
>> (LP: #1396421)

Definitely you could enable this via the configuration API. In this case you would get raw absolute axis events, and rather than (in addition to?) a synthetic touch event would receive a synthetic pointer event.

Globally: how difficult would it be to use a 24.8 fixed-point scheme? We do want subpixel motion precision, but we don't need all that much precision.

+ /* Axis values or button state has changed for the pointer */
122 + mir_pointer_input_event_action_change = 4

This appears to be a lie¹? It seems to only be for motion; perhaps it should be mir_pointer_input_event_motion?

I think mir_pointer_input_event_action_up/down might be better as mir_pointer_input_event_action_*button*_up/down? It doesn't make sense for a pointer to be up down.

Additionally, there doesn't seem to be a way to figure out *which* button went up/down? You can get the current state through get_button_state, but unless the client maintains the previous button states it can't work out what button changed.

Likewise, we can get the current axis values but not their changes. It would seem sensible for the client to be able to know which ones have changed?

133 +/* Relative axis containing the speed of the vertical scroll wheel */
134 + mir_pointer_input_axis_vscroll = 2,

Are we really reporting the time-derivative of the scroll axis values? Why? We don't get that from the kernel; we get the values themselves.

input_axis_vscroll and input_axis_hscroll should be relative axes. (Most) scroll wheels aren't joysticks; you can just keep spinning them :).

¹: Relevant bit is:
357 +MirPointerInputEventAction mir_pointer_input_event_get_action(MirPointerInputEvent const* pev)
...
373 + case mir_motion_action_move:
374 + case mir_motion_action_hover_move:
375 + case mir_motion_action_outside:
376 + default:
377 + return mir_pointer_input_event_action_change;

Mark as needs-fixing, so it gets on my review list.

I think the ease with which I found the word "pointer" confusing suggests I won't be the last one. We have an opportunity to choose less ambiguous terminology now. Probably not so much later.

Chris:
I don't think fixed point is necessarily better than floats for input. If you have fixed 24.8 (32-bit) then to avoid overflows you need to remember to always work with 64-bit numbers. And of course, you need to shift in the end to get to your whole numbers. OK, yes there's a performance advantage in doing lots of ops with ints instead of floats, but not as great as it used to be. Can anyone cite a specific case for input where sub-pixel precision is important?...

"Needs Fixing" because I think we should seize the opportunity to avoid introducing confusing wording with "pointer". Less ambiguous alternatives mentioned above.

> >> speed -> change?
>
> Hey! You're back. Happy new years...we have some fun to have on evdev-input-
> platform ;)
>
> Hmm. I agree its kind of confusing.
>
> I think speed may be correct because change makes it seem like you are talking
> about change in the axis state over time. Imagine you are scrolling up and
> receive a +7...then you reverse the direction of your scroll...at one point
> you will hit +0 which has to be interpreted as a current scroll speed of zero,
> not a change in the scroll axis state over time (because the change would be
> zero). Likewise if this entire reveral of speed (say from +7 to -3) happens in
> a single input frame, -3 is not your change (which would be 10) but rather
> just your new negative speed.
>
> I have this sinking feeling that I am turning things over wrong in my head
> though so will continue to think some today.

Yes in the example above I expect to find a change expressed as the value "-10" attached to the event. I believe you are exposing relative changes along the vertical and horizontal axis.
I do not think that you plan to measure the velocity here.. and expect users to integrate speed change events over time diffs, to get to the amount of ticks scrolled.

So I really think that you mean something like a delta expressed in ticks. How about "ticks"?

Fixed vscroll and hscroll comments...I got in to a weird state of confusion imagining spinning scroll balls ;)

Chris other comments:

Updated the actions now button_up and button_down and change...documentation should likewise be more accurate.

Chris: With respect to previous button and axis state I agree including it would be useful. Until we change the internal MirEvent representaiton it's difficult though. In the meantime clients will have to track the previous button state (just as they do now). Following the transition, we can add backwards compatible new functions to query the collection of changed axis and buttons.

PASSED: Continuous integration, rev:2204
http://jenkins.qa.ubuntu.com/job/mir-ci/2652/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/858
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/858
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/820
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/649
        deb: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/649/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/820
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/820/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/3938
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/17088

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/2652/rebuild

On Wed, Jan 14, 2015 at 6:35 PM, Daniel van Vugt
<email address hidden> wrote:
> I think the ease with which I found the word "pointer" confusing
> suggests I won't be the last one. We have an opportunity to choose
> less ambiguous terminology now. Probably not so much later.

I don't think we have a better candidate terminology available yet?

The thing is, pointer events *don't* come from any physical input
device; they come from a synthesised virtual pointer.

It shouldn't be called MirRelativeInputEvent because the (the x,y axes
of the event) event is absolute (and the vscroll, hscroll axes are
relative ☺).

And, as you note, absolute devices like the stylus on a tablet *can*
produce MirPointerInputEvents; it would be pretty weird if they
produced RelativeInputEvents.

We *could* have a different model; make everything MirInputDeviceEvent
and then have a special “device 0” virtual pointer device. I'm not
sure if that's any better, though.

>
> Chris:
> I don't think fixed point is necessarily better than floats for
> input. If you have fixed 24.8 (32-bit) then to avoid overflows you
> need to remember to always work with 64-bit numbers. And of course,
> you need to shift in the end to get to your whole numbers. OK, yes
> there's a performance advantage in doing lots of ops with ints
> instead of floats, but not as great as it used to be. Can anyone cite
> a specific case for input where sub-pixel precision is important?...

Sub-pixel precision is important for two cases:

For relative motion events it's flat out necessary - otherwise, it's
possible to move the mouse at a slow but reasonable speed without
generating any non-zero input events.

For absolute events it's necessary if you're doing any sort of
transformation.

Hm. I was suggesting fixed point in part because that's what we get out
of libinput, but it seems that libinput switched to double since I last
looked at it.

Since float will still give us 8bits of subpixel precision all the way
out to 65,000x65,000 pixel outputs that might be enough. The reasons
why we might want doubles include: that's what we get from libinput,
and it gives nested servers some headroom.

If we need to we can double later.

Hm. Rather than “ticks” I think it would be better to describe the scroll axes as measured in pixels. The other axes are measured in pixels, and pixels are a useful unit for scrolling.

Otherwise, ok.

LGTM.

Alright, I trust all confusion will be resolved later. When we finish designing the client API ;)

OK