[RFC wayland 1/1] protocol: Add summary attributes to request params and enum entries

Pekka Paalanen ppaalanen at gmail.com
Tue Jun 7 14:36:13 UTC 2016


On Wed,  1 Jun 2016 10:49:55 -0500
Yong Bakos <junk at humanoriented.com> wrote:

> From: Yong Bakos <ybakos at humanoriented.com>
> 
> Signed-off-by: Yong Bakos <ybakos at humanoriented.com>
> ---
>  protocol/wayland.xml | 334 ++++++++++++++++++++++++++-------------------------
>  1 file changed, 172 insertions(+), 162 deletions(-)
> 
> diff --git a/protocol/wayland.xml b/protocol/wayland.xml
> index 700ef03..d0db6cc 100644
> --- a/protocol/wayland.xml
> +++ b/protocol/wayland.xml
> @@ -48,7 +48,8 @@
> 
>  	The callback_data passed in the callback is the event serial.
>        </description>
> -      <arg name="callback" type="new_id" interface="wl_callback"/>
> +      <arg name="callback" type="new_id" interface="wl_callback"
> +	   summary="callback object for the sync request"/>

It's a new_id, so summary should probably contain "new". And here it
really is a whole new object, but just a handle (see below).

>      </request>
> 
>      <request name="get_registry">
> @@ -57,7 +58,8 @@
>  	to list and bind the global objects available from the
>  	compositor.
>        </description>
> -      <arg name="registry" type="new_id" interface="wl_registry"/>
> +      <arg name="registry" type="new_id" interface="wl_registry"
> +	   summary="global registry object"/>

I was first confused about "global" here, but it does fit. The registry
is a global and a singleton for the display. "global" might be confused
with globals though (the ones advertised with wl_registry.global
event). Maybe call it "a handle to the registry"?

All protocol objects are handles in a sense, but I would not call them
all handles. wl_surface is a wl_surface, it's not a handle to some
other object. wl_registry OTOH is always a handle to the compositor's
registry. wl_compositor is always a handle to the compositor, not
something a client creates when it creates the protocol object.

Does this makes sense?

>      </request>
> 
>      <event name="error">
> @@ -129,8 +131,8 @@
>  	Binds a new, client-created object to the server using the
>          specified name as the identifier.
>        </description>
> -      <arg name="name" type="uint" summary="unique name for the object"/>
> -      <arg name="id" type="new_id"/>
> +      <arg name="name" type="uint" summary="unique numeric name of the global object"/>
> +      <arg name="id" type="new_id" summary="global object to bind"/>

In the C API, a new_id is actually a return value. I wouldn't say it's
the object to bind, rather it is a new object, already bound to the
global.

>      </request>
> 
>      <event name="global">
> @@ -187,14 +189,14 @@
>        <description summary="create new surface">
>  	Ask the compositor to create a new surface.
>        </description>
> -      <arg name="id" type="new_id" interface="wl_surface"/>
> +      <arg name="id" type="new_id" interface="wl_surface" summary="surface to create"/>

That's another interesting choice of words. Is the surface already
created or to be created? Maybe just punt that difference and say "the
new surface"?

>      </request>
> 
>      <request name="create_region">
>        <description summary="create new region">
>  	Ask the compositor to create a new region.
>        </description>
> -      <arg name="id" type="new_id" interface="wl_region"/>
> +      <arg name="id" type="new_id" interface="wl_region" summary="region to create"/>
>      </request>
>    </interface>
> 
> @@ -224,12 +226,12 @@
>  	a buffer from it.
>        </description>
> 
> -      <arg name="id" type="new_id" interface="wl_buffer"/>
> -      <arg name="offset" type="int"/>
> -      <arg name="width" type="int"/>
> -      <arg name="height" type="int"/>
> -      <arg name="stride" type="int"/>
> -      <arg name="format" type="uint" enum="wl_shm.format"/>
> +      <arg name="id" type="new_id" interface="wl_buffer" summary="buffer to create"/>
> +      <arg name="offset" type="int" summary="buffer byte offset within the pool"/>
> +      <arg name="width" type="int" summary="buffer width, in bytes"/>
> +      <arg name="height" type="int" summary="buffer height, in bytes"/>

Width and height are in pixels, not bytes.

offset is bytes, stride is bytes.

Very good to clarify those. :-)

