XI2 events' subpixel precision may come slightly out of window positions

Douglas said:

It has been reported that it affects windows placed to the left and top of the screen. I just want to add that it's affecting panels (maybe it's the same thing. Is it?) The wingpanel of elementary OS doesn't always recognize the pointer passing over and clicking, same with Budgie's top panel and MATE's. Unity works fine, and so does Cinnamon and Gnome.

I've tested on three absolutely different machines. All affected. Does it affect all users? Why isn't this getting the proper attention...

Most importantly right now, is there a workaround? Can I replace this version of Xinput?

Douglas said:

This bug will die with Xorg. No activity. No reports elsewhere. No way to set a bounty on it. Wayland is not affected, it seems.

Adam Purkrt said:

Xfce4-panel 4.13 and higher - the new version relying on gtk3 - is hit by this too. Really annoying.

Adam Purkrt uploaded an attachment:

A simple application based on gtk3 with a button in the upper left corner. Move the mouse pointer along the screen edge near the top left corner of the screen and watch as the button gets selected and deselected, randomly (changes shade). It should always be selected, when the mouse is over it. This is not always the case.

Something really should done about this bug, it is a major annoyance.

Attachment 136946, "app with button in the top left corner":
example-1.c

Adam Purkrt uploaded an attachment:

Video demonstrating the bug in MATE destkop. The item under mouse cursor is not always selected. Also, the blinking is the result of this bug.

Attachment 136974, "effect of the bug on menu in MATE desktop":
Bug_92681_in__mate_desktop.mkv

Egmont Koblinger said:

(In reply to Douglas from comment 1)

Why isn't this getting the proper attention...

(In reply to Adam Purkrt from comment 4)

it is a major annoyance.

I can totally feel your pain, guys. I love Linux and open source and stuff, but there are a couple of things I truly dislike, one of them being usability annoyances getting way lower attention than they'd deserve. I'm not talking about X.Org in particular, it's across all the pieces. If it was me, I'd say such bugs are release critical. Devs usually say these are minor.

(In reply to Douglas from comment 2)

No way to set a bounty on it.

How many beers would you have paid? :-)

Egmont Koblinger submitted a patch:

After 4 hours of debugging a totally unknown source, here's a fix that works for me. Could you guys please test it?

It's against 1.19.5 because I recompiled my distro's packages and that's the version my distro ships. I hope it still more or less applies against git master.

There are two things to the story, and I think either segment of my patch fixes the issue, but it's probably better to have both parts in place.

One is an ugly use of trunc() instead of floor(). trunc() rounds towards zero, and as such, with slightly negative (temporary??, before clamping??) mouse coordinates, root_x_frac becomes negative (e.g. a value of -0.6 is stored as root_x = 0, root_x_frac = -0.6, as opposed to root_x = -1, root_x_frac = 0.4 which would be the logical, following the pattern of the rest of the code with the FP1616 and FP3232 data types).

Another is that at certain places, root_x is set to a certain value, but root_x_frac remains unchanged, still containing the previous value. This is probably how the previous negative fraction can leak to places it shouldn't.

[Same for root_y and root_y_frac, of course.]

My patch (especially this latter part, resetting root_x_frac whenever root_x is set) may not be complete, and I have a slight feeling that my patch still doesn't address the real root cause. I'm not sure about it.

Meta:

I think I've done my fair share to fix this issue, I won't have any more time to work on it. Douglas, Adam, could you please nag the devs on mailing lists and such if they don't notice this patch here in a reasonable time? It should be a piece of cake for them to polish this up and commit based on my findings.

Patch 136979, "Fix":
xorg-92681-negative-mouse-coordinate.patch

Adam Purkrt said:

Thank you very much Egmont for your time. Just testing the patch against 1.19.6 (it is also applicable to current git version). So far so good. I will try to post a short memo of this bug at xorg-devel, hopefully it will get some notice.

Adam Purkrt said:

Testing a bit more, I found it is not necessary to substitute trunc with floor; the patch works when only the part setting root_x_frac and root_y_frac to zero is left in.

Jason Gerecke said:

Gave this a quick test in the hopes that it might fix (or at least improve) bug 92186. Unfortunately, it looks like all this does is just result in the loss of fractional coordinate data, causing errors to increase slightly across the board. E.g., the first row of the table in the linked bug would now report a root coordinate of "164.00" making the delta from expected "-3.70" instead of "-3.00".

Egmont Koblinger said:

Jason, I'm sorry to hear that my patch makes another bug a bit worse. My patch consists of two parts, either one is sufficient to fix this bug here. Could you please maybe try which of its two parts cause that regression?

Taking a quick look at that other bug, you seem to be much more familiar with this topic and the intended behavior than I am. Either you or some other X.Org developer should take a closer look, I'm afraid I'm unable to further help, it would require much more time than I can voluntarily devote to this issue.

Both parts of my patch fix a "code smell", where the code was just fishy, didn't fit in with the rest of the picture. As I already mentioned, I had a guts feeling that I did not address the core issue, I mean why does that number become negative in the first place?

