On 05.07.2016 13:42, Pekka Paalanen wrote: > On Thu, 30 Jun 2016 06:29:48 +0200 > Armin Krezović <[email protected]> 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ć <[email protected]> >> --- >> 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.
signature.asc
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/wayland-devel