> +      <arg name="stride" type="int" summary="number of bytes from the beginning of one row to the beginning of the next row"/>
> +      <arg name="format" type="uint" enum="wl_shm.format" summary="buffer pixel format"/>
>      </request>
> 
>      <request name="destroy" type="destructor">
> @@ -250,7 +252,7 @@
>  	used to make the pool bigger.
>        </description>
> 
> -      <arg name="size" type="int"/>
> +      <arg name="size" type="int" summary="new size of the pool, in bytes"/>
>      </request>
>    </interface>
> 
> @@ -289,62 +291,62 @@
>        <!-- The drm format codes match the #defines in drm_fourcc.h.
>             The formats actually supported by the compositor will be
>             reported by the format event. -->
> -      <entry name="c8" value="0x20203843"/>
> -      <entry name="rgb332" value="0x38424752"/>
> -      <entry name="bgr233" value="0x38524742"/>
> -      <entry name="xrgb4444" value="0x32315258"/>
> -      <entry name="xbgr4444" value="0x32314258"/>
> -      <entry name="rgbx4444" value="0x32315852"/>
> -      <entry name="bgrx4444" value="0x32315842"/>
> -      <entry name="argb4444" value="0x32315241"/>
> -      <entry name="abgr4444" value="0x32314241"/>
> -      <entry name="rgba4444" value="0x32314152"/>
> -      <entry name="bgra4444" value="0x32314142"/>
> -      <entry name="xrgb1555" value="0x35315258"/>
> -      <entry name="xbgr1555" value="0x35314258"/>
> -      <entry name="rgbx5551" value="0x35315852"/>
> -      <entry name="bgrx5551" value="0x35315842"/>
> -      <entry name="argb1555" value="0x35315241"/>
> -      <entry name="abgr1555" value="0x35314241"/>
> -      <entry name="rgba5551" value="0x35314152"/>
> -      <entry name="bgra5551" value="0x35314142"/>
> -      <entry name="rgb565" value="0x36314752"/>
> -      <entry name="bgr565" value="0x36314742"/>
> -      <entry name="rgb888" value="0x34324752"/>
> -      <entry name="bgr888" value="0x34324742"/>
> -      <entry name="xbgr8888" value="0x34324258"/>
> -      <entry name="rgbx8888" value="0x34325852"/>
> -      <entry name="bgrx8888" value="0x34325842"/>
> -      <entry name="abgr8888" value="0x34324241"/>
> -      <entry name="rgba8888" value="0x34324152"/>
> -      <entry name="bgra8888" value="0x34324142"/>
> -      <entry name="xrgb2101010" value="0x30335258"/>
> -      <entry name="xbgr2101010" value="0x30334258"/>
> -      <entry name="rgbx1010102" value="0x30335852"/>
> -      <entry name="bgrx1010102" value="0x30335842"/>
> -      <entry name="argb2101010" value="0x30335241"/>
> -      <entry name="abgr2101010" value="0x30334241"/>
> -      <entry name="rgba1010102" value="0x30334152"/>
> -      <entry name="bgra1010102" value="0x30334142"/>
> -      <entry name="yuyv" value="0x56595559"/>
> -      <entry name="yvyu" value="0x55595659"/>
> -      <entry name="uyvy" value="0x59565955"/>
> -      <entry name="vyuy" value="0x59555956"/>
> -      <entry name="ayuv" value="0x56555941"/>
> -      <entry name="nv12" value="0x3231564e"/>
> -      <entry name="nv21" value="0x3132564e"/>
> -      <entry name="nv16" value="0x3631564e"/>
> -      <entry name="nv61" value="0x3136564e"/>
> -      <entry name="yuv410" value="0x39565559"/>
> -      <entry name="yvu410" value="0x39555659"/>
> -      <entry name="yuv411" value="0x31315559"/>
> -      <entry name="yvu411" value="0x31315659"/>
> -      <entry name="yuv420" value="0x32315559"/>
> -      <entry name="yvu420" value="0x32315659"/>
> -      <entry name="yuv422" value="0x36315559"/>
> -      <entry name="yvu422" value="0x36315659"/>
> -      <entry name="yuv444" value="0x34325559"/>
> -      <entry name="yvu444" value="0x34325659"/>
> +      <entry name="c8" value="0x20203843" summary="8-bit color index format"/>
> +      <entry name="rgb332" value="0x38424752" summary="8-bit RGB format"/>
> +      <entry name="bgr233" value="0x38524742" summary="8-bit BGR format"/>
> +      <entry name="xrgb4444" value="0x32315258" summary="16-bit xRGB format"/>
> +      <entry name="xbgr4444" value="0x32314258" summary="16-bit xBGR format"/>
> +      <entry name="rgbx4444" value="0x32315852" summary="16-bit RGBx format"/>
> +      <entry name="bgrx4444" value="0x32315842" summary="16-bit BGRx format"/>
> +      <entry name="argb4444" value="0x32315241" summary="16-bit ARGB format"/>
> +      <entry name="abgr4444" value="0x32314241" summary="16-bit ABGR format"/>
> +      <entry name="rgba4444" value="0x32314152" summary="16-bit RBGA format"/>
> +      <entry name="bgra4444" value="0x32314142" summary="16-bit BGRA format"/>
> +      <entry name="xrgb1555" value="0x35315258" summary="16-bit xRGB format"/>
> +      <entry name="xbgr1555" value="0x35314258" summary="16-bit xBGR 1555 format"/>
> +      <entry name="rgbx5551" value="0x35315852" summary="16-bit RGBx 5551 format"/>
> +      <entry name="bgrx5551" value="0x35315842" summary="16-bit BGRx 5551 format"/>
> +      <entry name="argb1555" value="0x35315241" summary="16-bit ARGB 1555 format"/>
> +      <entry name="abgr1555" value="0x35314241" summary="16-bit ABGR 1555 format"/>
> +      <entry name="rgba5551" value="0x35314152" summary="16-bit RGBA 5551 format"/>
> +      <entry name="bgra5551" value="0x35314142" summary="16-bit BGRA 5551 format"/>
> +      <entry name="rgb565" value="0x36314752" summary="16-bit RGB 565 format"/>
> +      <entry name="bgr565" value="0x36314742" summary="16-bit BGR 565 format"/>
> +      <entry name="rgb888" value="0x34324752" summary="24-bit RGB format"/>
> +      <entry name="bgr888" value="0x34324742" summary="24-bit BGR format"/>
> +      <entry name="xbgr8888" value="0x34324258" summary="32-bit xBGR format"/>
> +      <entry name="rgbx8888" value="0x34325852" summary="32-bit RGBx format"/>
> +      <entry name="bgrx8888" value="0x34325842" summary="32-bit BGRx format"/>
> +      <entry name="abgr8888" value="0x34324241" summary="32-bit ABGR format"/>
> +      <entry name="rgba8888" value="0x34324152" summary="32-bit RGBA format"/>
> +      <entry name="bgra8888" value="0x34324142" summary="32-bit BGRA format"/>
> +      <entry name="xrgb2101010" value="0x30335258" summary="32-bit xRGB 2:10:10:10 format"/>
> +      <entry name="xbgr2101010" value="0x30334258" summary="32-bit xBGR 2:10:10:10 format"/>
> +      <entry name="rgbx1010102" value="0x30335852" summary="32-bit RGBx 10:10:10:2 format"/>
> +      <entry name="bgrx1010102" value="0x30335842" summary="32-bit BGRx 10:10:10:2 format"/>
> +      <entry name="argb2101010" value="0x30335241" summary="32-bit ARGB 2:10:10:10 format"/>
> +      <entry name="abgr2101010" value="0x30334241" summary="32-bit ABGR 2:10:10:10 format"/>
> +      <entry name="rgba1010102" value="0x30334152" summary="32-bit RGBA 10:10:10:2 format"/>
> +      <entry name="bgra1010102" value="0x30334142" summary="32-bit BGRA 10:10:10:2 format"/>
> +      <entry name="yuyv" value="0x56595559" summary="packed YCbCr Cr0:Y1:Cb0:Y0 format"/>
> +      <entry name="yvyu" value="0x55595659" summary="packed YCbCr Cb0:Y1:Cr0:Y0 format"/>
> +      <entry name="uyvy" value="0x59565955" summary="packed YCbCr Y1:Cr0:Y0:Cb0 format"/>
> +      <entry name="vyuy" value="0x59555956" summary="packed YCbCr Y1:Cb0:Y0:Cr0 format"/>
> +      <entry name="ayuv" value="0x56555941" summary="packed AYCbCr format"/>
> +      <entry name="nv12" value="0x3231564e" summary="2 plane YCbCr Cr:Cb format"/>
> +      <entry name="nv21" value="0x3132564e" summary="2 plane YCbCr Cb:Cr format"/>
> +      <entry name="nv16" value="0x3631564e" summary="2 plane YCbCr Cr:Cb format"/>
> +      <entry name="nv61" value="0x3136564e" summary="2 plane YCbCr Cb:Cr format"/>
> +      <entry name="yuv410" value="0x39565559" summary="3 plane YCbCr format"/>
> +      <entry name="yvu410" value="0x39555659" summary="3 plane YCbCr format"/>
> +      <entry name="yuv411" value="0x31315559" summary="3 plane YCbCr format"/>
> +      <entry name="yvu411" value="0x31315659" summary="3 plane YCbCr format"/>
> +      <entry name="yuv420" value="0x32315559" summary="3 plane YCbCr format"/>
> +      <entry name="yvu420" value="0x32315659" summary="3 plane YCbCr format"/>
> +      <entry name="yuv422" value="0x36315559" summary="3 plane YCbCr format"/>
> +      <entry name="yvu422" value="0x36315659" summary="3 plane YCbCr format"/>
> +      <entry name="yuv444" value="0x34325559" summary="3 plane YCbCr format"/>
> +      <entry name="yvu444" value="0x34325659" summary="3 plane YCbCr format"/>

