Mir

Code review comment for lp://staging/~mir-team/mir/introduce-pointer-event

Revision history for this message
Chris Halse Rogers (raof) wrote :

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;

« Back to merge proposal