<div dir="ltr"> <div class="gmail_extra"> <div class="gmail_quote">On Mon, Jul 21, 2014 at 1:47 AM, Sjoerd Simons <<a href="mailto:sjoerd.simons@collabora.co.uk" target="_blank">sjoerd.simons@collabora.co.uk</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Remove the explicit destroy method from xdg_surface and xdg_popup as neither of them can be re-created after being destroyed. As a results a wl_surface gets into an odd sort of odd limbo state after a xdg_{surface,popup}.destroy call where it not only no longer has an xdg_{surface,popup} associated with it but where it's also not possible to re-created one. This also happens to align the life-time rules of xdg_{surface,popup} to match those of wl_shell_surface, which is probably good for consistency. </blockquote><div> </div><div>Sjoerd, First of all, making xdg_shell consistent with wl_shell is a non-reason for doing anything. The objective to xdg_shell is to *replace* wl_shell so they are going to have different symantics. </div><div>Second, regarding your original patch. I'm sorry that it never got reviewed, but implementation details of desktop-shell are, again, not a good driving force for xdg_shell. If weston's desktop-shell has wl_shell_surface symantics too closely tied to xdg_shell, that's a bug in weston's desktop-shell, not a bug in xdg_shell. Right now, weston's desktop-shell is kind of a mess and, yes, it makes a lot of assumptions about the lifecycles of objects both internal and external. When I did the surface/view split, it was a pain because it made the tacit assumption that a shell surface was a surface was a place on-screen. Separating these assumptions will make desktop-shell much cleaner. </div><div>Third is the question of whether we want destructors or not. One of the things we have learned is that destructors are very hard to properly add later. If there is any chance that we will need them, we have to add them before we stabilize the protocol. Also, I don't think there's any real harm except for the possible case where the xdg_surface is hanging around after the wl_surface is destroyed. I'd be 100% ok with defining any requests on the xdg_surface to result in a protocol error if the wl_surface is destroyed. It's possible that we could remove the destructor and tie the two lifecycles together. However, we need to be *really* careful about doing so. </div><div>--Jason Ekstrand </div> <div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> Signed-off-by: Sjoerd Simons <<a href="mailto:sjoerd.simons@collabora.co.uk" target="_blank">sjoerd.simons@collabora.co.uk</a>> --- desktop-shell/shell.c | 25 ++----------------------- protocol/xdg-shell.xml | 24 ++++-------------------- 2 files changed, 6 insertions(+), 43 deletions(-) diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c index 593c7f3..efec100 100644 --- a/desktop-shell/shell.c +++ b/desktop-shell/shell.c @@ -3370,13 +3370,6 @@ static const struct wl_shell_interface shell_implementation = { * xdg-shell implementation */ static void -xdg_surface_destroy(struct wl_client *client, - struct wl_resource *resource) -{ - wl_resource_destroy(resource); -} - -static void xdg_surface_set_parent(struct wl_client *client, struct wl_resource *resource, struct wl_resource *parent_resource) @@ -3534,7 +3527,6 @@ xdg_surface_set_minimized(struct wl_client *client, } static const struct xdg_surface_interface xdg_surface_implementation = { - xdg_surface_destroy, xdg_surface_set_parent, xdg_surface_set_title, xdg_surface_set_app_id, @@ -3662,18 +3654,6 @@ shell_surface_is_xdg_surface(struct shell_surface *shsurf) } /* xdg-popup implementation */ - -static void -xdg_popup_destroy(struct wl_client *client, - struct wl_resource *resource) -{ - wl_resource_destroy(resource); -} - -static const struct xdg_popup_interface xdg_popup_implementation = { - xdg_popup_destroy, -}; - static void xdg_popup_send_configure(struct weston_surface *surface, int32_t width, int32_t height) @@ -3753,8 +3733,7 @@ xdg_get_xdg_popup(struct wl_client *client, wl_resource_create(client, &xdg_popup_interface, 1, id); wl_resource_set_implementation(shsurf->resource, - &xdg_popup_implementation, - shsurf, shell_destroy_shell_surface); + NULL, shsurf, shell_destroy_shell_surface); } static void @@ -3771,7 +3750,7 @@ shell_surface_is_xdg_popup(struct shell_surface *shsurf) { return wl_resource_instance_of(shsurf->resource, &xdg_popup_interface, - &xdg_popup_implementation); + NULL); } static const struct xdg_shell_interface xdg_implementation = { diff --git a/protocol/xdg-shell.xml b/protocol/xdg-shell.xml index bd36231..ea005bc 100644 --- a/protocol/xdg-shell.xml +++ b/protocol/xdg-shell.xml @@ -127,16 +127,6 @@ the wl_surface object. </description> - <request name="destroy" type="destructor"> - <description summary="remove xdg_surface interface"> - The xdg_surface interface is removed from the wl_surface object - that was turned into a xdg_surface with - xdg_shell.get_xdg_surface request. The xdg_surface properties, - like maximized and fullscreen, are lost. The wl_surface loses - its role as a xdg_surface. The wl_surface is unmapped. - </description> - </request> - <request name="set_parent"> <description summary="surface is a child of another surface"> Child surfaces are stacked above their parents, and will be @@ -388,17 +378,11 @@ parent surface, in surface local coordinates. xdg_popup surfaces are always transient for another surface. - </description> - <request name="destroy" type="destructor"> - <description summary="remove xdg_surface interface"> - The xdg_surface interface is removed from the wl_surface object - that was turned into a xdg_surface with - xdg_shell.get_xdg_surface request. The xdg_surface properties, - like maximized and fullscreen, are lost. The wl_surface loses - its role as a xdg_surface. The wl_surface is unmapped. - </description> - </request> + On the server side the object is automatically destroyed when + the related wl_surface is destroyed. On client side, xdg_popup.destroy() + must be called before destroying the wl_surface object. + </description> <event name="popup_done"> <description summary="popup interaction is done"> -- 2.0.1 _______________________________________________ wayland-devel mailing list <a href="mailto:wayland-devel@lists.freedesktop.org" target="_blank">wayland-devel@lists.freedesktop.org</a> <a href="http://lists.freedesktop.org/mailman/listinfo/wayland-devel" target="_blank">http://lists.freedesktop.org/mailman/listinfo/wayland-devel</a> </blockquote></div> </div></div>