Mir

Code review comment for lp:~vanvugt/mir/parenting-client-api

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

I've just reviewed the Unity design doc with the misleading title "MIR: Surface Management v0.2" (an issue I've raised before because it's actually the design of Unity8, not Mir). It outlines 8 surface types/roles and 7 of them can have parents. Some required and some optional parenting.

So despite the fact I said I was going to look for a compromise, the design doc actually supports this current proposal rather strongly. Certainly we don't want to write 7 different set_parent client functions. Best case then is you've made the API 7 times more complex and still strictly support only one shell. More client functions might then be required for other shells or other surface types. It doesn't scale.

The design docs also detail complex state transitions and all the various combinations allowed. It's very complex for both surface_state and surface_type combinations, in concert with surface position and size etc. Some of these attributes whose consistency needs to be maintained for valid states exist only on the server side. So the client API can't enforce everything by itself obviously. You always have to "ask" the shell if any setting is allowed or has been accepted.

State validity and transitions will indeed be implemented on the server side because they're defined by the shell. There might be some sane defaults (e.g. Set state_fullscreen results in a resize), however the shell will have the power to wrap any state changes to guarantee its own desired semantics. And semantics may change at runtime; e.g. Unity8 in the phone form-factor only supports the "maximized" state for apps.

Back on the "menu" topic, I've proposed that as a real surface type today, separately. However since "menu" is only one of the seven Unity surface types that need parenting, and only some of those use parenting for relative positioning, it's clear we need a generic mir_surface_set_parent as a minimum.

« Back to merge proposal