On Mar 6, 2014 7:08 PM, "Nobuhiko Tanibata" <<a href="mailto:nobuhiko_tanibata@xddp.denso.co.jp">nobuhiko_tanibata@xddp.denso.co.jp</a>> wrote: > > 2014-03-07 00:21 に Jason Ekstrand さんは書きました: > >> On Mar 6, 2014 3:53 AM, "Nobuhiko Tanibata" >> <<a href="mailto:NOBUHIKO_TANIBATA@xddp.denso.co.jp">NOBUHIKO_TANIBATA@xddp.denso.co.jp</a>> wrote: >> > >> > Add interface ivi_application, which creates ivi_surface objects >> tied >> > to a given wl_surface with a given id. The given id can be used in >> a >> > shell to identify which application is assigned to a wl_surface and >> > layout the surface wherever the shell wants. ivi_surface objects >> can >> > be used to receive status of wl_surface in the scenegraph of the >> > compositor. >> >> In general, I think this looks pretty good. It's nice and simple and >> only adds what's needed for clients. I do have a couple comments >> below. >> --Jason Ekstrand >> >>> >> > Signed-off-by: Nobuhiko Tanibata >> <<a href="mailto:NOBUHIKO_TANIBATA@xddp.denso.co.jp">NOBUHIKO_TANIBATA@xddp.denso.co.jp</a>> >> > --- >> > protocol/ivi-application.xml | 88 >> ++++++++++++++++++++++++++++++++++++++++++++ >> > 1 file changed, 88 insertions(+) >> > create mode 100644 protocol/ivi-application.xml >> > >> > diff --git a/protocol/ivi-application.xml >> b/protocol/ivi-application.xml >> > new file mode 100644 >> > index 0000000..e58ad26 >> > --- /dev/null >> > +++ b/protocol/ivi-application.xml >> > @@ -0,0 +1,88 @@ >> > +<?xml version="1.0" encoding="UTF-8"?> >> > +<protocol name="ivi_application"> >> > + >> > + <copyright> >> > + Copyright (C) 2013 DENSO CORPORATION >> > + Copyright (c) 2013 BMW Car IT GmbH >> > + >> > + Permission is hereby granted, free of charge, to any person >> obtaining a copy >> > + of this software and associated documentation files (the >> "Software"), to deal >> > + in the Software without restriction, including without >> limitation the rights >> > + to use, copy, modify, merge, publish, distribute, >> sublicense, and/or sell >> > + copies of the Software, and to permit persons to whom the >> Software is >> > + furnished to do so, subject to the following conditions: >> > + >> > + The above copyright notice and this permission notice shall >> be included in >> > + all copies or substantial portions of the Software. >> > + >> > + THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY >> KIND, EXPRESS OR >> > + IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF >> MERCHANTABILITY, >> > + FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO >> EVENT SHALL THE >> > + AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, >> DAMAGES OR OTHER >> > + LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR >> OTHERWISE, ARISING FROM, >> > + OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR >> OTHER DEALINGS IN >> > + THE SOFTWARE. >> > + </copyright> >> > + >> > + <interface name="ivi_surface" version="1"> >> > + <description summary="application interface to surface >> in ivi compositor"/> >> > + >> > + <request name="destroy" type="destructor"> >> > + <description summary="destroy ivi_surface"/> >> > + </request> >> > + >> > + <event name="visibility"> >> > + <description summary="visibility of surface in >> ivi compositor has changed"> >> > + The new visibility state is provided in >> argument visibility. >> > + If visibility is 0, the surface has become >> invisible. >> > + If visibility is not 0, the surface has >> become visible. >> > + </description> >> > + <arg name="visibility" type="int"/> >> > + </event> >> > + >> > + </interface> >> > + >> > + <interface name="ivi_application" version="1"> >> > + <description summary="interface for ivi applications >> to use ivi compositor features"/> >> > + >> > + <request name="surface_create"> >> > + <description summary="create surface in ivi >> compositor"> >> > + surface_create will create a new surface >> with surface_id in ivi compositor, >> > + if it does not yet exists. If the surface >> with surface_id already exists in >> > + ivi compositor, the application content >> provided in argument surface will >> > + be used as surface content. If an other >> ivi application already registered >> > + content for surface with surface_id, an >> error event will indicate the problem. >> > + </description> >> > + <arg name="id_surface" type="uint"/> >> > + <arg name="surface" type="object" >> interface="wl_surface"/> >> > + <arg name="id" type="new_id" >> interface="ivi_surface"/> >> > + </request> >> > + >> > + <enum name="error_code"> >> > + <description summary="possible error codes >> returned by ivi compositor"> >> > + These error codes define all possible >> error codes returned by ivi compositor >> > + on server-side errors. >> > + </description> >> > + <entry name="unknown_error" value="1" >> summary="unknown error encountered"/> >> >> What kinds of things do you use "unknown error" for? I'm not sure >> how that's useful to the client. >> > > Hi Jason, > > Thank you for reviewing. > > I see. From this review point, it shall not return no useful value to the client. > I break down more here. > - id in use: specified ID for ivi_surface is already used by itself or other client. > - resource in use: specified ID of wl_surface is already tie to another ID of ivi_surface. > - invalid resource: specified ID of wl_surface is not valid in server. > > I think these three is sufficient for client. > > >>> + <entry name="resource_in_use" value="2" >> >> summary="resource is in use and can not be shared"/> >> >> What kind of resource does this refer to? It looks like you use it >> if someone tries to call surface_create twice on the same surface, but >> you don't specify. >> >>> + </enum> >> >> > + >> > + <event name="error"> >> > + <description summary="server-side error >> detected"> >> > + The ivi compositor encountered error while >> processing a request by this >> > + application. The error is defined by >> argument error_code and optional >> > + error_text. >> > + If the application requires to associate >> this error event to a request, >> > + it can >> > + 1. send request >> > + 2. force display roundtrip >> > + 3. check, if error event was >> received >> > + but this restricts the application to >> have only one open request at a time. >> > + </description> >> > + <arg name="error_code" type="int"/> >> > + <arg name="error_text" type="string" >> allow-null="true"/> >> > + </event> >> >> Why are you using this rather than the built-in wl_display.error? >> The built-in wl_display.error also terminates the client when an error >> is sent. Under what error conditions would you want the client to >> continue? >> > > Good suggestion! > According to wayland.xml, error code from 0 to 2 is reserved for global. > Please teach me there is any manner which number I can use for the above object? In the feature, if wayland.xml updated to add new global error code to 3, the local number may need to be incremented. The wl_display.error event is a bit confusing. The wl_resource_post_error function takes three arguments. The first argument is an object; the last two are an error code and a human-readable message string. The error code should be interpreted relative to the given object. Therefore, you don't need to worry about colliding with the codes defined in wl_display. If the error is defined in wl_application you should pass the wl_application resource into the error event. One warning about wl_display.error event: all errors are considered fatal and the client gets automatically disconnected. Given the errors you listed, this is probably OK. If you want a non-fatal error, you should use something else And probably not call it "error" (to avoid confusion). Hope that helps, --Jason Ekstrand > > BR, > Nobuhiko >>> >>> + >> >> > + </interface> >> > + >> > +</protocol> >> > -- >> > 1.8.3.1 >> > >> > _______________________________________________ >> > wayland-devel mailing list >> > <a href="mailto:wayland-devel@lists.freedesktop.org">wayland-devel@lists.freedesktop.org</a> >> > <a href="http://lists.freedesktop.org/mailman/listinfo/wayland-devel">http://lists.freedesktop.org/mailman/listinfo/wayland-devel</a> [1] >> >> >> Links: >> ------ >> [1] <a href="http://lists.freedesktop.org/mailman/listinfo/wayland-devel">http://lists.freedesktop.org/mailman/listinfo/wayland-devel</a>