Client display configuration notification

[Alexandros Frantzis]

On Wed, Sep 30, 2015 at 12:50:14PM +0300, Alexandros Frantzis wrote:
> On Wed, Sep 30, 2015 at 02:04:00PM +1000, Christopher James Halse Rogers wrote:
> > Forked from review¹.
> > 
> > I think we're currently handling display configuration events incorrectly,
> > or at least in a way that will be surprising to clients.
> 
> For a bit of background, the current scheme is:
> 
> 1. User changes the hardware config (e.g. adds/removes a monitor)
> 2. Update base config
> 3.1 If base config was applied before then apply the new base config
> 3.2 If per-session config was applied, do nothing, expect the client
>     to respond to the configuration_changed event by setting a new session
>     config based on the updated base config.
> 
> The reason for "do nothing" in 3.2 is to avoid applying the base
> configuration before the client has applied its own new client config,
> in order to avoid extra flickering.
> 
> > When the hardware configuration changes we send the new configuration to all
> > clients. However, if a client has specified a different display
> > configuration then we *don't* change it. The upshot is that
> > mir_connection_create_display_configuration() will *not* return the
> > currently active display configuration.

A few additional thoughts and realizations after investigating LP
#1498021 ([1]) and pondering on the current code some more.

mir_connection_create_display_configuration() was originally created
with a single purpose in mind, which was to allow clients to get a valid
configuration on which to base their own custom configurations. It was
not intended to give back either the current session config or the
active config. The only guarantee was that it accurately represented
the hardware state (connected monitors and available modes). The base config
was used for this purpose although we could have used the more bare
configuration as returned by the mg::Display class.

Until recently, MediatingDisplayChanger honored this purpose by sending
new configurations to clients only as a response to hardware changes.
This was enough (sans bugs) for the use cases we had until then, which
involved clients responding to configuration changes from the server.

However, since then we have made changes that try to turn the value
returned by mir_connection_create_display_configuration() to something
it wasn't designed to be, i.e., an accurate representation of the
current state.  These changes, as Alan mentioned in an previous email,
have conflated the various types of display configurations we have
in Mir (base, active, session). 

A few questions and my take on them:

1. Which configuration types and changes to them does a normal
   (non-modesetting) client need to know about?

None of the types are particularly useful.

Perhaps it would be useful for the client to know about the active
config (e.g. when another session with a custom config has the focus),
but I can't think of any interesting use cases for this. I am all ears,
though.

2. Which configuration types and changes to them does a
   modesetting client need to know about?

In order for a client to set a custom config it needs at least a config
that accurately represents the hardware state. The base config is useful
if we want to allow clients to take it into account when setting a
custom config, e.g., don't turn a screen on if it's off in the base
config.

Again I don't see an interesting use case for following the active config.

3. Which configuration types and changes does the display
   settings app need to know about
   modesetting) client need to know about?

It needs to be able to get and set the current base configuration.


So, my take is that the only config that clients need to be notified
about is the base one. Clients also need to be able to set the session
and base configs (subject to permission restrictions):

// The same as the current mir_connection_create_display_configuration
// with a more accurate name and guaranteed to always return the base config,
// not just a valid hardware config
mir_connection_create_base_display_config() 

// Set base configuration, used only be display settings app
mir_connection_apply_base_display_config()

// Set session configuration
mir_connection_apply_display_config()

The display config change notification callback will notify only about
base config changes (like it did before the recent changes).

If we decide we also want active configs too, we need to
differentiate between the two kinds in notifications and add
mir_connection_create_active_display_config().

Thanks,
Alexandros