On Sat, 8 Mar 2014 00:01:49 +0100 Daniel Stone <[email protected]> wrote:
> Hi, > > On 7 March 2014 13:03, Pekka Paalanen <[email protected]> wrote: > > Merge more code into a common function. No functional changes. > > Quick nitpick: does this not break all the pixman_region32_*() calls > in weston_surface_commit(), which rely on surface->{width,height}? > Should be pretty easy to see this breakage when resizing larger with a > steady-state (as opposed to constantly-attaching) client. If it is broken after this, it must have been broken also before this. This patch really does no functional changes: it moves *all* call sites of weston_surface_set_size_from_buffer() into weston_surface_attach(), and all those call sites have a weston_surface_attach() call right before the set_size call. Furthermore, no other place ever calls weston_surface_attach(). The two functions were always called together. That's why I think there are no functional changes. Can you claim otherwise? How could a client ever be resized if it does not attach at all? You cannot change the size of an existing wl_buffer, and changing buffer_scale or _transform without an attach is nonsense, IMO. But, I see one failure path here: wl_viewport changing dst rect should change the surface size regardless of attach. Patch 2 adds a comment pointing out that problem. I just didn't fix it in this series. I don't think patch 1 makes fixing that harder... you can still call set_size_from_buffer(), if wl_viewport dst rect is set without an attach. We cannot call set_size_from_buffer() unconditionally, because the buffer may be gone but the surface still has valid contents, and so the size should stay. This would happen e.g. with frame+commit without an attach, and the last buffer being a wl_shm buffer with gl-renderer so copied and released already. Or did I miss something? :-) Thanks, pq _______________________________________________ wayland-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/wayland-devel