There must be levels of architectural complexity that I'm totally unaware of... But just looking at the overall picture: Handling the motion of a value within a certain interval causes problems, really?? Just sounds totally ridiculous.

While my patch makes your bug even somewhat worse, it at least fixes one of the two bugs, so still it's not obvious whether it's more useful or more harmful to apply it on its own.

Daniel Stone @daniels said:

A few suggestions.

Firstly, Xorg patches are reviewed on the mailing list (xorg-devel@lists.x.org), with patches sent through git send-email; you might want to try that to get some more attention.

Secondly, the changes to CheckVirtualMotion() seem like they would drop fractions on the floor in too many cases. Presumably we only want to do that when the co-ordinates actually hit the limits, rather than unconditionally drop the fractional component even when it's within bounds.

One way to do that would be an explicit check against the limits, and only set ev->root_* to the sprite co-ordinates if they're out of bounds. Alternatively, you could make the sprite's HotSpot struct actually carry the fractional parts as well. I would have a slight preference for the latter, but am not involved in Xorg or review anymore, so if anyone else suggests otherwise, you're probably better off listening to them. :)

I'm curious about the changing of rounding from trunc() to floor(). Previously, -0.86 would be expressed as (0, -0.86), and this changes it to (-1, 0.14). It's not necessarily a bad change, but it is a user-visible one and I'm curious why it's necessary or better.

You might also want to put some tests for negative co-ordinates in test/xi2/protocol-eventconvert.c or similar.

Rafael Kitover said:

Any news about this?

I just hit this bug here:

https://bugs.launchpad.net/bugs/1757209

assigned to @whot

I encountered this bug via weird edge behaviour in XFCE's xfdesktop, but ended up in a different code path while tracing it. The end of miPointerSetPosition (mi/mipointer.c) is currently:

   /* In the event we actually change screen or we get confined, we just
     * drop the float component on the floor
     * FIXME: only drop remainder for ConstrainCursorHarder, not for screen
     * crossings */
    if (x != trunc(*screenx))
        *screenx = x;
    if (y != trunc(*screeny))
        *screeny = y;

However, the use of trunc there (which rounds values such as -0.5 to 0) prevents negative values of *screenx and *screeny between -1 and 0 from being detected as out of bounds and the float component from being dropped, while apparently GTK+ does consider those outside of the screen causing the event weirdness described above.

This codebase is largely unknown to me so I cannot say with any confidence that this is the core issue here, but the code here behaves differently for constraining values to 0 than it would when constraining values to any positive integer.

I've tested changing these trunc calls to floor, and that resolves the xfdesktop issue I was seeing myself. Also xinput test-xi2 with a window overlapping the left edge of the screen no longer shows Motion events with root coordinates between -1 and 0 after that change.

P.S. If this is indeed the core issue here, it would probably be good to also replace the other two instances of trunc in miPointerSetPosition by floor to prevent inconsistencies there from losing fractional components unintentionally. The xfdesktop issue I ran into with coordinates between -1 and 0 causing random dropped events on the screen edge remains resolved with those extra changes.

Is there any chance to get this fixed?
I know, the future is Wayland, but quite some desktop environments are not read yet.

The most practically annoying thing about this issue (or I assume it is this one, after Egmont has pointed me to it) is that on a maximised/fullscreen gnome-terminal window, I can no longer just move the pointer to e.g. the leftmost position, click-hold and start selecting lines.

May sound like a minor nuisance, but is extremely annoying in practise.

Interestingly though, this doesn't seem to happen when using xterm, so maybe after all it is another issue.

Is there any chance to get this fixed?

No-one's currently working on this, please feel free to take it and submit an MR

Not sure whether @wjp still intends on submitting a PR for this, but would you accept his patch?

Sorry for the delay; had to refresh my memory on the rationale behind the patch. I have written it up now and submitted MR !1451 (merged) .

For what it's worth, I've been running with the attached patch (replacing trunc by floor in miPointerSetPosition) for the past year or so, and haven't encountered this issue in XFCE's xfdesktop since, and have noticed no related new issues.

@calestyo, it would be interesting to know if this solves your problem with fullscreen gnome-terminal as well. I think it could, since the specific issue I patch here was due to a mismatch in boundary behaviour between xorg-server and GTK, which would affect gnome-terminal but not xterm.

If this also fixes your issue I'd be willing to submit this patch as an MR; still with the disclaimer that this codebase is largely unknown to me...

xorg-server-floor.patch

@wjp Just checked it with a rebuild of Debian’s xorg-server source package in version 2:21.1.11-2 from unstable with the patch applied.

Seems to indeed fix the issue (and I tried like 5 times or so to move left most, click+hold and select) with gnome-terminal (and it still works with xterm as well).
Downgrading and the problem is back.

mentioned in commit wjp/xserver@653e7b70

XI2 events' subpixel precision may come slightly out of window positions

Submitted by Carlos Garnacho `@carlosg`

Description

Child items ...

Activity

Admin message

Admin message

XI2 events' subpixel precision may come slightly out of window positions

Submitted by Carlos Garnacho @carlosg

Description

Activity

Submitted by Carlos Garnacho `@carlosg`