Fix facttree touchpad scrolling on Wayland by removing custom scroll code #702
Conversation
This was created and maintained, but not actually made visible, so effectively this code is unused. Remove it to avoid having to maintain it. Turns out this code was added in 2014 in commit 5a5f94f but has been inactive and incomplete since it was first added.
It was unused, and removing simplifies future refactoring.
This removes the lib.graphics.Scene superclass, since pretty much nothing of that class was used and it did complicate an upcoming refactor. Removing it makes FactTree a plain Gtk widget. Some of the code (e.g. managing width/height) is now duplicated from Scene, but this will be removed in an upcoming simplification anyway.
When finding the fact under the cursor, this now no longer looks at the list of currently visible days, but instead looks at all of them. This prepares for an upcoming refactoring where the list of visible days is no longer easily available. To ensure that this does not result in any slowdowns, this now uses bisect rather than looking at each day in turn.
This simplifies FactTree by no longer making it a gtk.Scrollable. This means that rather than having to manually do scrollbar management and adjust coordinates for drawing and input, FactTree will just set its size to fit all facts in the list and rely on its ScrolledWindow and Viewport containers to handle scrolling and simply change the drawingarea position to only show the relevant portion. Where the old code knew the scroll position exactly and used that to only draw visible facts, the new code just uses gtk's dirty clip rectangle for detecting what facts to draw. It turns out this is actually more efficient, since any content that was already visible before scrolling does not need to be redrawn now. The incentive for this change is that scrolling with a touchpad (and probably also a touch screen) did not work (at least on Wayland), because that produces "smooth scrolling" events that the old code did not handle. Rather than further increasing complexity by handling these, this just reduces complexity by deferring to Gtk default code, fixing touchpad scrolling in the process.
Previously, the mouse move handler would figure out the fact under the cursor whenever it moved, filling the hover_day and hover_fact attributes. However, in practice hover_day was unused, and hover_fact only used when clicking, so this wasted resources for no good reason. This changes the mouse handler into a get_hover_fact() helper that is just called on from the mouse button down handler instead.
Ah, good catch. This worked when I tested it during development earlier, but it seems a later changes broke this. They are supposed to be handled by ScrolledWindow, not sure why they are not. I'll have a closer look soon. |
|
Just in the way of encouragement, I have also tested this PR under xorg+xfce4 and also found it works fine (except for PgUp/PgDown). |
|
Another minor imperfection (on xorg at least), is that scrolling to the bottom with either KEY_End or repeated KEY_Down, the Total line for the last day doesn't show. It does, however, when scrolling to the bottom with the mouse wheel. (Followup: the same is true for current master, but a final PgDown does show the last line.) |
I see the same with 3.0.3, also on Wayland. It doesn't seem to be a regression in this PR, but maybe something to fix while working on that code anyway indeed. |
This PR was prompted by touchpad scrolling not working on wayland, because this is handled using different ("smooth") scroll events that were not handled by the custom scrolling code.
This is fixed by removing all the custom scrolling code, making the facttree DrawingArea just big enough to fit all facts and just letting gtk handle scrolling by moving the facttree inside a viewport. By only redrawing dirty areas, invisible facts are (still) not drawn (and it seems that when scrolling, there is now less need to redraw previously drawn areas as well).
Leading up to this change are a few more commits with additional cleanups, mostly removing unused code or small refactorings that make the main change easier to implement.
There might be subtle changes (i.e. in how far things are scrolled when pressing pgdown), but there should be nothing that is actually significant.
Tested on Wayland and Xorg (both inside Gnome), seems to work as expected.