On Thu, 26 Jun 2014 11:19:05 -0700 Jason Ekstrand <[email protected]> wrote:
> From: Jason Ekstrand <[email protected]> > > This new structure is used for both weston_surface.pending and > weston_subsurface.cached. > > Signed-off-by: Jason Ekstrand <[email protected]> > --- > src/compositor.c | 270 > +++++++++++++++++++++++-------------------------------- > src/compositor.h | 80 +++++++---------- > 2 files changed, 142 insertions(+), 208 deletions(-) Nice diffstat. :-) > diff --git a/src/compositor.c b/src/compositor.c > index 4ccae79..be33a36 100644 > --- a/src/compositor.c > +++ b/src/compositor.c ... > @@ -389,6 +379,72 @@ struct weston_frame_callback { > struct wl_list link; > }; > > +static void > +surface_state_handle_buffer_destroy(struct wl_listener *listener, void *data) > +{ > + struct weston_surface_state *state = > + container_of(listener, struct weston_surface_state, > + buffer_destroy_listener); > + > + state->buffer = NULL; > +} > + > +static void > +weston_surface_state_init(struct weston_surface_state *state) > +{ > + state->newly_attached = 0; > + state->buffer = NULL; > + state->buffer_destroy_listener.notify = > + surface_state_handle_buffer_destroy; > + state->sx = 0; > + state->sy = 0; > + > + pixman_region32_fini(&state->damage); > + pixman_region32_fini(&state->opaque); fini? Should it not be init? > + region_init_infinite(&state->input); > + > + wl_list_init(&state->frame_callback_list); > + > + state->buffer_viewport.buffer.transform = WL_OUTPUT_TRANSFORM_NORMAL; > + state->buffer_viewport.buffer.scale = 1; > + state->buffer_viewport.buffer.src_width = wl_fixed_from_int(-1); > + state->buffer_viewport.surface.width = -1; > + state->buffer_viewport.changed = 0; > +} ... > @@ -2390,7 +2365,9 @@ weston_subsurface_commit_to_cache(struct > weston_subsurface *sub) > > if (surface->pending.newly_attached) { > sub->cached.newly_attached = 1; > - weston_buffer_reference(&sub->cached.buffer_ref, > + weston_surface_state_set_buffer(&sub->cached, > + surface->pending.buffer); > + weston_buffer_reference(&sub->cached_buffer_ref, > surface->pending.buffer); Alright, you solved it this way. Seems to work, I think. > } > sub->cached.sx += surface->pending.sx; ... > @@ -2849,7 +2803,7 @@ weston_subsurface_create(uint32_t id, struct > weston_surface *surface, > sub, subsurface_resource_destroy); > weston_subsurface_link_surface(sub, surface); > weston_subsurface_link_parent(sub, parent); > - weston_subsurface_cache_init(sub); > + weston_surface_state_init(&sub->cached); Sould we explicitly init cached_buffer_ref too? For consistency and doc value? I think I would prefer yes. > sub->synchronized = 1; > > return sub; > diff --git a/src/compositor.h b/src/compositor.h > index 06f8b03..bef5e1d 100644 > --- a/src/compositor.h > +++ b/src/compositor.h > @@ -789,6 +789,32 @@ struct weston_view { > uint32_t output_mask; > }; > > +struct weston_surface_state { > + /* wl_surface.attach */ > + int newly_attached; > + struct weston_buffer *buffer; > + struct wl_listener buffer_destroy_listener; > + int32_t sx; > + int32_t sy; > + > + /* wl_surface.damage */ > + pixman_region32_t damage; > + > + /* wl_surface.set_opaque_region */ > + pixman_region32_t opaque; > + > + /* wl_surface.set_input_region */ > + pixman_region32_t input; > + > + /* wl_surface.frame */ > + struct wl_list frame_callback_list; > + > + /* wl_surface.set_buffer_transform */ > + /* wl_surface.set_scaling_factor */ > + /* wl_viewport.set */ > + struct weston_buffer_viewport buffer_viewport; > +}; > + > struct weston_surface { > struct wl_resource *resource; > struct wl_signal destroy_signal; > @@ -833,31 +859,7 @@ struct weston_surface { > struct wl_resource *viewport_resource; > > /* All the pending state, that wl_surface.commit will apply. */ > - struct { > - /* wl_surface.attach */ > - int newly_attached; > - struct weston_buffer *buffer; > - struct wl_listener buffer_destroy_listener; > - int32_t sx; > - int32_t sy; > - > - /* wl_surface.damage */ > - pixman_region32_t damage; > - > - /* wl_surface.set_opaque_region */ > - pixman_region32_t opaque; > - > - /* wl_surface.set_input_region */ > - pixman_region32_t input; > - > - /* wl_surface.frame */ > - struct wl_list frame_callback_list; > - > - /* wl_surface.set_buffer_transform */ > - /* wl_surface.set_scaling_factor */ > - /* wl_viewport.set */ > - struct weston_buffer_viewport buffer_viewport; > - } pending; > + struct weston_surface_state pending; > > /* > * If non-NULL, this function will be called on > @@ -895,31 +897,9 @@ struct weston_subsurface { > int set; > } position; > > - struct { > - int has_data; > - > - /* wl_surface.attach */ > - int newly_attached; > - struct weston_buffer_reference buffer_ref; > - int32_t sx; > - int32_t sy; > - > - /* wl_surface.damage */ > - pixman_region32_t damage; > - > - /* wl_surface.set_opaque_region */ > - pixman_region32_t opaque; > - > - /* wl_surface.set_input_region */ > - pixman_region32_t input; > - > - /* wl_surface.frame */ > - struct wl_list frame_callback_list; > - > - /* wl_surface.set_buffer_transform */ > - /* wl_surface.set_buffer_scale */ > - struct weston_buffer_viewport buffer_viewport; > - } cached; > + int has_cached_data; > + struct weston_surface_state cached; > + struct weston_buffer_reference cached_buffer_ref; > > int synchronized; > The first two patches apply cleanly, but this third fails to apply, maybe because the empty_region -> pixman_region32_clear patch. If you agree with all my comments, you are welcome to have my reviewed-by, and push the fixed series to master. Only patch 3 needs fixing. Good work! Thanks, pq _______________________________________________ wayland-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/wayland-devel
