From 71431bd5dea8564d5b18dd556541110f9601bc71 Mon Sep 17 00:00:00 2001 From: sizzlesloth <64416644+sizzlesloth@users.noreply.github.com> Date: Wed, 24 Jul 2024 23:17:09 +0100 Subject: [PATCH] Text box can be used to set DPI (#960) * Add dpi_entry GtkEntry objects to UI file and Python. 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. Remove focus methods. * Calculate DPI text box width based on max supported resolution. * Renamed methods/signals to be consistent with current convention. * Add a 'focus-in-event' signal to the DPI entry field. 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 * Add type hints. Prefix unused vars with underscores. Actioned suggested import order from linter. * Actually apply the closest res value (oops). Apply the res value on focus-out-event. * Toggle allow editing of DPI entry if 'disable' button clicked. * Use set_sensitive() on DPI entry. * rm pointless var. * Create custom class for DPI text entry to circumvent assertion warning. 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. * Line lengths * Removed unused import. Underscored unused parameter (PEP convention). * Reverted underscore convention as to not break existing stuff. * Use underscore convention, but also use it when invoking the method. * Renamed handler name for ResolutionsPage to match method. * Clarified docstring. --------- Co-authored-by: Dan <64416644+brittle-bones@users.noreply.github.com> --- data/ui/ResolutionRow.ui | 13 +++++- data/ui/ResolutionsPage.ui | 2 +- piper/resolutionrow.py | 89 +++++++++++++++++++++++++++++++++----- piper/resolutionspage.py | 6 +-- 4 files changed, 94 insertions(+), 16 deletions(-) diff --git a/data/ui/ResolutionRow.ui b/data/ui/ResolutionRow.ui index 0fca3c81..071d26d6 100644 --- a/data/ui/ResolutionRow.ui +++ b/data/ui/ResolutionRow.ui @@ -16,12 +16,21 @@ False 12 6 + + + True + start + + + + + True False start - 0 DPI + DPI True @@ -66,7 +75,7 @@ True True - Set this resolution's dpi + Set this resolution's DPI 0 0 False diff --git a/data/ui/ResolutionsPage.ui b/data/ui/ResolutionsPage.ui index 9f7807c8..8c6a5103 100644 --- a/data/ui/ResolutionsPage.ui +++ b/data/ui/ResolutionsPage.ui @@ -38,7 +38,7 @@ True False none - + True diff --git a/piper/resolutionrow.py b/piper/resolutionrow.py index 8c295e8f..2e2e37c6 100644 --- a/piper/resolutionrow.py +++ b/piper/resolutionrow.py @@ -11,6 +11,34 @@ from gi.repository import GObject, Gdk, Gtk # noqa +class _DPIEntry(Gtk.Entry, Gtk.Editable): + """ + A subclass of Gtk.Entry with an overridden method for input validation. + + This class addresses a specific bug in the handling of parameters in pygobject when + using signals. For Gtk.Entry, the 'position' parameter is incorrect when handling + the 'insert-text' signal, causing an assertion warning to be logged. By using this + subclass, we can override the 'insert-text' logic to correctly handle the position + parameter and avoid the warning. + + Inheriting from Gtk.Editable in addition to Gtk.Entry ensures that the overridden + method applies only to instances of this class, rather than affecting all + Gtk.Entry objects in the application. + """ + + __gtype_name__ = "DPIEntry" + + def do_insert_text(self, new_text: str, new_text_length: int, position: int) -> int: + """Overrides the default Gtk.Editable insert-text handler to validate numerical + input.""" + if not new_text.isdigit(): + self.stop_emission("insert-text") + return position + + self.get_buffer().insert_text(position, new_text, new_text_length) + return position + new_text_length + + @Gtk.Template(resource_path="/org/freedesktop/Piper/ui/ResolutionRow.ui") class ResolutionRow(Gtk.ListBoxRow): """A Gtk.ListBoxRow subclass containing the widgets to configure a @@ -22,15 +50,19 @@ class ResolutionRow(Gtk.ListBoxRow): active_label: Gtk.Label = Gtk.Template.Child() # type: ignore disable_button: Gtk.Button = Gtk.Template.Child() # type: ignore dpi_label: Gtk.Label = Gtk.Template.Child() # type: ignore + dpi_entry: _DPIEntry = Gtk.Template.Child() # type: ignore 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 + ) -> None: Gtk.ListBoxRow.__init__(self, *args, **kwargs) + self.resolutions_page = resolutions_page self._resolution = resolution self.resolutions = resolution.resolutions self._scale_handler = self.scale.connect( @@ -60,13 +92,19 @@ def __init__(self, resolution: RatbagdResolution, *args, **kwargs) -> None: res = resolution.resolution[0] minres = resolution.resolutions[0] maxres = resolution.resolutions[-1] + res_num_chars = len(str(maxres)) + + # Set the max width and length for the DPI text boxes, based on the max res value. + self.dpi_entry.set_width_chars(res_num_chars) + self.dpi_entry.set_max_length(res_num_chars) + with self.scale.handler_block(self._scale_handler): self.scale.props.adjustment.configure(res, minres, maxres, 50, 50, 0) self.scale.set_value(res) if resolution.is_disabled: with self.disable_button.handler_block(self._disabled_button_handler): self.disable_button.set_active(True) - self._on_status_changed(resolution, pspec=None) + self._on_status_changed(resolution, _pspec=None) @Gtk.Template.Callback("_on_change_value") def _on_change_value( @@ -82,6 +120,7 @@ def _on_change_value( value = lo if value - lo < hi - value else hi scale.set_value(value) + self.dpi_entry.set_text(str(value)) # libratbag provides a fake-exponential range with the deltas # increasing as the resolution goes up. Make sure we set our @@ -99,15 +138,15 @@ def _on_disable_button_toggled(self, togglebutton: Gtk.Button) -> None: self._resolution.set_disabled(togglebutton.get_active()) # Update UI - self._on_status_changed(self._resolution, pspec=None) + self._on_status_changed(self._resolution, _pspec=None) @Gtk.Template.Callback("_on_active_button_clicked") - def _on_active_button_clicked(self, togglebutton: Gtk.Button) -> None: + def _on_active_button_clicked(self, _togglebutton: Gtk.Button) -> None: # The set active button has been clicked, update RatbagdResolution. self._resolution.set_active() @Gtk.Template.Callback("_on_scroll_event") - def _on_scroll_event(self, widget: Gtk.Widget, event: Gdk.EventScroll) -> bool: + def _on_scroll_event(self, widget: Gtk.Widget, _event: Gdk.EventScroll) -> bool: # Prevent a scroll in the list to get caught by the scale. GObject.signal_stop_emission_by_name(widget, "scroll-event") return False @@ -118,7 +157,7 @@ def _on_scale_value_changed(self, scale: Gtk.Scale) -> None: self._on_dpi_values_changed(res=res) def _on_status_changed( - self, resolution: RatbagdResolution, pspec: Optional[GObject.ParamSpec] + self, resolution: RatbagdResolution, _pspec: Optional[GObject.ParamSpec] ) -> None: # The resolution's status changed, update UI. self._on_dpi_values_changed() @@ -136,33 +175,63 @@ def _on_status_changed( self.disable_button.set_active(True) self.active_button.set_sensitive(False) self.dpi_label.set_sensitive(False) + self.dpi_entry.set_sensitive(False) self.scale.set_sensitive(False) else: self.disable_button.set_active(False) self.dpi_label.set_sensitive(True) + self.dpi_entry.set_sensitive(True) self.scale.set_sensitive(True) def toggle_revealer(self) -> None: # Toggles the revealer to show or hide the configuration widgets. reveal = not self.revealer.get_reveal_child() self.revealer.set_reveal_child(reveal) + if reveal: + self.dpi_entry.grab_focus() def _on_dpi_values_changed(self, res: Optional[int] = None) -> None: # Freeze the notify::resolution signal from firing and - # update dpi label and resolution values. + # update DPI text box and resolution values. if res is None: res = self._resolution.resolution[0] new_res = (res, res) if self.CAP_SEPARATE_XY_RESOLUTION else (res,) - self.dpi_label.set_text(f"{res} DPI") + self.dpi_entry.set_text(str(res)) # Only update new resolution if changed if new_res != self._resolution.resolution: self._resolution.resolution = new_res + @Gtk.Template.Callback("_on_dpi_entry_activate") + def _on_dpi_entry_activate(self, entry: Gtk.Entry) -> None: + # The DPI entry has been changed, update the scale and RatbagdResolution's resolution. + try: + res = int(entry.get_text()) + # Get the resolution closest to what has been entered + closest_res = min(self.resolutions, key=lambda x: abs(x - res)) + entry.set_text(str(closest_res)) + self._on_dpi_values_changed(res=closest_res) + + with self.scale.handler_block(self._scale_handler): + self.scale.set_value(res) + except ValueError: + # If the input is not a valid integer, reset to the current value. + entry.set_text(str(self._resolution.resolution[0])) + + @Gtk.Template.Callback("_on_dpi_entry_focus_in") + def _on_dpi_entry_focus_in(self, _entry: Gtk.Entry, _event_focus: Gdk.EventFocus): + if not self.revealer.get_reveal_child(): + self.resolutions_page.on_row_activated(None, self) + + @Gtk.Template.Callback("_on_dpi_entry_focus_out") + def _on_dpi_entry_focus_out(self, entry: Gtk.Entry, _event_focus: Gdk.EventFocus): + # Apply the DPI value on focus out + self._on_dpi_entry_activate(entry) + def _on_profile_resolution_changed( - self, resolution: RatbagdResolution, pspec: GObject.ParamSpec + self, resolution: RatbagdResolution, _pspec: GObject.ParamSpec ) -> None: with self.scale.handler_block(self._scale_handler): res = resolution.resolution[0] self.scale.set_value(res) - self.dpi_label.set_text(f"{res} DPI") + self.dpi_entry.set_text(str(res)) diff --git a/piper/resolutionspage.py b/piper/resolutionspage.py index d5e3477f..7823f17b 100644 --- a/piper/resolutionspage.py +++ b/piper/resolutionspage.py @@ -63,11 +63,11 @@ def __init__( self.listbox.foreach(Gtk.Widget.destroy) for resolution in profile.resolutions: - row = ResolutionRow(resolution) + row = ResolutionRow(resolution, self) self.listbox.insert(row, resolution.index) - @Gtk.Template.Callback("_on_row_activated") - def _on_row_activated(self, listbox: Gtk.ListBox, row: ResolutionRow) -> None: + @Gtk.Template.Callback("on_row_activated") + def on_row_activated(self, _listbox: Gtk.ListBox, row: ResolutionRow) -> None: if row is self._last_activated_row: self._last_activated_row = None row.toggle_revealer()