[PATCH weston-ivi-shell 02/15] ivi application protocol:
Jason Ekstrand
jason at jlekstrand.net
Thu Mar 6 18:37:07 PST 2014
On Mar 6, 2014 7:08 PM, "Nobuhiko Tanibata" <
nobuhiko_tanibata at xddp.denso.co.jp> wrote:
>
> 2014-03-07 00:21 に Jason Ekstrand さんは書きました:
>
>> On Mar 6, 2014 3:53 AM, "Nobuhiko Tanibata"
>> <NOBUHIKO_TANIBATA at xddp.denso.co.jp> 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
>> <NOBUHIKO_TANIBATA at xddp.denso.co.jp>
>> > ---
>> > 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
>> > wayland-devel at lists.freedesktop.org
>> > http://lists.freedesktop.org/mailman/listinfo/wayland-devel [1]
>>
>>
>> Links:
>> ------
>> [1] http://lists.freedesktop.org/mailman/listinfo/wayland-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20140306/eee29eee/attachment-0001.html>
More information about the wayland-devel
mailing list