[PATCH weston v3 3/8] compositor: If the output is inhibited, don't idle it off
Pekka Paalanen
ppaalanen at gmail.com
Thu May 26 15:01:37 UTC 2016
On Thu, 7 Apr 2016 16:44:18 -0700
Bryce Harrington <bryce at osg.samsung.com> wrote:
> Adds a helper routine weston_output_inhibited_outputs() which returns a
> mask of outputs that should inhibit screen idling.
>
> Use this routine to check for inhibiting outputs for handling of idle
> behaviors in core: In sleep mode, only halt repainting outputs that
> don't have valid inhibits. Don't send these monitors DPMS off commands
> either, if the system would otherwise be powering them down.
In sleep mode...
so we have one global idle timer that will trigger the sleep mode
compositor-wide. When that happens, you pick specific outputs to not
turn off, but still the compositor stays in sleep mode, right?
So if something removes the inhibitor from an inhibited output that was
left on, how do you trigger it going off after another idle timeout?
> Signed-off-by: Bryce Harrington <bryce at osg.samsung.com>
> ---
> src/compositor.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++-------
> src/compositor.h | 2 ++
> 2 files changed, 57 insertions(+), 7 deletions(-)
>
> diff --git a/src/compositor.c b/src/compositor.c
> index 9531a0a..8e01d38 100644
> --- a/src/compositor.c
> +++ b/src/compositor.c
> @@ -2389,15 +2389,48 @@ weston_output_schedule_repaint_reset(struct weston_output *output)
> weston_compositor_read_input, compositor);
> }
>
> +/** Retrieves a mask of outputs that should inhibit screensaving
> + *
> + * \param compositor The compositor instance.
> + * \return An output mask indicating the ids of all inhibiting outputs
> + *
> + * Checks for surfaces whose clients have requested that they
> + * disable the screenserver and display powersaving while they are
> + * 'active' (as defined by the particular shell being used). Note
> + * the output ids for these surfaces.
> + *
> + */
> +WL_EXPORT uint32_t
> +weston_output_inhibited_outputs(struct weston_compositor *compositor)
I think you meant weston_compositor_inhibited_outputs()?
The main argument is a compositor.
Hrm, "inhibited output" sounds like the output is inhibited, as in, the
output is forbidden to work. Here we mean outputs that are prevented
from turning off. We could really use a better term.
Could we use "pinned outputs"? Like you "pin" something so it is always
on or on every page or such?
"idle-inhibited output" would be the obvious choice...
> +{
> + struct weston_view *view;
> + uint32_t inhibited_outputs_mask = 0;
> +
> + wl_list_for_each(view, &compositor->view_list, link) {
> + /* Only look at views whose surfaces are considered "active" by the shell */
> + if (!view->surface->active)
This should use the weston_surface_is_active() you added in the
previous patch.
Why do you skip inactive surfaces?
Is this here the only reason to track the "active" stuff?
If a surface is inactive, like you have video player with an inhibitor,
and temporarily visited an IRC window leaving that active instead of
the video player, why should be video player not be able to inhibit
anymore? It is still visible and being watched.
Or is "active" something different than what xdg_shell calls active?
In that case, please use another word, because it is totally confusing,
and probably causing me to give lots on misguided feedback.
Was this not supposed to be tested against visibility only? That is,
surfaces that have views on the global view_list and occupy the given
output. That should suffice, no? At least for starters. If we later get
things live like thumbnail copies of windows, one should exclude those
views and only check the "main" views, but that can be dealt then.
One thing view_list + output does not cover is windows completely
occluded, but we could dig that information from the scenegraph too,
and occlusion info would be useful also for further throttling frame
callbacks.
Hmm, now I get some very vague memories about talking with you about
activeness, but I cannot remember at all how it was related to
idle-inhibition. I think you wanted it, and I explained how to get it,
but I did not question its use at the time? The active we talked about
was indeed the "active" exposed in xdg-shell.
> + continue;
> +
> + /* Does the view's surface inhibit this output? */
> + if (!view->surface->inhibit_screensaving)
> + continue;
> +
> + inhibited_outputs_mask |= view->output_mask;
> + }
> + return inhibited_outputs_mask;
> +}
> +
> static int
> output_repaint_timer_handler(void *data)
> {
> struct weston_output *output = data;
> struct weston_compositor *compositor = output->compositor;
> + uint32_t inhibited_outputs_mask = weston_output_inhibited_outputs(compositor);
>
> if (output->repaint_needed &&
> - compositor->state != WESTON_COMPOSITOR_SLEEPING &&
> compositor->state != WESTON_COMPOSITOR_OFFSCREEN &&
> + (compositor->state != WESTON_COMPOSITOR_SLEEPING
> + || inhibited_outputs_mask & (1 << output->id)) &&
> weston_output_repaint(output) == 0)
> return 0;
>
> @@ -2519,9 +2552,15 @@ weston_output_schedule_repaint(struct weston_output *output)
> {
> struct weston_compositor *compositor = output->compositor;
> struct wl_event_loop *loop;
> + uint32_t inhibited_outputs_mask = weston_output_inhibited_outputs(compositor);
>
> - if (compositor->state == WESTON_COMPOSITOR_SLEEPING ||
> - compositor->state == WESTON_COMPOSITOR_OFFSCREEN)
> + /* If we're offscreen, or if we're sleeping and the monitor
> + * isn't currently being inhibited by an active surface, then
> + * skip repainting.
> + */
> + if (compositor->state == WESTON_COMPOSITOR_OFFSCREEN ||
> + (compositor->state == WESTON_COMPOSITOR_SLEEPING &&
> + !(inhibited_outputs_mask & (1 << output->id))))
> return;
>
> if (!output->repaint_needed)
> @@ -3863,11 +3902,20 @@ static void
> weston_compositor_dpms(struct weston_compositor *compositor,
> enum dpms_enum state)
> {
> - struct weston_output *output;
> + struct weston_output *output;
> + struct weston_view *view;
> + uint32_t inhibited_outputs_mask = weston_output_inhibited_outputs(compositor);
>
> - wl_list_for_each(output, &compositor->output_list, link)
> - if (output->set_dpms)
> - output->set_dpms(output, state);
> + wl_list_for_each(output, &compositor->output_list, link) {
> + if (!output->set_dpms)
> + continue;
> +
> + /* If output is inhibited, don't do DPMS. */
> + if (inhibited_outputs_mask & (1 << output->id))
> + continue;
This has a side-effect of also inhibiting the outputs from coming out
of DPMS, as this function is also used with WESTON_DPMS_ON. So if an
output went off and became inhibited afterwards, it would never wake up?
> +
> + output->set_dpms(output, state);
> + }
> }
>
> WL_EXPORT void
> diff --git a/src/compositor.h b/src/compositor.h
> index d8d5368..98f73b6 100644
> --- a/src/compositor.h
> +++ b/src/compositor.h
> @@ -1206,6 +1206,8 @@ void
> weston_output_finish_frame(struct weston_output *output,
> const struct timespec *stamp,
> uint32_t presented_flags);
> +uint32_t
> +weston_output_inhibited_outputs(struct weston_compositor *compositor);
> void
> weston_output_schedule_repaint(struct weston_output *output);
> void
Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 811 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20160526/c1f74a28/attachment-0001.sig>
More information about the wayland-devel
mailing list