| [ Return to Bugs & Features | Roadmap 1.3 | SVN ⇄ GIT ]
STR #3352
Application: | FLTK Library |
Status: | 1 - Closed w/Resolution |
Priority: | 3 - Moderate, e.g. unable to compile the software |
Scope: | 3 - Applies to all machines and operating systems |
Subsystem: | Core Library |
Summary: | Tiny window problem if child group larger than window |
Version: | 1.3.4 |
Created By: | greg.ercolano |
Assigned To: | AlbrechtS |
Fix Version: | 1.4.0 |
Fix Commit: | 091712bea8ff5aab89f0f8483ca572c118ca5715 |
Update Notification: | |
Trouble Report Files:
Trouble Report Comments:
|
#1 | greg.ercolano 13:02 Nov 13, 2016 |
| With the attached program, a very tiny window opens (instead of 100x100). OS: Ubuntu 14.04 + Metacity. Can also replicate with Ubuntu 16.04 + KDE.
Seems to be if the child group is 100x100 more in size than the window, this problem occurs.
So a 400x400 window with a 500x500 group has the same problem, but if the group is 499x499, then it opens fine.
Ditto for a 100x100 window with a 200x200 group is bad, but a 199x199 group works OK. | |
|
#2 | greg.ercolano 13:50 Nov 13, 2016 |
| Some investigation info:
It seems the reason the window can't be resized is perhaps the size_range() is set to a really low value.
I don't think there's a public way to access the size_range() values once set, so I added a few methods to Fl_Window.H to let me access them so I can print them (see attached patch add_size_range_accessors.patch).
What I found is the that the minw/h values are set to negative values (-150,-150) and maxw/h values are set to 0,0 when this problem happens, which explains why one can't resize the window once open. | |
|
#3 | greg.ercolano 14:03 Nov 13, 2016 |
| OK, yes, seems like it might have to do with the size_range() calculations for the minw/h values.
Seems I can 'fix' the problem by adding the following to the winsize_problem.cxx example code:
win->resizable(group); win->size_range(100,100,0,0); // ADD THIS win->end(); win->show();
(Apparently the size_range() maxw/maxh values can be 0,0 to allow the maximum window size, so I don't think those being zero are the problem as I first thought in Comment #2.. the issue seems more to do perhaps with the minimum sizes being <0 unless changed) | |
|
#4 | greg.ercolano 21:26 Nov 13, 2016 |
| Assigning this to myself, and suggesting the attached solution: fix_str_3352_v1.patch
Note that code for all three platforms are being modified here, as I think this problem applies to all three.
Basically, this focuses on the logic that occurs when no size_range() has been specified by the user, and FLTK tries to derive a default, which includes being influenced by the window's resizable().
Seeking feedback from devs before a commit; the existing logic is very old (r1), so it's been the way it is for >15 years. But it also seems to be wrong WRT to the following docs for Fl_Window::size_range():
""" If size_range() is not called, FLTK tries to figure out the range from the setting of resizable():
If resizable() is NULL (this is the default) then the window cannot be resized and the resize border and max-size control will not be displayed for the window.
If either dimension of resizable() is less than 100, then that is considered the minimum size. Otherwise the resizable() has a minimum size of 100.
If either dimension of resizable() is zero, then that is also the maximum size (so the window cannot resize in that direction). """
..the part that seems inconsistent is the last sentence:
""" If either dimension of resizable() is zero, then that is also the maximum size (so the window cannot resize in that direction). """
I find that is not the case with the current logic, which is executed if the size_range() hasn't been set yet, and a resizable() has been set for the window:
---- snip if (w->resizable()) { Fl_Widget *o = w->resizable(); // the resizable int minw = o->w(); if (minw > 100) minw = 100; // minw for resizable int minh = o->h(); if (minh > 100) minh = 100; // minh for resizable w->size_range(w->w() - o->w() + minw, w->h() - o->h() + minh, 0, 0); } ---- snip
..so if either dimension of the resizable() is zero, the size_range() minw/h value still ends up being the window's current dimension. Example: window's w()=100, window's resizable w()=0:
w->size_range(w->w() - o->w() + minw, w->h() - o->h() + minh, 0, 0); // ------ ------ ---- // | | | // 100 - 0 + 0
..the result for size_range()'s minw will be 100, not zero, and therefore /will/ be resizable, since the width size range is minw=100, maxw=0.
I don't think the new logic I'm adding affects this.
But something is wrong here: either the logic or the docs are incorrect, not sure which. | |
|
#5 | greg.ercolano 22:47 Nov 13, 2016 |
| I've stared at this a bit, and I just don't get the old logic:
Fl_Widget *o = w->resizable(); int minw = o->w(); if (minw > 100) minw = 100; int minh = o->h(); if (minh > 100) minh = 100; w->size_range(w->w() - o->w() + minw, w->h() - o->h() + minh, 0, 0);
A common case would be if the window w=400 and the resizable w=380, why set the default minimum width for the window to 120? (400-380+100)=120
I just don't see why that would be desirable behavior. And note this same logic is for all platforms (mac/linux/windows).
Actually, I'm not sure I understand why the resizable should influence the window's default size range sizes at all (while perhaps the odd exception of the resizable's w() or h() being zero might be useful).
I'd think the default minimum width/height should be the window's initial size, and anything else would be up to the app to set size_range() by itself. The exception being the allowance for the resizable's w() or h() == 0, disabling resizing on that dimension.
To implement /that/ logic, I'd suggest:
Fl_Widget *o = w->resizable(); int minw = w->w(); // minw is window's initial width int minh = w->h(); // minh is window's initial height int maxw = (o->w() == 0) ? minw : 0; // special case if resizable w()==0: disable resizing width int maxh = (o->h() == 0) ? minh : 0; // special case if resizable h()==0: disable resizing height w->size_range(minw, minh, maxw, maxh);
..I did some tests with that code, and that /seems/ the most desirable "default" behavior for window sizing, IMHO. Not sure what that'd break, but it sure seems easy to understand. And again, an app that wants other behavior can set size_range() to what it wants. | |
|
#6 | greg.ercolano 22:57 Nov 13, 2016 |
| Attaching v2 patch, in case the bottom part of Comment #5 is useful. But I imagine v1 patch, or something like it, is the path of least resistance.
Awaiting dev feedback before any commits. | |
|
#7 | AlbrechtS 02:52 Nov 14, 2016 |
| Greg, I don't have enough time to investigate more or to test the given patch(es), but I see that the v2 patch doesn't apply cleanly to FLTK 1.4. Please switch to branch-1.4 because we don't want to continue development on branch-1.3, particularly not on code that is so old and that would introduce behavioral changes.
However, I think that the actual behavior is strange, and reading your descriptions I agree that it should be changed (in 1.4 ;-)). I can't tell if v1 or v2 would be the way to go though... | |
|
#8 | greg.ercolano 05:53 Nov 14, 2016 |
| Attaching a v2 patch against 1.4.x: fix_str_3352_v2_fltk-1.4.x.patch | |
|
#9 | greg.ercolano 06:01 Nov 14, 2016 |
| Attaching the v1 patch against 1.4.x, just for completeness. | |
|
#10 | matt 07:00 Feb 02, 2019 |
| I find a completely random value of 100 for any minimum or maximum a really bad idea. IMHO, the minimum should be the width of the window minus the width of the resizable widget after clipping it to the window size. Everything else is just a bunch of assumption of the developers intention, which leads to exactly what we saw in this STR: unexpected behavior and a 10 postings long hunt for what the original devs may have though.
Do you want to fix this, Greg, or shall I do it? | |
|
#11 | AlbrechtS 11:50 Dec 05, 2021 |
| +1 on fix_str_3352_v2_fltk-1.4.x.patch
I agree with Greg (and this patch version and Matt in comment #10) that setting an arbitrary minimum value for the window size is something we shouldn't do. OTOH, if the WM forces a certain minimum size this would be OK.
I'm uploading 0001-Fix-Tiny-window-problem-.-STR-3352.patch which is the same as the above mentioned patch adjusted to Git current (1.4.0).
Note: Tested with 'winsize-problem.cxx' and different group and window sizes on Linux only. It would be interesting what happens on macOS and Windows if the window size is set to (w=2, h=2), for instance.
Greg, please feel free to use this patch and commit yourself, since it's your work. Otherwise I can merge my branch with *my* commit message, see https://github.com/Albrecht-S/fltk/tree/str-3352_v2 and https://github.com/Albrecht-S/fltk/commit/0f15df24eca6d81eee7544501b3b8633c57644cc (the patch).
It's your choice...
PS: note to readers: the GitHub links will go stale after a while once the branch is deleted. | |
|
#12 | AlbrechtS 12:22 Dec 05, 2021 |
| Tested on macOS. Setting a window size of 2x2 makes the window really hard to find but it's disputable whether we should force a minimal window size in general.
However, I believe that these are two different issues and the given fix is fine.
I'm leaving tests on Windows to others... | |
|
#13 | greg.ercolano 12:31 Dec 05, 2021 |
| Thanks Albrecht. OK, fixed in 74dd5164d.
Applied the patch (with tabs removed), and tested to make sure the test program doesn't exhibit the described behavior.
(Reading this STR is like reading someone else's report; I can't remember any of this, lol)
It does seem like there's other problems mentioned in this STR that should maybe be reported as a separate issue (regarding the closing sentence in comment #4). If so I'll open a new RFE issue for that. | |
|
#14 | AlbrechtS 12:34 Dec 05, 2021 |
| Note: I removed "Linux:" from STR title (aka summary) and set scope to "... all machines and OS's". | |
|
#15 | chris 19:48 Dec 05, 2021 |
| >> [Greg #13] (Reading this STR is like reading someone else's report; I can't >> remember any of this, lol)
Don't worry, you are right! You opened it on behalf of my report in fltkgeneral:
https://groups.google.com/g/fltkgeneral/c/m-d79BEQmXw/m/305ElD7GBAAJ
(But I forgot till now ;) | |
|
#16 | AlbrechtS 10:52 Dec 06, 2021 |
| Fixed in Git repository.
Commit: 39b5ae9e6e957582eddf16c86829d01473458e5d
I fixed the docs according to the corrected behavior.
Please check, and if this is OK I believe we can close this STR. | |
|
#17 | greg.ercolano 11:56 Dec 06, 2021 |
| Looks good! Glad that extra bit is solved.
Reading the doc mods, I'd just suggest a few small tweaks to the Fl_Window::size_range() docs. This one for extra clarity:
- - If either dimension of resizable() is zero, then.. + - If either dimension of the resizable() widget is zero, then..
..and this one for grammar:
- is unlimited like if size_range(w(), h(), 0, 0) was called. + is unlimited as if size_range(w(), h(), 0, 0) was called. | |
|
#18 | AlbrechtS 05:11 Feb 13, 2022 |
| Note: the attempted fix in commit 74dd5164d3da59c99efc451438091c6a40327e0b introduced a regression.
Please see GitHub Issue #392: https://github.com/fltk/fltk/issues/392 | |
|
#19 | greg.ercolano 12:27 Feb 13, 2022 |
| @Albrecht, I think this is what the bottom part of Comment #5 alludes to:
> v1 patch being path of least resistance > v2 patch which redefines the min size behavior
Seems like the V2 path was taken (redefines resize behavior), but V1 is perhaps "better" if we want to continue the old behavior. | |
|
#20 | AlbrechtS 05:35 Mar 01, 2022 |
| Summary of changes:
(1) 74dd5164d3da59c99efc451438091c6a40327e0b: first attempt (regression, see GitHub issue #392)
(2) 39b5ae9e6e957582eddf16c86829d01473458e5d: documentation updates according to (1)
(3) 091712bea8ff5aab89f0f8483ca572c118ca5715: new fix (see below), documentation modified accordingly
The third commit changes internal calling sequences and goes back to the original behavior as supposedly intended by the original author(s).
The key point in calculation of the default size_range() values is to clip the resizable() widget to the bounds of the window as suggested by Matthias in comment #10.
The "magic 100" is a plausible value: it is used as the lower limits of the resizable() widget (width and height) if it is larger than 100 in any direction. This makes the resizable() widget shrinkable and thus the window resizable below its original size (the regression mentioned above was that windows couldn't become smaller than their original sizes).
The old size calculation was broken only for already broken widget layouts, i.e. if the resizable() widget was not entirely inside the window. The new algorithm produces the same results as before if the resizable() widget doesn't need to be clipped (backwards compatibility).
The previously documented but not implemented feature "If either dimension of resizable() is zero, then that is also the maximum size (so the window cannot resize in that direction)" has been implemented. The new documentation clarifies this in much simpler wording:
"If either dimension of resizable() is zero, then the window cannot resize in that direction."
Finally, this applies only to the default size_range() calculation, i.e. if a resizable() widget has been set but size_range() has not been called. The recommendation is to always call size_range() if a resizable() widget of the window is set so the programmer has full control. | |
[ Return to Bugs & Features ]
|
| |