[RFCv3 weston 01/15] compositor: refactor more into weston_surface_attach

[Pekka Paalanen]

On Sat, 8 Mar 2014 00:01:49 +0100
Daniel Stone <daniel at fooishbar.org> wrote:

> Hi,
> 
> On 7 March 2014 13:03, Pekka Paalanen <ppaalanen at gmail.com> 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