From 15b0adf59a4bb1086328bbcb95c5a355e15c46db Mon Sep 17 00:00:00 2001 From: Simon Ser Date: Fri, 22 Dec 2023 10:07:04 +0100 Subject: [PATCH 1/2] compositor: wrap cached lock seq in struct This improves type safety: the struct wlr_surface_cached_lock value can't be mixed with other integer values. --- include/wlr/types/wlr_compositor.h | 11 +++++++++-- include/wlr/types/wlr_subcompositor.h | 2 +- types/wlr_compositor.c | 10 +++++----- types/wlr_subcompositor.c | 10 +++++----- 4 files changed, 20 insertions(+), 13 deletions(-) diff --git a/include/wlr/types/wlr_compositor.h b/include/wlr/types/wlr_compositor.h index cb9f9292f..383fbd702 100644 --- a/include/wlr/types/wlr_compositor.h +++ b/include/wlr/types/wlr_compositor.h @@ -381,6 +381,13 @@ void wlr_surface_get_effective_damage(struct wlr_surface *surface, void wlr_surface_get_buffer_source_box(struct wlr_surface *surface, struct wlr_fbox *box); +/** + * A lock preventing cached state from being applied. + */ +struct wlr_surface_cached_lock { + uint32_t seq; +}; + /** * Acquire a lock for the pending surface state. * @@ -390,7 +397,7 @@ void wlr_surface_get_buffer_source_box(struct wlr_surface *surface, * * Returns a surface commit sequence number for the cached state. */ -uint32_t wlr_surface_lock_pending(struct wlr_surface *surface); +struct wlr_surface_cached_lock wlr_surface_lock_pending(struct wlr_surface *surface); /** * Release a lock for a cached state. @@ -398,7 +405,7 @@ uint32_t wlr_surface_lock_pending(struct wlr_surface *surface); * Callers should not assume that the cached state will immediately be * committed. Another caller may still have an active lock. */ -void wlr_surface_unlock_cached(struct wlr_surface *surface, uint32_t seq); +void wlr_surface_unlock_cached(struct wlr_surface *surface, struct wlr_surface_cached_lock lock); /** * Set the preferred buffer scale for the surface. diff --git a/include/wlr/types/wlr_subcompositor.h b/include/wlr/types/wlr_subcompositor.h index d20fef949..2ea5a1787 100644 --- a/include/wlr/types/wlr_subcompositor.h +++ b/include/wlr/types/wlr_subcompositor.h @@ -32,7 +32,7 @@ struct wlr_subsurface { struct wlr_subsurface_parent_state current, pending; - uint32_t cached_seq; + struct wlr_surface_cached_lock cached_lock; bool has_cache; bool synchronized; diff --git a/types/wlr_compositor.c b/types/wlr_compositor.c index a3e02655e..58fb631e1 100644 --- a/types/wlr_compositor.c +++ b/types/wlr_compositor.c @@ -815,13 +815,13 @@ static void surface_destroy_role_object(struct wlr_surface *surface) { wl_list_init(&surface->role_resource_destroy.link); } -uint32_t wlr_surface_lock_pending(struct wlr_surface *surface) { +struct wlr_surface_cached_lock wlr_surface_lock_pending(struct wlr_surface *surface) { surface->pending.cached_state_locks++; - return surface->pending.seq; + return (struct wlr_surface_cached_lock){surface->pending.seq}; } -void wlr_surface_unlock_cached(struct wlr_surface *surface, uint32_t seq) { - if (surface->pending.seq == seq) { +void wlr_surface_unlock_cached(struct wlr_surface *surface, struct wlr_surface_cached_lock lock) { + if (surface->pending.seq == lock.seq) { assert(surface->pending.cached_state_locks > 0); surface->pending.cached_state_locks--; return; @@ -830,7 +830,7 @@ void wlr_surface_unlock_cached(struct wlr_surface *surface, uint32_t seq) { bool found = false; struct wlr_surface_state *cached; wl_list_for_each(cached, &surface->cached, cached_state_link) { - if (cached->seq == seq) { + if (cached->seq == lock.seq) { found = true; break; } diff --git a/types/wlr_subcompositor.c b/types/wlr_subcompositor.c index c66d6f1de..8ed24c65f 100644 --- a/types/wlr_subcompositor.c +++ b/types/wlr_subcompositor.c @@ -26,7 +26,7 @@ static const struct wl_subsurface_interface subsurface_implementation; static void subsurface_destroy(struct wlr_subsurface *subsurface) { if (subsurface->has_cache) { - wlr_surface_unlock_cached(subsurface->surface, subsurface->cached_seq); + wlr_surface_unlock_cached(subsurface->surface, subsurface->cached_lock); } wlr_surface_unmap(subsurface->surface); @@ -177,7 +177,7 @@ static void subsurface_handle_set_desync(struct wl_client *client, if (!subsurface_is_synchronized(subsurface) && subsurface->has_cache) { wlr_surface_unlock_cached(subsurface->surface, - subsurface->cached_seq); + subsurface->cached_lock); subsurface->has_cache = false; } } @@ -247,9 +247,9 @@ static void subsurface_handle_surface_client_commit( return; } subsurface->has_cache = true; - subsurface->cached_seq = wlr_surface_lock_pending(surface); + subsurface->cached_lock = wlr_surface_lock_pending(surface); } else if (subsurface->has_cache) { - wlr_surface_unlock_cached(surface, subsurface->cached_seq); + wlr_surface_unlock_cached(surface, subsurface->cached_lock); subsurface->has_cache = false; } } @@ -275,7 +275,7 @@ void subsurface_handle_parent_commit(struct wlr_subsurface *subsurface) { } if (subsurface->synchronized && subsurface->has_cache) { - wlr_surface_unlock_cached(surface, subsurface->cached_seq); + wlr_surface_unlock_cached(surface, subsurface->cached_lock); subsurface->has_cache = false; } From 6ebe38b759f396d3177564d16ff1d889b449451a Mon Sep 17 00:00:00 2001 From: Simon Ser Date: Fri, 22 Dec 2023 10:13:35 +0100 Subject: [PATCH 2/2] compositor: make state seq an uint64_t Saves us from having to think about overflow cases. --- include/wlr/types/wlr_compositor.h | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/include/wlr/types/wlr_compositor.h b/include/wlr/types/wlr_compositor.h index 383fbd702..200d180f8 100644 --- a/include/wlr/types/wlr_compositor.h +++ b/include/wlr/types/wlr_compositor.h @@ -33,9 +33,8 @@ enum wlr_surface_state_field { struct wlr_surface_state { uint32_t committed; // enum wlr_surface_state_field - // Sequence number of the surface state. Incremented on each commit, may - // overflow. - uint32_t seq; + // Sequence number of the surface state. Incremented on each commit. + uint64_t seq; struct wlr_buffer *buffer; int32_t dx, dy; // relative to previous position @@ -385,7 +384,7 @@ void wlr_surface_get_buffer_source_box(struct wlr_surface *surface, * A lock preventing cached state from being applied. */ struct wlr_surface_cached_lock { - uint32_t seq; + uint64_t seq; }; /**