Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

wxGUI: fix 'SpinCtrl' widget size #1339

Merged
merged 8 commits into from
Apr 18, 2022

Conversation

tmszi
Copy link
Member

@tmszi tmszi commented Feb 14, 2021

To Reproduce
Steps to reproduce the behavior:

  1. Launch Attribute Table Manager e.g. g.gui.dbmgr map=roadsmajor
  2. Switch to Manage Layers page (tab) -> Add layer page (tab), and click inside SpinCtrl widget
  3. See error message in the virtual terminal

Error message

(g.gui.dbmgr:17305): Gtk-CRITICAL **: 15:49:23.810: gtk_box_gadget_distribute: assertion 'size >= 0' failed in GtkSpinButton

Additional info

Bug related with size of SpinCtrl widget (wxGTK only) which is different according input param arg min=, max=. According my testing, width threshold value for SpinCtrl widget with default param arg min=1, max=100 is 118 (bug not occur).

I also fixed this in other wxGUI components using this widget.

  • GRASS GIS Preference dialog
  • Attribute Table Manager
  • Georectify
  • Vector digitizer
  • Supervised Classification Tool

@tmszi tmszi added bug Something isn't working GUI wxGUI related linux Linux specific labels Feb 14, 2021
@petrasovaa
Copy link
Contributor

To Reproduce
Steps to reproduce the behavior:

  1. Launch Attribute Table Manager e.g. g.gui.dbmgr map=roadsmajor
  2. Switch to Manage Layers page (tab) -> Add layer page (tab), and click inside SpinCtrl widget
  3. See error message in the virtual terminal

Error message

(g.gui.dbmgr:17305): Gtk-CRITICAL **: 15:49:23.810: gtk_box_gadget_distribute: assertion 'size >= 0' failed in GtkSpinButton

Additional info

Bug related with size of SpinCtrl widget (wxGTK only) which is different according input param arg min=, max=. According my testing, width threshold value for SpinCtrl widget with default param arg min=1, max=100 is 118 (bug not occur).

So I am wondering, why do you use other values (e.g. 137) in this PR?
I am not really sure how to deal with this, it seems it depends on the theme. With some themes I don't get the gtk error even now. Also how will the changes look on other operating systems?

@tmszi
Copy link
Member Author

tmszi commented Feb 17, 2021

So I am wondering, why do you use other values (e.g. 137) in this PR?

Because SpinCtrl widget width depend on min= max= arg (with default min=0, max=100, I derived threshold widget width which is 118) and other SpinCtrl widget has another min= max= and they needs another width value e.g. 137

I am not really sure how to deal with this, it seems it depends on the theme. With some themes I don't get the gtk error even now.

I tested on one of the default GTK+ theme Adwaita

Also how will the changes look on other operating systems?

I haven't tested it on other OS yet.

@tmszi
Copy link
Member Author

tmszi commented Feb 17, 2021

I am not really sure how to deal with this, it seems it depends on the theme. With some themes I don't get the gtk error even now.

If it's a bug in GTK+ or in the theme, then it doesn't make sense to solve it at the wxPython level.

@neteler neteler added this to the 8.0.1 milestone Dec 9, 2021
@neteler
Copy link
Member

neteler commented Jan 12, 2022

@tmszi would you mind to rebase this PR?

@tmszi
Copy link
Member Author

tmszi commented Jan 13, 2022

@tmszi would you mind to rebase this PR?

It seems this bug related with wxGTK+ theme, I'll take another look at it.

@ninsbl ninsbl modified the milestones: 8.0.1, 8.0.2 Feb 20, 2022
@ninsbl
Copy link
Member

ninsbl commented Feb 20, 2022

Bumping up milestone as 8.0.1 is due in two days, while this has not been part of RC1 and there has not been activity for some time.

@wenzeslaus wenzeslaus modified the milestones: 8.0.2, 8.4.0 Feb 27, 2022
@petrasovaa petrasovaa force-pushed the wxgui_set-optimal-spinctrl-widget-width branch from a31b1ff to 97f8fa3 Compare April 7, 2022 04:11
@petrasovaa
Copy link
Contributor

In general, using sizers and not setting size explicitly works best, so I changed the SpinCtrl wrapper to (on gtk) ignore size parameter if it's smaller than the minimum you identified, so that we don't have to remove all the size parameters directly. I removed couple size settings which were greater than the minimum and would likely cause troubles. I haven't seen any problems so far and it shouldn't break anything. This PR definitely helps with getting rid of many Gtk-critical messages.

@petrasovaa petrasovaa modified the milestones: 8.4.0, 8.2.0 Apr 13, 2022
@neteler
Copy link
Member

neteler commented Apr 13, 2022

This PR definitely helps with getting rid of many Gtk-critical messages.

This is great news!
I just locally patched relbr80 and many "Gtk-critical" messages seem to have disappeared. 😄

@petrasovaa petrasovaa merged commit d7c3b5a into OSGeo:main Apr 18, 2022
@tmszi tmszi deleted the wxgui_set-optimal-spinctrl-widget-width branch April 18, 2022 18:26
@neteler neteler modified the milestones: 8.2.0, 8.0.2 Apr 18, 2022
neteler pushed a commit that referenced this pull request Apr 21, 2022
In general, using sizers and not setting size explicitly works best, so I changed the SpinCtrl wrapper to (on gtk) ignore size parameter if it's smaller than the minimum you identified, so that we don't have to remove all the size parameters directly. I removed couple size settings which were greater than the minimum and would likely cause troubles. This should help with getting rid of many Gtk-critical messages.
@neteler
Copy link
Member

neteler commented Apr 21, 2022

Backport done

ninsbl pushed a commit to ninsbl/grass that referenced this pull request Oct 26, 2022
In general, using sizers and not setting size explicitly works best, so I changed the SpinCtrl wrapper to (on gtk) ignore size parameter if it's smaller than the minimum you identified, so that we don't have to remove all the size parameters directly. I removed couple size settings which were greater than the minimum and would likely cause troubles. This should help with getting rid of many Gtk-critical messages.
ninsbl pushed a commit to ninsbl/grass that referenced this pull request Feb 17, 2023
In general, using sizers and not setting size explicitly works best, so I changed the SpinCtrl wrapper to (on gtk) ignore size parameter if it's smaller than the minimum you identified, so that we don't have to remove all the size parameters directly. I removed couple size settings which were greater than the minimum and would likely cause troubles. This should help with getting rid of many Gtk-critical messages.
neteler pushed a commit to nilason/grass that referenced this pull request Nov 7, 2023
In general, using sizers and not setting size explicitly works best, so I changed the SpinCtrl wrapper to (on gtk) ignore size parameter if it's smaller than the minimum you identified, so that we don't have to remove all the size parameters directly. I removed couple size settings which were greater than the minimum and would likely cause troubles. This should help with getting rid of many Gtk-critical messages.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working GUI wxGUI related linux Linux specific
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants