Shell wants API to listen for client pixel format changes

[Gerry Boland]

AFAICS there is no way for shell to notice if a Window's pixel format changes.

The only way I can see is on every buffer returned from generate_renderables that it checks the pixel_format() of that buffer to ensure it doesn't change.

Shell would like a nicer way of being notified of any such change. It mostly cares about if the buffer is opaque or not to do a little rendering optimisation.

Question: since a Window can have multiple renderables, must all those renderables have the same pixel format?

[Alan Griffiths (alan-griffiths)]

I can answer the question part of this: The existence of a pixel format on window is a legacy of it starting as a surface and having additional functionality added there instead of creating a window class. The Mir team is in the process of cleaning this up.

Following the cleanup it won't make sense to talk about the pixel format of a window, only of any renderables.

[Daniel van Vugt (vanvugt)]

A pixel format would only change if a window changed its streams configuration. So that's pretty uncommon. I'm not sure if we have any software which actually does that.

So it's not a pixel format change you want to observe. It's a change to a window's streams that you need to observe. That appears to be published as a "move_to" event (!?)

void ms::BasicSurface::set_streams(std::list<scene::StreamInfo> const& s)
{
    {
        std::unique_lock<std::mutex> lk(guard);
        for(auto& layer : layers)
            observers.for_each([&](std::shared_ptr<SurfaceObserver> const& observer)
            {
                layer.stream->remove_observer(observer);
            });

        layers = s;

        for(auto& layer : layers)
            observers.for_each([&](std::shared_ptr<SurfaceObserver> const& observer)
            {
                layer.stream->add_observer(observer);
            });
    }
    observers.moved_to(surface_rect.top_left);
}

That all said, I urge you to not optimise your scene based on occlusions. GL does that already, and doing it at a higher level doesn't always save time, but the added complexity does actually cost some CPU, it creates problems like bug 1666363, and could conflict with some effects in future like window previews. Be careful to not spend time optimizing things unless supported by careful analysis and/or profiling.

If you do implement some kind of occlusion testing, then it should be for the simple (and important) case only. Check if a renderable is fullscreen, !shaped() [an opaque rectangle], and untransformed, and if so call DisplayBuffer::overlay() bypassing all rendering completely. For games and benchmarks at least it is important that Unity8 supports DisplayBuffer::overlay() if not already.

[Daniel van Vugt (vanvugt)]

And yes, a window can contain any number of renderables with different pixel formats (and different opacity). A window may also contain holes where none of its renderables cover, so there isn't even a pixel format to test. So it's renderables, not windows, to use in occlusion testing.

[Daniel van Vugt (vanvugt)]

In your compositor/renderer you should be calling "mir::graphics::Renderable::shaped()", which tells you when it is something other than an opaque rectangle.

[Daniel van Vugt (vanvugt)]

Incomplete.

Isn't the only way pixel formats might change for a window a 'streams' change? That is a general scene change, which you're already notified of.

Nothing to fix?

[Gerry Boland (gerboland)]

move_to event? Okay

I'll have to read the Mir code to decide if that is all I need.

@duflu btw I'm not trying to do any fancy occlusion culling, I just need to know if the surface needs blending enabled or not. As you say, I trust GL to optimize overdraw.

[Daniel van Vugt (vanvugt)]

Observing move_to probably is not the right idea. The important thing is that a pixel format can change in theory on any frame. And the right answer is to check on every frame. The name is not great but we provide you that flag in Renderable:
    virtual bool shaped() const = 0; // meaning the pixel format has alpha

Although I keep thinking this approach is insufficient. We fail to distinguish between a surface having an alpha channel on purpose, and having one by accident (e.g. bug 1667577).

In the spirit of glEnable(GL_BLEND), I think we need an extra flag that clients must set in order for them to get blended. That way the case of clients getting an alpha channel unwittingly and failing to initialize it correctly is solved. Although you could argue that's just an app or toolkit bug.

[Daniel van Vugt (vanvugt)]

In case you hadn't already found it, Mir's (correct?) default blending implementation is in the function "mrg::Renderer::draw".

[Gerry Boland (gerboland)]

> In the spirit of glEnable(GL_BLEND), I think we need an extra flag
> that clients must set in order for them to get blended.
Well I was working under the assumption that MirPixelFormat is for that, and I have this working. If a Qt client requests an opaque EGL surface but get one with an alpha channel, the client still specifies that the surface MirPixelFormat has no alpha. In that case Unity8 will not enable blending while compositing that surface.

Back on this topic though:
I'm still confused. Mir clients set the pixel format for the window with

mir_window_spec_set_pixel_format(MirWindowSpec* spec, MirPixelFormat format)

But I do see that clients can allocate pixel buffer(s) with a possibly different pixel format:

void mir_connection_allocate_buffer(
    MirConnection* connection,
    int width, int height,
    MirPixelFormat format,
    MirBufferCallback available_callback, void* available_context);

Android buffers can also have their own pixel formats. And presumably GBM?

So is the buffer pixel format the only value of pixel format I can actually trust? And therefore the window pixel format is useless?

[Kevin DuBois (kdub)]

mir_window_spec_set_pixel_format is being deprecated, and only currently applies to the MirBufferStream auto-created with MirSurfaceSpec.

In the new 1.0 world, the MirSurfaceSpec is given MirRenderSurfaces, which have MirBuffers with pixel format.

[Daniel van Vugt (vanvugt)]

I agree with Kevin's statement but it appears that disagrees with the documentation in the Mir headers. Seems the final correct client function for setting pixel format (of software streams) doesn't yet exist. As for hardware clients, they don't choose a pixel format at all in the new world. They just call eglChooseConfig with or without EGL_ALPHA_SIZE.

In summary:
 * Client functions for selecting an alpha channel are changing/deprecating and the final function may not even exist yet.
 * The correct server function for testing if a scene element/renderable needs blending is 'Renderable::shaped()'.

[Daniel van Vugt (vanvugt)]

Isn't this bug invalid (comment #5)?

There's nothing to fix(?)

[Gerry Boland (gerboland)]

With the knowledge that mir_window_spec_set_pixel_format is being deprecated, this request is therefore invalid.