Input shapes with holes don't correctly allow input through

[gandalf3]

Output of i3 --moreversion 2>&- || i3 --version:

Binary i3 version:  4.13-non-git © 2009 Michael Stapelberg and contributors
Running i3 version: 4.13-non-git (pid 699)rl-c to abort…)
Loaded i3 config: /home/gandalf3/.config/i3/config (Last modified: Wed 12 Apr 2017 11:17:14 PM PDT, 220408 seconds ago)

The i3 binary you just called: /usr/bin/i3
The i3 binary you are running: i3

URL to a logfile as per http://i3wm.org/docs/debugging.html:

http://logs.i3wm.org/logs/5744863563743232.bz2

What I did:

Started peek and clicked on its click-through-able region during recording.

What I saw:

The click was received by peek and not the program underneath.
For what its worth, I observed the same behavior with licecap running in wine.

What I expected instead:

Mouse events to be received by the program under peek.

---

According the the peek developer, this is likely an upstream issue.

[Airblader]

Note that this is not a bug since i3 doesn't claim to support the Shape extension. In general there's also no reason to: tiling window managers don't have overlapping windows, that's the entire point. Floating windows in i3 are meant for dialogs and popups, not long-living applications.

That said, I see why in this specific case a floating window makes sense even for such an application. I think implementing Shape in i3 would be quite a lot of work, and would of course introduce a new dependency. I'm not really sure yet whether we'd want this.

[pciavald]

+1 to support Shape in i3, even just for this peek killer-app.

[stapelberg]

peek is indeed pretty slick and useful.

Two observations:

1. peek requires a compositor, so it won’t work on i3 out of the box anyway.

2. I think peek’s use of floating windows makes sense, even when applying i3’s strict philosophy — a floating window is the only way to overlay a tiling window which you want to record.

So, pull requests which implement just enough of the shape extension so that clicks are routed correctly are welcome :).

[phw]

peek requires a compositor, so it won’t work on i3 out of the box anyway.

This requirement will go away in the next release :) Only the click through won't work with i3 then.

[stapelberg]

Interesting. How are you going to implement transparency without a compositor?

[phw]

Sorry for raising false hopes, won't work on i3 still. Of course it uses the shape extension, which is not supported, hence this issue here :D

[psychon]

@stapelberg phw/peek#147 and phw/peek@1843473 and https://developer.gnome.org/gdk3/stable/gdk3-Windows.html#gdk-window-shape-combine-region (so, as @phw says: By using the SHAPE extension)

[McKean]

Please have a look, you can have peek recording stacked and it will work perfectly fine.
phw/peek#200
Perhaps some other i3 users could verify :)

[algmyr]

Sorry to raise a year old issue, but it's still very much relevant.

I'm curious what in i3 actually makes this not work, especially seeing that something as simplistic as tinywm which is a super bare bones wm with just 50 lines of C code somehow manages to behave as expected with the likes of peek and xeyes.

I'm not well versed in Xlib and xcb (and the differences between them). It just seems odd that it appears to work out of the box with Xlib, while it doesn't work in i3.

[Airblader]

It works for tinywm because it doesn't do reparenting, I'd guess.

[psychon]

I would claim that tinywm is not a WM. It doesn't do any of the things ICCCM requires from a WM.

@algmyr Let's say you want your WM to add window decorations to windows (=titlebar). The way to do this is for the WM to create its own window that is slightly larger than the window that should get the decorations. Then, the WM reparents the target window into its own window. This means that the target window is now inside of the WM's window and e.g. moves with it. Now, the WM can draw stuff on its target window to e.g. add a close button.

Now, if the target window has a hole in it (as in xeyes and peek), then that hole just allows you to see the WM's window. Since the WM does not expect this part to be visible, you usually end up with a black background here.

In a non-reparenting WM, you do not get things like a close button. And since the WM does not add its own frame window around the window that some program creates, a hole in the window allows to see through to whatever is behind that window.

[algmyr]

@Airblader I guess that makes sense. Does that mean that the garbage with something like xeyes comes from the parent window (which I guess handles the window decoration) rather than the application itself? It would make sense if the garbage comes from an uninitialized pixmap where only the decoration is actually drawn.
Edit: Based on what @psychon wrote I guess the answer is yes

[Airblader]

Edit: Based on what @psychon wrote I guess the answer is yes

Yes, it is.

[psychon]

Uhm. Is that "garbage screenshot" with i3?
@Airblader How about i3 initialises its pixmap with some black content here (or some lines later after the xcb_change_gc):

i3/src/x.c

Line 855 in 64e7646

xcb_create_pixmap(conn, win_depth, con->frame_buffer.id, con->frame.id, width, height);

