Bug 497131

Summary: Wrong implementation on NotifyPointerAxisDiscrete
Product: [Plasma] xdg-desktop-portal-kde Reporter: jackyzy823
Component: generalAssignee: Plasma Bugs List <plasma-bugs>
Status: REPORTED ---    
Severity: normal CC: ahiemstra, akselmo, aleixpol, kde, nate, nicolas.fella
Priority: NOR    
Version First Reported In: unspecified   
Target Milestone: ---   
Platform: Other   
OS: Other   
Latest Commit: Version Fixed In:
Sentry Crash Report:

Description jackyzy823 2024-12-06 14:55:52 UTC 
SUMMARY

xdg-desktop-portal-kde implement `org.freedesktop.portal.RemoteDesktop.NotifyPointerAxisDiscrete` 's variable `step` wrongly.

Here: https://invent.kde.org/plasma/xdg-desktop-portal-kde/-/blob/8ef972777a8b20cf626c91f47d1505b10b6b9bdc/src/remotedesktop.cpp#L323

https://invent.kde.org/plasma/xdg-desktop-portal-kde/-/blob/8ef972777a8b20cf626c91f47d1505b10b6b9bdc/src/waylandintegration.cpp#L411-435

1)
It simply pass `step` as relative axis movement (like dx dy) in `NotifyPointerAxis` -> `requestPointerAxis`

So it cause that scrolling in KRDP is very very slow. https://invent.kde.org/plasma/krdp/-/blob/9c7d722ee8236377b55f7e01e279ef6f38a377e4/src/PortalSession.cpp#L162

3) 
About converting `steps` to `dx/dy` for fakeInput->axis . `Mutter` and `gnome-remote-desktop` use a fixed value 10. 

See https://gitlab.gnome.org/GNOME/gnome-remote-desktop/-/commit/54cc935d8bc1872b0cab204f6672d2a29913a4da

and  https://gitlab.gnome.org/search?search=DISCRETE_SCROLL_STEP&nav_source=navbar&project_id=547&group_id=8&search_code=true&repository_ref=main

It would better  `step` could reflect Plamsa settings' "Mouse -> Scrolling speed' . But since RemoteDesktop.NotifyPointerAxisDiscrete is for remote-desktop, this may not be possible.

3)
It passes positive value to fakeinput->axis in y axis, while requestPointerAxis doing a reverse one ( passing  -y to fakeinput->axis). https://invent.kde.org/plasma/xdg-desktop-portal-kde/-/blob/8ef972777a8b20cf626c91f47d1505b10b6b9bdc/src/waylandintegration.cpp#L431






STEPS TO REPRODUCE
1.  Enable KRDP service.
2.  Use rdp client to connect.
3.  Scroll on any scrollable  page.

OBSERVED RESULT
tiny scrolling distance

EXPECTED RESULT
scrolling like local one.
Comment 2 David Redondo 2024-12-16 13:59:14 UTC 
> It passes positive value to fakeinput->axis in y axis, while requestPointerAxis doing a reverse one ( passing  -y to fakeinput->axis). 

I noticed this as well, probably because it was developed with kdeconnect - I checked and  it just passes the Qt value (wayland and Qt direction are reversed). However I am wary of fixing it since it will break things that relied plasma having the broken impl and we can't sync the kdeconnect release.
Comment 3 jackyzy823 2024-12-17 01:16:44 UTC 
(In reply to David Redondo from comment #2)
> > It passes positive value to fakeinput->axis in y axis, while requestPointerAxis doing a reverse one ( passing  -y to fakeinput->axis). 
> 
> I noticed this as well, probably because it was developed with kdeconnect -
> I checked and  it just passes the Qt value (wayland and Qt direction are
> reversed). However I am wary of fixing it since it will break things that
> relied plasma having the broken impl and we can't sync the kdeconnect
> release.

We could not break it by adding some documentation about the meaning (direction) of the value for further developers to follow the same rule.
Comment 4 David Redondo 2024-12-17 07:45:17 UTC 
I think we should probably fix it after all and make a not in visible places. People that cared about that and had conditional code should be fine and portal users that didnt know wiill be fixed.
Comment 5 David Redondo 2024-12-17 07:45:40 UTC 
And do some version dependent code in kdeconnect I guess