Code review comment for lp:~unity-team/unity-api/add-surfacemanager-and-item

> On 01.05.2014 15:48, Michael Zanetti wrote:
> > + enum State {
> > + Unknown = mir_surface_state_unknown,
> > + Restored = mir_surface_state_restored,
> > + Minimized = mir_surface_state_minimized,
> > + Maximized = mir_surface_state_maximized,
> > + VertMaximized = mir_surface_state_vertmaximized,
> > + /* SemiMaximized = mir_surface_state_semimaximized, // see
> mircommon/mir_toolbox/common.h*/
> > + Fullscreen = mir_surface_state_fullscreen,
> > + };
>
> We've been playing with the idea of expressing "stage-ness" with those
> (since we probably want to support sidestage on desktop). I understand
> Mir probably doesn't want explicit MainStage, SideStage, but maybe
> something could be discussed?

I haven't been part of that so I might not know all details, but seems to me that Stage-ness is more related to the whole application instead of a single surface. Or do we want to allow a single Application to paint to the Main and Side stage at the same time?

>
> > + void surfaceCreated(SurfaceItemInterface* surface);
> > + void surfaceDestroyed(SurfaceItemInterface* surface);
>
> Wouldn't those be redundant to row additions/removals? What's the use
> case here?

I've added them in ApplicationManager because its really nasty to figure which one has been added/removed without having knowledge of the QModelIndex. So I thought they'd be useful here too. They'll probably change to "surfaceAdded(surfaceId)" and "surfaceRemoved(surfaceId)" as I'm pushing for allowing the QML context to create and destroy SurfaceItems as required and binding them to a surfaceId. That way we can have multiple SurfaceItems (App stack, alt+tab switcher, launcher preview etc) painting the same surface.

>
> > +protected:
> > + /// @cond
> > + QHash<int, QByteArray> m_roleNames;
>
> I know this is early, but didn't we decide that creating the hash
> dynamically in roleNames() is better, since there's no use for this
> member other than returning it from there (and only once in most cases)?

right... will change, thanks for the pointer.