After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 675290 - Broken sizing when inside a scrolled window
Broken sizing when inside a scrolled window
Status: RESOLVED OBSOLETE
Product: gtk+
Classification: Platform
Component: Widget: GtkIconView
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2012-05-02 11:14 UTC by Bastien Nocera
Modified: 2018-05-02 15:24 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
testiconview.c (2.31 KB, text/plain)
2012-05-02 11:14 UTC, Bastien Nocera
  Details
too small (9.00 KB, image/png)
2012-05-02 11:21 UTC, Bastien Nocera
  Details
correct size (25.09 KB, image/png)
2012-05-02 11:22 UTC, Bastien Nocera
  Details
expanded testcase (2.92 KB, text/x-csrc)
2012-05-03 12:18 UTC, Matthias Clasen
  Details
current result (3.51 KB, image/png)
2012-05-03 12:27 UTC, Matthias Clasen
  Details
a scrolledwindow patch (2.25 KB, patch)
2012-05-03 12:28 UTC, Matthias Clasen
none Details | Review
with the patch (5.63 KB, image/png)
2012-05-03 12:29 UTC, Matthias Clasen
  Details
forcing padding and border to 0 (597 bytes, patch)
2012-05-03 12:30 UTC, Matthias Clasen
none Details | Review
look ma, no scrollbar (5.46 KB, image/png)
2012-05-03 12:31 UTC, Matthias Clasen
  Details
the scrolled window can still be resized smaller (3.94 KB, image/png)
2012-05-03 12:31 UTC, Matthias Clasen
  Details
and setting a min-content-height still forces the initial height (4.87 KB, image/png)
2012-05-03 12:32 UTC, Matthias Clasen
  Details
iconview: prove a point (1.30 KB, patch)
2012-05-03 13:32 UTC, Benjamin Otte (Company)
none Details | Review

Description Bastien Nocera 2012-05-02 11:14:12 UTC
Created attachment 213265 [details]
testiconview.c

GTK+ git master

With the attached test application, the size of the window should be the same in both cases, as the natural and minimum size of the window should be the same.

Toggle the #if 0 to see both cases.

See also bug 673326 and bug 672173.
Comment 1 Bastien Nocera 2012-05-02 11:21:36 UTC
Created attachment 213268 [details]
too small
Comment 2 Bastien Nocera 2012-05-02 11:22:07 UTC
Created attachment 213269 [details]
correct size
Comment 3 Matthias Clasen 2012-05-03 12:18:43 UTC
Created attachment 213370 [details]
expanded testcase

Here is an expansion of the testcase. It can now be controlled by env vars:

SCROLLEDWINDOW=1 - use a scrolled window
ICONVIEW=1 - use an icon view as child, else use a label
MINHEIGHT=1 - set min-content-height on the scrolled window
Comment 4 Matthias Clasen 2012-05-03 12:27:32 UTC
Created attachment 213372 [details]
current result

Here is how the current code sizes

SCROLLEDWINDOW=1 ./testiconview
Comment 5 Matthias Clasen 2012-05-03 12:28:46 UTC
Created attachment 213373 [details] [review]
a scrolledwindow patch

I feel that this is wrong - the scrolledwindow should still request enough size to show its child. Here is a patch  that does that.
Comment 6 Matthias Clasen 2012-05-03 12:29:40 UTC
Created attachment 213374 [details]
with the patch

With the patch, things are much better, but it turns out the viewport gets its allocation messed up with padding and border...
Comment 7 Matthias Clasen 2012-05-03 12:30:33 UTC
Created attachment 213375 [details] [review]
forcing padding and border to 0

With this test patch to force padding and borders to zero, things work as expected.
Comment 8 Matthias Clasen 2012-05-03 12:31:15 UTC
Created attachment 213376 [details]
look ma, no scrollbar
Comment 9 Matthias Clasen 2012-05-03 12:31:55 UTC
Created attachment 213377 [details]
the scrolled window can still be resized smaller
Comment 10 Matthias Clasen 2012-05-03 12:32:54 UTC
Created attachment 213378 [details]
and setting a min-content-height still forces the initial height
Comment 11 Matthias Clasen 2012-05-03 12:33:49 UTC
Of course, these patches don't fix the icon view size request code, which is still broken and doesn't layout before requesting a height.
Comment 12 Benjamin Otte (Company) 2012-05-03 13:12:25 UTC
(Summarized paste from an IRC discussion)
So the thing is:

(1) How size requests interact with scrollable widgets is completely undefined
The icon view currently requests a random size and then the scrolled window assigns something roughly unrelated to it. I'm not sure there is completely no interaction, there might be. But even the values in those scrollbars are in no way related to the size requests.

(2) Iconview knows exactly the sizes it should have inside a scrolled window
The iconview can request a proper minimum and natural width and height and width for height and height for width. It computes the column width and height it's going to use in advance and from that, it can pretty much compute everything. So minimum width is 1 column, natural width is X columns (figure out a sane number here or just use N_ICONS). And for height ROWS, it's 1 and MIN (X, N_ICONS / ROWS). And the same math goes for height and height for width. This should be roughly trivial.

The problem is that there's no way to communicate this to the scrolled window.

But the solution we want to have in the end is to keep the max_cell_width/height current at all times (or rather: for size requests and allocations) and then go from there.
Comment 13 Matthias Clasen 2012-05-03 13:14:35 UTC
And to summarize some more: I dispute (1) - it may not work perfectly in practice, but it is far from 'completely undefined'.

The way to communicate your sizing needs is by implementing get_preferred_width/height, etc...
Comment 14 Bastien Nocera 2012-05-03 13:20:53 UTC
The main problem in trying to force a size on the scrolled window is that it's impossible to get useful data out of the GtkIconView for sizing. For example, I know the width of the window (therefore the GtkIconView), and I want to compute the height myself to check whether it's above a threshold, and size up the window myself. I cannot, because GtkIconView thinks it's fine to have a natural size where neither its icon nor the associated label can fit. That's ludicrous.
Comment 15 Benjamin Otte (Company) 2012-05-03 13:32:02 UTC
Created attachment 213382 [details] [review]
iconview: prove a point

Feel free to apply this patch and test how much it effects icon views
inside scrolled windows. All I managed to do with this is make
applications behave weird that don't use scrolled windows.
Comment 16 Bastien Nocera 2012-05-08 17:12:57 UTC
With the recent changes to GtkIconView in master, gtk_widget_get_preferred_height_for_width() seems to work, which means we can size our windows correctly.
Comment 17 Daniel Boles 2017-08-31 11:15:55 UTC
What's still outstanding here?
Comment 18 GNOME Infrastructure Team 2018-05-02 15:24:47 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/gtk/issues/394.