Text box can be used to set DPI #960

sizzlesloth · 2024-05-25T20:44:44Z

resolves #423

I've found clicking and dragging the sliders to set DPIs to be a bit finnicky, so I have added GtkEntry objects where the DPI labels would appear. This allows for setting the DPI via keyboard input.

Changes

Text boxes show DPIs that have been previously set
Text boxes and sliders affect each others' values
Text box is set to nearest supported DPI if a random value is set
Text boxes only allow numerical input
ResolutionRow() init takes reference to ResolutionsPage as to call _on_row_activated() when the DPI text box is focused.

Find and apply closest resolution in self.resolutions. [Needed?] Store value on textbox focus in. [Needed?] Do not apply DPI value if the same as previous value. DPI entry and resolution sliders affect each others' values.

Remove focus methods.

Pass reference to ResolutionsPage when initialising ResolutionRow. Use reference to invoke `ResolutionsPage._on_row_activated()`. Do not toggle Revealer on DPI entry text box focus-in-event if Revealer is already expanded. Grammar: dpi -> DPI

Prefix unused vars with underscores. Actioned suggested import order from linter.

Apply the res value on focus-out-event.

piper/resolutionrow.py

staticssleever668

Some quick testing shows:
Piper started:

/path/to/piper/piper/resolutionrow.py:170: Warning: g_value_get_int: assertion 'G_VALUE_HOLDS_INT (value)' failed
  self.dpi_entry.set_text(str(res))

Slider moved:

/path/to/piper/piper/resolutionrow.py:218: Warning: g_value_get_int: assertion 'G_VALUE_HOLDS_INT (value)' failed
  self.dpi_entry.set_text(str(res))
/path/to/piper/piper/resolutionrow.py:96: Warning: g_value_get_int: assertion 'G_VALUE_HOLDS_INT (value)' failed
  self.dpi_entry.set_text(str(value))

Typing non-numeric characters:

/usr/lib/python3.12/site-packages/gi/overrides/Gio.py:42: Warning: g_value_get_int: assertion 'G_VALUE_HOLDS_INT (value)' failed
  return Gio.Application.run(self, *args, **kwargs)

Not sure after which action:

/path/to/piper/piper/resolutionrow.py:191: Warning: g_value_get_int: assertion 'G_VALUE_HOLDS_INT (value)' failed
  entry.set_text(str(closest_res))
/path/to/piper/piper/resolutionrow.py:200: Warning: g_value_get_int: assertion 'G_VALUE_HOLDS_INT (value)' failed
  entry.set_text(str(self._resolution.resolution[0]))

staticssleever668 · 2024-05-27T22:38:32Z

Even a simple set_text call on Gtk.Entry when signal callbacks are present is enough to trigger those warnings. Seems to be a pygobject bug: https://gitlab.gnome.org/GNOME/pygobject/-/issues/12

Looks like Lutris has had same warnings at some point, and they fixed it by sub-classing Gtk.Entry: lutris/lutris#394
Piper uses UI files for defining widgets, so in our case work-around would be more ugly

sizzlesloth · 2024-05-27T23:31:34Z

Even a simple set_text call on Gtk.Entry when signal callbacks are present is enough to trigger those warnings. Seems to be a pygobject bug: https://gitlab.gnome.org/GNOME/pygobject/-/issues/12

Looks like Lutris has had same warnings at some point, and they fixed it by sub-classing Gtk.Entry: lutris/lutris#394 Piper uses UI files for defining widgets, so in our case work-around would be more ugly

Following the advice from https://bugzilla.gnome.org/show_bug.cgi?id=708676 I have suppressed all warnings except for the one originating from Gio.py. I'll need to investigate this more, but the warning occurs before anything in the method to handle text input is run, so probably something to do with the signalling.

with warnings.catch_warnings():
    warnings.simplefilter("ignore")
    self.dpi_entry.set_text(str(value))

If we made a class to wrap around Gtk.Entry, would we not be able to use it in the UI files? I'm not familiar with GTK or UI files. What would make it uglier? While suppressing the warnings isn't too bad currently because we're not doing it much, I think it would be a good idea to do something like Lutris' solution in the future if Gtk Entry components end up being used more.

sizzlesloth · 2024-05-29T21:57:28Z

Seems like the issue occurs due to another instance of this same bug, where some parameters - like position, are invalid when handled using signals. The last assertion warning that I haven't suppressed yet is in Gio.py (when handling the insert-text signal). It seems like a solution is to make a class that inherits both Gtk.Entry and Gtk.Editable with a method inside for handling the insert-text signal. The position parameter is handled correctly by doing this, and inheriting Editable stops the warnings from occurring.

https://stackoverflow.com/questions/38815694/gtk-3-position-attribute-on-insert-text-signal-from-gtk-entry-is-always-0

A problem I see already is that the components are of a new Gtk.Template.Child(), so I'm not sure how to make this work with this custom class. Any advice is appreciated!

Grab DPIEntry focus when revealing ResolutionRow. Revert import order. Removed unused 'previous DPI value' var. Remove 'private' convention underscore from ResolutionsPage._on_row_activate(). Remove callback method for DPI entry on ResolutionRow. UI file uses DPIEntry. Remove insert-text signal for DPIEntry from UI file.

