Add more documentation.

Submitted by Thomas Fischer on Aug. 26, 2018, 5:14 a.m.

Details

Message ID 20180826051424.18613-1-tjfischer98@gmail.com
State New
Series "Add more documentation."
Headers show

Commit Message

Thomas Fischer Aug. 26, 2018, 5:14 a.m.
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(-)

Patch hide | download patch | download mbox

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.

Comments

Uli Schlachter Sept. 16, 2018, 4:33 p.m.
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
> 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
Thomas Fischer Sept. 16, 2018, 10:36 p.m.
On Sun, Sep 16, 2018 at 9:33 AM Uli Schlachter <psychon@znc.in> wrote:

> 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
> > 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 Sept. 17, 2018, 6:51 a.m.
On 17.09.2018 00:36, Thomas Fischer wrote:
> On Sun, Sep 16, 2018 at 9:33 AM Uli Schlachter <psychon@znc.in> wrote:
> 
>> 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
>>> 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}

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.
Thomas Fischer Sept. 27, 2018, 9:46 p.m.
On Sun, Sep 16, 2018 at 11:51 PM Uli Schlachter <psychon@znc.in> wrote:

> On 17.09.2018 00:36, Thomas Fischer wrote:
> > On Sun, Sep 16, 2018 at 9:33 AM Uli Schlachter <psychon@znc.in> wrote:
> >
> >> 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
> >>> 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}
>
> 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.

> 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.


> 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