On Wed, 22 Jun 2016 12:16:31 +0200 Armin Krezović <krezovic.ar...@gmail.com> wrote:
> On 21.06.2016 14:05, Pekka Paalanen wrote: > > On Sat, 18 Jun 2016 19:15:23 +0200 > > Armin Krezović <krezovic.ar...@gmail.com> wrote: > > > >> Currently, weston assumes a surface/view is mapped if > >> it has an output assigned. In a zero outputs scenario, > >> this isn't really desirable. > >> > >> This patch introduces a new flag to weston_surface and > >> weston_view, which has to be set manually to indicate > >> that a surface/view is mapped. > >> > >> Signed-off-by: Armin Krezović <krezovic.ar...@gmail.com> > > > > Hi Armin, > > > > there is a slight problem with the split and ordering in patches 8 - > > 11. Patch 8 removes the old conditions, which breaks "everything" and > > the follow-up patches fix things up piece by piece. The problem is that > > we now have known-broken commits from the start. Those will make > > bisecting unrelated issues harder. > > > > Ugh, that's a valid concern. > > > There are two possible solutions: > > > > - split patch 8 to to only add the new fields, and make a new patch as > > the final one in the series that switches the > > weston_{view,surface}_is_mapped() to use the new flags > > > > - squash all patches 8 - 11 into one > > > > Because the changes in patches 9 - 11 are fairly trivial and unlikely > > to break anything in themselves, the patch that switches the > > *_is_mapped() functions will be the one causing the regressions if any. > > Hence it would be also ok to just squash these into one patch. It > > doesn't make too much difference for reviews, and no difference for > > bisecting. > > > > I wanted to split the shells stuff mainly because different people work(ed) > on different shells. I like the first approach. Hi, ok, that is the way I would consider more proper too. > >> --- > >> src/compositor.c | 29 ++++++++++------------------- > >> src/compositor.h | 4 ++++ > >> src/data-device.c | 2 ++ > >> src/input.c | 2 ++ > >> 4 files changed, 18 insertions(+), 19 deletions(-) > > > > There is also a more subtle change of semantics in this patch, which > > would be good to underline in the commit message. > > > > Previously when mappedness was determined by outputs, unmapping the > > last view unmapped also the surface, or so I believe. That no longer > > happens, which is actually a good thing. This allows a shell to unmap > > all views without losing the mappedness information of the surface. > > > > A practical example of this is hiding a desktop-shell top-level window. > > When the window is shown again, it should appear on the position where > > it was hidden, but that will not happen if desktop-shell thinks the > > surface is not mapped. Instead, desktop-shell will randomize the window > > position again. I haven't looked if this actually happens or if there > > is code to work around that, but I think the change by this patch is > > for the better. > > > > The mappedness states of a surface and its views are no longer > > implicitly connected. > > > > I'm not sure I'm getting all this. We can discuss about it more on > IRC or I can just copy/paste most of the text above :). > >> @@ -2152,6 +2149,7 @@ view_list_add(struct weston_compositor *compositor, > >> struct weston_subsurface *sub; > >> > >> weston_view_update_transform(view); > >> + view->is_mapped = true; > > > > This I am not so sure about. Isn't it the responsibility of whoever > > puts a view on a layer to set is_mapped to true? > > > > Doing it here might paper over some overlooked places. > > > > Again, I was trying to mimic the previous behavior. > weston_view_update_transform assigns an output to the view, and when > the output got assigned it was assumed the view was mapped. Mimicing the old behaviour would require you to make weston_surface_assing_output() set the surface->is_mapped based on the various view->is_mapped. You don't do that, and I don't think we should either because of what I explained above. That makes us change the behaviour more than strictly necessary, but I think it's worth it. Thanks, pq
pgpLIsx3Fn_WN.pgp
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel