On 16/07/15 13:50, Gerry Boland wrote:
>>> +++ include/unity/shell/application/MirSurfaceInterface.h
>>> All the methods on a MirSurface, you only add type to the interface? What's
>> the point of having this, if you then have another MirSurfaceInterface in
>> qtmir?
>>
>> The MirSurfaceInterface in qtmir defines the *internal* interface
>> between qtmir::MirSurface and other qtmir components, like
>> qtmir::Session, qtmir::Application, qtmir::SurfaceManager and
>> qtmir::MirSurfaceItem. They're qtmir implementation details.
>> Unity.Application users (ie, unity8) are oblivious to all of the methods
>> there. All unity8 cares about is taking a
>> unity::shell::application::MirSurface and putting it in a
>> unity::shell::application::MirSurfaceItem.
>>
>>
>>> Also I will want to be able to inspect properties of the MirSurface (size,
>> state, type, etc) to make decisions before creating a MirSurfaceItem.
>>
>> This is just a conjecture at this point and does not reflect how unity8
>> uses it currently. I'm being pragmatic, only adding the API that is
>> *actually needed*. Once unity8 actually needs all the properties you
>> mentioned in MirSurface, we just add them. No big deal. This is a
>> constantly evolving API. I'm not setting in stone that
>> unity::shell::application::MirSurface will only have a type property for
>> all eternity and it does seems odd, but that's the current reality. We
>> should keep APIs as lean as possible. The best API is no API. You have
>> to prove that you need an API before you create it and untiy8 is our proof.
> I see where you're coming from. It's a fair argument. Unity8 is our main consumer.
>
> But a position I don't want to be in is for us to require Unity8 justify every API addition to QtMir. Especially for obvious properties a mir surface will definitely have, like width/height, state. Adding these things has a small cost if done right now, but will most likely make our future lives easier.

It's like SurfaceManager being a model (list of surfaces). Yeah, it
makes sense, might be useful and all. But it's not, we never used it.
It's just dead code and I'm now finally removing it.

> I think of us getting properties added to Mir classes and how long it takes. I'd rather take the hit to make the Unity8 team's job faster.
>
> "The best API is no API." is totally untrue from an API consumer's point of view. We don't know who other future consumers might be and what they'd want, but they certainly will review an API before trying it. If it's missing obvious stuff, it's not inviting.
>
> QtMir isn't just for Unity8. If it is flexible and powerful, other shells may start using it, which would be a great win.

You run the risk of having multiple ways/APIs to achieve the same thing,
of having redundant APIs.

>>> + virtual bool consumesInput() const = 0;
>>> (Discussion) Item has an "enabled" property to stop it processing input. I
>> suspect "enabled" is a more consistent QML name than "consumesInput". If we
>> used the existing property, it has the bonus of updating activeFocus
>> correctly:
>>> http://doc.qt.io/qt-5/qquickitem.html#enabled-prop
>> I thought about doing it, but found safer to use a separate property.
>> enabled() already has several meanings and consequences and I'm not sure
>> we want all of them. Feels like overloading it too much.
> Sure, but as it's a property we inherited from QQuickItem, I was hoping to put it to use in some sensible way. Otherwise it's just sitting there being a cause of confusion.
>
>
>> Eg: a disabled item will also cause its children to not receive any
>> input. Imagine an alt-tab UI that displays live thumbnails of windows
>> and that you can click on them to foreground the respective window. With
>> the current API, you would do it like that (remember that consumesInput
>> is false by default):
>>
>> MirSurfaceItem {
>>     MouseArea {}
>> }
>>
>> But with the API you propose it would be like that instead:
>>
>> MirSurfaceItem {
>>     enabled: false
>>     MouseArea {}
>> }
> We can have it so enabled is false by default too.
>
>> And it would not work because disabled items also cause its children to
>> not receive any input. Some implementations even gray-out disabled items
>> (like buttons) to show they're inactive. But that's really not the case
>> with a MirSurfaceItem that does not forward input to the MirSurface it's
>> displaying.
> I don't really follow your point here. MirSurfaceItem is an QML Item, if I set enabled false, I would expect its children to also not get input. That's not surprising. I don't expect a visual consequence to that tho unless explicitly coded.
>
> If I have a WindowContainer which is the parent of my MirSurfaceItem, and draws window decoration around it, setting enabled:false to disable all input to it (while in Spread for example) sounds right to me. If my MirSurfaceItem has consumesInput:true, will it get the input anyway? Or not? Now I'm confused how enabled & consumesInput work together.

enabled <- whether QQuickWindow sends input to the item
consumesInput <- whether the item forwards the input it receives to the
contained surface

Those are two distinct things.