To update the surface label based on client-controllable fields like
app_id and title we need to use a listener and update the label
accordingly using weston_surface_set_label() added previously.
The added weston_desktop_surface_make_label() will eventually supersede
weston_shell_utils_surface_get_label() by having a more fluent API.
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Subsurfaces labels do not have dynamic fields that change so we can just
use it as is.
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
This is an alternative to using set_label_func to avoid using get_label()
callback. get_label() is inconvenient to set up and to call. It incurs
the cost of creating the string every time it is needed. During
debug logging, the string is needed much more often than it changes.
The new label field simply stores the string, making it easy and cheap
to use. As the trade-off, components that set the label string must
re-create the string when it changes, whether it is needed or not.
For the migration to the new label field, get_label_member callback is
used. It will be deleted once the migration is done.
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
On the quest to delete the get_label() vfunc completely, a temporary
default get_label implementation needs to be plugged in. This would
affect all the code that checks whether the get_label vfunc is NULL.
Let's make get_label vfunc non-NULL always intentionally first. We can
delete all the code that checked for NULL, and the bespoke label
replacements in that case. Now all those different "no label" cases are
unified.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Doing
weston_log_subscription_printf(sub, "%s", str);
would malloc a new buffer, copy str into it, flush it our to the
subscriber, and free the buffer before returning.
Using weston_log_subscription_write() instead there is no malloc and
copy. Only open_memstream() has a malloc'd temporary buffer.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Add a new gl-renderer scope (gl-renderer-paint-nodes) to print out
repaint (damage) regions, opaque and blended regions as well as other
optimizations we might be taking (clear region, color effects), and
skipping repaints.
It includes as well the elapsed time being used by GPU timeline
(GL_EXT_disjoint_timer_query) as well.
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
Further clarify that is printing the GL/EGL-extension by renaming
the internal scope name to extensions_scope and the advertised name
is gl-renderer-ext.
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
yuv_coefficients and yuv_offsets should get optimized away by shader
compilers as the related code paths can never be reached. This seems to
work well on Mesa but not necessarily with other drivers.
While on it, assert that the uniforms *are* present, unless the
yuv-to-rgb conversion is handled by the driver.
Signed-off-by: Robert Mader <robert.mader@collabora.com>
The read() function expects a pointer to buffer, not address of
pointer. Three instances were incorrectly passing &buffer instead
of buffer to read().
Signed-off-by: Wang Yu <wangyu@uniontech.com>
These were simply leaked before. Now they get destroyed together with
the primary DRM device, too.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Replace all the open-coding in drm_backend_create() with a call to
drm_device_create(). This is much more maintainable.
drm_source is not needed, it is tracked in struct
drm_device::drm_event_source.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Otherwise we cannot clean it up.
The special backend->drm device has the special backend->drm_source for
the same purpose.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Will need to destroy more than the special b->drm. All the additional
devices are currently leaked.
Move the code into a new function to re-use it.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
I want to consolidate 'struct drm_device' destruction into a single
function in the next patch, and that function needs to be usable for
both the primary device drm_backend::drm, and the secondary KMS devices
drm_backend::kms_list.
Sprites, a.k.a drm_planes, are objects fundamentally owned by their
drm_device. To encapsulate drm_device destruction into a single function
(that assumes that all outputs on that device are already destroyed),
one cannot call destroy_sprites() from drm_shutdown() anymore - it
logically belongs in drm_device_destroy().
The problem is that that will reverse the order of destroying drm_planes
vs. output (GBM) surfaces. drm_plane_state holds a reference to drm_fb
which points to a GBM BO that was received from a GBM surface. When the
GBM surface is destroyed, all its GBM BOs are destroyed as well, which
will cause all drm_fb pointing to them to be destroyed as well
regardless of drm_fb reference count. Therefore, if any object was
thought to hold a reference to a drm_fb, it now holds a dangling
pointer.
Why is drm_output_deinit() not clearing the drm_plane state it used,
releasing all the drm_fb references? And do that before destroying the
GBM surface?
Because drm_output_deinit_planes() explicitly skips clearing the
drm_plane state during compositor shutdown, because during shutdown the
drm_planes have already been destroyed.
If we change both, we get a much simpler tear-down logic:
- drm_output_deinit_planes() will always clear drm_plane states, no
special handling for shutdown (which is an elaborate way of saying
"before outputs are destroyed").
- drm_output_deinit() calls drm_output_deinit_planes() before it
destroys the GBM surface, which ensures no dangling drm_fb pointers
are left.
- destroy_sprites() call is moved *after* destruction of outputs where
it logically belongs.
This also means that the per-renderer fini functions do not need to
clean up manually when the compositor is not shutting down. They can
just assert the scanout_plane has already been cleaned up.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
This makes it possible to use drm_device_create() from
drm_backend_create() in the future.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
find_primary_gpu() and open_specific_drm_device() are modified to return
drm_kms_device instead of modifying fields inside the given drm_device.
Likewise, there is no need to pass the whole drm_backend in (which is
not fully initialized, maybe), but just the needed things.
This should make the code a little easier to follow.
Accidentally, this seems to fix a reference leak of udev_device in
drm_device_create(), because the udev_device is now tracked inside
drm_kms_device and properly unref'd. drm_backend_create() had the unref
that drm_device_create() missed.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
find_primary_gpu() and open_specific_drm_device() are currently
returning the udev_device while modifying the passed in drm_device. To
stop the modification, these functions should return drm_kms_device
directly, but the udev_device is also needed.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
drm_device_is_kms() used to populate a few fields of the passed in
struct drm_device, releasing old values. This was inconvenient, because
one must allocate struct drm_device first, then call drm_device_kms() in
a loop, and finally end up with only some fields of drm_device
populated, then continue to populate it more.
The new function drm_kms_device_create() will malloc a new struct
drm_kms_device every time. RAII. No need to populate and re-populate the
same fields in drm_device until the right KMS device is found.
One can no longer check the fd >= 0 for device validity, now one checks
if kms_device is not NULL.
The fd_owner field is added because it would be inconvenient to pass in
the launcher to the destroy function. It's cleaner to save it, make sure
the right one used for releasing an fd (as if we had multiple launcher
instances, haa haa). Can't release it properly without the launcher.
This is just a minor step to facilitate more refactoring.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
This gives the anonymous struct at struct drm_device::drm a name.
drm_device::drm is renamed to kms_device, which I think is a better
name, lacking an even better one.
The type of kms_device field is changed into a pointer, so that struct
drm_kms_device could be created and destroyed separately from struct
drm_device. Actually implementing this is left for the next patch, and
here the pointer is temporarily initialized with
device->kms_device = &device->kms_device_allocd;
Changing the name and type causes tons of trivial changes all over the
DRM-backend. This patch does only that, and the next patch will be more
interesting.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
There are failure paths here that may return without resetting the
variables, leaving dangling pointers behind.
Make sure there cannot be dangling pointers.
Found by manual code review.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
drm_gbm_dmabuf was the type allocated, so that is the type that should
be freed, too.
This was only a semantical bug. It did work before because 'base' was
the first member of struct drm_gbm_dmabuf and therefore the two pointers
were identical.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Having the options printed in addition to the layout and variant is
essential to trace how keyboard assignments ended up as they did.
Signed-off-by: Roland Kaufmann <rlndkfmn+freerdp@gmail.com>
The structure that convert_rdp_keyboard_to_xkb_rule_names fills is
next sent as argument to xkb_keymap_new_from_names, which constructs
the complete keymap.
Signed-off-by: Roland Kaufmann <rlndkfmn+freerdp@gmail.com>
Initially just allocate an extra data field and initialize this to
zero (meaning no particular options) for all keybords in the mapping
table.
Signed-off-by: Roland Kaufmann <rlndkfmn+freerdp@gmail.com>
weston_output_init() should be setting all members to a default value,
it can't assume anything is already 0.
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
Functions that implement the xdg_surface, toplevel and popup
interfaces may depend on struct weston_desktop_client::resource.
In normal circumstances, this resource can't be NULL. The xdg protocol
extension forbids destroying xdg_wm_base before destroying all its child
surfaces. If that happens, we disconnect the client with protocol error
DEFUNCT_SURFACES (see commit "xdg-shell: handle xdg_wm_base being
destroyed before its children").
But during client teardown resources are destroyed in arbitrary order,
so it is possible that client resource becomes NULL before its surfaces
are destroyed.
This commit adds checks to avoid using NULL client resource, in order to
avoid crashes. It is safe to silently do nothing in these cases, as the
client is being destroyed anyway.
Signed-off-by: Leandro Ribeiro <leandro.ribeiro@collabora.com>
Currently if weston_desktop_surface_add_resource() fails to create a
wl_resource, it disconnects the client and destroys the desktop surface
that we pass.
It is odd to do that, callers should be responsible for destroying the
desktop surface when it is reasonable to do so. This is dangerous and
can leave dangling pointers.
Besides that, in many cases callers should not even destroy the desktop
surface, as they are not the owners of it. When we are giving the role
of toplevel/popup to a xdg_surface and we fail to do so, client gets
disconnected and the base desktop surface will get destroyed by the
destructor of the xdg_surface resource.
This commit fixes these issues, bringing a more consistent behavior.
Signed-off-by: Leandro Ribeiro <leandro.ribeiro@collabora.com>
According to the xdg-shell protocol specification, if the xdg_wm_base
object is destroyed while there are still xdg_surface objects associated
with it, the compositor must post a protocol error (DEFUNCT_SURFACES) to
the client. In this patch we do that.
Signed-off-by: Leandro Ribeiro <leandro.ribeiro@collabora.com>
This signal is never emitted when touch focus changes. In this patch we
start emitting it, allowing listeners to react to touch focus changes.
Signed-off-by: Leandro Ribeiro <leandro.ribeiro@collabora.com>
We can get this right from the plane state/paint node now, so let's do
that and get rid of a bunch of crufty stuff.
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
Right now, any non-renderer plane only has a single paint node on it. It
can be useful to know what paint node that was after the damage flush.
This will be used shortly to simplify cursor handling in the drm backend.
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
I found it confusing to read:
[view] view 3-2-1 will be placed on the renderer: no color transform
The view definitely has a color transform. What this means is:
[view] view 3-2-1 will be placed on the renderer: cannot off-load color transform
I hope that's more clear.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
The scene-graph debug already uses these names, and they are much more
human-friendly to read than memory addresses.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Makes the print-out a little bit tidier. Passing NULL for "%s" is
handled by glibc gracefully, but I'm not sure what standard requires it
if any.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
We have a consistent and more human-friendly name for views. Use it
instead of pointer values, view_idx, and protocol object ID. This makes
the scene-graph print more readable.
I presume the view_idx was just an ad hoc human-readable addressing for
views. It was stable only as long as the scene-graph didn't change. The
view internal_name is always stable and unique.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Views did not have any identification of their own, except the memory
address "%p" (not human-readable) and very likely assumption that a
surface would have only one view (but we support multiple views).
For trace and debug print purposes, give views nice names like we just
added for surfaces. The owning surface is apparent from the view name.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
So far we have had two ways of identifying a weston_surface: by its
memory address "%p", and by its get_label function. The memory address
is not human-friendly and can get recycled. get_label() is not unique,
and in some cases it is client-controllable.
Oh, we also have the protocol object ID, but that does not exist for
internally created weston_surfaces.
We also have weston_surface::s_id, damage_track_id and flow_id. These
are used by some Perfetto instrumentation. s_id comes from a
compositor-wide counter rather than per-client counter, the others are
probably not what I'm looking for.
None of these are really nice for trace and debug prints for identifying
surfaces for human reading. Therefore, let's add one more ID, and with
it, a nice name for each surface.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
This adds the framework to track clients with custom data. The first use
is to associate an ID with each client. The ID is much better suited for
debug printing than a pointer value.
The string representation is stored so that it can be overridden if
desired for compositor forked clients like Xwayland or shell helpers.
Change to a struct in the public header forces a major version bump for
this development cycle.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
