Mir

Code review comment for lp:~alan-griffiths/mir/first-pass-of-surface-spec-modification

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

This looks mostly sensible, but does mean that we need a separate API for introspecting the state of surfaces. IIRC the longer-term plan was to move all the surface introspection into MirSurfaceSpec accessors and deprecate things such as MirSurfaceParameters.

That said, now that I think of it some more, I think being able to get a SurfaceSpec from an existing surface and apply it would probably a misfeature, as the possibility for clients to accidentally override user changes would be high.

Hm. If we need to support morphing from different surface types - dialog to normal, for example - we'll need to have a mir_surface_spec_set_type and this will be somewhat redundant with the mir_connection_surface_spec_for_* family of functions. A client would be able to mir_create_spec_for_changes(), fill in all the same details as mir_connection_surface_spec_for_normal(), and then pass it to mir_surface_create() - whereupon they'd somewhat unexpectedly get an abort(), because the MirConnection isn't set.

I'm not sure what I think about this. On the one hand, it might be nice to not have the same type for both. On the other, that's obvious duplication. I guess I lean towards the former; one type with perhaps surprising behaviour.

17 +mir_connection_create_spec_for_changes(MirConnection* connection);
Why does this take a MirConnection? It's not used for anything, and it doesn't seem likely that it'll be used for anything in the future.

review: Needs Information

« Back to merge proposal