A simple call to xcb_poly_fill_rectangles(conn, con->frame_buffer.id, con->frame_buffer.gc, 1, (xcb_rectangle_t[]) { x = 0, y, = 0, width = width, height = height }); should do the job.

(Also, I will not ask why this seems to have one GC per container and I also will not ask why this pixmap even is as large as it is, instead of just having a pixmap for the area that is actually used (which seems to be enough given that @algmyr has a screenshot suggesting that the remaining area is never filled with anything))

[Airblader]

@Airblader How about i3 initialises its pixmap with some black content here (or some lines later after the xcb_change_gc):

I wouldn't mind, though for the record on my machine xeyes works just fine (in the sense that the background is black).

(Also, I will not ask why this seems to have one GC per container […])

Aren't gcs per drawable? You know much more about this stuff, so I'm sure that if you say it's unnecessary, it is. :-)

([…] I also will not ask why this pixmap even is as large as it is)

You mean why it covers the entire window rather than just the titlebar? We'll need it for the borders, right?

[algmyr]

Uhm. Is that "garbage screenshot" with i3?

i3-gaps, but I suspect the behavior would be the same in plain i3. Edit: it is

I can get the xeyes background to be black in some circumstances, like launching it on an empty workspace. If I launch it on a non-empty workspace the background will be some existing part of the screen. The "garbage" shows up when resizing, that particular screenshot was after toggling the window to floating (which consistently seems to give that kind of thing). I suppose the cairo surface is reallocated and never overwritten, so old data just stays there.

[psychon]

(Also, I will not ask why this seems to have one GC per container […])

Aren't gcs per drawable? You know much more about this stuff, so I'm sure that if you say it's unnecessary, it is. :-)

From https://www.x.org/releases/X11R7.5/doc/x11proto/proto.html (look for "CreateGC"):

The gcontext can be used with any destination drawable having the same root and depth as the specified drawable; use with other drawables results in a Match error.

So, assuming i3 does not support zaphod mode and only cares about a single root window, GCs are per depth. Which, in practice, likely means that you need one GC for depth 24 and one for depth 32.

You mean why it covers the entire window rather than just the titlebar? We'll need it for the borders, right?

Sorry, but that sounds like a waste of memory. Memory is cheap these days, but still... A pixmap of size 800x600 and 4 byte per pixel needs about 1.8 MiB (800*600*4). A pixmap of size... let's say 800x50 needs 156 KiB. That's about 8% of the memory for the full pixmap.
(Are the borders single-color? That could be drawn just directly to the target window instead of going through a pixmap, but I guess this would make the code slightly more complicated to avoid some visible flickering)
(Slightly unrelated: Awesome uses four separate pixmaps; one for each "side" of the window (and corners get assigned to one of the sides))

If I launch it on a non-empty workspace the background will be some existing part of the screen. The "garbage" shows up when resizing, that particular screenshot was after toggling the window to floating (which consistently seems to give that kind of thing).

Actually... The code that draws the titlebars and copies the pixmap to the window does not run for floating containers:

i3/src/x.c

Line 419 in 64e7646

* • floating containers (they don’t have a decoration)

Thus, these end up being not painted (and since, I guess, i3 uses a background of None for its frame windows, this means whatever was visible here before ends up being visible).
(Oh, which means that the pixmap allocated for floating containers is completely unused...? ;-) )

I suppose the cairo surface is reallocated and never overwritten, so old data just stays there.

Hm, that's not "consistent" with the available symptoms. Cairo clears image surfaces that it allocates, unless you use cairo_image_surface_create_for_data. i3 does not seem to be using cairo image surfaces.
X11 windows get their background applied "when needed". If a window has no background, whatever was visible here before stays visible. This seems to be the "some existing part of the screen"-behaviour that you sometimes see.
X11 pixmaps are the only thing that I can come up with right now which end up with uninitialized memory.
Perhaps x_draw_decoration should just fill the target surface/pixmap with black before beginning to "draw things" to it?

