On Wed, 15 Feb 2017 12:17:10 +0800 Jonas Ådahl <jad...@gmail.com> wrote:
> On Tue, Feb 14, 2017 at 10:20:06AM -0600, Derek Foreman wrote: > > Attaching a NULL wl_buffer to a surface is not always valid, but > > the previous text indicated it was. > > > > Instead, let's define what NULL attachment does for all the surface > > roles defined in wayland.xml, stop giving a blanket definition of > > its behavior in wl_surface.attach, and warn developers that they > > should refer to other protocol documentation for a full understanding > > of wl_surface.attach. > > Good to see these things cleared up. Have some comments on the wording > below, and the "cursor" role behaviour seems still undefined when > attaching a NULL buffer. > > > > > Signed-off-by: Derek Foreman <der...@osg.samsung.com> > > --- > > > > Changes from v1: > > pretty much everything - define NULL attach for wl_shell specifically > > and remove the generic statement from wl_surface.attach > > refer readers to "other documentation" > > > > protocol/wayland.xml | 10 +++++++--- > > 1 file changed, 7 insertions(+), 3 deletions(-) > > > > diff --git a/protocol/wayland.xml b/protocol/wayland.xml > > index 29b63be..1a76e60 100644 > > --- a/protocol/wayland.xml > > +++ b/protocol/wayland.xml > > @@ -1002,6 +1002,10 @@ > > the related wl_surface is destroyed. On the client side, > > wl_shell_surface_destroy() must be called before destroying > > the wl_surface object. > > + > > + Attaching a NULL wl_buffer to a surface assigned a role by > > + wl_shell will remove the content from the surface after the > > + next commit. > > How is "removes the content" compared to unmaps? Does this mean a shell > implementation need to keep track of the position when the shell surface > is later remapped? That would be a new requirement that was previously > undefined behaviour, and is not what mutter does at the moment. Maintaining position has been how I have always read it, but then I shouldn't be allowed near desktop shell protocols. I even thought that unmap also maintains the position. > > </description> > > > > <request name="pong"> > > @@ -1324,6 +1328,9 @@ > > <description summary="set the surface contents"> > > Set a buffer as the content of this surface. > > > > + The role of the surface influences the behaviour of attach, > > + so this documentation is incomplete without further reading. > > + > > Possible alternative wording, possibly in the end of the <description/>, > as it talks about things that is specified below: > > The effect committing an attached buffer to a surface depends on > what role the surface has been or is going to be assigned to. > See the corresponding role specification for further details. Is it missing the case for "is currently"? I forget our rules about changing an assigned role, but I think the wording you want here is for current or the next assigned role, to cover the case where a wl_surface.commit makes the wl_surface illegal for a following role assignment, not just for doing an illegal thing while the role is already assigned. What role it may have had in the past should not matter. > > This makes read more as the behaviour of *attach* does not change, but > the effect of having attached and committed a buffer. Yes, it is important to make that distiction. Attach alone does nothing, since a following attach may overwrite the pending buffer again before it is committed. Not that clients would be sane to do that, but for completeness. With the fixed wording: Acked-by: Pekka Paalanen <pekka.paala...@collabora.co.uk> Not R-b because I have not read all the affected role specs in wayland.xml. Thanks, pq > > > Jonas > > > The new size of the surface is calculated based on the buffer > > size transformed by the inverse buffer_transform and the > > inverse buffer_scale. This means that the supplied buffer > > @@ -1358,9 +1365,6 @@ > > the surface contents. However, if the client destroys the > > wl_buffer before receiving the wl_buffer.release event, the surface > > contents become undefined immediately. > > - > > - If wl_surface.attach is sent with a NULL wl_buffer, the > > - following wl_surface.commit will remove the surface content. > > </description> > > <arg name="buffer" type="object" interface="wl_buffer" > > allow-null="true" > > summary="buffer of surface contents"/> > > -- > > 2.11.0 > >
pgpLbJSikpjEl.pgp
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel