[KDE/Mac] Review Request 120319: Make Kate mousewheel zoom feature respect the corresponding global setting (KGlobalSettings::wheelMouseZooms())

[René J.V. Bertin]

> On Sept. 23, 2014, 5:01 p.m., Thomas Lübking wrote:
> > Qt cannot "distiguish" because there's nothing to distinguish - the driver generates synthetic wheel event for the inertia.
> > You can btw. turn that <censored> off.
> > 
> > Seems an issue with inertial scrolling on X11 as well https://bugs.freedesktop.org/show_bug.cgi?id=38909
> > Otherwise i'd have opted for "the driver shall please stop this when you hit a key".
> > 
> > On a random note, I can't find that setting in systemsettings?
> > If there's a config GUI for this, aligning to it seems reasonable, BUT does no way fix the actual issue w/ inertia (ie. "you don't have control over your input device")
> 
> René J.V. Bertin wrote:
>     You may not believe it, but I actually prefer the UE with the feature on. Probably because it saves me some movements, which is always good (less RSI).
>     
>     Yes, I imagine that the issue can occur on Linux as well if inertial scrolling works the same way there. It just never bit me on Linux - and yet I run that on underpowered machines ...
>     Might be a thought to define the modifier key to get wheelMouseZooms in that case, or at least make that possible somewhere in systemsettings?
>     
>     You're right, I haven't been able to find the setting in systemsettings. The setting *is* part of the standard settings, though, no idea why it slipped through and remained a hidden setting. But because it's part of the standard settings I went with aligning to it, instead of hacking in a new switch as in Konsole.
>     NB, seems likely that Konsole offers its own switch because the authors didn't go the length I did to find out about the one in `kdeglobalrc`?
> 
> Thomas Lübking wrote:
>     UX isn't my concern - but doing that in the driver is simply idiotic.
>     (Sorry, but hey - Peter H. shares that opinion ;-)
>     What eg. happens when you reached the end of the document? The driver keeps scrolling and you don't even notice that.
>     
>     Either the view provides inertic scrolling or you purchase a rodant that can alter between swing and precise wheel (w/o trying to advertise here, eg. Logitech M500 is an affordable device with this capability)
>     
>     Choosing another modifier won't help, but at best move the problem around (ok, there's no META key on OSX, but it is indeed used for shortcuts on linux/windows - often global ones. So instead of scaling the text, you end up scaling the entire desktop ;-)
>     
>     While I personally don't use text scaling in kate (this way), if there's no present UI for the global config, it can hardly be used and we can't (?) introduce config GUI for the kate part in SC4, I assume (kate devs will know better)
>     Also simply disabling a feature because it is *one* occasion of a driver issue doesn't sound too reasonable - what if I'd in general like to scale the text this way but am still annoyed by the driver behavior?
>     
>     One way to deal with this is to measure the time between the last unmodified wheel event and the now modified wheel event. If that is too low to be reasonably caused by a human being (ie. the scroll "started" unmodified and suddenly a modifier kicks in) one could ignore the modifier.
>     That would however have to apply to all items.
>     Stupid question: how does (Qt)WebKit behave (eg. in qt-assistant?) in this regard?
> 
> René J.V. Bertin wrote:
>     I use Apple's Magic Trackpad, no spamming intended, but I wouldn't want to change for anything else anymore... 
>     
>     > ok, there's no META key on OSX
>     
>     There is. META is mapped to Ctrl by default on OS X (and CTRL to Command, which is the real Meta key), but there is a setting to switch that back. I presume the default is conceived to reflect the fact that OS X uses the Command key in place of Ctrl for shortcuts. It apparently never occurred to them to define a special code (`ACCEL`) for the standard accelerator "opcode" ;)
>     
>     > Also simply disabling a feature because it is one occasion of a driver issue doesn't sound too reasonable 
>     
>     How do you think I found the global setting and how to read it out? It's used exactly this way in KTextEdit (see `KTextEdit::wheelEvent`) which again raises the question why the setting isn't exposed in systemsettings.
>     
>     > - what if I'd in general like to scale the text this way but am still annoyed by the driver behavior?
>     
>     The only solution in that case would be to configure the feature to use another key or key combination, one that works for you and your workflow and doesn't risk to get triggered unexpectedly. 
>     
>     > Stupid question: how does (Qt)WebKit behave (eg. in qt-assistant?) in this regard?
>     
>     Not so stupid: Qt does exactly the same thing, and it doesn't appear to be optional in any way there. This is probably why `KTextEdit` overrides the `wheelEvent` it inherits from the parent Qt widget.
> 
> René J.V. Bertin wrote:
>     BTW:
>     > What eg. happens when you reached the end of the document? The driver keeps scrolling and you don't even notice that.
>     
>     But the view knows. On iOS and newer OS X versions it actually bounces when this happens. Which is why I learned that I prefer the inertial scrolling, because I tried switching it off to get rid of the bouncing and felt like handicapped.
>     
>     BTW, the OS X driver's inertial feature can be stopped very easily: swipe to get the view to scroll; place back 2 fingers and the scrolling stops at once. Exactly as it did with the big thumb trackball I had before.
> 
> Dominik Haumann wrote:
>     Did you reach any consensus about this patch? If this is not going in, it should be discarded.
> 
> René J.V. Bertin wrote:
>     Good question, I guess that we both forgot about the whole thing ... Thomas?
> 
> Thomas Lübking wrote:
>     I still oppose the idea to try to fix this in client code.
>     
>     a) the setting is KDE4/support only, ie. deprecated code anyway
>     b) there's apparently no config GUI for that setting?!
>        => secret workaround
>     c) You suggested that (all?) Qt5 applications suffer from this problem (where we cannot even involve this solution -> problem globally remains. Let alone gtk+ etc. clients)
>     d) The setting can no more be used to prevent eg. accelerated scrolling (which it originally selected)
>     e) There may be random custom handling of modifiers on wheel events everywhere - and they all needed to be stripped/made configurable at least.
>     
>     
>     Let alone that faking scroll inertia in the server is silly itfp.
>     
>     a) The server must either simply stop faking inertia with the next keyboard/crossing event (while apparently that's not simple on evdev) or (rather) scrolling inertia has to be done in the client (w/o faking input events)
>     b) Eg. when I scroll and move the mouse to another window or just scrollview while the inertia faking is going on, I suddenly scroll the other window/view? (the view is not "fixable" on serverside inertia faking)
>     c) Do I really want scrolling inertia on the destkop to change virtual desktops?
> 
> René J.V. Bertin wrote:
>     a) yay, stable release code that actually allows you to get some work done without fearing whatever tomorrow's update might break. Fixing naggles in KDE4 is like showing love to unknown hordes of users who stick with LTS or non-Linux distributions for good reasons.
>     b) So? If that's really a serious issue for you (you know what vi[m] is, right? :P) then there's always the possibility to add a setting to Kate that is first-time initialised off the global setting and then maintained independently.
>     c) I haven't checked with Qt5 for this since I created the RR, nor KF5 applications so I don't know to what extent my claim still stands, nor why the proposed fix wouldn't work there. (I'd hope there's something comparable, in fact, I seem to recall that Qt also has a "should the mouse wheel zoom" setting.) But we're talking about a KDE4 application here in which the issue is particularly noticeable (to me). KDE. 4. Not GTk or anything else.
>     d) eh? The setting I use is called `KGlobalSettings::wheelMouseZooms()`. I don't see what that has got to do with accelerated scrolling, nor how checking the setting prevents that kind of scrolling?
>     e) again, so? This is a patch for Kate. It's not intented to cure all pain in the world, but it could show how this issue can be tackled to others who're bothered by it in a random other application.
>     
>     a) I don't agree and also think neither of us gets to dictate what the server may not or must do. Besides ... what server, on platforms that don't use X11?
>     b) I don't know what happens to you, but in my case on OS X this is not possible because the inertial scrolling stops as soon as I move the pointer. It does not when I press or release a key, and I think that's rightly so. I could in fact find a use for such an event in Kate (like the accelerated scrolling you mentioned).
>     c) ditto. (In fact, you actually want to disable virtual desktop change coupled to mouse *movement* that's not part of a multi-touch gesture, trust me ;))
> 
> Thomas Lübking wrote:
>     a) and will see maybe one more release - at best.
>     b) this typically means "my secret pet feature workaround with no use for anyone else" - if it's so important, it must be exposed to the common user.
>     c) kdeglobals is gone (ok, deprecated to kdelibs support) and this setting isn't invoked by any QPA - what you set here has no whatsoever impact on plain Qt5 applications.
>     d) look up the code, the comment describes the history of the feature.
>     e) this means you seriously want to fix clients one-by-one by stripping features instead of simply addressing to core culprit?
>     
>     --
>     
>     a) I don't dictate, but merely state sane behavior. If OSX btw. fakes this through a common input lib, it could just as well fix it there (despite the feature belongs into the clients)
>     b) I frankly would have to try, but then the solution is simple: slightly move the mouse before pressing a modifier...
>     c) what "ditto"? if i'm above the desktop and rotate the wheel by one/two/three iteration/s, I expect to end up one/two/three desktops ahead, not 13 or 25. I'm talking about a mouse wheel, not a touch device (which isn't controllable itr. anyway)
>     
>     
>     You may have a point though in that altering the font scale (or anything like that) is of no use from a touch device, because it can/does not generate clean wheel clicks.
>     However, the proper™ solution is then to base this behavior on the nature of the input device and/or eg. ignore scroll events for such behavior when many™ of them occur in short™ time.
>     Not some obscure and deprecated setting that you luckily happen to know.
> 
> René J.V. Bertin wrote:
>     I've got better things to do than keep up a tit-for-tat dialogue, but I do not agree that simulating inertial scrolling is anything but the "event server's" business. That's not any less the case than that it's some server's role to provide access to any other kind of events, or fonts.
>     What's more, that event server probably has access to lower-level information from the input device anyway, and can use that information to determine whether simulation is needed or not (a trackball wouldn't require it). And above all, it can provide the simulated events in a consistent manner across all clients.
>     
>     But I'm not going to insist ... nor feel inclined to discard this request because there's only been feedback from a single person who has failed to come up with (AFAIC) convincingly valid arguments against my change.
> 
> Thomas Lübking wrote:
>     > that event server probably has access to lower-level information from the input device anyway
>     
>     Which it can (and does) easily pass downstream while it completely lacks (by nature) private client detail informations.
>     
>     > it can provide the simulated events in a consistent manner across all clients
>     
>     What apparently leads to glitches.
>     
>     You however completely ignored any technical issues - and suggestions.
>     Reading across https://bugs.freedesktop.org/show_bug.cgi?id=38909 again, it seems the faked events have a higher valuator (scrollDistance) than usual. If that's the same on OSX, it could be used (next to an absurd frequency?) to control the feature.
>     
>     Relying on a deprecated and hidden config key is no solution at all - I'm sorry if that doesn't convince you.
> 
> René J.V. Bertin wrote:
>     I've been ignoring technical issues mostly because they are moot, but I do realise I could have made it clearer that this whole thing concerns OS X primarily. It's not that the issue cannot be reproduced on X11/xcb set-ups, I just don't run into it.
>     
>     I have not been able to reproduce the issue with Qt 5.4.2 on OS X which somehow manages to dissociate keypresses and simulated inertial scrolling.
>     So the patch proposed here is only a fix for Kate 4.14 and most likely not required for newer Qt5-based versions. It brings Kate's behaviour in synch with applications that are based on KTextEdit, like all KDE PIM applications, by using the same config key (cf `KTextBrowser::wheelEvent` and `KTextEdit::wheelEvent`). In my world that makes a perfectly fine fix for a branch that is no longer under active development. I don't see a valid reason to make it OS X only though; if you mentioned things that are definitely broken because of it I missed them.
>     
>     There's a valid point that could have been made is that my patch should do as the wheelEvent handlers cited above, that is provide an `else` for when `wheelMouseZooms()` returns false. Maybe that was implied; if so it wasn't explicit enough. I'll look into that possibility, off the top of my head I'd say that might actually reinstate the accelerated scrolling referred to above (which I think is impossible to obtain in Kate's current implementation).
> 
> Thomas Lübking wrote:
>     Did you check whether you can make QTextBrowser from Qt5 zoom text at all - for this doesn't work here.
>     Have not checked why, but
>     
>         float delta = e->angleDelta().y() / 120.f;
>         zoomInF(delta);
>         
>     looks very suspicious, since 120.0 is the typical delta for one wheel event...
>     
>     Therefore i'm not convinced this problem would not re-popup on kate/5 and KTextBrowser is a deprecated thin wrapper around QTextBrowser. It's in kdelibs4support, so using the deprecated key *there* is ok, but only schedules the problem in kate.
>     
>     In case that's not clear: I give a shit on what happens to kate/4, I just want to ensure that we get an actual solution to this problem that has a lifetime beyond next month.
>     
>     The core problem as I perceive it:
>     1. some devices/systems fake input events to provide inertial scrolling
>     2. touch devices to not generate precise wheel clicks
>     
>     Leaving completely aside what me or the evdev maintainer thinks about one or the other cause, I assume in neither case anything like font scaling should ever happen for such imprecise input, right?
>     
>     => Can we detect this kind of input and prevent unwanted action?
>     
>     Possible detection vectors:
>     a) higher valuator (suggested to apply for X11, but you must check on OSX)
>     b) many events in short time (eg. it's fair to assume that users would first check the result of a font size change?)
>     c) suddenly modified event after a very short time since the last unmodified event (related to b)
>     
>     If we can detect the "unwanted" (modified) input, we can avoid the undesired action, agree?
>     And this will neither be a KDE/4 only solution, nor require invocation of inaccessible config keys nor re-tie kate with kdelibs4support, correct? Do you agre such solution would be favorable to this approach?
> 
> René J.V. Bertin wrote:
>     Text zoom works for me in Qt5's Assistant, and in a Qt5 build of xxdiff on OS X and X11/xcb, as well as in Kate/5 using Qt 5.3.2 on Linux.
>     I have raised the issue on Qt's development ML, with the hope that I can find a fix to backport to Qt4 on OS X, which I now think would be the best solution (again, it would seem that Qt5-based applications aren't affected by this issue on OS X). Hopefully this will also raise some awareness to the issue on X11/xcb .
>     The issue is actually worse under X11 in the sense that the coasting continues after the view reached the top or bottom, whereas on OS X simulated inertial scrolling is implemented really well and determined by the kind of widget the events are sent to.
>     
>     >The core problem as I perceive it:
>     >
>     >1. some devices/systems fake input events to provide inertial scrolling
>     
>     It is not on OS X (and possibly MS Windows) because Qt 5.4 clearly demonstrates that it is somehow possible to handle the simulated events separately from the keypress events: press the Meta (Command) key while the view is coast-scrolling, and it will keep scrolling. Press the same key during an active, "real" scroll, and zooming ensues. Apparently I will have to start looking at `- (void)scrollWheel:(NSEvent *)theEvent` in qnsview.mm (cocoa platform plugin).
>     >2. touch devices to not generate precise wheel clicks
>     
>     This may be true on X11/xcb (where I only ever encountered imprecise trackpads with all kinds of unwanted side-effects), but not on OS X. 2-finger scrolling causes text views to scroll by the number of lines set with "mouse wheel scrolls by". It is not necessarily easy to move your 2 fingers the distance that corresponds to exactly 1 such click, but that's an operator limitation, not by definition a device limitation.
>     
>     I don't really understand your detection vector, but the main hurdle I have is that ATM I can only do rather limited KF5 *testing* on Linux, no development on any platform. Qt5 is a different matter (cocoa and xcb platform plugins both on OS X).
> 
> Thomas Lübking wrote:
>     Qt assistant is QWebKit, xxfiff has a custom textview/zoom implementation and kate as well, non of them is a QTextBrowser, ie. what KTextBrowser would behave like.
>     
>     > whereas on OS X simulated inertial scrolling is implemented really well and determined by the kind of widget the events are sent to.
>     
>     Either OS X exposes client side widget types to the input (faking) server or (i'd say: rather) Qt5 actually just performs client side inertial scrolling (and makes the system aware of that to not fake anything for this window) - Qt does know that you've a touch input device (on any backend; touch events are different from and only translated to wheel events)
>     
>     
>     > It is not necessarily easy to move your 2 fingers the distance that corresponds to exactly 1 such click
>     
>     That is actually what I meant by "touch devices do not generate precise wheel clicks"; changing the font size 1 or 2 steps up or down should be an obstacle game - no matter of the system. Isn't for you? (And if it's for any operator, it's not an operator problem, touchpads scroll-by-slide is just hardly controllable)
>     
>     The detection can be performed by a plain Qt application, I'll write a test case tonight.
> 
> René J.V. Bertin wrote:
>     > Either OS X exposes client side widget types to the input (faking) server
>     > or (i'd say: rather) Qt5 actually just performs client side inertial scrolling
>     
>     I'm pretty damn sure it's neither (or maybe a special case of the 1st option). The system provides a basic widget/class called `NSView` that just about any widget that provides a view on content descends from, and that widget already knows how to scroll. Clients typically have to concern themselves only with preparing the content for display in those NSViews, not with how to scroll them. It clearly is configurable because not all clients show the bouncing behaviour when scrolling hits the top or bottom, but AFAIK the basic operation is handled by the system.
>     
>     Oh, and I checked since I'm typing this in Chrome where "wheel scrolling" doesn't happen on a line-by-line basis: I'm getting what looks like single pixel scrolling precision from my trackpad. You do have a point though: there's imprecision in that it isn't perfectly clear how far you have to slide your finger tips to scroll 1 line or N pixel(s).
> 
> René J.V. Bertin wrote:
>     Another thought, independent of where inertial scrolling should be implemented and what kind of fix is implemented ultimately against text zooming during scroll coasting:
>     
>     KDE4 provides a global config parameter that controls whether or not the mouse wheel should zoom text. Applications (Kate included) should take that setting into account, or provide their own switch to override the global setting (and/or compensate for the fact there is no UI to set the global switch).
> 
> Thomas Lübking wrote:
>     "KDE4" has seen it's last release last December and I'm not sure any more releases are scheduled.
>     Kate is based upon KF/5 since the Applications 14.12 release (last december...) and does not require kdelibs4support.
>     
>     You can of course do whatever you want downstream, but I severely doubt there'll be some KDE SC4 kate release in the future.
>     
>     => If you spot a problem (and it exists in KF/5 - mind that you maybe want to use kate on linux as well ;-) you should seek to fix it for KF/5 and/or in a way that covers both versions.
> 
> René J.V. Bertin wrote:
>     Again, it's not like nobody is still using KDE4, or even that we are a dwindling minority (and as long as even Kubuntu doesn't manage to provide a stable KF5-based version where everything works as it does under KDE4 I can only agree with anyone deciding not to switch).
>     IMVHO it doesn't matter whether there will still be official releases; fixes can be committed in git, from where they can be picked up by (LTS) distro maintainers.
> 
> Thomas Lübking wrote:
>     If you desperately want to face the same problems again next year (at least on non-OSX or touchpads(?), got to try inertia emulation on evdev) instead of addressing them once and for all, just go for it. *shrug*
>     
>     As long as you understood the conditions, discussion about this aspect is pointless, I will very likely not understand the stance anyway (to my mind, it's completely absurd, sorry)
>     
>     
>     You're however not introducing a bug, so the patch itself is fine to me. I just don't think it's a viable solution to the problem.
> 
> René J.V. Bertin wrote:
>     But yeah, I agree I'll want to be able to use applications that use the Kate editor part also on Linux some day, and that it'd be better to solve this issue at a lower level than in each individual app (e.g. in the xcb plugin). It's a pity that the config parameter disappeared that allowed for the easiest fix (even if at the app level).
>     
>     BTW, I added a checkbox to KDE4's kcm_input; I guess I'll create a RR for that despite the chances that it'll go in.

Oops, our comments crossed. Yes, I've been telling myself I ought to go dig out my trackball that has real inertial scrolling and see how that one behaves. I'm guessing one would indeed have to check inter-event timing to reject modifier state changes. And yes, I've understood that my fix won't port over to KF5 (not why the config parameter disappeared though), and I also realised that respecting the setting in an app is less useful if you don't provide the intended "else" behaviour. I stick to my impression though that this is the quickest way to get rid of the unwanted text zooming in kate4-based applications, one that doesn't require tinkering with Qt (which may be an argument, seeing how distribution releases tend to stick to a single Qt version).


- René J.V.


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120319/#review67302
-----------------------------------------------------------


On Sept. 3, 2015, 8:03 p.m., René J.V. Bertin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120319/
> -----------------------------------------------------------
> 
> (Updated Sept. 3, 2015, 8:03 p.m.)
> 
> 
> Review request for Kate, KDE Software on Mac OS X and kdelibs.
> 
> 
> Repository: kate
> 
> 
> Description
> -------
> 
> KDE has a global text editor option that can be used to let Ctrl-MouseWheel events zoom the text font being used. Kate does not respect this setting, which is an omission that can lead to unexpected behaviour.
> 
> On OS X, the feature works slightly differently in the sense that `Qt::ControlModifier` does not designate the control key, but the command (?, Apple) key. In addition, Qt's event handling does not appear to be able to distinguish between scrolling under direct control, and residual scroll movement that's due to simulated inertia. As a result, any attempt to use a keyboard shortcut while a text view has not stopped moving completely will lead to text zooming.
> See https://bugreports.qt-project.org/browse/QTBUG-41475 .
> 
> At first I thought to replace `Qt::ControlModifier` with `Qt::MetaModifier` on OS X but that would probably require changes in many locations, and thus best be preceded by a design decision if the standard shortcut modifier key ought not be referenced via a symbolic platform constant not named after a specific key, instead of being hardcoded (and using a key name).
> 
> Therefore, I present a small patch that checks `KGlobalSettings::wheelMouseZooms()` when the platform's `ControlModifier` is held and a wheel event received.
> 
> An alternative solution could introduce a Kate-specific setting (just like Konsole has one), but that would require far more changes.
> 
> 
> Diffs
> -----
> 
>   part/view/kateviewinternal.cpp a2906f3 
> 
> Diff: https://git.reviewboard.kde.org/r/120319/diff/
> 
> 
> Testing
> -------
> 
> On OS X against kdelibs 4.14.1 (git/kde4). The change consists of an additional call to a standard kdelibs function which I do not expect to introduce regressions on any platform.
> 
> I looked at kate's git/master code, which lacks a `wheelEvent` handler suggesting the feature works differently there. However, Qt 5.3.1 applications (like Digia's own Qt Creator) still suffer from the phenomenon described above, so a fix would be beneficial for KF5 too)
> 
> 
> File Attachments
> ----------------
> 
> Qt4 patch to prevent unwanted/unexpected text zoom while scrolling
>   https://git.reviewboard.kde.org/media/uploaded/files/2015/09/03/ae8f63f3-4289-40bb-8b97-61a934bb33ec__patch-avoid-unwanted-fontzooming.diff
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-mac/attachments/20150904/5a569f0f/attachment-0001.html>