After another look at GtkScrolledWindow, it seems clear what the swindow
is doing with the aux_info.

Basically if the swindow has (automatic or always) scrollbars for a 
said orientation, then instead of going down to the minimum possible
scrolled window size, it tries to fall-back on a manually 
set set_size_request of its child.

However this is still weird code and should be easily removable,
if its important to have the scrolled window at least of a
specific size, the user should just set_size_request() on the
actual scrolled-window to ensure it gets enough space; not
on the child.

Let me try a patch....

I guess the reason you'd want to set size request on the child, rather than the scrolled window, is that you don't know the size of the scrollbars which may depend on the theme or the scrollbars may not be showing.

My problem with this is, I don't think set_size_request should set a "special" minimum size that has extra effects. It should just set the minimum size.

If extra effects or special treatment of the child's min size are needed, there could be a property on ScrolledWindow for example to enable that.

hmmm good point.

However the minimum width of the widget is probably not what should
be used here, then the user could just set the swindow to never scroll
horizontaly and be done with it.

Well, unless the user required the scrolled window to start scrolling
when size drops < natural width... this is a property I'd like to add
to GtkScrollableIface (i.e. the child scrollable should decide whether
it wants to be scrolled when dropping below minimum, or natural size
in each orientation).

Sorry for repeated comments, the relevant scrollable iface bug to link
in here is bug 468689.

Created attachment 172329 [details] [review]
Patch to remove aux_info handling

I think we can start by removing the aux_info special-casing.

This patch does that, then I'd really like to land that scrollable iface
and add the relevant properties to scrolledwindowclass/scrollableiface.

Review of attachment 172329 [details] [review]:

Looks good to me

Alright lets go with this for now.

If there's need for more features here we'll go 
and implement them properly ;-)

Committed.

I think this patch must be clearly documented somewhere, since this will regress any application that currently sets a size request on a child widget of a scrolled window.  (The tree view scrolling tests being the first victim :).


