On Fri, 3 Feb 2017 16:10:39 +0100 Emilio Pozuelo Monfort <poch...@gmail.com> wrote:
> We were calling weston_surface::committed on surfaces with > no buffer attached. Stop doing that, since surface::committed > will map the surfaces and put them in a visible layer. That may > not be a problem for a single surface as it wouldn't be visible > anyway because it's got no contents, but it is a problem if the > surface has subsurfaces. > > This fixes the subsurface_mapped test, so mark it as expected > to succeed. > > https://bugs.freedesktop.org/show_bug.cgi?id=94735 > > Signed-off-by: Emilio Pozuelo Monfort <emilio.pozu...@collabora.co.uk> > --- > libweston/compositor.c | 10 +++++++++- > tests/subsurface-shot-test.c | 2 +- > 2 files changed, 10 insertions(+), 2 deletions(-) > > diff --git a/libweston/compositor.c b/libweston/compositor.c > index 81392063..8a018897 100644 > --- a/libweston/compositor.c > +++ b/libweston/compositor.c > @@ -1589,6 +1589,12 @@ weston_surface_is_mapped(struct weston_surface > *surface) > return surface->is_mapped; > } > > +static bool > +weston_surface_has_content(struct weston_surface *surface) > +{ > + return surface->width > 0 && surface->height > 0; > +} > + > static void > surface_set_size(struct weston_surface *surface, int32_t width, int32_t > height) > { > @@ -2928,7 +2934,9 @@ weston_surface_commit_state(struct weston_surface > *surface, > > if (state->newly_attached || state->buffer_viewport.changed) { > weston_surface_update_size(surface); > - if (surface->committed) > + if (surface->committed && > + (state->newly_attached && > + weston_surface_has_content(surface))) > surface->committed(surface, state->sx, state->sy); Hi, I think this isn't right. The shells need to know when a surface unmaps or changes dimensions, and they get that via the ->committed hook (currently less well named, as ->configure was more descriptive, but I think in the long term we'd like to eliminate the two hooks and have just one called on every commit regardless of state - maybe have the hook call weston_surface_apply_state so it can inspect weston_surface state before and after). If the viewport changed, it must lead to a call to ->committed, with or without an attach. Another thing is that weston_surface_update_size() updates the width, height you use in weston_surface_has_content(). This means that a surface first having content, receiving attach NULL + commit, will not call ->committed. When I was last thinking about the committed() hook, I came up with: https://docs.google.com/spreadsheets/d/1b5azX4_gq9ZlYe4Y6UyHnHF8Wsiqnf1uHt9hR4whni4/edit#gid=0 but that just proves how complicated that design is, while not really saving much in the committed() implementations. Given my comments to the previous patch (the test), I think the fix might be completely elsewhere. Thanks, pq > } > > diff --git a/tests/subsurface-shot-test.c b/tests/subsurface-shot-test.c > index e7da1e0e..275d4726 100644 > --- a/tests/subsurface-shot-test.c > +++ b/tests/subsurface-shot-test.c > @@ -261,7 +261,7 @@ TEST(subsurface_z_order) > buffer_destroy(bufs[i]); > } > > -FAIL_TEST(subsurface_mapped) > +TEST(subsurface_mapped) > { > const char *test_name = get_test_name(); > struct client *client;
pgpvSML_E2MAm.pgp
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
