Re: [PATCH 1/4] Popup var inspector size requisition code rework

[Dodji Seketeli <dodji seketeli org>]

Kalev Lember <kalev smartlink ee> a écrit:


> I'd rather leave the horizontal shrinking out of this patch for two
> reasons:
> a) this patch doesn't (or at least shouldn't) change the current
>    behaviour;
> b) size request code is noticeably different in gtk3, and putting
>    the horizontal size calculations here here would make the following
>    gtk3 patch slightly less straight forward.

OK.

>> Now that we are doing this on the scrolled window (the container)
>> rather than on the popup variable inspector itself, it seems to me
>> that it is the height of the scrolled window that we want to stay
>> smaller than max_height, not necessarily the height of its child
>> widget, or am I missing something ?
>> 
>> FWIW, I have tested the code snipped below and it seems to behave
>> fine, unless I have missed a corner case:
>> 
>>             // If the height of the container is too big so that
>>             // it overflows the max usable height, clip it.
>>             if (req.height > max_height) {
>>                 req->height = max_height;
>>             } else {
>>                 req->height = child_req.height;
>>             }
>
> No, that would let it grow past the max_height.
>
> As an example, lets say that the container itself thinks it should be 20
> pixels high, the child widget thinks it should be 600 pixels high, and
> we've calculated that the max_heigh is 400. When we check if
> (req.height > max_height), we are checking if (20 > 400), which is
> false. But we should really be comparing the child widget's height
> request here.

My understanding is that size_request on the container (the call to
Gtk::ScrolledWindow::on_size_request at the beginning of the function)
should invoke the size_request function of its children and of all the
relevant visual elements that contribute to its size.  So by default, we
should not have 20 pixels high for the container when its child is
requesting 600.  We should at least have 600 pixels for the container as
well.

Or if we have less, it would mean there is some extra constraint being
applied to the size container, and in that case, it should present
scroll bars.  And we are precisely adding such a constraint.

So I think that by doing it your way, we are precisely risking to let
the container grow past the max_height because we possibly don't take in
account other visual elements of the containers that might contribute to
its size.

-- 
		Dodji