(In reply to comment #2)
> I guess the reason you'd want to set size request on the child, rather than the
> scrolled window, is that you don't know the size of the scrollbars which may
> depend on the theme or the scrollbars may not be showing.

Yes, for example you might want to set the minimum size of such a widget to exactly n tree view rows, or n icon view items.  This is something that would ideally be supported in tree view or icon view itself, but this patch makes it impossible to do this yourself in an easy way by just calling set size request (but I guess you can still do this by overriding size_request, or the modern variant of that, yourself).

How about just a boolean property min-size-from-child, if true, size request of the scrolled window is child min size + scrollbar size. if false, just scrollbar size. However unlike the old setup, it uses child min size whether or not that size was set manually with set_size_request.

Then tree view should have some way to set the min size in rows/cols rather than pixels probably.

The natural size of sw would still be the natural size of the child always.

I bet in a lot of cases where people were setting size request on child, they really wanted to set the default size of the window, so if patching GtkWindow to use natural size as default size before 3.0, things may just work - the min size of scrolled window won't really matter.

A gtk_widget_set_natural_size() that works like set_size_request() would probably be nice. That would allow overriding the default size too.

(In reply to comment #9)
Wait, the implementation is now such that a child widget of a scrolled window does not have anything to say.  So it is currently impossible to implement what I mentioned in comment 9: API to have the tree view set n rows as either minimum or natural size.

(In reply to comment #10)
> How about just a boolean property min-size-from-child, if true, size request of
> the scrolled window is child min size + scrollbar size. if false, just
> scrollbar size. However unlike the old setup, it uses child min size whether or
> not that size was set manually with set_size_request.
> 
> Then tree view should have some way to set the min size in rows/cols rather
> than pixels probably.

Such a boolean property would work, but I think it is a bit awkward that in case we introduce a min size in rows/cols API for tree/icon view you would have to explicitly enable that property on the scrolled window as well.

Maybe scrolled window should default to showing the entire min size of its contents? 

That could work on anything that properly implements the new size request APIs, though it looks like maybe TextView does not for example.

Ok keeping in mind this is a 2:00am comment before I sleep, I'll
give this more thought tomorrow.

First of all, I get the feeling what you want is for treeviews and
icon views to report a minimum size that is clearly not the minimum
size (i.e. it's less than the minimum size, but you want only a portion
of the minimum size to be displayed in the scrolled window, this is
another detail that is different from the childs actual minimum size).

I think it's important to have treeviews, as specially after 
height-for-width is implemented, to only ever report their
real minimum and natural width (i.e. minimum width of a treeview
is the *whole* content before unwrapping, natural is generally
the fully unwrapped treeview rows).

I'd also really like to highlight previous comments about bug 468689 here.

Currently I have scrolled window scrolling only below minimum size,
which means that if you set the hscrollbar to POLICY_NEVER, than the
scrolled window will always display at least the minimum width, and only
unwrap the content when expanded or achieving natural size (I think this
is the right default behavior, because when the scrolled window has
a small size the child widget gets a chance to rewrap and show more
content before scrollbars appear below the child's minimum width).

However it makes sense that in some child configurations that you 
want to scroll below natural size instead, meaning if the child
scrollable widget wants to always have it's natural width, an
hscrollbar policy of "never" would cause the scrolled window to
always request the child's natural width for it's own minimum.

If I understand the requirement that is unsatisfied correctly:
what you want is to define a minimum space that the scrolled window
requests space for, that is possibly not the minimum size of the
content, nor is it the natural size of the content (I suppose
the minimum size of a wrapping icon view would be a single icon
width, while the natural width would by default be the collective
width of all icons for every row of data).

I.e. you might even want to configure your wrapping icon view
to have a minimum width of say, 5 icons. then you might want the
scrolled window to request space to always display at least 3
icons.. in which case an AUTOMATIC hscrollbar is visible at the 
scrolled window's minimum width... but goes away after space is
available for 5 icons, then additional space lets the icon view
unwrap... is this the kind of functionality we need ?

It's definitely a reasonable feature IMO, but I think it would
be more cleanly implemented if the scrolled window were to
export some gtk_scrolled_window_set_minimum_content_size() API,
since the desired size for the scrolled window's content in
these cases are not the real minimum or natural size of the
child (this would be better than having the scrollable child
play mind tricks with the scrolled window by reporting false
minimum sizes).

again... it's 2:00am here and I may have missed something important,
let me know what you think I'll be back up tomorrow ;-)

Actually even more intuitive than 
   gtk_scrolled_window_set_minimum_content_size(),

might be yet another good excuse for GtkScrollableIface:
   gtk_scrollable_set_minimum_display_[width/height]()

(In reply to comment #14)
> First of all, I get the feeling what you want is for treeviews and
> icon views to report a minimum size that is clearly not the minimum
> size (i.e. it's less than the minimum size, but you want only a portion
> of the minimum size to be displayed in the scrolled window, this is
> another detail that is different from the childs actual minimum size).
> 
> I think it's important to have treeviews, as specially after 
> height-for-width is implemented, to only ever report their
> real minimum and natural width (i.e. minimum width of a treeview
> is the *whole* content before unwrapping, natural is generally
> the fully unwrapped treeview rows).

I am actually not so concerned about width, but more so about the height.  See also below:

> Currently I have scrolled window scrolling only below minimum size,
> which means that if you set the hscrollbar to POLICY_NEVER, than the
> scrolled window will always display at least the minimum width, and only
> unwrap the content when expanded or achieving natural size (I think this
> is the right default behavior, because when the scrolled window has
> a small size the child widget gets a chance to rewrap and show more
> content before scrollbars appear below the child's minimum width).

For width this might actually be fine and the behavior as it is right now might make sense.  I am hitting the biggest problem with height.  You have to set some minimum or natural size (what is a minimum or natural size in terms of height?) and you also have to set the scroll bar to ALWAYS or AUTOMATIC, since typically there will be many more rows in the tree view than can be displayed on the screen at once.  In this case you really want to have tree or icon view show more than part of the first row or item.  And this is where a future n rows API would come in, and the amount of rows you would like to show by default depends on the context of the tree view in the UI I think.

> I.e. you might even want to configure your wrapping icon view
> to have a minimum width of say, 5 icons. then you might want the
> scrolled window to request space to always display at least 3
> icons.. in which case an AUTOMATIC hscrollbar is visible at the 
> scrolled window's minimum width... but goes away after space is
> available for 5 icons, then additional space lets the icon view
> unwrap... is this the kind of functionality we need ?

Again this is seen horizontally, and then my issue might be awkward.

> It's definitely a reasonable feature IMO, but I think it would
> be more cleanly implemented if the scrolled window were to
> export some gtk_scrolled_window_set_minimum_content_size() API,
> since the desired size for the scrolled window's content in
> these cases are not the real minimum or natural size of the
> child (this would be better than having the scrollable child
> play mind tricks with the scrolled window by reporting false
> minimum sizes).

With a scrollable interface we would indeed make it possible for tree view and other widgets to tell the scrolled window what kind of minimum size to set.

I would really propose that this scrolled window patch is reverted, until we have the scrollable interface in place.  I am very afraid that this patch is breaking much more code than anticipated as I said in comment 9.

Isn't gtk_scrolled_window_set_minimum_content_size() exactly equivalent to the current behavior with set_size_request() except that it doesn't magically overload setting size request to also do some other unrelated thing?

If so I would propose that gtk_scrolled_window_set_minimum_content_size() is the trivial quick fix if we don't have time to "do it right," and is better than reverting to the old broken hack. The porting for old apps is just s/set_size_request/set_minimum_content_size/ isn't it?

If someone does have time to do a Scrollable interface for 3.0, all the better.

The reason I'd advocate not putting the hack back, is that there seem to be lots of issues shaking out of the gtk layout stuff including the issue Owen raised on the list about toplevel window sizing, the issue of not properly setting GtkWindow default size to natural size, the issue of documenting exactly what min and natural mean (see e.g. http://mail.gnome.org/archives/gtk-devel-list/2010-October/msg00103.html ), and the issue of adjust request/allocation not adjusting the for_size as Tristan just brought up on the list. We are finding layout issues left and right, bottom line. This is just complex.

It's going to be tough to figure this all out on 3.0 timeframe - so weird hacks that confuse matters are unhelpful.

The scrollable interface is discussed in bug 468689

(In reply to comment #17)
> Isn't gtk_scrolled_window_set_minimum_content_size() exactly equivalent to the
> current behavior with set_size_request() except that it doesn't magically
> overload setting size request to also do some other unrelated thing?
> 
> If so I would propose that gtk_scrolled_window_set_minimum_content_size() is
> the trivial quick fix if we don't have time to "do it right," and is better
> than reverting to the old broken hack. The porting for old apps is just
> s/set_size_request/set_minimum_content_size/ isn't it?
> 
> If someone does have time to do a Scrollable interface for 3.0, all the better.

Right, it's practically already done, the patch just needs a little work
(it adds some redundant public apis that need removing from the patch).

I think we should just fix/land that patch and implement a
gtk_scrollable_set_minimum_display_width/height() from there.

The somewhat annoying part of that is if I understand correctly,
every scrollable widget will need to actually bookkeep the parameter
(unless maybe I can figure a way to make the default implementation
use some qdata instead).

(In reply to comment #17)
> Isn't gtk_scrolled_window_set_minimum_content_size() exactly equivalent to the
> current behavior with set_size_request() except that it doesn't magically
> overload setting size request to also do some other unrelated thing?
> 
> If so I would propose that gtk_scrolled_window_set_minimum_content_size() is
> the trivial quick fix if we don't have time to "do it right," and is better
> than reverting to the old broken hack. The porting for old apps is just
> s/set_size_request/set_minimum_content_size/ isn't it?

If the application sets the size request, then yes.  But if there are third-party widgets out there that try to accomplish this by returning some minimum width from a size-request handler, then this logic needs to be moved elsewhere (though perhaps one can say that changes in size-request handlers are required anyway and this is part of that).  Either to the code embedding that widget in a scrolled window, or of course ideally keep it in the widget itself and get it done through this new scrollable interface.

> If someone does have time to do a Scrollable interface for 3.0, all the better.

Yep!

> 
> The reason I'd advocate not putting the hack back, is that there seem to be
> lots of issues shaking out of the gtk layout stuff including the issue Owen
> raised on the list about toplevel window sizing, the issue of not properly
> setting GtkWindow default size to natural size, the issue of documenting
> exactly what min and natural mean (see e.g.
> http://mail.gnome.org/archives/gtk-devel-list/2010-October/msg00103.html ), and
> the issue of adjust request/allocation not adjusting the for_size as Tristan
> just brought up on the list. We are finding layout issues left and right,
> bottom line. This is just complex.

I understand that and agree, but people porting applications to gtk+ 3 and testing them at this very moment might be suffering from this ...  (I think it was actually quite common to set a size request on a scrolled window's child).

Kristian, returning min width from a size request handler didn't work, did it? The whole problem I had with this hack is that it was specific to set_size_request() and ignored any other minimum size, thus breaking the abstraction.

(In reply to comment #21)
> Kristian, returning min width from a size request handler didn't work, did it?
> The whole problem I had with this hack is that it was specific to
> set_size_request() and ignored any other minimum size, thus breaking the
> abstraction.

Ah, that's the catch!  I didn't realize these would be different.

The GtkScrollable interface is now available in the scrollable branch.

Tristan, whats in the scrollable branch now addresses the remaining problems here, correct ?

yes it does.

Should we get it committed then ?

desrt is actually working on doing this without the scrollable interface, by turning set_scroll_adjustments into a regular vfunc, since we thought that this property-only interface which forces everybody to implement the properties sucks too much.

Eeek.

Ok maybe it sucks that everybody has to implement the property, so

  a.) Ideally GInterface should handle states for basic typed properties
      that were not handled by g_object_class_override_property()

  b.) As an irreversible hack around the above shortcoming, the property 
      should go on scrolled window.

The other downside of "b" is that it means treeview or icon view cant even
potentially set the minimum display size in terms of a number of rows
or icons (i.e. icon_view_set_minimum_displayed_icons() could potentially
update the property).

But regardless of that, does this mean that we don't want the scrollable 
iface at all ?

I think that without the property the scrollable iface is still much
nicer than a vfunc on the widget class.

Nevermind, halfway through writing a set-scroll-adjustment -> vfunc patch I discovered that all the a11y stuff is connecting to set-scroll-adjustments.
So we clearly want something beyond just a vfunc, and should probably just suck it up and go with the interface. The interface also has the nice advantage that you can see from just looking at the type whether you need a viewport or not (nice in glade, e.g.).

But the interface patch needs to fix the a11y stuff by listening for propery notify.