[patch]: fix the download GUI resize bug
There's a bug in the downloads GUI that stops the download items resizing when the window does. The resizable of the window should be the scrollgroup, not the packedgroup. Fixing this means we don't need to explicitly resize the scrollgroup after removing an item (because the window has already resized it) so I took that out. Patch attached. Regards, Jeremy Henty
On Wed, Oct 29, 2008 at 02:28:48PM +0000, Jeremy Henty wrote:
There's a bug in the downloads GUI that stops the download items resizing when the window does. The resizable of the window should be the scrollgroup, not the packedgroup. Fixing this means we don't need to explicitly resize the scrollgroup after removing an item (because the window has already resized it) so I took that out. Patch attached.
Looks good to me! Cheers, Johannes
Regards,
Jeremy Henty
From 0e241cbce318768aa8ad8f7715b80dbb3a38a6f3 Mon Sep 17 00:00:00 2001 From: Jeremy Henty <onepoint@starurchin.org> Date: Wed, 29 Oct 2008 14:14:58 +0000 Subject: [PATCH] Fix the download GUI resize behaviour.
--- dpi/downloads.cc | 5 +---- 1 files changed, 1 insertions(+), 4 deletions(-)
diff --git a/dpi/downloads.cc b/dpi/downloads.cc index d2f3a54..b88deed 100644 --- a/dpi/downloads.cc +++ b/dpi/downloads.cc @@ -1029,9 +1029,6 @@ void DLWin::del(int n_item)
// Remove the widget from the scroll group mPG->remove(dl_item->get_widget()); - // Resize the scroll group - mPG->resize(mWin->w(), 1); - mDList->del(n_item); delete(dl_item); } @@ -1093,7 +1090,7 @@ DLWin::DLWin(int ww, int wh) { mScroll->end(); mScroll->type(ScrollGroup::VERTICAL); mWin->end(); - mWin->resizable(mPG); + mWin->resizable(mScroll); mWin->callback(dlwin_esc_cb, NULL); mWin->show();
-- 1.6.0.2
_______________________________________________ Dillo-dev mailing list Dillo-dev@dillo.org http://lists.auriga.wearlab.de/cgi-bin/mailman/listinfo/dillo-dev
On Wed, Oct 29, 2008 at 02:28:48PM +0000, Jeremy Henty wrote:
There's a bug in the downloads GUI that stops the download items resizing when the window does. The resizable of the window should be the scrollgroup, not the packedgroup. Fixing this means we don't need to explicitly resize the scrollgroup after removing an item (because the window has already resized it) so I took that out. Patch attached.
Oh, please excuse me not understanding what the bug is and how to reproduce it. Please explain with apples and pears! :-) (yes, I get the explicit resize is strange) -- Cheers Jorge.-
On Wed, Oct 29, 2008 at 01:38:14PM -0300, Jorge Arellano Cid wrote:
On Wed, Oct 29, 2008 at 02:28:48PM +0000, Jeremy Henty wrote:
There's a bug in the downloads GUI that stops the download items resizing when the window does. The resizable of the window should be the scrollgroup, not the packedgroup. Fixing this means we don't need to explicitly resize the scrollgroup after removing an item (because the window has already resized it) so I took that out. Patch attached.
Oh, please excuse me not understanding what the bug is and how to reproduce it. Please explain with apples and pears! :-)
OK, fruit 'n' veg coming up! :-) Symptom: If you change the width of the download GUI window its contents do not respond. This is annoying because (a) if you narrow the window to save space then it truncates its contents at the right-hand side rather than gracefully resizing them, and (b) if you widen the window hoping that it will make the display more legible by widening the text labels etc. then nothing happens. It's an easy bug to miss because the result is not obviously wrong, just poor and inconvenient. Once you've seen the correct behaviour the improvement is obvious. Cause: FLTK's resize mechanism is controlled by the resizable members of the Group widgets. This is a pointer to the child of the group widget which the group should resize when its own size changes. The downloads GUI is buggy because it sets the resizable widget of the Window to be the PackedGroup that contains the download items, which is wrong because the PackedGroup is *not* a child of the Window, it's a child of the the ScrollGroup (which is in turn a child of the Window). The result is that the contents of the Window don't resize at all, except when the code tries to work around the bug by explicitly calling resize(). Fix: The code should make the ScrollGroup the resizable widget of the Window. (The ScrollGroup then properly resizes the PackedGroup without any further changes. I think this is because its type is set to VERTICAL.) This makes the width of the ScrollGroup (and hence the PackedGroup too) follow the width of the Window in a way that is beautiful to behold. :-) Now we don't need to explicitly resize the any of the widgets because FLTK does it all automatically whenever the width of the Window changes, so the call to resize() should be deleted. Apples and pears enough? :-) :-) Regards, Jeremy Henty
On Wed, Oct 29, 2008 at 09:16:07PM +0000, Jeremy Henty wrote:
On Wed, Oct 29, 2008 at 01:38:14PM -0300, Jorge Arellano Cid wrote:
On Wed, Oct 29, 2008 at 02:28:48PM +0000, Jeremy Henty wrote:
There's a bug in the downloads GUI that stops the download items resizing when the window does. The resizable of the window should be the scrollgroup, not the packedgroup. Fixing this means we don't need to explicitly resize the scrollgroup after removing an item (because the window has already resized it) so I took that out. Patch attached.
Oh, please excuse me not understanding what the bug is and how to reproduce it. Please explain with apples and pears! :-)
OK, fruit 'n' veg coming up! :-)
Symptom:
If you change the width of the download GUI window its contents do not respond. This is annoying because (a) if you narrow the window to save space then it truncates its contents at the right-hand side rather than gracefully resizing them, and (b) if you widen the window hoping that it will make the display more legible by widening the text labels etc. then nothing happens. It's an easy bug to miss because the result is not obviously wrong, just poor and inconvenient. Once you've seen the correct behaviour the improvement is obvious.
I still don't get it... Here, with the unpatched code, the download dpi resizes without problems when narrowing the window (fltk-r6403), and labels are widened too. And more, the unpatched code responds to clicking "done" by removing the corresponding item (when alone and when in a list). The patched code, here, sometimes doesn't remove the widget when clicking "done", and others leaves a background with the to-be-removed item as a window shadow (partial remove). I'm baffled.
Cause:
FLTK's resize mechanism is controlled by the resizable members of the Group widgets. This is a pointer to the child of the group widget which the group should resize when its own size changes. The downloads GUI is buggy because it sets the resizable widget of the Window to be the PackedGroup that contains the download items, which is wrong because the PackedGroup is *not* a child of the Window, it's a child of the the ScrollGroup (which is in turn a child of the Window). The result is that the contents of the Window don't resize at all, except when the code tries to work around the bug by explicitly calling resize().
Fix:
The code should make the ScrollGroup the resizable widget of the Window. (The ScrollGroup then properly resizes the PackedGroup without any further changes. I think this is because its type is set to VERTICAL.) This makes the width of the ScrollGroup (and hence the PackedGroup too) follow the width of the Window in a way that is beautiful to behold. :-) Now we don't need to explicitly resize the any of the widgets because FLTK does it all automatically whenever the width of the Window changes, so the call to resize() should be deleted.
-- Cheers Jorge.-
On Thu, Oct 30, 2008 at 09:49:23AM -0300, Jorge Arellano Cid wrote:
Here, with the unpatched code, the download dpi resizes without problems when narrowing the window (fltk-r6403), and labels are widened too.
OK, I am embarrassed. There certainly *used* to be the problem I described but it has gone. I thought I had checked that but I obviously didn't. Apologies. However, it is definitely weird to set the window's resizable to something that is not a child of the window. Also, to my eye the resizing seems smoother when the resizable is the scrollgroup. Without the patch the contents seem to jiggle sideways more as the width changes.
And more, the unpatched code responds to clicking "done" by removing the corresponding item (when alone and when in a list). The patched code, here, sometimes doesn't remove the widget when clicking "done", and others leaves a background with the to-be-removed item as a window shadow (partial remove).
Strange, I deliberately tested clicking "done" (both with one download and several downloads) before submitting and I saw no problems. What happens if you call relayout() on the widget instead of resize()? (patch attached) I'm using fltk2-r6403, gcc-4.1.2 and running on Linux. Regards, Jeremy Henty
On Thu, Oct 30, 2008 at 05:36:42PM +0000, Jeremy Henty wrote:
On Thu, Oct 30, 2008 at 09:49:23AM -0300, Jorge Arellano Cid wrote:
Here, with the unpatched code, the download dpi resizes without problems when narrowing the window (fltk-r6403), and labels are widened too.
OK, I am embarrassed. There certainly *used* to be the problem I described but it has gone. I thought I had checked that but I obviously didn't. Apologies.
We, human beans, make mistakes by definition! :-)
However, it is definitely weird to set the window's resizable to something that is not a child of the window. Also, to my eye the resizing seems smoother when the resizable is the scrollgroup. Without the patch the contents seem to jiggle sideways more as the width changes.
And more, the unpatched code responds to clicking "done" by removing the corresponding item (when alone and when in a list). The patched code, here, sometimes doesn't remove the widget when clicking "done", and others leaves a background with the to-be-removed item as a window shadow (partial remove).
Strange, I deliberately tested clicking "done" (both with one download and several downloads) before submitting and I saw no problems. What happens if you call relayout() on the widget instead of resize()? (patch attached)
Same problem... :( The test that fails here is: - load the dillo downloads page - download a tarball - download another - resize the window to 1.25 the height of both download panels - Once both are done, press done in the lower panel then I get the panel drawn over the old one (like a shadow). -- Cheers Jorge.-
On Thu, Oct 30, 2008 at 02:54:57PM -0300, Jorge Arellano Cid wrote:
The test that fails here is:
- load the dillo downloads page - download a tarball - download another - resize the window to 1.25 the height of both download panels - Once both are done, press done in the lower panel
then I get the panel drawn over the old one (like a shadow).
I cannot reproduce this at all!!! But if I write a FLTK2 app with a scrolled list of buttons that delete themselves when pressed then I *do* get redisplay problems: the deleted buttons don't disappear. And the fix is to tell the scroll to redraw itself. I guess that's because the scroll is responsible for the blank space left behind when the button disappears. It would be interesting to know if this patch fails the test. (I've also attached the FLTK2 app. If you comment out the redraw() call in the callback you'll see that the bottom button does not disappear after clicking any button, even though the list has actually shrunk.) Regards, Jeremy Henty
On Thu, Oct 30, 2008 at 07:17:27PM +0000, Jeremy Henty wrote:
On Thu, Oct 30, 2008 at 02:54:57PM -0300, Jorge Arellano Cid wrote:
The test that fails here is:
- load the dillo downloads page - download a tarball - download another - resize the window to 1.25 the height of both download panels - Once both are done, press done in the lower panel
then I get the panel drawn over the old one (like a shadow).
I cannot reproduce this at all!!! But if I write a FLTK2 app with a scrolled list of buttons that delete themselves when pressed then I *do* get redisplay problems: the deleted buttons don't disappear. And the fix is to tell the scroll to redraw itself. I guess that's because the scroll is responsible for the blank space left behind when the button disappears. It would be interesting to know if this patch fails the test. (I've also attached the FLTK2 app. If you comment out the redraw() call in the callback you'll see that the bottom button does not disappear after clicking any button, even though the list has actually shrunk.)
Committed. Yes, the example program showed exactly what I was getting here. This looks like a bug in FLTK2 (AFAIU, the scroll group should take care of its background when an item is removed). -- Cheers Jorge.-
On Thu, Oct 30, 2008 at 05:32:58PM -0300, Jorge Arellano Cid wrote:
On Thu, Oct 30, 2008 at 07:17:27PM +0000, Jeremy Henty wrote:
... if I write a FLTK2 app with a scrolled list of buttons that delete themselves when pressed then I *do* get redisplay problems: the deleted buttons don't disappear. And the fix is to tell the scroll to redraw itself.
Committed.
Good! I'm glad something useful came out of this.
Yes, the example program showed exactly what I was getting here. This looks like a bug in FLTK2 (AFAIU, the scroll group should take care of its background when an item is removed).
IIRC I've sometimes had to manually tell a widget to update/redraw itself and I thought this was a deliberate policy from the FLTK devs. I'll send them my example and ask. Regards, Jeremy Henty
participants (3)
-
jcid＠dillo.org -
Johannes.Hofmann＠gmx.de -
onepoint＠starurchin.org