ortk95 / planetmapper

PlanetMapper: An open source Python package for visualising, navigating and mapping Solar System observations
https://planetmapper.readthedocs.io
MIT License
10 stars 1 forks source link

Tk `ScrolledText` can sometimes lock up GUI elements when scrolled with inertial scroll over X11 and `grab_set()` #339

Closed ortk95 closed 6 months ago

ortk95 commented 6 months ago

If scrolling e.g. a HeaderDisplay quickly (using the touchpad on my mac) when running the PlanetMapper GUI over SSH/X11, the GUI occasionally locks up and refuses to respond. When hovering over the title bar, the close button appears to rapidly flicker (i.e. seems to be related to input being sent between the two windows) and the process has to be killed from the command line.

This appears to be some bug with Tk over X11, as I can reproduce with very simple code (below), so it isn't a PlanetMapper issue specifically. However, it is obviously not great, so should probably work around it.

The issue only seems to occur for

Given that grab_set isn't strictly necessary (and may even occasionally be annoying for users), the easiest solution is probably just to not use it for windows containing scrolledtext where possible.

#!/usr/bin/env python3
# -*- coding: utf-8 -*-
import sys
import tkinter as tk
import tkinter.scrolledtext
from tkinter import ttk

def main():
    root = tk.Tk()
    window = tk.Toplevel(root)
    window.grab_set()
    window.transient(root)

    frame = ttk.Frame(window)
    frame.pack(expand=True, fill='both')
    scrolled_text = tkinter.scrolledtext.ScrolledText(frame)

    scrolled_text.pack(expand=True, fill='both')

    scrolled_text.insert('1.0', '\n'.join(chr(i) * 100 for i in range(32, 127)))
    window.mainloop()

if __name__ == '__main__':
    main(*sys.argv[1:])
ortk95 commented 6 months ago

Removed grab_set for header and GUI customisation popups in d76ec0362023af7bc895fdde0e51f0e627317c4c. I've kept grab_set on the various save popups, as this ensures the state isn't changed while saving, and they don't contain any scrolled text anyway.

Still have a scrolled text element in the OpenObservation popup which needs fixing. This will take a little more thought, as this could change the state and mean e.g. the header popup contains stale data. Options are probably to either:

  1. Close all popups when changing the observation.
  2. Add some method to update the contents of any popup when changing the observation.

These would both probably require having some list of active popups registered to GUI that can then be dealt with as necessary. Option 2 is probably the cleanest and easiest to implement.

There is also an edge case where the Other body of interest and Other body of interest label popups share state with the list of other bodies. This could cause inconsistencies where both are open, and then the list is edited in one popup. This is a very edge case though, and would probably be unlikely to occur in reality, so may not be worth the effort/complexity to fix.