On 9/9/16 4:31 AM, Bryce Harrington wrote:
Updated to include review feedback from Quentin on the v5. That
involves refinements to the destructor behavior, reorganizing patches a
bit, and noting that if the idle manager is destroyed by the client, the
individual inhibitor objects remain active. The libweston-desktop API
is renamed from surface_drop_idle_inhibitor to just drop_idle_inhibitor.
I’ve been trying to make up my mind of this series for quite some time
now, and I’m sad that we hit v6 and still not have something usable.
Let’s start with global patches comments:
1. I merged it because it was nice and standalone, so we can stop
worrying about it.
2.
3. This one seems odd to me. It patches stuff added in 2, that would be
better squashed directly. And the protocol handling code looks too
different from what it usually is (e.g. emitting the signal in the
destroy request, not in the resource destroy handler).
4. As I said in v5, this patch is wrong. It will not work if you add an
inhibitor to a popup, it will just crash (didn’t test that, but I’m
pretty confident).
5. Mostly good, except for the libweston-desktop stuff.
6. It should allow to test for popup inhibition, and subsurface too.
To get the implementation right, we first need to define what it should do.
The first question is: when is an inhibitor “active”? The answer from
the protocol file is:
“If the surface is destroyed, unmapped, becomes occluded, loses
visibility, or otherwise becomes not visually relevant for the user, the
idle inhibitor will not be honored by the compositor”
This means only visibility is taken into account. The shell has no
checks to add to the inhibitor.
In libweston, this effectively means we can just check the
weston_surface and ignore role-specific structures.
So all the *inhibitor* related code should be hidden in libweston.
Now, we want the compositor (the shell in weston’s case) to drive the
fade in/out animation.
That means we need the shell to know when an *output* is being inhibited
or not. Not individual surfaces.
I suggest a few things for the next round.
First, implement the client directly, this way we can merge it quickly
to help other compositors to run tests. (And implement subsurface/popup
testing.)
Then, reworking the output code to something like that (in pseudo-code):
output_set_on(output) {
if output.compositor.inhibitor_handler != NULL
output.compositor.inhibitor_handler(output, ON)
else
output.set_dpms(ON)
}
output_set_off(output) {
if output.inhibited || output.compositor.idle
return
if output.compositor.inhibitor_handler != NULL
output.compositor.inhibitor_handler(output, OFF)
else
output.set_dpms(OFF)
}
idle_handler(compositor) {
for output in compositor.outputs {
if compositor.idle
output_set_off(output)
else
output_set_on(output)
}
}
output_check_inhibit(output) {
was_inhibited = output.inhibited
output.inhibited = false
for v in output.views {
if v.surface.inhibit {
output.inhibited = true
break
}
}
if was_inhibited
output_set_off(output)
}
protocol_create_inhibitor(surface) {
surface.inhibit = true
for v in surface.views {
output_check_inhibit(v.output)
}
}
protocol_destroy_inhibitor(surface) {
surface.inhibit = false
for v in surface.views {
output_check_inhibit(v.output)
}
}
The shell/compositor would set the compositor.inhibitor_handler to drive
fade in/out.
Does that seem sensible to you?
Cheers,
--
Quentin “Sardem FF7” Glidic
_______________________________________________
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
