Bug 357692 - Choppy screen and mouse movement with Zoom Desktop Effect
Summary: Choppy screen and mouse movement with Zoom Desktop Effect
Status: RESOLVED FIXED
Alias: None
Product: kwin
Classification: Plasma
Component: effects-various (show other bugs)
Version: 5.5.2
Platform: Arch Linux Linux
: NOR normal
Target Milestone: ---
Assignee: KWin default assignee
URL: https://git.reviewboard.kde.org/r/126...
Keywords:
Depends on:
Blocks:
 
Reported: 2016-01-08 10:08 UTC by Qbert
Modified: 2016-01-14 16:25 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed In: 5.6.0
thomas.luebking: ReviewRequest+


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Qbert 2016-01-08 10:08:03 UTC
The screen and mouse pointer movement is choppy/blurry when the cursor is moved. Is very apparent when the cursor is moved fast during very high zoom levels. It can however be seen as blurry movements at lower zoom levels as well. This unfortunately makes the zoom effect unusable for people, like me, who use the zoom all the time to read text.

The issue is likely related to the mousepoll rate. See this comment in cursor.cpp "// TODO: How often do we really need to poll?". I solved the very same issue in Gnome Classic (on older hardware) by increasing the mousepoll speed in compiz. There is no similar setting in KDE.

I suggest a custom mousepoll rate in the zoom effect settings, because the rate is dependent on the used zoom levels, cursor speed, resolution of the screen and CPU load. It was discussed in 2013 here: https://git.reviewboard.kde.org/r/111875/ and was supposed to be solved by XInput2 but as far as I know it has not been implemented.

Reproducible: Always

Steps to Reproduce:
1. Enable the zoom desktop effect
2. Zoom in to 6x.
3. Move the cursor fast.

Actual Results:  
Choppy screen and cursor movement.

Expected Results:  
Smooth screen and cursor movement.

Hardware:
Core i7 Broadwell-U-based NUC
Iris Graphics 6100
16 Gb RAM

Reason for high severity level:
I know that this seems to be a minor or perhaps wishlist type of bug. For people like me however it's not. I simply cannot use a computer all day without sufficient zoom and I'm not alone. The bigger issue here is that NO current window manager meets the need of visually impaired users like me. Gnome 3 suffers from the very same mouse poll issue and Unity cannot zoom the dash and menus.
Comment 1 Martin Flöser 2016-01-11 11:38:44 UTC
I'm sorry that you have a bad experience. Accessibility features are unfortunately not really being worked on. Like in many other areas it needs devs affected to fix it.

Anyway: I think that increasing the polling interval is wrong. This cannot and must not be the solution. At least on Wayland we have instant cursor position updates. If XInput2 provides it, we should look into adding support for that. Unfortunately using XInput2 is annoying as there are no xcb bindings yet.
Comment 2 Thomas Lübking 2016-01-11 13:40:25 UTC
More than annoying: see bug #353587 (the issue is there in Qt already, but we'd add to it)
About polling, present state is somehow suboptimal, we poll too often but it's insufficient for smooth movements.