That's a good try. Did you look these up from drm_fourcc.h?

You may want to explain somewhere what endianess is being used in this
notation, and understand the array-of-bytes vs. bits-of-unsigned
difference which often causes confusion with 8 bits-per-channel
formats. Those are the usual confusions with pixel formats.

Even better if you can copy the notation from somewehere like
drm_fourcc.h and also explain how it is read.

Also the existing summaries for argb8888 and xrgb8888 could use the
same treatment, even when the value does not match with drm_fourcc.h.

>      </enum>
> 
>      <request name="create_pool">

> @@ -2279,12 +2289,12 @@
>  	This enumeration describes how the physical
>  	pixels on an output are laid out.
>        </description>
> -      <entry name="unknown" value="0"/>
> -      <entry name="none" value="1"/>
> -      <entry name="horizontal_rgb" value="2"/>
> -      <entry name="horizontal_bgr" value="3"/>
> -      <entry name="vertical_rgb" value="4"/>
> -      <entry name="vertical_bgr" value="5"/>
> +      <entry name="unknown" value="0" summary="unkown geometry"/>
> +      <entry name="none" value="1" summary="no geometry"/>

I suppose "none" would be more like where each pixel changes its color
uniformly, without being divided into red, green and blue regions.
Maybe it could apply to Pentile, too.

> +      <entry name="horizontal_rgb" value="2" summary="horizontal RGB"/>
> +      <entry name="horizontal_bgr" value="3" summary="horizontal BGR"/>
> +      <entry name="vertical_rgb" value="4" summary="vertical RGB"/>
> +      <entry name="vertical_bgr" value="5" summary="vertical BGR"/>
>      </enum>
> 
>      <enum name="transform">
> @@ -2302,14 +2312,14 @@
>  	surfaces.
>        </description>
> 
> -      <entry name="normal" value="0"/>
> -      <entry name="90" value="1"/>
> -      <entry name="180" value="2"/>
> -      <entry name="270" value="3"/>
> -      <entry name="flipped" value="4"/>
> -      <entry name="flipped_90" value="5"/>
> -      <entry name="flipped_180" value="6"/>
> -      <entry name="flipped_270" value="7"/>
> +      <entry name="normal" value="0" summary="no transform"/>
> +      <entry name="90" value="1" summary="90 degrees counter-clockwise"/>
> +      <entry name="180" value="2" summary="180 degrees counter-clockwise"/>
> +      <entry name="270" value="3" summary="270 degrees counter-clockwise"/>
> +      <entry name="flipped" value="4" summary="180 degree flip around a vertical axis"/>
> +      <entry name="flipped_90" value="5" summary="flip and rotate 90 degrees counter-clockwise"/>
> +      <entry name="flipped_180" value="6" summary="flip and rotate 180 degrees counter-clockwise"/>
> +      <entry name="flipped_270" value="7" summary="flip and rotate 270 degrees counter-clockwise"/>

Unfortunately, counter-clockwise still does not say what is being
rotated: the framebuffer or the display?

This, and also wl_surface.set_buffer_transform could use a rigorous
definition on which way the transformation goes, but I can't tell you
off-hand what is the right answer.

Also flip is problematic: is it always around vertical axis? Before or
after the rotation?

>      </enum>
> 
>      <event name="geometry">
> @@ -2423,10 +2433,10 @@
>  	Add the specified rectangle to the region.
>        </description>
> 
> -      <arg name="x" type="int"/>
> -      <arg name="y" type="int"/>
> -      <arg name="width" type="int"/>
> -      <arg name="height" type="int"/>
> +      <arg name="x" type="int" summary="TODO"/>
> +      <arg name="y" type="int" summary="TODO"/>
> +      <arg name="width" type="int" summary="rectangle width"/>
> +      <arg name="height" type="int" summary="rectangle height"/>
>      </request>
> 
>      <request name="subtract">
> @@ -2434,10 +2444,10 @@
>  	Subtract the specified rectangle from the region.
>        </description>
> 
> -      <arg name="x" type="int"/>
> -      <arg name="y" type="int"/>
> -      <arg name="width" type="int"/>
> -      <arg name="height" type="int"/>
> +      <arg name="x" type="int" summary="TODO"/>
> +      <arg name="y" type="int" summary="TODO"/>
> +      <arg name="width" type="int" summary="rectangle width"/>
> +      <arg name="height" type="int" summary="rectangle height"/>
>      </request>
>    </interface>
> 

A region has its own coordinate space. It is completely arbitrary. Only
when a region is attached to something, e.g. with
wl_surface.set_opaque_region, the coordinate space gets a tangible
meaning. Until then its just numbers in unspecified units.


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 811 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20160607/ff8e28d9/attachment.sig>


More information about the wayland-devel mailing list