sizzlesloth · 2024-06-01T14:58:04Z

Using this new class as suggested in the StackOverflow link results in no warnings anymore. Feel free to test.

Underscored unused parameter (PEP convention).

staticssleever668 · 2024-07-24T22:00:29Z

piper/resolutionrow.py

    revealer: Gtk.Revealer = Gtk.Template.Child()  # type: ignore
    scale: Gtk.Scale = Gtk.Template.Child()  # type: ignore

    CAP_SEPARATE_XY_RESOLUTION = False
    CAP_DISABLE = False

-    def __init__(self, resolution: RatbagdResolution, *args, **kwargs) -> None:
+    def __init__(
+        self, resolution: RatbagdResolution, resolutions_page, *args, **kwargs


Nit: I wonder if we could type-hint 'resolutions_page'. My Python is rusty.

staticssleever668

Comments seem superfluous (and so are existing ones; explain the reason behind code if needed, instead of rephrasing code into words), but otherwise looks good to me. Glad you figured out how to work-around GTK. :)

staticssleever668 · 2024-07-24T22:10:43Z

It's possible to highlight text in a textbox and select another entry. Not a blocker.

staticssleever668 · 2024-07-24T22:20:30Z

You seem not to have allowed maintainer push access, I prefer rebasing manually which requires that.

To github.com:sizzlesloth/piper
 ! [remote rejected]           HEAD -> dpi-text-entry (permission denied)
error: failed to push some refs to 'github.com:sizzlesloth/piper

Regardless, merged with GitHub UI instead. Thank you for your contribution. :)

sizzlesloth added 5 commits May 25, 2024 19:42

Add dpi_entry GtkEntry objects to UI file and Python.

149f0c3

Find and apply closest resolution in self.resolutions. [Needed?] Store value on textbox focus in. [Needed?] Do not apply DPI value if the same as previous value. DPI entry and resolution sliders affect each others' values.

Remove pointless recursive logic.

d426383

Remove focus methods.

Calculate DPI text box width based on max supported resolution.

845465d

Renamed methods/signals to be consistent with current convention.

e0ffb07

sizzlesloth changed the title ~~Replace DPI labels with text boxes~~ Text box can be used to set DPI May 25, 2024

sizzlesloth added 5 commits May 25, 2024 23:22

Add type hints.

095308e

Prefix unused vars with underscores. Actioned suggested import order from linter.

Actually apply the closest res value (oops).

9c076c9

Apply the res value on focus-out-event.

Toggle allow editing of DPI entry if 'disable' button clicked.

a13acce

Use set_sensitive() on DPI entry.

2226ae6

rm pointless var.

280d7e6

staticssleever668 reviewed May 27, 2024

View reviewed changes

piper/resolutionrow.py Show resolved Hide resolved

staticssleever668 reviewed May 27, 2024

View reviewed changes

piper/resolutionrow.py Outdated Show resolved Hide resolved

staticssleever668 reviewed May 27, 2024

View reviewed changes

sizzlesloth added 2 commits June 1, 2024 15:55

Line lengths

2fef94b

sizzlesloth added 5 commits June 1, 2024 16:04

Removed unused import.

1fad6ff

Underscored unused parameter (PEP convention).

Reverted underscore convention as to not break existing stuff.

60975bc

Use underscore convention, but also use it when invoking the method.

9565ff9

Renamed handler name for ResolutionsPage to match method.

07c642b

Clarified docstring.

ac607f3

staticssleever668 reviewed Jul 24, 2024

View reviewed changes

staticssleever668 approved these changes Jul 24, 2024

View reviewed changes

staticssleever668 merged commit 71431bd into libratbag:master Jul 24, 2024

staticssleever668 mentioned this pull request Sep 24, 2024

Piper 0.8 #948

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Text box can be used to set DPI #960

Text box can be used to set DPI #960

sizzlesloth commented May 25, 2024 •

edited

Loading

staticssleever668 left a comment

staticssleever668 commented May 27, 2024 •

edited

Loading

sizzlesloth commented May 27, 2024 •

edited

Loading

sizzlesloth commented May 29, 2024 •

edited

Loading

sizzlesloth commented Jun 1, 2024

staticssleever668 Jul 24, 2024

staticssleever668 left a comment

staticssleever668 commented Jul 24, 2024

staticssleever668 commented Jul 24, 2024

Text box can be used to set DPI #960

Text box can be used to set DPI #960

Conversation

sizzlesloth commented May 25, 2024 • edited Loading

Changes

staticssleever668 left a comment

Choose a reason for hiding this comment

staticssleever668 commented May 27, 2024 • edited Loading

sizzlesloth commented May 27, 2024 • edited Loading

sizzlesloth commented May 29, 2024 • edited Loading

sizzlesloth commented Jun 1, 2024

staticssleever668 Jul 24, 2024

Choose a reason for hiding this comment

staticssleever668 left a comment

Choose a reason for hiding this comment

staticssleever668 commented Jul 24, 2024

staticssleever668 commented Jul 24, 2024

sizzlesloth commented May 25, 2024 •

edited

Loading

staticssleever668 commented May 27, 2024 •

edited

Loading

sizzlesloth commented May 27, 2024 •

edited

Loading

sizzlesloth commented May 29, 2024 •

edited

Loading