(Even more off-topic: Why does

i3/libi3/draw_util.c

Line 216 in db0add0

CAIRO_SURFACE_FLUSH(src->surface);

flush the source of a copy? The source is not modified when copying pixels around... I should stop looking at i3's code.)

[Airblader]

I should stop looking at i3's code.

Please don't :-) I've created issues for all of the raised points:

#3478
#3479
#3480

The point about x_draw_decoration filling the background first would be void if we used separate pixmaps, though, right? I've created an issue nonetheless since that change is much easier to do:

#3481

[algmyr]

I suppose the cairo surface is reallocated and never overwritten, so old data just stays there.

Hm, that's not "consistent" with the available symptoms. Cairo clears image surfaces that it allocates, unless you use cairo_image_surface_create_for_data. i3 does not seem to be using cairo image surfaces.

This codebase is a bit of a maze to me right now. I think that resizing containers trigger reallocs of pixmaps/surfaces at around

i3/src/x.c

Line 855 in 64e7646

xcb_create_pixmap(conn, win_depth, con->frame_buffer.id, con->frame.id, width, height);

. You're right that cairo should clear the surface created in draw_util_surface_init, but then I suspect that it's the xcb_create_pixmap that is not cleared? Otherwise I have no clue why there would be garbage data in the pixmap.

On the other hand, wouldn't

i3/src/x.c

Line 889 in 64e7646

draw_util_copy_surface(&(con->frame_buffer), &(con->frame), 0, 0, 0, 0, con->rect.width, con->rect.height);

overwrite the pixmap with the one that should have been cleared by cairo? I'm confused.

[psychon]

@algmyr First: Yupp, this xcb_create_pixmap creates a pixmap and does not initialise its content. Thus, one ends up with "random memory data".
However, for cairo, I think it needs to be cleared up that cairo can draw to different things: An in-memory buffer (image surface), an X11 surface (via xlib or xcb), but also e.g. a PDF surface. Only with an image surface does cairo initialise things on its own. With an X11 surface, there is no own "state of things" in cairo. Instead, cairo draws to the given X11 drawable, which is here for i3 the pixmap that was created by the xcb_create_pixmap call.
Your second code example, the call to draw_util_copy_surface, just uses cairo to copy the contents of the pixmap to the window, i.e. to make actually visible what was drawn to the surface.

[psychon]

@Airblader Here is some start of proper(?) shape support: https://github.com/psychon/i3/tree/shape_support
The first commit in that branch just does the shape and xfixes extension initialisation, the second commit actually shapes some windows. At this point I would now have to understand enough of i3 to figure out the geometry of window decorations (so that they can be added to the shape) and find all the places that have to cause a re-shape.
Oh and this could be optimised so that the shaping is only done for windows which actually have a shape (which can be checked via xcb_shape_query_extents, the field bounding_shaped of the answer is a boolean).

I'm out of time for today and if anyone wants, they can pick this up and continue working on it.

(For how the actual shapeing is done here: The XFixes extension introduces the concept of "regions" in version 2. A region can e.g. be created from the shape of a window and can also be used to set the shape of a window. Thus, this allows to "copy" the shape from one window to another without actually having to download the shape from the X11 server. This just needs to add the geometry of window decorations to the shape first so that these stay visible.)

[Airblader]

@psychon Thank you so much for this already, that's a really great starting point for someone to work on this! I appreciate it.

[xzfc]

I've continued the work of @psychon: https://github.com/xzfc/i3/tree/2742-shape. Anyone feel free to take my commit.

Changes made:

• Proper handling of i3 decorations geometry.
• XCB_SHAPE_NOTIFY handling. Also, do not shape windows that should not be shaped.
• Added code for handling input shapes (XCB_SHAPE_SK_INPUT).
  However it doesn't work as intended, and I can't figure why.
  I can click through input-shaped area, but the click is handled by i3 (resizing) and not by the window underneath.

For now peek is usable, but you should disable compositing.

Some clumsy test app: https://gist.github.com/xzfc/d3b28cc800e477a89cb70b7bf081f3cd

[psychon]

@xzfc Thanks. Very nice.

One quick comment: Input shapes were added in shape version 1.1. My initial commit does not do any version checks because version 1.0 was "good enough" and there is no older version. For input shape handling, this should test if the server actually supports shape 1.1 and otherwise stay away from XCB_SHAPE_SK_INPUT and only use the bounding and clip shape. (Does this even have to worry about the clip shape? Bounding might be enough...)

[xzfc]

@psychon

this should test if the server actually supports shape 1.1

OK, but I think this is a minor issue now, since the X.Org supports 1.1.

Does this even have to worry about the clip shape? Bounding might be enough...

Not sure if this is the right way, but I just made it in the same way as Openbox does: it handles both bounding and clip as the bounding.
https://github.com/danakj/openbox/blob/9e8813e111cbe6c1088f6abbc771a29470f05fc2/openbox/event.c#L1728-L1729

[psychon]

Oh, hey... https://github.com/danakj/openbox/blob/9e8813e111cbe6c1088f6abbc771a29470f05fc2/openbox/frame.c#L293

xcb_shape_combine allows to directly copy the shape from one window to another, without the indirection through a region... xcb_shape_rectangles(c, XCB_SHAPE_SO_UNION, ...) can then add something to that, also without using a region. Interesting and useful. Seems like I was doing this unnecessarily complicated. :-)

[xzfc]

Yay, switching to xcb_shape_combine + xcb_shape_rectangles fixed handling input shapes.

[orestisfl]

#3514