Yes, On Jun 18, 2014 11:57 PM, "Pekka Paalanen" <<a href="mailto:ppaalanen@gmail.com">ppaalanen@gmail.com</a>> wrote: > > On Wed, 18 Jun 2014 12:38:50 -0700 > Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> wrote: > > > On Fri, Jun 13, 2014 at 9:14 AM, George Kiagiadakis < > > <a href="mailto:george.kiagiadakis@collabora.com">george.kiagiadakis@collabora.com</a>> wrote: > > > > > This fixes at least the case where you want to do > > > wl_viewport.set_destination > > > to resize the surface but without attaching new content in it. > > > --- > > > src/compositor.c | 60 > > > +++++++++++++++++++++++++++++++++++++++++++------------- > > > src/compositor.h | 2 ++ > > > 2 files changed, 48 insertions(+), 14 deletions(-) > > > > > > diff --git a/src/compositor.c b/src/compositor.c > > > index 2fbfdbf..973c7e4 100644 > > > --- a/src/compositor.c > > > +++ b/src/compositor.c > > > @@ -404,6 +404,7 @@ weston_surface_create(struct weston_compositor > > > *compositor) > > > surface->buffer_viewport.buffer.scale = 1; > > > surface->buffer_viewport.buffer.src_width = wl_fixed_from_int(-1); > > > surface->buffer_viewport.surface.width = -1; > > > + surface->pending.buffer_viewport.changed = 0; > > > surface->pending.buffer_viewport = surface->buffer_viewport; > > > surface->output = NULL; > > > surface->pending.newly_attached = 0; > > > @@ -1211,13 +1212,12 @@ fixed_round_up_to_int(wl_fixed_t f) > > > } > > > > > > static void > > > -weston_surface_set_size_from_buffer(struct weston_surface *surface) > > > +weston_surface_calculate_size_from_buffer(struct weston_surface *surface) > > > { > > > struct weston_buffer_viewport *vp = &surface->buffer_viewport; > > > int32_t width, height; > > > > > > if (!surface->buffer_ref.buffer) { > > > - surface_set_size(surface, 0, 0); > > > surface->width_from_buffer = 0; > > > surface->height_from_buffer = 0; > > > return; > > > @@ -1239,14 +1239,24 @@ weston_surface_set_size_from_buffer(struct > > > weston_surface *surface) > > > > > > surface->width_from_buffer = width; > > > surface->height_from_buffer = height; > > > +} > > > + > > > +static void > > > +weston_surface_update_size(struct weston_surface *surface) > > > +{ > > > + struct weston_buffer_viewport *vp = &surface->buffer_viewport; > > > + int32_t width, height; > > > + > > > + width = surface->width_from_buffer; > > > + height = surface->height_from_buffer; > > > > > > - if (vp->surface.width != -1) { > > > + if (width != 0 && vp->surface.width != -1) { > > > surface_set_size(surface, > > > vp->surface.width, vp->surface.height); > > > return; > > > } > > > > > > - if (vp->buffer.src_width != wl_fixed_from_int(-1)) { > > > + if (width != 0 && vp->buffer.src_width != wl_fixed_from_int(-1)) { > > > int32_t w = fixed_round_up_to_int(vp->buffer.src_width); > > > int32_t h = fixed_round_up_to_int(vp->buffer.src_height); > > > > > > @@ -1351,6 +1361,7 @@ weston_surface_reset_pending_buffer(struct > > > weston_surface *surface) > > > surface->pending.sx = 0; > > > surface-><a href="http://pending.sy">pending.sy</a> = 0; > > > surface->pending.newly_attached = 0; > > > + surface->pending.buffer_viewport.changed = 0; > > > } > > > > > > struct weston_frame_callback { > > > @@ -1520,7 +1531,7 @@ weston_surface_attach(struct weston_surface *surface, > > > > > > surface->compositor->renderer->attach(surface, buffer); > > > > > > - weston_surface_set_size_from_buffer(surface); > > > + weston_surface_calculate_size_from_buffer(surface); > > > } > > > > > > > I think we have an issue here. If a client attaches a new buffer but does > > not commit for a while, the width/height_from_buffer and width/height will > > be out of sync. That said, I think having surface_set_size_from_buffer > > here was wrong in the first place. The new surface size should only take > > over after the commit. Other than that, the patch looks fine to me. > > Hmm, but weston_surface_attach() is only called from > weston_surface_commit() and weston_subsurface_commit_from_cache()... > > The wl_surface.attach protocol handler is surface_attach() IIRC. Was > that a confusion? Yes, I think it was. In that case, this patch looks good. Somewhere, I've got a branch that cleans that mess stuff up. --Jason > > > Thanks, > pq