Store the output presentation timestamp as struct timespec.
This commit is part of a larger effort to transition the Weston codebase
to struct timespec.
Signed-off-by: Alexandros Frantzis <alexandros.frantzis@collabora.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Change code related to animations to use struct timespec to represent
time.
This commit is part of a larger effort to transition the Weston codebase
to struct timespec.
Signed-off-by: Alexandros Frantzis <alexandros.frantzis@collabora.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
This bumps the libweston major version due to breakage in the animation
ABI. The commits following this one break more ABI in other parts.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Check and ensure that a compositor can only load one backend
successfully. If a backend fails to load, it is theoretically possible
to try another backend. Once loading succeeds, only destroying the
compositor would allow "unloading" a backend.
If backend init fail, ensure the backend pointer remains NULL to avoid
calling into a half-loaded backend on e.g. compositor destruction.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Ian Ray <ian.ray@ge.com>
Acked-by Daniel Stone <daniels@collabora.com>
'release' is a more appropriate name because the function does not free
the underlying memory. The main reason for this is that we need the name
weston_output_destroy() for new API that actually will free also the
underlying memory.
Since the function is only used in backends and external backends are
not a thing, this does not cause libweston major version bump, even
though it does change the ABI. There is no way external users could have
successfully used this function.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Ian Ray <ian.ray@ge.com>
Acked-by Daniel Stone <daniels@collabora.com>
A client may have bound the same wl_output multiple times, for who knows
what reason. As the server cannot know which wl_output resource to use
for which wl_surface, send enter/leave events for all of them.
This is a protocol correctness fix.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Ian Ray <ian.ray@ge.com>
Acked-by Daniel Stone <daniels@collabora.com>
Add 'name' argument to weston_output_init(). This is much more obvious
than the assert inside weston_output_init() to ensure the caller has set
a field in weston_output first.
Now weston_output_init() will strdup() the name itself, which means we
can drop a whole bunch of strdup()s in the backends. This matches
weston_output_destroy() which was already calling free() on the name.
All backends are slightly reordered to call weston_output_init() before
accessing any fields of weston_output, except the Wayland backend which
would make it a little awkward to do it in this patch. Mind, that
weston_output_init() still does not reset the struct to zero - it is
presumed the caller has done it, since weston_output is embedded in the
backend output structs.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Ian Ray <ian.ray@ge.com>
Reviewed-by: David Fort <contact@hardening-consulting.com>
[Daniel: document name copying]
Acked-by Daniel Stone <daniels@collabora.com>
Initialize the list in weston_output_init() instead of doing it
separately in each backend.
One would expect weston_output_init() to initialize all weston_output
members, at least those that are not NULL.
We rely on the set_size() functions to be called only once, as is
assert()'d. If set_size() becomes callable multiple times, this patch
will force them to be fixed to properly manage the mode list instead of
losing all members.
compositor-wayland.c is strange in
wayland_output_create_for_parent_output(): it first called
wayland_output_set_size() that initialized the mode list with a single
mode manufactured from width and height and set that mode as current.
Then it continued to reset the mode list and adding the list of modes
from the parent output, leaving the current mode left to point to a mode
struct that is no longer in the mode list and with a broken 'link'
element. This patch changes things such that the manufactured mode is
left in the list, and the parent mode list is added. This is probably
not quite right either.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Emre Ucan <eucan@de.adit-jv.com>
Reviewed-by: Ian Ray <ian.ray@ge.com>
Acked-by Daniel Stone <daniels@collabora.com>
This change replaces a queued emission of buffer-release events (which
is prone to starvation) with a regular event emission. This means that
client programs no longer need to secretly install surface frame
listeners just to guarantee that they get correctly notified of buffer
lifecycle events.
v2:
More information about the historical reasons why this change hadn't
happened yet, and the consensus to finally move ahead with it can be
found at the discussion terminating in this message:
https://lists.freedesktop.org/archives/wayland-devel/2017-September/035147.html
Signed-off-by: Matt Hoosier <matt.hoosier@gmail.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Derek Foreman <derekf@osg.samsung.com>
This is a simple wrapper for casting the user data of a wl_resource into
a struct weston_output pointer. Using the wrapper clearly marks all the
places where a wl_output protocol object is used.
Replace ALL wl_output related calls to wl_resource_get_user_data() with
a call to weston_output_from_resource().
v2: add type assert in weston_output_from_resource().
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Armin Krezović <krezovic.armin@gmail.com>
It's a little awkward to try to keep the weston_output::region and
weston_output::previous_damage allocate exactly only when the output is
enabled. There was also a leak: weston_output_move() was calling
weston_output_init_geometry() on an already allocated regions without
fini in between.
Fix both issues by allocating the regions in weston_output_init(),
always fini/init'ing them in weston_output_init_geometry(), and fini'ing
for good in weston_output_destroy().
This nicely gets rid of weston_output_enable_undo() so I do not need to
try to figure out what to do with it later.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Armin Krezović <krezovic.armin@gmail.com>
Move the wl_output global management into weston_compositor_add_output()
and weston_compositor_remove_output().
If weston_output_enable() fails, there is no need to clean up the global
and the clients will not see a wl_output come and go.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Armin Krezović <krezovic.armin@gmail.com>
Move the output id management into weston_compositor_add_output() and
weston_compositor_remove_output(). This is a more logical place, and
works towards assimilating weston_output_enable_undo().
The output id is no longer available to the backend enable() vfuncs, but
it was not used there to begin with.
v2: moved assert earlier in weston_compositor_add_output()
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Armin Krezović <krezovic.armin@gmail.com>
Enabling an already enabled output is an error, at least with the
current implementation.
However, disabling an output that has not been enabled is ok.
Cope with the first and document the second.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Armin Krezović <krezovic.armin@gmail.com>
It was ambiguous what this flag meant - it did not mean whether the
backend is considering this output to be enabled, because
weston_output_destroy() unsets it while deliberately not calling the
backend disable() vfunc.
Perhaps the most clear definition is with respect to the output's
assignment in the pending vs. enabled output lists. There is also a whole
bunch of variables that are allocated only when enabled is true.
Since the flag is related to the list membership, set and clear the flag
only when manipulating the lists.
Assert that weston_compositor_add_output() and
weston_compositor_remove_output() are not called in a wrong state.
v2:
- talk about "list of enabled outputs"
- clear 'enabled' in weston_compositor_remove_output() earlier
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Armin Krezović <krezovic.armin@gmail.com>
weston_compositor_add_pending_output() is the point through which all
backends must go when creating a new output. The enable and disable
vfuns are essential for anything to be done with the output, so it makes
sense to check them here, rather than when actually enabling or
disabling.
Particularly the disable vfunc is rarely called, so this gets the check
better excercised.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Armin Krezović <krezovic.armin@gmail.com>
Only used internally in core. Needs to happen automatically when
something changes, so there should no need to call it from outside.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Armin Krezović <krezovic.armin@gmail.com>
Only used by weston_output_enable().
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Armin Krezović <krezovic.armin@gmail.com>
Document two more functions of the weston_output API.
Exported functions marked internal are meant for backends only.
Exported functions not marked internal are meant for libweston users.
v2: talk about "list of enabled outputs".
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Armin Krezović <krezovic.armin@gmail.com>
A weston_output available to the compositor should always be either in
the list of pending outputs or the list of enabled outputs. Let
weston_compositor_add_output() and weston_compositor_remove_output()
handle the moves between the lists.
This way weston_output_enable() does not need to remove and
oops-it-failed-add-it-back. weston_output_disable() does not need to
manually re-add the output back to the pending list.
To make everything nicely symmetric and fix any unbalancing caused by
this:
- weston_output_destroy() explicitly wl_list_remove()s
- weston_compositor_add_pending_output() first removes then inserts, as
we have the assumption that the link is always valid, even if empty.
Update the documentations, too.
v2:
- talk about "list of enabled outputs"
- keep wl_list_remove in weston_compositor_remove_output in its old
place
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Armin Krezović <krezovic.armin@gmail.com>
Trying to make it more readable. Things that happen in the same step are
kept in the same paragraph.
v2: talk about "list of enabled outputs"
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Armin Krezović <krezovic.armin@gmail.com>
At the bottom of weston_output_finish_frame(), code exists to account
for flips which have missed the repaint window, by shifting them to lock
on to the next repaint window rather than repainting immediately.
This code only accounted for flips which missed their target by one
repaint window. If they miss by multiples of the repaint window, adjust
them until the next repaint timestamp is in the future. This will only
happen in fairly extreme situations, such as Weston being scheduled out
for a punitively long period of time. Nevertheless, try to help recovery
by still aiming for more predictable timings.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reorder some paragraphs to be more logically ordered. Rewrite the
description of the backend-specific disable function to explain the
semantics instead of the mechanics. Remove the paragraph about
pending_output_list as unnecessary details.
Add a big fat comment on why we call output->disable() always instead of
only for actually enabled outputs.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Armin Krezović <krezovic.armin@gmail.com>
Implement new repaint_begin and repaint_flush hooks inside
weston_backend, allowing backends to gang together repaints which
trigger at the same time.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
In preparation for grouping output repaint together where possible,
switch the per-output repaint timer, to a global timer which iterates
across all outputs.
This is implemented by storing the absolute time for the next repaint
for each output locally, and maintaining a global timer which iterates
all of them, scheduling the repaint for the first available time.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Cc: Mario Kleiner <mario.kleiner.de@gmail.com>
Cc: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
[Pekka: The comment about 1 ms delay.]
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
repaint_scheduled is actually cleverly a quad-state, disguised as a
boolean. There are four possible conditions for the repaint loop to be
in at any time:
- loop idle; no repaint will occur until specifically requested, which
may be never (repaint_scheduled == 0)
- loop schedule to begin: the loop was previously idle, but due to a
repaint-schedule request, we will call the start_repaint_loop hook
in the next idle task
- repaint scheduled: the compositor has definitively scheduled a
repaint request for this output, which will occur in fixed time
- awaiting repaint completion: the backend has not yet signaled
completion of the last repaint request, and the compositor will not
schedule another until it does so
All but the first condition were previously conflated as
repaint_scheduled == 1, but break them out into separate conditions to
aid clarity, backed up by some asserts.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
On startup, we cannot lock on to the repaint timer because it is unknown
to us. We deal with this by claiming that the moment of entry into the
repaint loop is the moment a frame returned, causing finish_frame to
delay our initial repaint to (refresh_time - repaint_delay), typically
around 9ms of utterly wasted time.
Add an explicit stamp == NULL, to determine that we are just beginning
our repaint loop, that the timings are in fact totally invalid, and that
it would be beneficial to repaint the output immediately. This will only
trigger when the display had previously been disabled or the previous
state is unknown, e.g. at startup, or coming back from DPMS off.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Rather than determining the time until next-frame repaint in relative
space (time until repaint), determine it first in absolute space, and
then later convert this to relative.
This will later allow us to store these per-output, so we can have a
single idle timer which will allow us to aggregate multiple repaints
together when timing allows.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Paralleling timespec_to_nsec, converts to milliseconds.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
[Pekka: added doc about flooring]
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
When a client changes the subsurfaces state, we need to damage
them so the result is visible. We do that by flagging the surfaces
when the state changes and causing damage when committing the
state. This prevents normal repaints from considering these changes
until a commit has happened, and allows the client to atomically
schedule several changes.
This fixes the subsurface_z_order test, which is now marked as expected
to succeed.
Signed-off-by: Emilio Pozuelo Monfort <emilio.pozuelo@collabora.co.uk>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Micah Fedke <micah.fedke@collabora.co.uk>
repaint_needed / repaint_scheduled are surprisingly subtle. Explode the
conditional with side-effects into more obvious separate calls, and
document what they do.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Use different functions so we cannot load a libweston common module in
weston directly or the other way around.
Signed-off-by: Quentin Glidic <sardemff7+git@sardemff7.net>
Reviewed-by: Daniel Stone <daniels@collabora.com>
This prevents loading a backend as a simple module. This will avoid
messing up with backends when we will introduce libweston common
modules.
Signed-off-by: Quentin Glidic <sardemff7+git@sardemff7.net>
Reviewed-by: Daniel Stone <daniels@collabora.com>
Currently, layers’ order depends on the module loading order and it does
not survive runtime modifications (like shell locking/unlocking).
With this patch, modules can safely add their own layer at the expected
position in the stack, with runtime persistence.
v4 Reviewed-by: Giulio Camuffo <giuliocamuffo@gmail.com>
Signed-off-by: Quentin Glidic <sardemff7+git@sardemff7.net>
Acked-by: Daniel Stone <daniels@collabora.com>
[Pekka: fix three whitespace issues]
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
When we create a new view, assign it to the primary plane from the
beginning.
Currently, every view across the compositor will be assigned to a plane
during every repaint cycle of every output: the DRM renderer's
assign_planes hook will either move a view to a drm_plane, or to the
primary plane if a suitable drm_plane could not be found for the output
it is on. There are no other assign_planes implementation, and the
fallback when none is provided, is to assign every view to the primary
plane.
DRM's behaviour is undesirable in multi-output situations, since it
means that views which were on a plane on one output will be demoted to
the primary plane; doing this causes damage, which will cause a spurious
repaint for the output. This spurious repaint will have no effect on the
other output, but it will do the same demotion of views to the primary
plane, which will again provoke a repaint on the other output.
With a simple fix for this behaviour (i.e. not moving views which are
only visible on other outputs), the following behaviour is observed:
- outputs A and B are present
- views A and B are created for those outputs respectively, with SHM
buffers attached; view->plane == NULL for both
- current buffer content for views A and B are uploaded to the
renderer
- output A runs its repaint cycle, and sets keep_buffer to false on
surface B's output, as it can never be promoted to a plane; it does
not move view B to another plane
- output B runs its repaint cycle, and moves view B to the primary
plane
- weston_view_assign_to_plane has work to do (as the plane is changing
from NULL to the primary plane), calls weston_surface_damage and
calls weston_surface_damage
- weston_surface_damage re-uploads buffer content, possibly from
nowhere at all; e508ce6a notes that this behaviour is broken
Assigning views to the primary plane when created makes it possible to
fix the DRM assign_planes implementation: assign_planes will always set
keep_buffer to true if there is any chance the buffer can ever be
promoted to a plane, regardless of view configruation. If the buffer
cannot be promoted to a plane, it must by definition never migrate from
the primary plane. This means that there is no opportunity to hit the
same issue, where the buffer content has already been discarded, but
weston_view_assign_to_plane is not a no-op.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Reviewed: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Forcing DPMS on when we lose our session may force an expensive modeset
operation, which is pointless if the next consumer (another compositor,
or the console) is going to do a modeset. These should force DPMS on
regardless.
This actively causes problems for the DRM backend, in that it may
actually require a repaint to set coherent state for DPMS off -> DPMS on
transitions, which is very much not what we want when going offscreen.
As DRM is the only backend which actually implements DPMS, just remove
this call.
Differential Revision: https://phabricator.freedesktop.org/D1483
Signed-off-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Quentin Glidic <sardemff7+git@sardemff7.net>
Avoid any buffer overflows here by checking we don't go over PATH_MAX
with stupid module names.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
The parent of a subsurface can be used as a sibling in the place_below
and place_above calls. However this did not work when the parent is
nested, so fix the sibling check and add a test to check this case.
Signed-off-by: Daniel Stone <daniels@collabora.com>
They were required for transitional phase in order not to
break previous weston_output_init(). Now, they can even
be initialized on enable, or left with defaults if backend
doesn't support them.
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Signed-off-by: Armin Krezović <krezovic.armin@gmail.com>
This patch implements additional functionality that will be used
for configuring, enabling and disabling weston's outputs. Its
indended use is by the compositors or user programs that want to
be able to configure, enable or disable an output at any time. An
output can only be configured while it's disabled.
The compositor and backend specific functionality is required
for these functions to be useful, and those will come later in
this series.
All the new functions have been documented, so I'll avoid
describing them here.
v2:
- Minor documentation improvements.
- Rename output-initialized to output->enabled.
- Split weston_output_disable() further into
weston_compositor_remove_output().
- Rename weston_output_deinit() to weston_output_enable_undo().
- Make weston_output_disable() call two functions mentioned
above instead of calling weston_output_disable() directly.
This means that backend needs to take care of doing backend
specific disable in backend specific destroy function.
v3:
- Require output->name to be set before calling
weston_output_init_pending().
- Require output->destroying to be set before
calling weston_compositor_remove_output().
- Split weston_output_init_pending() into
weston_compositor_add_pending_output() so pending outputs
can be announced separately.
- Require output->disable() to be set in order for
weston_output_disable() to be usable.
- Fix output removing regression that happened when
weston_output_disable() was split.
- Minor documentation fix.
v4:
- Bump libweston version to 2 as this patch breaks the ABI.
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Signed-off-by: Armin Krezović <krezovic.armin@gmail.com>
If the transform on a view is only a translation we can trivially
set the opaque region for it so to optimize the rendering.
Reviewed-by: Jonas Ådahl <jadahl@gmail.com>
Alexandros Frantzis
Pekka Paalanen
Matt Hoosier
Daniel Stone
Sergi Granell
Emilio Pozuelo Monfort
Bryce Harrington
Quentin Glidic
Abdur Rehman
Arnaud Vrac
Armin Krezović
Giulio Camuffo