ui: actually apply the hover preview size cap - #494
Conversation
c4d6ce9 to
1b7cf86
Compare
|
Not a huge fan of adding yet another setting for that. What about caping it to, say, 90% of the window size or something like that? |
Worth separating two things in this PR, because I think the setting is the less important half. The 400px cap already on master doesn't actually work: it's applied with gtk_widget_set_size_request(), which sets a minimum, not a maximum. GtkPicture still reports the paintable's full intrinsic size as its natural size, so a 3000px screenshot gets a 3000px tooltip. The fix is re-rendering the paintable at the capped size via GtkSnapshot — that part is a bugfix and applies whatever we cap to. On the cap source: 90% of the window works fine, and the numbers are reasonable (window defaults to 800×600, min 400×300, so 720×540 down to 360×270 — close to the current 400). One wrinkle: the preview is cached per thumbnail and only rebuilt when the thumbnail changes, so a window-relative cap also needs invalidating on window resize. Easy enough, just not free. My reasoning for the setting was that there's already a dedicated Images page with a "Preview size" range next to it, so a second range felt consistent rather than new surface area. But I don't feel strongly. Happy to drop it and go window-relative — or split this into the cap fix now and leave the setting for later, whichever you prefer. |
|
Let's go with the cap fix first. Maybe keeping the already existing 400 limit that we fail to apply. I thought we had such a limit but didn't double check before replying, the failure to properly apply it is unfortunate |
The hover-preview tooltip already declares a 400px cap, but never applies it: gtk_widget_set_size_request () establishes a minimum, not a maximum, so it cannot bound the tooltip's natural size. GtkPicture keeps reporting the source paintable's full intrinsic size, and a large image gets a tooltip to match -- a 3000px screenshot spans several monitors rather than the intended 400px. Re-render the paintable at the capped size with GtkSnapshot instead, so the preview's intrinsic size is the one the tooltip lays out with. This keeps the scaling on the GSK side: no pixel readback of the full-size texture, and no gdk-pixbuf round trip (gdk_pixbuf_get_from_texture () is deprecated since 4.12, gdk_texture_new_for_pixbuf () since 4.20, and GTK explicitly discourages the texture -> pixbuf -> texture path this would otherwise take). The 400px cap itself is unchanged; it just takes effect now. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1b7cf86 to
50bba0d
Compare
madicnikola
changed the title
done |
|
Thanks. Something I don't understand though... we do it the same way for the thumbnail, so why does it work for the thumbnail and not for the tooltip? |
| (gdouble) G_PASTE_UI_ITEM_SKELETON_PREVIEW_SIZE / height)); | ||
| gtk_widget_set_size_request (preview, width * scale, height * scale); | ||
| gint target_width = MAX (1, (gint) (width * scale)); | ||
| gint target_height = MAX (1, (gint) (height * scale)); |
There was a problem hiding this comment.
These two are not necessary, we can use widthscale and heightscale directly here as it's already ceil'd underneath
| gint target_width = MAX (1, (gint) (width * scale)); | ||
| gint target_height = MAX (1, (gint) (height * scale)); | ||
| g_autoptr (GtkSnapshot) snapshot = gtk_snapshot_new (); | ||
| graphene_size_t size = GRAPHENE_SIZE_INIT (target_width, target_height); |
There was a problem hiding this comment.
This is not necessary, let's avoid explicit graphene API usage as we don't use it directly anywhere else. We can just pass NULL and it will use the right one taken from the snapshot
|
After digging, thumbnail works because the list box row itself caps the size. |
2 participants
Split out of the original PR, per review: this is the cap fix on its own, keeping the existing 400px limit unchanged.
The hover preview already declares a 400px cap, but never applies it.
gtk_widget_set_size_request ()establishes a minimum, not a maximum, so it cannot bound the tooltip's natural size —GtkPicturekeeps reporting the source paintable's full intrinsic size. A 3000px screenshot gets a tooltip to match, spanning several monitors instead of the intended 400px.Re-render the paintable at the capped size with
GtkSnapshotinstead, so the preview's intrinsic size is the one the tooltip lays out with.This keeps the scaling on the GSK side: the resulting paintable wraps the render node, which still references the original texture, so it rasterizes from the full-resolution source at whatever device scale the display uses — no pixel readback, no blur on HiDPI. It also avoids the gdk-pixbuf round trip, which is doubly deprecated (
gdk_pixbuf_get_from_texture ()since 4.12,gdk_texture_new_for_pixbuf ()since 4.20) and which GTK explicitly discourages.The 400px value itself is untouched; it just takes effect now.
Tested on a 5760x3420 image at 150% scaling: the tooltip is capped and stays sharp.
The follow-up in #499 proposes replacing the fixed 400 with 90% of the window, as you suggested — kept separate so this one can go in on its own.