Merge lp://staging/~macslow/qtmir/fix-1475678 into lp://staging/qtmir

m_surface->visible()
this isn't doing what you'd expect. Looking into the Mir code: src/server/scene/basic_surface.cpp:

bool ms::BasicSurface::visible() const
{
    std::unique_lock<std::mutex> lk(guard);
    return visible(lk);
}

bool ms::BasicSurface::visible(std::unique_lock<std::mutex>&) const
{
    bool visible{false};
    for (auto const& info : layers)
        visible |= info.stream->has_submitted_buffer();
    return !hidden && visible;
}

This is a place where the integration of QtMir and Mir isn't so good. Mir kinda assumes that we're using its Scene, and thus its compositor. But we're not. Our Scene is defined in QML, and our compositor is the Qt scenegraph renderer.

BasicSurface::visible() defines if the surface is visible in Mir's scene. It doesn't notify the client that its surface is exposed or not.

So instead you should ignore this, and read the surface attribute to learn if the client thinks it is exposed or not.

I'm unclear why you're introducing "visibility" - do you want to distinguish QML's visible state from the client's exposure state?

I also don't see the point of an enum for it, it's just true/false.

void MirSurfaceManager::displayOff()
void MirSurfaceManager::displayOn()

I disapprove of this design. A surface manager should not care about display state. It's the shell's decision if a client surface should be marked invisible when the display is off.

Also this throws away any visibility state that the shell may have set on surfaces.

A remote desktop would be an example where we don't care about the actual display state.

> ...
> So instead you should ignore this, and read the surface attribute to learn if
> the client thinks it is exposed or not.

This has been addressed.

> I'm unclear why you're introducing "visibility" - do you want to distinguish
> QML's visible state from the client's exposure state?
>
> I also don't see the point of an enum for it, it's just true/false.

I got rid of visibility.

> void MirSurfaceManager::displayOff()
> void MirSurfaceManager::displayOn()
>
> I disapprove of this design. A surface manager should not care about display
> state. It's the shell's decision if a client surface should be marked
> invisible when the display is off.
>
> Also this throws away any visibility state that the shell may have set on
> surfaces.
>
> A remote desktop would be an example where we don't care about the actual
> display state.

displayOn()/displayOff() are just temp. methods for quicker testing of the correct working of the visible-flag propagation. They will not stay... it's a WIP-branch after all :)