[PATCH weston v5 16/36] libweston: add weston_head_is_device_changed() API

[Derek Foreman]

On 2017-12-14 05:40 AM, Pekka Paalanen wrote:
> From: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> 
> Reacting to DRM hotplug events is racy. It is theoretically possible to
> get hotplug events for a quick swap from one monitor to another and
> process both only after the new monitor is connected. Hence it is
> possible for display device information to change without going through
> a disconnected state for the head.

I figured you'd have to address this somewhere. :)

> To support such cases, add API to allow detecting it in the compositor.
> 
> Signed-off-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> ---
>   libweston/compositor.c | 88 ++++++++++++++++++++++++++++++++++++++++++++++----
>   libweston/compositor.h |  7 ++++
>   2 files changed, 89 insertions(+), 6 deletions(-)
> 
> diff --git a/libweston/compositor.c b/libweston/compositor.c
> index 86efcfad..0d3e185b 100644
> --- a/libweston/compositor.c
> +++ b/libweston/compositor.c
> @@ -4714,6 +4714,31 @@ weston_head_release(struct weston_head *head)
>   	wl_list_remove(&head->compositor_link);
>   }
>   
> +static void
> +weston_head_condition_device_changed(struct weston_head *head, bool condition)
> +{
> +	if (!condition)
> +		return;

It feels odd to me to check the condition here instead of having the 
callers do something like:
if (condition) weston_head_device_changed();

But I guess that's a style thing, so my personal preference doesn't hold 
much weight here...

Either way,
Reviewed-by: Derek Foreman <derekf at osg.samsung.com>

> +
> +	head->device_changed = true;
> +
> +	if (head->compositor)
> +		weston_compositor_schedule_heads_changed(head->compositor);
> +}
> +
> +/** String non-equal comparison with NULLs being equal */
> +static bool
> +str_null_neq(const char *a, const char *b)
> +{
> +	if (!a && !b)
> +		return false;
> +
> +	if (!!a != !!b)
> +		return true;
> +
> +	return strcmp(a, b);
> +}
> +
>   /** Store monitor make, model and serial number
>    *
>    * \param head The head to modify.
> @@ -4724,6 +4749,8 @@ weston_head_release(struct weston_head *head)
>    * \param serialno The monitor serial number, a made-up string, or NULL for
>    * none.
>    *
> + * This may set the device_changed flag.
> + *
>    * \memberof weston_head
>    * \internal
>    */
> @@ -4733,6 +4760,11 @@ weston_head_set_monitor_strings(struct weston_head *head,
>   				const char *model,
>   				const char *serialno)
>   {
> +	weston_head_condition_device_changed(head,
> +				str_null_neq(head->make, make) ||
> +				str_null_neq(head->model, model) ||
> +				str_null_neq(head->serial_number, serialno));
> +
>   	free(head->make);
>   	free(head->model);
>   	free(head->serial_number);
> @@ -4748,6 +4780,8 @@ weston_head_set_monitor_strings(struct weston_head *head,
>    * \param mm_width Image area width in millimeters.
>    * \param mm_height Image area height in millimeters.
>    *
> + * This may set the device_changed flag.
> + *
>    * \memberof weston_head
>    * \internal
>    */
> @@ -4755,6 +4789,10 @@ WL_EXPORT void
>   weston_head_set_physical_size(struct weston_head *head,
>   			      int32_t mm_width, int32_t mm_height)
>   {
> +	weston_head_condition_device_changed(head,
> +					     head->mm_width != mm_width ||
> +					     head->mm_height != mm_height);
> +
>   	head->mm_width = mm_width;
>   	head->mm_height = mm_height;
>   }
> @@ -4770,6 +4808,8 @@ weston_head_set_physical_size(struct weston_head *head,
>    * - WL_OUTPUT_SUBPIXEL_VERTICAL_RGB,
>    * - WL_OUTPUT_SUBPIXEL_VERTICAL_BGR
>    *
> + * This may set the device_changed flag.
> + *
>    * \memberof weston_head
>    * \internal
>    */
> @@ -4777,6 +4817,8 @@ WL_EXPORT void
>   weston_head_set_subpixel(struct weston_head *head,
>   			 enum wl_output_subpixel sp)
>   {
> +	weston_head_condition_device_changed(head, head->subpixel != sp);
> +
>   	head->subpixel = sp;
>   }
>   
> @@ -4812,7 +4854,7 @@ weston_head_set_internal(struct weston_head *head)
>    * connection to the parent display server.
>    *
>    * When the connection status changes, it schedules a call to the heads_changed
> - * hook.
> + * hook and sets the device_changed flag.
>    *
>    * \sa weston_compositor_set_heads_changed_cb
>    * \memberof weston_head
> @@ -4821,13 +4863,9 @@ weston_head_set_internal(struct weston_head *head)
>   WL_EXPORT void
>   weston_head_set_connection_status(struct weston_head *head, bool connected)
>   {
> -	if (head->connected == connected)
> -		return;
> +	weston_head_condition_device_changed(head, head->connected != connected);
>   
>   	head->connected = connected;
> -
> -	if (head->compositor)
> -		weston_compositor_schedule_heads_changed(head->compositor);
>   }
>   
>   /** Is the head currently connected?
> @@ -4871,6 +4909,44 @@ weston_head_is_enabled(struct weston_head *head)
>   	return head->output->enabled;
>   }
>   
> +/** Has the device information changed?
> + *
> + * \param head The head to query.
> + * \return True if the device information has changed since last reset.
> + *
> + * The information about the connected display device, e.g. a monitor, may
> + * change without being disconnected in between. Changing information
> + * causes a call to the heads_changed hook.
> + *
> + * The information includes make, model, serial number, physical size,
> + * and sub-pixel type. The connection status is also included.
> + *
> + * \sa weston_head_reset_device_changed, weston_compositor_set_heads_changed_cb
> + * \memberof weston_head
> + */
> +WL_EXPORT bool
> +weston_head_is_device_changed(struct weston_head *head)
> +{
> +	return head->device_changed;
> +}
> +
> +/** Acknowledge device information change
> + *
> + * \param head The head to acknowledge.
> + *
> + * Clears the device changed flag on this head. When a compositor has processed
> + * device information, it should call this to be able to notice further
> + * changes.
> + *
> + * \sa weston_head_is_device_changed
> + * \memberof weston_head
> + */
> +WL_EXPORT void
> +weston_head_reset_device_changed(struct weston_head *head)
> +{
> +	head->device_changed = false;
> +}
> +
>   /** Get the name of a head
>    *
>    * \param head The head to query.
> diff --git a/libweston/compositor.h b/libweston/compositor.h
> index af61215e..1a9aac7f 100644
> --- a/libweston/compositor.h
> +++ b/libweston/compositor.h
> @@ -170,6 +170,7 @@ struct weston_head {
>   	char *serial_number;		/**< monitor serial */
>   	uint32_t subpixel;		/**< enum wl_output_subpixel */
>   	bool connection_internal;	/**< embedded monitor (e.g. laptop) */
> +	bool device_changed;		/**< monitor information has changed */
>   
>   	char *name;			/**< head name, e.g. connector name */
>   	bool connected;			/**< is physically connected */
> @@ -2041,6 +2042,12 @@ weston_head_is_connected(struct weston_head *head);
>   bool
>   weston_head_is_enabled(struct weston_head *head);
>   
> +bool
> +weston_head_is_device_changed(struct weston_head *head);
> +
> +void
> +weston_head_reset_device_changed(struct weston_head *head);
> +
>   const char *
>   weston_head_get_name(struct weston_head *head);
>   
>