=> dynamic poll rate?
Ie. create some kind of gliding mean over the manhatten length between two polls and if it crosses a threshold, switch between eg. 10fps and 30fps? (presently we've 20fps)
Comment 3 Thomas Lübking 2016-01-11 15:38:18 UTC
Do we have some a11y group?
Qbert (Hubert? ;-), can you try the patch on your scenario?
Comment 4 Qbert 2016-01-11 20:41:57 UTC
Tank you all for the comments. I've got full respect for the limited resources available and understand that these niche issues are not top priority.

I'm not a developer but I'm willing to do whatever it takes to help out. My programming skills are limited but I've got a lot of experience related to these kind of issues from a usability perspective and I know my way around Linux.

I'll gladly test out patches or contribute in any way I can. Just let me know or point me in the right direction.
Comment 5 Thomas Lübking 2016-01-11 21:59:23 UTC
Apply the patch from https://git.reviewboard.kde.org/r/126716/ to the Plasma/5.5 branch of https://quickgit.kde.org/?p=kwin.git

Briefly:
------
git clone git://anongit.kde.org/kwin.git
cd kwin
git checkout Plasma/5.5
patch -p1 < /path/to/downloaded/patch.diff
mkdir build
cd build
cmake -DCMAKE_INSTALL_PREFIX=/usr -DKDE_INSTALL_USE_QT_SYS_PATHS=ON ..
[optionally run "ccmake .." for nice curses based configuration]
make && sudo make install
kwin_x11 --replace &


----
Watch out for config or compile errors (though Arch installs all required headers you might lack gcc, cmake or extra-cmake-modules)
Comment 6 Martin Flöser 2016-01-12 07:30:52 UTC
> I'll gladly test out patches or contribute in any way I can. Just let me know or point me in the right direction.

My suggestion is do what you did here: report good bug reports. As I explained: we don't know the issue and if you can reason why it's a problem for you, it's much more likely to get fixed. Good bug reports are read and a good reasoned problem description is more likely to get fixed.
Comment 7 Qbert 2016-01-12 21:21:31 UTC
(In reply to Thomas Lübking from comment #3)
> Qbert (Hubert? ;-), can you try the patch on your scenario?

Nice! I tried the path and yes it's now very smooth at higher mouse speeds. But I was perhaps a bit unclear in my description. The issue is easily noticed at higher speeds but the real use case is at lower speeds. Try zooming in parts of a sentence and the slowly move the screen while reading from one line to another.

I tried changing the unpatched source to various values finding a sweetspot at: m_mousePollingTimer->setInterval(30);

It might be to often for regular usecases. Perhaps it could be conditional based on the enabled effects?
Comment 8 Thomas Lübking 2016-01-12 21:35:22 UTC
Try this on top, resp. to lower the "distance" threshold to other values.

diff --git a/cursor.cpp b/cursor.cpp
index 271eec9..e3b3111 100644
--- a/cursor.cpp
+++ b/cursor.cpp
@@ -327,7 +327,7 @@ void X11Cursor::mousePolled()
 
     static int distance = 0;
     distance = (2*distance + QPoint(currentPos() - lastPos).manhattanLength())/3;
-    m_mousePollingTimer->setInterval(distance > 16 ? 25 : 100);
+    m_mousePollingTimer->setInterval(distance > 8 ? 25 : 100);
 
     if (lastPos != currentPos() || lastMask != m_buttonMask) {
         emit mouseChanged(currentPos(), lastPos,


This lowers the required mouse speed (to keep fast polling) from 640px/s to 320px/s (unscaled pixels)


Depending on loaded (or rather active) effects is what https://git.reviewboard.kde.org/r/111875/ sought to do, but it also means to "usually poll-a-lot" (too much), even if you don't touch the mouse at all.
Comment 9 Qbert 2016-01-13 06:24:34 UTC
Great! I'll have a look at it tonight.

I estimate that it has to be as close to 'no movement at all' in order to prevent choppy behavior when moving the mouse slightly. But I guess that it's OK, right? Because the goal is to prevent unnecessary polling when the cursor is not moving at all?
Comment 10 Thomas Lübking 2016-01-13 11:11:32 UTC
(In reply to Qbert from comment #9)

> Because the goal is to prevent unnecessary polling when the
> cursor is not moving at all?
Yes. Some dead-zone would be good to protect against judder. One might als want to introduce a timer (QElapsedTimer) to keep the poll rate alive for eg. 250ms in case you perform non-continuous movements (what's nearly inavoidable on very slow actions)

Eg. this:

diff --git a/cursor.cpp b/cursor.cpp
index 271eec9..86f58cc 100644
--- a/cursor.cpp
+++ b/cursor.cpp
@@ -31,6 +31,7 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #include <KSharedConfig>
 // Qt
 #include <QDBusConnection>
+#include <QElapsedTimer>
 #include <QScreen>
 #include <QTimer>
 // xcb
@@ -322,12 +323,13 @@ void X11Cursor::mousePolled()
 {
     static QPoint lastPos = currentPos();
     static uint16_t lastMask = m_buttonMask;
+    static QElapsedTimer timeout;
 
     doGetPos(); // Update if needed
 
     static int distance = 0;
     distance = (2*distance + QPoint(currentPos() - lastPos).manhattanLength())/3;
-    m_mousePollingTimer->setInterval(distance > 16 ? 25 : 100);
+    m_mousePollingTimer->setInterval((distance > 3 || timeout.restart() < 250) ? 25 : 100);
 
     if (lastPos != currentPos() || lastMask != m_buttonMask) {
         emit mouseChanged(currentPos(), lastPos,
Comment 11 Martin Flöser 2016-01-13 14:43:56 UTC
different approach using XInput: https://git.reviewboard.kde.org/r/126733/

I have not tested with zoom effect, only with track mouse to see whether the mouse updates correctly.
Comment 12 Qbert 2016-01-13 20:17:58 UTC
(In reply to Thomas Lübking from comment #8)
> Try this on top, resp. to lower the "distance" threshold to other values.

Tried lowering the threshold as low as 1 but it still studders on slow mousemovements. Have not tried adjusting the timer. However i expect it to do a single jump when moving the cursor after the timeout.

(In reply to Martin Gräßlin from comment #11)
> different approach using XInput: https://git.reviewboard.kde.org/r/126733/
> 
> I have not tested with zoom effect, only with track mouse to see whether the
> mouse updates correctly.

It's perfect! Buttery smooth even at insane zoom levels in a way i haven't experienced before. Not on my old compiz setup, on windows at work or on any other window manager. They have all suffered from the same choppy behavour at very high zoom levels due to screen movements mapping to the magnified mousepointers position. This causes studder when the magnified pointer moves lager and larger distances due to the increased size of magnified pixels.

I would go for Martins approach unless there are significant drawbacks. Do you both agree?
Comment 13 Thomas Lübking 2016-01-13 20:28:58 UTC
The major problem with Xinput2 is that we don't (perfectly) know whether it can cause us deadlocks in Xlib.
Iff the problem is limited to XIQueryDevice, then it's the by far superior solution (no polling at all) - if not, well, we'll get more "plasma randomly freezes" reports :-(
Comment 14 Qbert 2016-01-13 20:33:22 UTC
(In reply to Thomas Lübking from comment #13)
> The major problem with Xinput2 is that we don't (perfectly) know whether it
> can cause us deadlocks in Xlib.
> Iff the problem is limited to XIQueryDevice, then it's the by far superior
> solution (no polling at all) - if not, well, we'll get more "plasma randomly
> freezes" reports :-(

OK. I will run the patch on my system for a while and report back if there are any issues.

Btw, cannot thank you enough for the patches and the help!
Comment 15 Martin Flöser 2016-01-14 16:25:35 UTC
Git commit 5a3161846177797d4a8d7c28c6ad4f00e2177206 by Martin Gräßlin.
Committed on 14/01/2016 at 16:25.
Pushed by graesslin into branch 'master'.

Use XInput for "polling" the mouse positing

Replaces the timer based polling approach. If XInput is available we
listen for the RawMotion event on the root window and use this to
trigger a mouse pointer position.
FIXED-IN: 5.6.0
REVIEW: 126733

M  +3    -0    CMakeLists.txt
M  +1    -0    config-kwin.h.cmake
M  +118  -2    cursor.cpp
M  +13   -0    cursor.h

http://commits.kde.org/kwin/5a3161846177797d4a8d7c28c6ad4f00e2177206