[PATCH weston RFC] compositor: Move all views to a new primary output

[Armin Krezović]

On 05.07.2016 13:42, Pekka Paalanen wrote:
> On Thu, 30 Jun 2016 06:29:48 +0200
> Armin Krezović <krezovic.armin at gmail.com> wrote:
> 
>> When primary output gets disconnected and there
>> were views created, they won't get assigned a
>> new output when a new primary output gets plugged
>> in. This will lead to crashes when weston tries
>> to use an already destroyed output.
> 
> Hi Armin,
> 
> I wonder if we could avoid keeping stale output pointers in the first
> place. The *_assing_output() machinery would already set them to NULL
> if it just got called.
> 
> Maybe removing an output should cause all views to be marked dirty?
> 
> Removing or adding outputs can cause shells to move views too.
> We want to avoid calling assign_outputs in the middle of a sequence of
> reorganizing views, so that clients don't receive unnecessary
> wl_surface.enter/leave events, which may cause redrawing.
> 
>> This fixes the problems by force-moving all views
>> to the newly attached output if there were no
>> outputs present.
>>
>> Signed-off-by: Armin Krezović <krezovic.armin at gmail.com>
>> ---
>>  libweston/compositor.c | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>>
>> diff --git a/libweston/compositor.c b/libweston/compositor.c
>> index eb9e8d9..62687cf 100644
>> --- a/libweston/compositor.c
>> +++ b/libweston/compositor.c
>> @@ -4313,8 +4313,18 @@ WL_EXPORT void
>>  weston_compositor_add_output(struct weston_compositor *compositor,
>>                               struct weston_output *output)
>>  {
>> +	struct weston_view *view;
>> +	int reassign_outputs = 0;
>> +
>> +	if (wl_list_empty(&compositor->output_list))
>> +		reassign_outputs = 1;
>> +
>>  	wl_list_insert(compositor->output_list.prev, &output->link);
>>  	wl_signal_emit(&compositor->output_created_signal, output);
>> +
>> +	if (reassign_outputs)
>> +		wl_list_for_each(view, &compositor->view_list, link)
>> +			weston_view_assign_output(view);
>>  }
>>  
>>  WL_EXPORT void
> 
> How about we do this every time an output get plugged in or out, not
> just when the first output gets plugged in?
> 
> That way the output assignments get automatically updated if e.g. a
> view is hanging off the edge of one output and into the region where
> the new output will be.
> 
> And instead of calling weston_view_assign_output() directly, call
> weston_view_geometry_dirty()?
> 
> Then *_assign_output() would get called as part of the repaint next
> time via weston_view_update_transform(), and it would coalesce any view
> manipulations done by the shell.
> 
> When outputs are removed, it is possible that a view from that output
> will no longer be on any output. That's not a problem if at least one
> output remains, because weston_output_repaint() will end up calling
> update_transform and assign_output. But when the very last output gets
> unplugged, there is no repaint to call those anymore. In that case we
> should probably call weston_view_assign_output() directly. It cannot
> cause glitching output assignments either, because even if the shell
> did move the view, it cannot get an output assigned anyway.
> 
> Or would it make better sense for weston_{surface,view} to also
> subscribe to the destruction of the assigned output, or even maintain a
> per-output list of assigned views and surfaces. I wonder...
> 
> 
> Thanks,
> pq
> 

Hi, setting transform.dirty to 1 makes sense. When an output is destroyed,
all views from that output are moved to another output. We can modify
the following snippet to set transform.dirty to 1 for a view if output
list is empty. That would make the patch even simpler.

        wl_list_for_each(view, &output->compositor->view_list, link) {
                if (view->output_mask & (1u << output->id))
                        weston_view_assign_output(view);
        }

Thanks, Armin.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 855 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20160708/99b36aa3/attachment-0001.sig>