Discussion:
[Xcb] [PATCH] Add more documentation.
Thomas Fischer
2018-08-26 05:14:25 UTC
Permalink
This patch touches up the example for map_window and adds some
documentation for TODO fields, mostly in create_gc.

I noticed the TODO in libxcb/c_client.py that said that including enums
is hardcoded 'until xproto.xml is fixed.' I changed the code to include
the "GC" enum for create_gc, but how could I go about "un-hardcoding"
it? I'm still a little new to xml.

---
src/xproto.xml | 156 ++++++++++++++++++++++++++++++++++---------------
1 file changed, 108 insertions(+), 48 deletions(-)

diff --git a/src/xproto.xml b/src/xproto.xml
index 9624700..f78f339 100644
--- a/src/xproto.xml
+++ b/src/xproto.xml
@@ -1828,6 +1828,10 @@ <xcb/xcb.h>
xcb_screen_iterator_t iter = xcb_setup_roots_iterator (setup);
xcb_screen_t *screen = iter.data;

+ uint32_t value_mask = XCB_CW_BACK_PIXEL | XCB_CW_EVENT_MASK;
+ uint32_t value_list[] = { screen->black_pixel,
+ XCB_EVENT_MASK_KEY_RELEASE | XCB_EVENT_MASK_EXPOSE
+ /* | XCB_EVENT_MASK_* ... */ };

/* Create the window */
xcb_window_t window = xcb_generate_id (connection);
@@ -1840,7 +1844,8 @@ <xcb/xcb.h>
10, /* border_width */
XCB_WINDOW_CLASS_INPUT_OUTPUT, /* class */
screen->root_visual, /* visual */
- 0, NULL ); /* masks, not used yet */
+ value_mask, value_list ); /* masks, creates window
+ that receives KEY_RELEASE and EXPOSE events */


