Code review comment for lp:~robertcarr/mir/client-focus-notifications

>> 1. Build failure:

Merged trunk

>>2._enum_mx_
mir_surface_attrib_enum_max_ is very literally the maximum valid member of the mir surface attribute enumeration :p. Chris requested this change.

3.
>> mir_surface_attrib_focus should not exist; focus is not a per-surface attribute. For reasons I have mentioned several >> times since June.

You keep saying this but haven't proposed an alternative architecture.

>> Focus is per input device (for non-pointing devices: keyboard, voice...). Though could probably be simplified to >> one focus per session. Something like: SurfaceID Session::focussed_surface() const;

Actually the design docs talk about input focus as a sort of separate concept. There is certainly some notion of a surface having a focused attribute:

1. Clients need to alter their presentation based on whether they are focused, i.e. blinking cursor. It seems like this is in some sense a 'property of the surface', i.e. has some sort of input focus.
2. Which surface does the HUD act on? This has nothing to do with pointing devices.
3. Which surface does the global menu bar display from?

I think it's likely that focus will require more refinement for some multi window, multi input device, multi monitor, etc scenarios on the desktop. So far, the design documents have talked more about a hierarchy of focus though than modelling focus only as per input device. I think these requirements are unclear and trying to over generalize the system now is fruitless. On the other hand the requirements for the phone and system compositor are very clear and satisfied here.