-
-
Notifications
You must be signed in to change notification settings - Fork 42
Fix GLib::GObject::CRITICAL on out-of-range zoom factor (Gtk3::ImageV… #775
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
base: master
Are you sure you want to change the base?
Fix GLib::GObject::CRITICAL on out-of-range zoom factor (Gtk3::ImageV… #775
Conversation
…iew) The zoom factor have a minimum value 0.001 by default. To fit high-resolution display (4K/8K), a minimum value 0.0001 is perferred. Or boring CRITICAL messages are frequently printed. This commit gets the minimum value tuned for ImageView used by screenshot tabs. Fix shutter-project#774
| 'zoom', #name | ||
| 'zoom', #nick | ||
| 'zoom level', #blurb | ||
| 0.0001, #minium, tuned to fit 4K/8K modern display |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not to fix it in Gtk3::ImageView itself?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another PR to Gtk3::ImageView?
If no change to upstream, derived class is the most convenient way to override GObject Property, AFAIK.
Alternative way: if extending with package Gtk3::ImageView; ... , some low-level C functions (e.g. g_object_class_install_property) may help. (see https://docs.gtk.org/gobject/class_method.Object.install_property.html). But this is not recommended and more complex.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's at https://github.com/carygravel/gtk3-imageview
Or you think only shutter can benefit from 8k display?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What we're trying to say is that it would be more beneficial to try a fix upstream in addition to the one here, so we get a fix right away plus other projects can be benefit long term as well :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it. PR for upstream on the way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After more investigation, it turns out the RCA is the lazy allocation of Gtk widgets:
New ImageView widgets created during invisible state would keep 1x1 dummy size allocation till first show. When this size applied in set_fitting(), pixbuf with height or width over 1000px would have zoom factor beyond the range.
…iew)
The zoom factor have a minimum value 0.001 by default. To fit high-resolution display (4K/8K), a minimum value 0.0001 is perferred. Or boring CRITICAL messages are frequently printed.
This commit gets the minimum value tuned for ImageView used by screenshot tabs.
Fix #774