/* Map the window on the screen */
@@ -3410,7 +3415,7 @@ <request name="WarpPointer" opcode="41">
relative to the current position of the pointer.
]]></field>
<error type="Window"><![CDATA[
-TODO: reasons?
+Either the `src_window` or the `dst_window` do not refer to a defined `window`.
]]></error>
<see type="request" name="SetInputFocus" />
</doc>
@@ -3859,9 +3864,11 @@ <request name="CreatePixmap" opcode="53">
<description><![CDATA[
Creates a pixmap. The pixmap can only be used on the same screen as `drawable`
is on and only with drawables of the same `depth`.
+
+The initial contents of the pixmap are undefined.
]]></description>
<field name="depth"><![CDATA[
-TODO
+Specifies the new pixmap's color depth in pits per pixel.
]]></field>
<field name="pid"><![CDATA[
The ID with which you will refer to the new pixmap, created by
@@ -3869,6 +3876,9 @@ <request name="CreatePixmap" opcode="53">
]]></field>
<field name="drawable"><![CDATA[
Drawable to get the screen from.
+
+It is `legal` to pass an `XCB_WINDOW_CLASS_INPUT_ONLY` drawable to this
+request.
]]></field>
<field name="width"><![CDATA[
The width of the new pixmap.
@@ -3877,7 +3887,8 @@ <request name="CreatePixmap" opcode="53">
The height of the new pixmap.
]]></field>
<error type="Value"><![CDATA[
-TODO: reasons?
+The `width` and `height` of the pixmap must be nonzero. The `depth` must be
+supported by the root of the specified drawable.
]]></error>
<error type="Drawable"><![CDATA[
The specified `drawable` (Window or Pixmap) does not exist.
@@ -3948,36 +3959,46 @@ <enum name="GC">
Background colorpixel.
]]></field>
<field name="LineWidth"><![CDATA[
-The line-width is measured in pixels and can be greater than or equal to one, a wide line, or the
-special value zero, a thin line.
+The line-width is measured in pixels and can be greater than or equal to one, a
+wide line, or the special value zero, a thin line.
]]></field>
<field name="LineStyle"><![CDATA[
The line-style defines which sections of a line are drawn:
+
Solid The full path of the line is drawn.
+
DoubleDash The full path of the line is drawn, but the even dashes are filled differently
than the odd dashes (see fill-style), with Butt cap-style used where even and
odd dashes meet.
+
OnOffDash Only the even dashes are drawn, and cap-style applies to all internal ends of
the individual dashes (except NotLast is treated as Butt).
]]></field>
<field name="CapStyle"><![CDATA[
The cap-style defines how the endpoints of a path are drawn:
+
NotLast The result is equivalent to Butt, except that for a line-width of zero the final
endpoint is not drawn.
+
Butt The result is square at the endpoint (perpendicular to the slope of the line)
with no projection beyond.
+
Round The result is a circular arc with its diameter equal to the line-width, centered
on the endpoint; it is equivalent to Butt for line-width zero.
+
Projecting The result is square at the end, but the path continues beyond the endpoint for
a distance equal to half the line-width; it is equivalent to Butt for line-width
zero.
]]></field>
<field name="JoinStyle"><![CDATA[
The join-style defines how corners are drawn for wide lines:
+
Miter The outer edges of the two lines extend to meet at an angle. However, if the
angle is less than 11 degrees, a Bevel join-style is used instead.
+
Round The result is a circular arc with a diameter equal to the line-width, centered
on the joinpoint.
+
Bevel The result is Butt endpoint styles, and then the triangular notch is filled.
]]></field>
<field name="FillStyle"><![CDATA[
@@ -3986,53 +4007,70 @@ <enum name="GC">
as well as for line requests with line-style Solid, (for example, PolyLine, PolySegment,
PolyRectangle, PolyArc) and for the even dashes for line requests with line-style OnOffDash
or DoubleDash:
+
Solid Foreground
+
Tiled Tile
+
OpaqueStippled A tile with the same width and height as stipple but with background
everywhere stipple has a zero and with foreground everywhere stipple
has a one
+
Stippled Foreground masked by stipple
+
For the odd dashes for line requests with line-style DoubleDash:
+
Solid Background
+
Tiled Same as for even dashes
+
OpaqueStippled Same as for even dashes
+
Stippled Background masked by stipple
]]></field>
<field name="FillRule"><![CDATA[
]]></field>
<field name="Tile"><![CDATA[
-The tile/stipple represents an infinite two-dimensional plane with the tile/stipple replicated in all
-dimensions. When that plane is superimposed on the drawable for use in a graphics operation,
-the upper-left corner of some instance of the tile/stipple is at the coordinates within the drawable
-specified by the tile/stipple origin. The tile/stipple and clip origins are interpreted relative to the
-origin of whatever destination drawable is specified in a graphics request.
-The tile pixmap must have the same root and depth as the gcontext (or a Match error results).
-The stipple pixmap must have depth one and must have the same root as the gcontext (or a
-Match error results). For fill-style Stippled (but not fill-style
-OpaqueStippled), the stipple pattern is tiled in a single plane and acts as an
-additional clip mask to be ANDed with the clip-mask.
-Any size pixmap can be used for tiling or stippling, although some sizes may be faster to use than
-others.
+The tile/stipple represents an infinite two-dimensional plane with the
+tile/stipple replicated in all dimensions. When that plane is superimposed on
+the drawable for use in a graphics operation, the upper-left corner of some
+instance of the tile/stipple is at the coordinates within the drawable
+specified by the tile/stipple origin. The tile/stipple and clip origins are
+interpreted relative to the origin of whatever destination drawable is
+specified in a graphics request.
+
+The tile pixmap must have the same root and depth as the gcontext (or a Match
+error results). The stipple pixmap must have depth one and must have the same
+root as the gcontext (or a Match error results). For fill-style Stippled (but
+not fill-style OpaqueStippled), the stipple pattern is tiled in a single plane
+and acts as an additional clip mask to be ANDed with the clip-mask.
+
+Any size pixmap can be used for tiling or stippling, although some sizes may be
+faster to use than others.
]]></field>
<field name="Stipple"><![CDATA[
-The tile/stipple represents an infinite two-dimensional plane with the tile/stipple replicated in all
-dimensions. When that plane is superimposed on the drawable for use in a graphics operation,
-the upper-left corner of some instance of the tile/stipple is at the coordinates within the drawable
-specified by the tile/stipple origin. The tile/stipple and clip origins are interpreted relative to the
-origin of whatever destination drawable is specified in a graphics request.
-The tile pixmap must have the same root and depth as the gcontext (or a Match error results).
-The stipple pixmap must have depth one and must have the same root as the gcontext (or a
-Match error results). For fill-style Stippled (but not fill-style
-OpaqueStippled), the stipple pattern is tiled in a single plane and acts as an
-additional clip mask to be ANDed with the clip-mask.
-Any size pixmap can be used for tiling or stippling, although some sizes may be faster to use than
-others.
+The tile/stipple represents an infinite two-dimensional plane with the
+tile/stipple replicated in all dimensions. When that plane is superimposed on
+the drawable for use in a graphics operation, the upper-left corner of some
+instance of the tile/stipple is at the coordinates within the drawable
+specified by the tile/stipple origin. The tile/stipple and clip origins are
+interpreted relative to the origin of whatever destination drawable is
+specified in a graphics request.
+
+The tile pixmap must have the same root and depth as the gcontext (or a Match
+error results). The stipple pixmap must have depth one and must have the same
+root as the gcontext (or a Match error results). For fill-style Stippled (but
+not fill-style OpaqueStippled), the stipple pattern is tiled in a single plane
+and acts as an additional clip mask to be ANDed with the clip-mask.
+
+Any size pixmap can be used for tiling or stippling, although some sizes may be
+faster to use than others.
]]></field>
<field name="TileStippleOriginX"><![CDATA[
-TODO
+Specifies the X origin in the drawable for the `upper-left` corner of the tile/stipple pixmap.
]]></field>
<field name="TileStippleOriginY"><![CDATA[
-TODO
+Specifies the Y origin in the drawable for the `upper-left` corner of the tile/stipple pixmap.
]]></field>
<field name="Font"><![CDATA[
Which font to use for the `ImageText8` and `ImageText16` requests.
@@ -4040,11 +4078,11 @@ <enum name="GC">
<field name="SubwindowMode"><![CDATA[
For ClipByChildren, both source and destination windows are additionally
clipped by all viewable InputOutput children. For IncludeInferiors, neither
-source nor destination window is
-clipped by inferiors. This will result in including subwindow contents in the source and drawing
-through subwindow boundaries of the destination. The use of IncludeInferiors with a source or
-destination window of one depth with mapped inferiors of differing depth is not illegal, but the
-semantics is undefined by the core protocol.
+source nor destination window is clipped by inferiors. This will result in
+including subwindow contents in the source and drawing through subwindow
+boundaries of the destination. The use of IncludeInferiors with a source or
+destination window of one depth with mapped inferiors of differing depth is not
+illegal, but the semantics is undefined by the core protocol.
]]></field>
<field name="GraphicsExposures"><![CDATA[
Whether ExposureEvents should be generated (1) or not (0).
@@ -4052,28 +4090,43 @@ <enum name="GC">
The default is 1.
]]></field>
<field name="ClipOriginX"><![CDATA[
-TODO
+Specifies the X origin in the drawable for the `upper-left` corner of the `ClipMask` pixmap.
]]></field>
<field name="ClipOriginY"><![CDATA[
-TODO
+Specifies the Y origin in the drawable for the `upper-left` corner of the `ClipMask` pixmap.
]]></field>
<field name="ClipMask"><![CDATA[
-The clip-mask restricts writes to the destination drawable. Only pixels where the clip-mask has
-bits set to 1 are drawn. Pixels are not drawn outside the area covered by the clip-mask or where
-the clip-mask has bits set to 0. The clip-mask affects all graphics requests, but it does not clip
-sources. The clip-mask origin is interpreted relative to the origin of whatever destination drawable is specified in a graphics request. If a pixmap is specified as the clip-mask, it must have
-depth 1 and have the same root as the gcontext (or a Match error results). If clip-mask is None,
-then pixels are always drawn, regardless of the clip origin. The clip-mask can also be set with the
+The clip-mask restricts writes to the destination drawable. Only pixels where
+the clip-mask has bits set to 1 are drawn. Pixels are not drawn outside the
+area covered by the clip-mask or where the clip-mask has bits set to 0. The
+clip-mask affects all graphics requests, but it does not clip sources. The
+clip-mask origin is interpreted relative to the origin of whatever destination
+drawable is specified in a graphics request. If a pixmap is specified as the
+clip-mask, it must have depth 1 and have the same root as the gcontext (or a
+Match error results). If clip-mask is None, then pixels are always drawn,
+regardless of the clip origin. The clip-mask can also be set with the
SetClipRectangles request.
]]></field>
<field name="DashOffset"><![CDATA[
-TODO
+Defines the phase of the dash pattern, specifying how many pixels into `dashes`
+the pattern should actually begin in any single graphics request. Dashing is
+continuous through path elements combined with a join-style but is reset to the
+`DashOffset` between each sequence of joined lines.
]]></field>
<field name="DashList"><![CDATA[
-TODO
+Specifies a list of `dash lengths` in pixels. All of the elements must be
+nonzero or a `Value` error occurs. The length of `DashList` must be nonzero or
+a `Value` error occurs.
+
]]></field>
<field name="ArcMode"><![CDATA[
-TODO
+The `ArcMode` controls filling in the `PolyFillArc` request.
+
+For `Chord`, the single line segment joining the endpoints of the arc is used.
+
+For `PieSlice`, the two line segments joining the endpoints of the arc with the
+center point are used.
+
]]></field>
</doc>

@@ -4252,6 +4305,13 @@ <request name="CreateGC" opcode="55">
]]></field>
<field name="drawable"><![CDATA[
Drawable to get the root/depth from.
+ ]]></field>
+ <!-- the enum documentation is good enough. -->
+ <field name="value_mask" />
+ <field name="value_list"><![CDATA[
+Values for each of the components specified in the bitmask `value_mask`. The
+order has to correspond to the order of possible `value_mask` bits. See the
+example in xcb_chane_gc(3).
]]></field>
<error type="Drawable"><![CDATA[
The specified `drawable` (Window or Pixmap) does not exist.
--
2.18.0
Uli Schlachter
2018-09-16 16:33:36 UTC
Permalink
Hi Thomas,

sorry for being awfully slow.

On 26.08.2018 07:14, Thomas Fischer wrote:
[...]> diff --git a/src/xproto.xml b/src/xproto.xml
Post by Thomas Fischer
index 9624700..f78f339 100644
--- a/src/xproto.xml
+++ b/src/xproto.xml
@@ -1828,6 +1828,10 @@ <xcb/xcb.h>
xcb_screen_iterator_t iter = xcb_setup_roots_iterator (setup);
xcb_screen_t *screen = iter.data;
+ uint32_t value_mask = XCB_CW_BACK_PIXEL | XCB_CW_EVENT_MASK;
+ uint32_t value_list[] = { screen->black_pixel,
+ XCB_EVENT_MASK_KEY_RELEASE | XCB_EVENT_MASK_EXPOSE
+ /* | XCB_EVENT_MASK_* ... */ };
[...]

What did you base your patch on? The string "xcb_setup_roots_iterator"
does not appear in the current(?) version of xproto.xml. Thus, your
patch does not apply here.

Cheers,
Uli
--
99 little bugs in the code
99 little bugs in the code
Take one down, patch it around
117 little bugs in the code
-- @irqed
Thomas Fischer
2018-09-16 22:36:54 UTC
Permalink
Post by Uli Schlachter
Hi Thomas,
sorry for being awfully slow.
[...]> diff --git a/src/xproto.xml b/src/xproto.xml
Post by Thomas Fischer
index 9624700..f78f339 100644
--- a/src/xproto.xml
+++ b/src/xproto.xml
@@ -1828,6 +1828,10 @@ <xcb/xcb.h>
xcb_screen_iterator_t iter = xcb_setup_roots_iterator (setup);
xcb_screen_t *screen = iter.data;
+ uint32_t value_mask = XCB_CW_BACK_PIXEL | XCB_CW_EVENT_MASK;
+ uint32_t value_list[] = { screen->black_pixel,
+ XCB_EVENT_MASK_KEY_RELEASE | XCB_EVENT_MASK_EXPOSE
+ /* | XCB_EVENT_MASK_* ... */ };
[...]
What did you base your patch on? The string "xcb_setup_roots_iterator"
does not appear in the current(?) version of xproto.xml. Thus, your
patch does not apply here.
Cheers,
Uli
Hello, it's no problem!

The example was based on the tutorial here:
https://xcb.freedesktop.org/tutorial/basicwindowsanddrawing/

The "xcb_setup_roots_iterator" function is generated in
libxcb/src/xproto.{h,c}
Most of the "*_iterator", "*_next" etc. functions are generated, and have
no documentation.
I think the functions "xcb_setup_roots_iterator", "xcb_setup_next",
"xcb_setup_end", etc. are generated by libxcb/src/c_client.py from the
struct "Setup" in xproto.xml:

<struct name="Setup">
<field type="CARD8" name="status" /> <!-- always 1 -> Success -->
<pad bytes="1" />
<field type="CARD16" name="protocol_major_version" />
<field type="CARD16" name="protocol_minor_version" />
<field type="CARD16" name="length" />
<field type="CARD32" name="release_number" />
<field type="CARD32" name="resource_id_base" />
<field type="CARD32" name="resource_id_mask" />
<field type="CARD32" name="motion_buffer_size" />
<field type="CARD16" name="vendor_len" />
<field type="CARD16" name="maximum_request_length" />
<field type="CARD8" name="roots_len" />
<field type="CARD8" name="pixmap_formats_len" />
<field type="CARD8" name="image_byte_order" enum="ImageOrder" />
<field type="CARD8" name="bitmap_format_bit_order" enum="ImageOrder" />
<field type="CARD8" name="bitmap_format_scanline_unit" />
<field type="CARD8" name="bitmap_format_scanline_pad" />
<field type="KEYCODE" name="min_keycode" />
<field type="KEYCODE" name="max_keycode" />
<pad bytes="4" />
<list type="char" name="vendor">
<fieldref>vendor_len</fieldref>
</list>
<pad align="4" />
<list type="FORMAT" name="pixmap_formats">
<fieldref>pixmap_formats_len</fieldref>
</list>
<list type="SCREEN" name="roots">
<fieldref>roots_len</fieldref>
</list>

Should I change the example, or maybe try to write documentation for these
functions?

Thanks for taking the time to review it,
Thomas
Uli Schlachter
2018-09-17 06:51:42 UTC
Permalink
Post by Thomas Fischer
Post by Uli Schlachter
Hi Thomas,
sorry for being awfully slow.
[...]> diff --git a/src/xproto.xml b/src/xproto.xml
Post by Thomas Fischer
index 9624700..f78f339 100644
--- a/src/xproto.xml
+++ b/src/xproto.xml
@@ -1828,6 +1828,10 @@ <xcb/xcb.h>
xcb_screen_iterator_t iter = xcb_setup_roots_iterator (setup);
xcb_screen_t *screen = iter.data;
+ uint32_t value_mask = XCB_CW_BACK_PIXEL | XCB_CW_EVENT_MASK;
+ uint32_t value_list[] = { screen->black_pixel,
+ XCB_EVENT_MASK_KEY_RELEASE | XCB_EVENT_MASK_EXPOSE
+ /* | XCB_EVENT_MASK_* ... */ };
[...]
What did you base your patch on? The string "xcb_setup_roots_iterator"
does not appear in the current(?) version of xproto.xml. Thus, your
patch does not apply here.
Cheers,
Uli
Hello, it's no problem!
https://xcb.freedesktop.org/tutorial/basicwindowsanddrawing/
The "xcb_setup_roots_iterator" function is generated in
libxcb/src/xproto.{h,c}
Sorry if I was not clear.

My problem is not the content of your patch, but that the patch does not
apply. Your patch touches code next to an example with
"xcb_setup_roots_iterator", but there is no such example in the current
version of xproto.xml.

After taking another look at this, I think I found the problem:

I have to first apply your patch "Add documentation" from beginning of
August and only afterwards can "Add more documentation" apply. I totally
forgot about that first patch (or assumed that the new patch replaces
the old one). Sorry about that.

I will take another look when I have some time.

Cheers,
Uli

P.S.: And the commit messages could be improved, but I can just reword
that while applying the patches.
--
I'd be delighted to offer any advice I can. When I have some, I'll let
you know.
Thomas Fischer
2018-09-27 21:46:17 UTC
Permalink
Post by Uli Schlachter
Post by Thomas Fischer
Post by Uli Schlachter
Hi Thomas,
sorry for being awfully slow.
[...]> diff --git a/src/xproto.xml b/src/xproto.xml
Post by Thomas Fischer
index 9624700..f78f339 100644
--- a/src/xproto.xml
+++ b/src/xproto.xml
@@ -1828,6 +1828,10 @@ <xcb/xcb.h>
xcb_screen_iterator_t iter = xcb_setup_roots_iterator (setup);
xcb_screen_t *screen = iter.data;
+ uint32_t value_mask = XCB_CW_BACK_PIXEL | XCB_CW_EVENT_MASK;
+ uint32_t value_list[] = { screen->black_pixel,
+ XCB_EVENT_MASK_KEY_RELEASE | XCB_EVENT_MASK_EXPOSE
+ /* | XCB_EVENT_MASK_* ... */ };
[...]
What did you base your patch on? The string "xcb_setup_roots_iterator"
does not appear in the current(?) version of xproto.xml. Thus, your
patch does not apply here.
Cheers,
Uli
Hello, it's no problem!
https://xcb.freedesktop.org/tutorial/basicwindowsanddrawing/
The "xcb_setup_roots_iterator" function is generated in
libxcb/src/xproto.{h,c}
Sorry if I was not clear.
My problem is not the content of your patch, but that the patch does not
apply. Your patch touches code next to an example with
"xcb_setup_roots_iterator", but there is no such example in the current
version of xproto.xml.
Sorry, I misunderstood there! It was late and I should have read the words
"based" and "apply" from a git perspective.
Post by Uli Schlachter
I have to first apply your patch "Add documentation" from beginning of
August and only afterwards can "Add more documentation" apply. I totally
forgot about that first patch (or assumed that the new patch replaces
the old one). Sorry about that.
I'm still slightly new to open source development -- in the future I'll
make sure to git commit --amend to prevent confusion.
Post by Uli Schlachter
P.S.: And the commit messages could be improved, but I can just reword
that while applying the patches.
I agree, should I have named it something like this?
"xproto: Add createWindow documentation and example"

Thanks for being patient. How does the documentation look? I'd say 70% is
from the X11 standard, the rest paraphrasing the standard or the Xlib
manuals.
If there's something I should change please let me know.

Thanks,
Thomas

Loading...